From 9e5115a9e0f58d95b9dd5410f8d2a768191b4add Mon Sep 17 00:00:00 2001 From: Jules Villard Date: Fri, 23 Aug 2019 09:44:52 -0700 Subject: [PATCH] [annotreach] support for new `"symbol_regexps"` matcher Summary: This is more powerful than `"symbols"` for more advanced use-cases. Keep `"symbols"` unchanged to make migrating easier. Differential Revision: D16985756 fbshipit-source-id: dfbb09393 --- infer/man/man1/infer-analyze.txt | 15 +++++---- infer/man/man1/infer-full.txt | 15 +++++---- infer/man/man1/infer.txt | 15 +++++---- infer/src/base/Config.ml | 17 +++++++--- infer/src/checkers/annotationReachability.ml | 33 +++++++++++++------ .../cpp/annotation-reachability/Makefile | 7 ++-- .../forbidden/library.cpp | 3 ++ .../forbidden/library.h | 3 ++ .../sources/client.cpp | 2 ++ 9 files changed, 71 insertions(+), 39 deletions(-) diff --git a/infer/man/man1/infer-analyze.txt b/infer/man/man1/infer-analyze.txt index 910fde6de..7cc64040d 100644 --- a/infer/man/man1/infer-analyze.txt +++ b/infer/man/man1/infer-analyze.txt @@ -337,26 +337,27 @@ CLANG OPTIONS Specify annotation reachability analyses to be performed on C/C++/ObjC code. Each entry is a JSON object whose key is the issue name. "sources" and "sinks" can be specified either by - symbol or path prefix. "sinks" optionally can specify "overrides" - (by symbol or path prefix) that block the reachability analysis - when hit. Example: { + symbol (including regexps) or path prefix. "sinks" optionally can + specify "overrides" (by symbol or path prefix) that block the + reachability analysis when hit. Example: { "ISOLATED_REACHING_CONNECT": { - "doc_url": "http:://optional/issue/doc/link.html", + "doc_url": + "http:://example.com/issue/doc/optional_link.html", "sources": { "desc": "Code that should not call connect [optional]", "paths": [ "isolated/" ] }, "sinks": { "symbols": [ "connect" ], - "overrides": { "symbols": [ "Trusted::" ] } + "overrides": { "symbol_regexps": [ ".*::Trusted::.*" ] } } } } + This will cause us to create a new ISOLATED_REACHING_CONNECT issue for every function whose source path starts with "isolated/" that may reach the function named "connect", ignoring paths that - go through a symbol starting with "Trusted::". - + go through a symbol matching the OCaml regexp ".*::Trusted::.*". --annotation-reachability-cxx-sources json Override sources in all cxx annotation reachability specs with the diff --git a/infer/man/man1/infer-full.txt b/infer/man/man1/infer-full.txt index 7ab4debfb..fb51f05f5 100644 --- a/infer/man/man1/infer-full.txt +++ b/infer/man/man1/infer-full.txt @@ -73,26 +73,27 @@ OPTIONS Specify annotation reachability analyses to be performed on C/C++/ObjC code. Each entry is a JSON object whose key is the issue name. "sources" and "sinks" can be specified either by - symbol or path prefix. "sinks" optionally can specify "overrides" - (by symbol or path prefix) that block the reachability analysis - when hit. Example: { + symbol (including regexps) or path prefix. "sinks" optionally can + specify "overrides" (by symbol or path prefix) that block the + reachability analysis when hit. Example: { "ISOLATED_REACHING_CONNECT": { - "doc_url": "http:://optional/issue/doc/link.html", + "doc_url": + "http:://example.com/issue/doc/optional_link.html", "sources": { "desc": "Code that should not call connect [optional]", "paths": [ "isolated/" ] }, "sinks": { "symbols": [ "connect" ], - "overrides": { "symbols": [ "Trusted::" ] } + "overrides": { "symbol_regexps": [ ".*::Trusted::.*" ] } } } } + This will cause us to create a new ISOLATED_REACHING_CONNECT issue for every function whose source path starts with "isolated/" that may reach the function named "connect", ignoring paths that - go through a symbol starting with "Trusted::". - + go through a symbol matching the OCaml regexp ".*::Trusted::.*". See also infer-analyze(1). --annotation-reachability-cxx-sources json diff --git a/infer/man/man1/infer.txt b/infer/man/man1/infer.txt index 9375b822c..5eac612a9 100644 --- a/infer/man/man1/infer.txt +++ b/infer/man/man1/infer.txt @@ -73,26 +73,27 @@ OPTIONS Specify annotation reachability analyses to be performed on C/C++/ObjC code. Each entry is a JSON object whose key is the issue name. "sources" and "sinks" can be specified either by - symbol or path prefix. "sinks" optionally can specify "overrides" - (by symbol or path prefix) that block the reachability analysis - when hit. Example: { + symbol (including regexps) or path prefix. "sinks" optionally can + specify "overrides" (by symbol or path prefix) that block the + reachability analysis when hit. Example: { "ISOLATED_REACHING_CONNECT": { - "doc_url": "http:://optional/issue/doc/link.html", + "doc_url": + "http:://example.com/issue/doc/optional_link.html", "sources": { "desc": "Code that should not call connect [optional]", "paths": [ "isolated/" ] }, "sinks": { "symbols": [ "connect" ], - "overrides": { "symbols": [ "Trusted::" ] } + "overrides": { "symbol_regexps": [ ".*::Trusted::.*" ] } } } } + This will cause us to create a new ISOLATED_REACHING_CONNECT issue for every function whose source path starts with "isolated/" that may reach the function named "connect", ignoring paths that - go through a symbol starting with "Trusted::". - + go through a symbol matching the OCaml regexp ".*::Trusted::.*". See also infer-analyze(1). --annotation-reachability-cxx-sources json diff --git a/infer/src/base/Config.ml b/infer/src/base/Config.ml index 34e896925..7b5206329 100644 --- a/infer/src/base/Config.ml +++ b/infer/src/base/Config.ml @@ -814,22 +814,29 @@ and { annotation_reachability and annotation_reachability_cxx = CLOpt.mk_json ~long:"annotation-reachability-cxx" ~in_help:InferCommand.[(Analyze, manual_clang)] - {|Specify annotation reachability analyses to be performed on C/C++/ObjC code. Each entry is a JSON object whose key is the issue name. "sources" and "sinks" can be specified either by symbol or path prefix. "sinks" optionally can specify "overrides" (by symbol or path prefix) that block the reachability analysis when hit. Example: - { + ( "Specify annotation reachability analyses to be performed on C/C++/ObjC code. Each entry is \ + a JSON object whose key is the issue name. \"sources\" and \"sinks\" can be specified \ + either by symbol (including regexps) or path prefix. \"sinks\" optionally can specify \ + \"overrides\" (by symbol or path prefix) that block the reachability analysis when hit. \ + Example:\n" + ^ {|{ "ISOLATED_REACHING_CONNECT": { - "doc_url": "http:://optional/issue/doc/link.html", + "doc_url": "http:://example.com/issue/doc/optional_link.html", "sources": { "desc": "Code that should not call connect [optional]", "paths": [ "isolated/" ] }, "sinks": { "symbols": [ "connect" ], - "overrides": { "symbols": [ "Trusted::" ] } + "overrides": { "symbol_regexps": [ ".*::Trusted::.*" ] } } } } -This will cause us to create a new ISOLATED_REACHING_CONNECT issue for every function whose source path starts with "isolated/" that may reach the function named "connect", ignoring paths that go through a symbol starting with "Trusted::". |} + ^ "\n\ + This will cause us to create a new ISOLATED_REACHING_CONNECT issue for every function \ + whose source path starts with \"isolated/\" that may reach the function named \"connect\", \ + ignoring paths that go through a symbol matching the OCaml regexp \".*::Trusted::.*\"." ) and annotation_reachability_cxx_sources = diff --git a/infer/src/checkers/annotationReachability.ml b/infer/src/checkers/annotationReachability.ml index d3c60640f..13ee312aa 100644 --- a/infer/src/checkers/annotationReachability.ml +++ b/infer/src/checkers/annotationReachability.ml @@ -263,22 +263,35 @@ module CxxAnnotationSpecs = struct L.d_printf "%b %s.@." r desc ; r + let at_least_one_nonempty ~src symbols symbol_regexps paths = + if List.is_empty symbols && Option.is_none symbol_regexps && List.is_empty paths then + L.die UserError "Must specify at least one of `paths`, `symbols`, or `symbols_regexps` in %s" + src + + let spec_from_config spec_name spec_cfg source_overrides = let src = option_name ^ " -> " ^ spec_name in let make_pname_pred entry ~src : Typ.Procname.t -> bool = let symbols = U.yojson_lookup entry "symbols" ~src ~f:U.string_list_of_yojson ~default:[] in + let symbol_regexps = + U.yojson_lookup entry "symbol_regexps" ~src ~default:None ~f:(fun json ~src -> + U.string_list_of_yojson json ~src |> String.concat ~sep:"\\|" |> Str.regexp + |> Option.some ) + in let paths = U.yojson_lookup entry "paths" ~src ~f:U.string_list_of_yojson ~default:[] in - let sym_pred pname = List.exists ~f:(symbol_match (Typ.Procname.to_string pname)) symbols in + at_least_one_nonempty ~src symbols symbol_regexps paths ; + let sym_pred pname_string = List.exists ~f:(symbol_match pname_string) symbols in + let sym_regexp_pred pname_string = + match symbol_regexps with + | None -> + false + | Some regexp -> + Str.string_match regexp pname_string 0 + in let path_pred pname = List.exists ~f:(path_match (src_path_of pname)) paths in - match (symbols, paths) with - | [], [] -> - L.die UserError "Must specify either `paths` or `symbols` in %s" src - | _, [] -> - sym_pred - | [], _ -> - path_pred - | _, _ -> - fun pname -> sym_pred pname || path_pred pname + fun pname -> + let pname_string = Typ.Procname.to_string pname in + sym_pred pname_string || sym_regexp_pred pname_string || path_pred pname in let sources, sources_src = if List.length source_overrides > 0 then (source_overrides, src_option_name) diff --git a/infer/tests/codetoanalyze/cpp/annotation-reachability/Makefile b/infer/tests/codetoanalyze/cpp/annotation-reachability/Makefile index 342021630..a3ceaa27d 100644 --- a/infer/tests/codetoanalyze/cpp/annotation-reachability/Makefile +++ b/infer/tests/codetoanalyze/cpp/annotation-reachability/Makefile @@ -28,11 +28,12 @@ INFER_OPTIONS = --annotation-reachability-only --debug-exceptions --project-root "sinks": { \ "desc": "a disallowed API", \ "overrides": { \ - "symbols": [ \ - "safewrapper::" \ + "symbol_regexps": [ \ + ".*::safewrapper::wrapper$$", \ + ".*::safewrapper::Wrapper::" \ ] \ }, \ - "symbols" : [ "details::" ], \ + "symbols" : [ "library::details::" ], \ "paths": [ "codetoanalyze/cpp/annotation-reachability/forbidden/" ] \ } \ } \ diff --git a/infer/tests/codetoanalyze/cpp/annotation-reachability/forbidden/library.cpp b/infer/tests/codetoanalyze/cpp/annotation-reachability/forbidden/library.cpp index b64783050..c4b28e41c 100644 --- a/infer/tests/codetoanalyze/cpp/annotation-reachability/forbidden/library.cpp +++ b/infer/tests/codetoanalyze/cpp/annotation-reachability/forbidden/library.cpp @@ -5,6 +5,8 @@ * LICENSE file in the root directory of this source tree. */ +namespace library { + // low-level implementation, clients shouldn't use directly namespace details { @@ -29,3 +31,4 @@ struct Wrapper { void wrapper() { details::low_level(); } } // namespace safewrapper +} // namespace library diff --git a/infer/tests/codetoanalyze/cpp/annotation-reachability/forbidden/library.h b/infer/tests/codetoanalyze/cpp/annotation-reachability/forbidden/library.h index fb1188886..46a7bd93d 100644 --- a/infer/tests/codetoanalyze/cpp/annotation-reachability/forbidden/library.h +++ b/infer/tests/codetoanalyze/cpp/annotation-reachability/forbidden/library.h @@ -4,6 +4,8 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. */ +namespace library { + namespace safewrapper { struct Wrapper { @@ -22,3 +24,4 @@ struct LowLevel { ~LowLevel(); }; } // namespace details +} // namespace library diff --git a/infer/tests/codetoanalyze/cpp/annotation-reachability/sources/client.cpp b/infer/tests/codetoanalyze/cpp/annotation-reachability/sources/client.cpp index f6a10a875..8eeb8726e 100644 --- a/infer/tests/codetoanalyze/cpp/annotation-reachability/sources/client.cpp +++ b/infer/tests/codetoanalyze/cpp/annotation-reachability/sources/client.cpp @@ -8,6 +8,8 @@ namespace client { +using namespace library; + void call_protected_api_bad() { details::low_level(); } void call_wrapper_ok() { safewrapper::wrapper(); }