diff --git a/.inferconfig b/.inferconfig index 90264e7cb..f203abe48 100644 --- a/.inferconfig +++ b/.inferconfig @@ -1,4 +1,6 @@ { + "siof-safe-methods": ["getGlobalNonPODWhitelisted", "whitelisted::getGlobalNonPOD", + "whitelisted::TemplatedObject::getGlobalNonPOD"], "skip-translation": [ { "language": "Java", diff --git a/infer/src/IR/Procname.re b/infer/src/IR/Procname.re index 11ba5ddfc..0426422f9 100644 --- a/infer/src/IR/Procname.re +++ b/infer/src/IR/Procname.re @@ -574,3 +574,52 @@ let module Set = Caml.Set.Make { /** Pretty print a set of proc names */ let pp_set fmt set => Set.iter (fun pname => F.fprintf fmt "%a " pp pname) set; + +let fuzzy_qualifiers_equal fuzzy_qualifiers::fuzzy_qualifiers qualifiers => { + let qual_equal q1 q2 => { + /* qual_name may have qualifiers with template parameters - + drop them to whitelist all instantiations */ + let no_template_name s => List.hd_exn (String.split on::'<' s); + String.equal (no_template_name q1) (no_template_name q2) + }; + let is_std_qual = String.equal "std"; + switch fuzzy_qualifiers { + | [first, ...rest] when is_std_qual first => + /* add special handling for std:: namespace to avoid problems with inconsistent + inline namespaces (such as __1 in libc++) */ + List.hd qualifiers |> Option.value_map default::false f::is_std_qual && + List.is_prefix (List.rev qualifiers) prefix::(List.rev rest) equal::qual_equal + | _ => List.equal equal::qual_equal fuzzy_qualifiers qualifiers + } +}; + +/* This is simplistic and will give the wrong answer in some cases, eg + "foo>::someMethod" will get parsed as ["foo>", + "someMethod"]. Ideally, we would keep the list of qualifiers in the procname, which would save us + from having to properly parse them. */ +let qualifiers_of_qual_name = { + let class_sep_regex = Str.regexp_string "::"; + /* wait until here to define the function so that [class_sep_regex] is only computed once */ + Str.split class_sep_regex +}; + +let qualifiers_of_fuzzy_qual_name qual_name => { + /* Fail if we detect templates in the fuzzy name. Template instantiations are not taken into + account when fuzzy matching, and templates may produce wrong results when parsing qualified + names. */ + if (String.contains qual_name '<') { + failwithf "Unexpected template in fuzzy qualified name %s." qual_name + }; + qualifiers_of_qual_name qual_name +}; + +let fuzzy_equal fuzzy_qualifiers::fuzzy_qualifiers pname => { + let qualifiers = + switch pname { + | C c => fst c |> qualifiers_of_qual_name + | ObjC_Cpp objc_cpp => + List.append (qualifiers_of_qual_name objc_cpp.class_name) [objc_cpp.method_name] + | _ => [] + }; + fuzzy_qualifiers_equal fuzzy_qualifiers::fuzzy_qualifiers qualifiers +}; diff --git a/infer/src/IR/Procname.rei b/infer/src/IR/Procname.rei index e05e4384a..438a7e1d3 100644 --- a/infer/src/IR/Procname.rei +++ b/infer/src/IR/Procname.rei @@ -275,3 +275,38 @@ let to_unique_id: t => string; /** Convert a proc name to a filename. */ let to_filename: t => string; + + +/** Return whether two qualified C++ procnames match up to namescapes and templating. In particular, + this deals with the following issues: + + 1. 'std::' namespace may have inline namespace afterwards: std::move becomes std::__1::move. This + happens on libc++ and to some extent on libstdc++. To work around this problem, make matching + against 'std::' more fuzzier: std::X::Y::Z will match std::.*::X::Y::Z (but only for the + 'std' namespace). + + 2. The names are allowed not to commit to a template specialization: we want std::move to match + std::__1::move and std::__1::move. To do so, comparison function for qualifiers + will ignore template specializations. + + For example: + ["std", "move"]: + matches: ["std", "blah", "blah","move"] + does not match: ["std","blah", "move", "BAD"] - we don't want std::.*::X::.* to pass + does not match: ["stdBAD", "move"], - it's not std namespace anymore + + ["folly", "someFunction"] + matches: ["folly","someFunction"] + matches: ["folly","someFunction"] + matches: ["folly","someFunction"] + does not match: ["folly", "BAD", "someFunction"] - unlike 'std' any other namespace needs all + qualifiers to match + does not match: ["folly","someFunction", "BAD"] - same as previous example + */ +let fuzzy_equal: fuzzy_qualifiers::list string => t => bool; + +let fuzzy_qualifiers_equal: fuzzy_qualifiers::list string => list string => bool; + + +/** parse the argument into a list::of::qualifiers::without::templates */ +let qualifiers_of_fuzzy_qual_name: string => list string; diff --git a/infer/src/base/CommandLineOption.ml b/infer/src/base/CommandLineOption.ml index da50aab10..3a49755b1 100644 --- a/infer/src/base/CommandLineOption.ml +++ b/infer/src/base/CommandLineOption.ml @@ -55,11 +55,13 @@ let warnf = else if not is_originator then fun fmt -> F.ifprintf F.err_formatter fmt else F.eprintf -type section = Analysis | BufferOverrun | Clang | Crashcontext | Driver | Java | Print | Quandary +type section = + Analysis | BufferOverrun | Checkers | Clang | Crashcontext | Driver | Java | Print | Quandary [@@deriving compare] let equal_section = [%compare.equal : section ] -let all_sections = [ Analysis; BufferOverrun; Clang; Crashcontext; Driver; Java; Print; Quandary ] +let all_sections = + [ Analysis; BufferOverrun; Checkers; Clang; Crashcontext; Driver; Java; Print; Quandary ] type 'a parse = Infer of 'a | Javac | NoParse [@@deriving compare] @@ -78,7 +80,8 @@ let to_parse_tag = function | Infer _ -> Infer () | Javac -> Javac | NoParse -> let accept_unknown_args = function | Infer Print | Javac | NoParse -> true - | Infer (Analysis | BufferOverrun | Clang | Crashcontext | Driver | Java | Quandary) -> false + | Infer (Analysis | BufferOverrun | Checkers | Clang | Crashcontext | Driver | Java | Quandary) -> + false type desc = { long: string; short: string; meta: string; doc: string; spec: spec; @@ -640,6 +643,7 @@ let set_curr_speclist_for_parse_action ~incomplete ~usage parse_action = curr_speclist := []; if equal_parse_action parse_action (Infer Driver) then ( add_to_curr_speclist ~add_help:true ~header:"Driver options" (Infer Driver); + add_to_curr_speclist ~header:"Checkers options" (Infer Checkers); add_to_curr_speclist ~header:"Clang-specific options" (Infer Clang); add_to_curr_speclist ~header:"Java-specific options" (Infer Java); add_to_curr_speclist ~header:"Quandary checker options" (Infer Quandary) diff --git a/infer/src/base/CommandLineOption.mli b/infer/src/base/CommandLineOption.mli index 57f20fb9b..75bda9635 100644 --- a/infer/src/base/CommandLineOption.mli +++ b/infer/src/base/CommandLineOption.mli @@ -12,7 +12,8 @@ open! IStd (** a section is a part of infer that can be affected by an infer option *) -type section = Analysis | BufferOverrun | Clang | Crashcontext | Driver | Java | Print | Quandary +type section = + Analysis | BufferOverrun | Checkers | Clang | Crashcontext | Driver | Java | Print | Quandary [@@deriving compare] val all_sections : section list diff --git a/infer/src/base/Config.ml b/infer/src/base/Config.ml index 8e7d35812..8e61742dc 100644 --- a/infer/src/base/Config.ml +++ b/infer/src/base/Config.ml @@ -222,17 +222,17 @@ let use_jar_cache = true let weak = "<\"Weak\">" let whitelisted_cpp_methods = [ - ["std"; "move"]; - ["std"; "forward"]; - ["std"; "min"]; - ["std"; "max"]; - ["google"; "CheckNotNull"]; + "std::move"; + "std::forward"; + "std::min"; + "std::max"; + "google::CheckNotNull"; ] let whitelisted_cpp_classes = [ - ["std"; "__less"]; - ["std"; "__wrap_iter"]; (* libc++ internal name of vector iterator *) - ["std"; "__normal_iterator"]; (* libstdc++ internal name of vector iterator *) + "std::__less"; + "std::__wrap_iter"; (* libc++ internal name of vector iterator *) + "std::__normal_iterator"; (* libstdc++ internal name of vector iterator *) ] type dynamic_dispatch_policy = [ @@ -1151,6 +1151,12 @@ and seconds_per_iteration = CLOpt.mk_float_opt ~deprecated:["seconds_per_iteration"] ~long:"seconds-per-iteration" ~meta:"float" "Set the number of seconds per iteration (see --iterations)" +and siof_safe_methods = + CLOpt.mk_string_list ~long:"siof-safe-methods" + ~parse_mode:CLOpt.(Infer [Checkers]) + "Methods that are SIOF-safe; \"foo::bar\" will match \"foo::bar()\", \"foo::bar()\", \ + etc. (can be specified multiple times)" + and skip_analysis_in_path = CLOpt.mk_string_list ~deprecated:["-skip-clang-analysis-in-path"] ~long:"skip-analysis-in-path" ~parse_mode:CLOpt.(Infer [Driver]) @@ -1575,6 +1581,7 @@ and save_analysis_results = !save_results and seconds_per_iteration = !seconds_per_iteration and show_buckets = !print_buckets and show_progress_bar = !progress_bar +and siof_safe_methods = !siof_safe_methods and skip_analysis_in_path = !skip_analysis_in_path and skip_translation_headers = !skip_translation_headers and sources = !sources diff --git a/infer/src/base/Config.mli b/infer/src/base/Config.mli index 90b31a3fa..220218a9f 100644 --- a/infer/src/base/Config.mli +++ b/infer/src/base/Config.mli @@ -130,8 +130,8 @@ val unsafe_unret : string val use_jar_cache : bool val version_string : string val weak : string -val whitelisted_cpp_methods : string list list -val whitelisted_cpp_classes : string list list +val whitelisted_cpp_methods : string list +val whitelisted_cpp_classes : string list val wrappers_dir : string @@ -266,6 +266,7 @@ val save_analysis_results : string option val seconds_per_iteration : float option val show_buckets : bool val show_progress_bar : bool +val siof_safe_methods : string list val skip_analysis_in_path : string list val skip_translation_headers : string list val spec_abs_level : int diff --git a/infer/src/base/Logging.ml b/infer/src/base/Logging.ml index 797a42add..7e2fe8ffb 100644 --- a/infer/src/base/Logging.ml +++ b/infer/src/base/Logging.ml @@ -30,7 +30,7 @@ let dup_formatter fmt1 fmt2 = (** Name of dir for logging the output in the specific executable *) let log_dir_of_action (action : CLOpt.parse_action) = match action with - | Infer (Analysis | BufferOverrun | Crashcontext | Quandary) -> "analyze" + | Infer (Analysis | BufferOverrun | Checkers | Crashcontext | Quandary) -> "analyze" | Infer Driver -> "driver" | Infer Clang | Infer Java diff --git a/infer/src/checkers/Siof.ml b/infer/src/checkers/Siof.ml index 142e1c995..010da957c 100644 --- a/infer/src/checkers/Siof.ml +++ b/infer/src/checkers/Siof.ml @@ -14,6 +14,15 @@ module L = Logging module GlobalsAccesses = SiofTrace.GlobalsAccesses +let whitelisted_models = + List.map Config.siof_safe_methods ~f:Procname.qualifiers_of_fuzzy_qual_name + +let is_whitelisted (pname : Procname.t) = + (* This is linear in the number of whitelisted models, which is not good if there are many + models... *) + List.exists whitelisted_models + ~f:(fun fuzzy_qualifiers -> Procname.fuzzy_equal pname ~fuzzy_qualifiers) + module Summary = Summary.Make (struct type summary = SiofDomain.astate @@ -77,6 +86,8 @@ module TransferFunctions (CFG : ProcCfg.S) = struct | Prune (exp, loc, _, _) -> let proc_loc = Procdesc.get_loc pdesc in get_globals pdesc loc exp |> add_globals astate proc_loc + | Call (_, Const (Cfun callee_pname), _, _, _) when is_whitelisted callee_pname -> + at_least_nonbottom astate | Call (_, Const (Cfun callee_pname), _::params_without_self, loc, _) when Procname.is_c_method callee_pname && Procname.is_constructor callee_pname && Procname.is_constexpr callee_pname -> diff --git a/infer/src/clang/cFrontend_decl.ml b/infer/src/clang/cFrontend_decl.ml index e313c46a9..d95dbc875 100644 --- a/infer/src/clang/cFrontend_decl.ml +++ b/infer/src/clang/cFrontend_decl.ml @@ -137,56 +137,21 @@ struct let process_methods trans_unit_ctx tenv cg cfg curr_class decl_list = IList.iter (process_one_method_decl trans_unit_ctx tenv cg cfg curr_class) decl_list - (** Given qualified name list and whitelist (list of list of qualifiers), return whether - qualified name matches whitelist. There are some complications due to following constraints: - 1. 'std::' namespace may have inline namespace afterwards - std::move becomes std::__1::move - This happens on libc++ and to some extent on libstdc++. To workaround this problem, make - matching against 'std::' whitelists more fuzzy: - std::X::Y::Z will match std::.*::X::Y::Z (but only for 'std' namespace) - 2. whitelist is specified without template specializations, but we want std::move to - match std::__1::move and std::__1::move. To do so, comparison function - for qualifiers will ignore template specializations. - - For example: - Whitelist: - [["std", "move"], ["folly", "someFunction"]] - Passing qualifiers: - ["std", "blah", "blah","move"] - ["folly","someFunction"] - ["folly","someFunction"] - ["folly","someFunction"] - NOT passing qualifiers: - ["std","blah", "move", "BAD"] - we don't want std::.*::X::.* to pass - ["stdBAD", "move"], - it's not std namespace anymore - ["folly", "BAD", "someFunction"] - unlike 'std' any other namespace needs all quals equal - ["folly","someFunction", "BAD"] - same as previous example - *) let is_whitelisted_qual_name qual_name whitelist = - let qual_equal q1 q2 = - (* qual_name may have qualifiers with template parameters - - drop them to whitelist all instantiations *) - let no_template_name s = List.hd_exn (String.split ~on:'<' s) in - String.equal (no_template_name q1) (no_template_name q2) in - let is_std_qual = String.equal "std" in - let method_matches whitelisted_method = - match whitelisted_method with - | first :: rest when is_std_qual first -> - (* add special handling for std:: namespace to avoid problems with inconsistent - inline namespaces (such as __1 in libc++) *) - List.hd qual_name |> Option.value_map ~default:false ~f:is_std_qual - && List.is_prefix (List.rev qual_name) ~prefix:(List.rev rest) ~equal:qual_equal - | _ -> List.equal ~equal:qual_equal whitelisted_method qual_name in - List.exists ~f:method_matches whitelist + List.exists whitelist + ~f:(fun fuzzy_qualifiers -> Procname.fuzzy_qualifiers_equal ~fuzzy_qualifiers qual_name) (** Given REVERSED list of method qualifiers (method_name::class_name::rest_quals), return whether method should be translated based on method and class whitelists *) - let is_whitelisted_cpp_method qual_method_rev = - (* method is either explictely whitelisted, or all method of a class are whitelisted *) - is_whitelisted_qual_name (List.rev qual_method_rev) Config.whitelisted_cpp_methods || - is_whitelisted_qual_name (List.tl_exn qual_method_rev |> List.rev) - Config.whitelisted_cpp_classes - - + let is_whitelisted_cpp_method = + let method_whitelist = + List.map ~f:Procname.qualifiers_of_fuzzy_qual_name Config.whitelisted_cpp_methods in + let class_whitelist = + List.map ~f:Procname.qualifiers_of_fuzzy_qual_name Config.whitelisted_cpp_classes in + fun qual_method_rev -> + (* method is either explictely whitelisted, or all method of a class are whitelisted *) + is_whitelisted_qual_name (List.rev qual_method_rev) method_whitelist || + is_whitelisted_qual_name (List.tl_exn qual_method_rev |> List.rev) class_whitelist let should_translate_decl trans_unit_ctx dec decl_trans_context = let info = Clang_ast_proj.get_decl_tuple dec in diff --git a/infer/tests/clang.make b/infer/tests/clang.make index 0e61202ed..0cde10e34 100644 --- a/infer/tests/clang.make +++ b/infer/tests/clang.make @@ -14,7 +14,7 @@ OBJECTS = $(foreach source,$(SOURCES),$(basename $(source)).o) include $(TESTS_DIR)/base.make include $(TESTS_DIR)/clang-base.make -infer-out$(TEST_SUFFIX)/report.json: $(CLANG_DEPS) $(SOURCES) $(HEADERS) +infer-out$(TEST_SUFFIX)/report.json: $(CLANG_DEPS) $(SOURCES) $(HEADERS) $(TESTS_DIR)/.inferconfig $(call silent_on_success,\ $(INFER_BIN) --results-dir $(@D) --check-duplicate-symbols \ $(INFER_OPTIONS) -a $(ANALYZER) -- \ diff --git a/infer/tests/codetoanalyze/cpp/checkers/siof/siof.cpp b/infer/tests/codetoanalyze/cpp/checkers/siof/siof.cpp index 67263f60f..a22125950 100644 --- a/infer/tests/codetoanalyze/cpp/checkers/siof/siof.cpp +++ b/infer/tests/codetoanalyze/cpp/checkers/siof/siof.cpp @@ -41,6 +41,14 @@ int X::static_pod_accesses_non_pod = access_to_non_pod(); // SIOF! SomeNonPODObject initWithStatic = getFunctionStaticNonPOD(); // OK SomeNonPODObject initWithGlobal = getGlobalNonPOD(); // SIOF! +SomeNonPODObject initWithGlobalWhitelisted = getGlobalNonPODWhitelisted(); // OK + +SomeNonPODObject initWithGlobalWhitelistedNamespaced = + whitelisted::getGlobalNonPOD(); // OK + +SomeNonPODObject initWithGlobalWhitelistedTemplated = + whitelisted::TemplatedObject::getGlobalNonPOD(); // OK + extern SomeConstexprObject& getGlobalConstexpr(); SomeConstexprObject initWithConstexpr = getGlobalConstexpr(); diff --git a/infer/tests/codetoanalyze/cpp/checkers/siof/siof_different_tu.cpp b/infer/tests/codetoanalyze/cpp/checkers/siof/siof_different_tu.cpp index cf7931766..d75139c13 100644 --- a/infer/tests/codetoanalyze/cpp/checkers/siof/siof_different_tu.cpp +++ b/infer/tests/codetoanalyze/cpp/checkers/siof/siof_different_tu.cpp @@ -33,6 +33,11 @@ SomeNonPODObject& getGlobalNonPOD() { return global_object2; } +SomeNonPODObject& getGlobalNonPODWhitelisted() { + some_other_global_object2.some_method(); + return global_object2; +} + // initialise static class field SomeConstexprObject SomeConstexprObject::instance_; @@ -40,6 +45,23 @@ SomeConstexprObject& getGlobalConstexpr() { return SomeConstexprObject::singletonMethod(); } +namespace whitelisted { + +SomeNonPODObject& getGlobalNonPOD() { + some_other_global_object2.some_method(); + return global_object2; +} + +template +SomeNonPODObject& TemplatedObject::getGlobalNonPOD() { + some_other_global_object2.some_method(); + return global_object2; +} + +// instantiate template so that infer analyses it +template struct TemplatedObject; +} // namespace whitelisted + // initialise static class field template SomeTemplatedConstexprObject SomeTemplatedConstexprObject::instance_; diff --git a/infer/tests/codetoanalyze/cpp/checkers/siof/siof_types.h b/infer/tests/codetoanalyze/cpp/checkers/siof/siof_types.h index 07c520943..d9eba7586 100644 --- a/infer/tests/codetoanalyze/cpp/checkers/siof/siof_types.h +++ b/infer/tests/codetoanalyze/cpp/checkers/siof/siof_types.h @@ -57,3 +57,13 @@ int access_to_templated_non_pod(); int access_to_non_pod(); SomeNonPODObject& getFunctionStaticNonPOD(); SomeNonPODObject& getGlobalNonPOD(); +SomeNonPODObject& getGlobalNonPODWhitelisted(); + +namespace whitelisted { +SomeNonPODObject& getGlobalNonPOD(); + +template +struct TemplatedObject { + static SomeNonPODObject& getGlobalNonPOD(); +}; +} // namespace whitelisted