diff --git a/infer/src/concurrency/ConcurrencyModels.ml b/infer/src/concurrency/ConcurrencyModels.ml index 9d3661f50..db8be1b70 100644 --- a/infer/src/concurrency/ConcurrencyModels.ml +++ b/infer/src/concurrency/ConcurrencyModels.ml @@ -55,8 +55,6 @@ let get_thread_assert_effect = function module Clang : sig val get_lock_effect : Procname.t -> HilExp.t list -> lock_effect - val lock_types_matcher : QualifiedCppName.Match.quals_matcher - val is_recursive_lock_type : QualifiedCppName.t -> bool end = struct type lock_model = @@ -133,11 +131,6 @@ end = struct , mk_matcher ["std::lock"] ) - let lock_types_matcher = - let class_names = List.map lock_models ~f:(fun mdl -> mdl.classname) in - QualifiedCppName.Match.of_fuzzy_qual_names class_names - - (** C++ guard classes used for scope-based lock management. NB we pretend all classes below implement the mutex interface even though only [shared_lock] and [unique_lock] do, for simplicity. The comments summarise which methods are implemented. *) @@ -399,8 +392,6 @@ let runs_on_ui_thread ~attrs_of_pname tenv pname = is_modeled_ui_method tenv pname || annotated_as_uithread_equivalent ~attrs_of_pname tenv pname -let cpp_lock_types_matcher = Clang.lock_types_matcher - let is_recursive_lock_type = function | Typ.CppClass (qname, _) -> Clang.is_recursive_lock_type qname diff --git a/infer/src/concurrency/ConcurrencyModels.mli b/infer/src/concurrency/ConcurrencyModels.mli index 13d6d2b12..2b70ca813 100644 --- a/infer/src/concurrency/ConcurrencyModels.mli +++ b/infer/src/concurrency/ConcurrencyModels.mli @@ -36,8 +36,6 @@ val get_thread_assert_effect : Procname.t -> thread val get_current_class_and_annotated_superclasses : (Annot.Item.t -> bool) -> Tenv.t -> Procname.t -> (Typ.name * Typ.name list) option -val cpp_lock_types_matcher : QualifiedCppName.Match.quals_matcher - val is_recursive_lock_type : Typ.name -> bool (** Type documenting why a method is considered as annotated with a certain annotation *) diff --git a/infer/src/concurrency/RacerD.ml b/infer/src/concurrency/RacerD.ml index 7e6311960..68cdd122f 100644 --- a/infer/src/concurrency/RacerD.ml +++ b/infer/src/concurrency/RacerD.ml @@ -850,21 +850,12 @@ let should_report_on_proc tenv procdesc = || Procdesc.get_access procdesc <> PredSymb.Private && (not (Procname.Java.is_autogen_method java_pname)) && not (Annotations.pdesc_return_annot_ends_with procdesc Annotations.visibleForTesting) - | ObjC_Cpp {kind; class_name} -> - ( match kind with - | CPPMethod _ | CPPConstructor _ | CPPDestructor _ -> - Procdesc.get_access procdesc <> PredSymb.Private - | ObjCClassMethod | ObjCInstanceMethod | ObjCInternalMethod -> - Tenv.lookup tenv class_name - |> Option.exists ~f:(fun {Struct.exported_objc_methods} -> - List.mem ~equal:Procname.equal exported_objc_methods proc_name ) ) - && - let matcher = ConcurrencyModels.cpp_lock_types_matcher in - Option.exists (Tenv.lookup tenv class_name) ~f:(fun class_str -> - (* check if the class contains a lock member *) - List.exists class_str.Struct.fields ~f:(fun (_, ft, _) -> - Option.exists (Typ.name ft) ~f:(fun name -> - QualifiedCppName.Match.match_qualifiers matcher (Typ.Name.qual_name name) ) ) ) + | ObjC_Cpp {kind= CPPMethod _ | CPPConstructor _ | CPPDestructor _} -> + Procdesc.get_access procdesc <> PredSymb.Private + | ObjC_Cpp {kind= ObjCClassMethod | ObjCInstanceMethod | ObjCInternalMethod; class_name} -> + Tenv.lookup tenv class_name + |> Option.exists ~f:(fun {Struct.exported_objc_methods} -> + List.mem ~equal:Procname.equal exported_objc_methods proc_name ) | _ -> false @@ -1106,6 +1097,28 @@ let make_results_table exe_env summaries = Payloads.racerd summary.payloads |> Option.fold ~init:acc ~f:(aggregate_post tenv procname) ) +let class_has_concurrent_method class_summaries = + let open RacerDDomain in + let method_has_concurrent_context (summary : Summary.t) = + Payloads.racerd summary.payloads + |> Option.exists ~f:(fun (payload : summary) -> + match (payload.threads : ThreadsDomain.t) with NoThread -> false | _ -> true ) + in + List.exists class_summaries ~f:method_has_concurrent_context + + +let should_report_on_class (classname : Typ.Name.t) class_summaries = + match classname with + | JavaClass _ -> + true + | CppClass _ | ObjcClass _ | ObjcProtocol _ | CStruct _ -> + class_has_concurrent_method class_summaries + | CUnion _ -> + false + + +let filter_reportable_classes class_map = Typ.Name.Map.filter should_report_on_class class_map + (* aggregate all of the procedures in the file env by their declaring class. this lets us analyze each class individually *) let aggregate_by_class exe_env procedures = @@ -1123,6 +1136,7 @@ let aggregate_by_class exe_env procedures = | None -> Some [summary] | Some summaries -> Some (summary :: summaries) ) acc ) ) |> Option.value ~default:acc ) + |> filter_reportable_classes (* Gathers results by analyzing all the methods in a file, then diff --git a/infer/tests/codetoanalyze/cpp/racerd/issues.exp b/infer/tests/codetoanalyze/cpp/racerd/issues.exp index f11570a49..b8dc983e4 100644 --- a/infer/tests/codetoanalyze/cpp/racerd/issues.exp +++ b/infer/tests/codetoanalyze/cpp/racerd/issues.exp @@ -24,3 +24,4 @@ codetoanalyze/cpp/racerd/unique_lock.cpp, basics::UniqueLock::get4, 55, LOCK_CON codetoanalyze/cpp/racerd/unique_lock.cpp, basics::UniqueLock::get4, 56, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,access to `this->suspiciously_read2`,,access to `this->suspiciously_read2`] codetoanalyze/cpp/racerd/unique_lock.cpp, basics::UniqueLock::get5, 64, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,access to `this->suspiciously_read1`,,access to `this->suspiciously_read1`] codetoanalyze/cpp/racerd/unique_lock.cpp, basics::UniqueLock::get6, 73, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,access to `this->suspiciously_read1`,,access to `this->suspiciously_read1`] +codetoanalyze/cpp/racerd/without_mutex.cpp, without_mutex::WithoutMutex::get, 15, LOCK_CONSISTENCY_VIOLATION, no_bucket, WARNING, [,access to `this->field_1`,,access to `this->field_1`] diff --git a/infer/tests/codetoanalyze/cpp/racerd/without_mutex.cpp b/infer/tests/codetoanalyze/cpp/racerd/without_mutex.cpp new file mode 100644 index 000000000..a3a186d85 --- /dev/null +++ b/infer/tests/codetoanalyze/cpp/racerd/without_mutex.cpp @@ -0,0 +1,25 @@ +/* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ +#include + +namespace without_mutex { + +class WithoutMutex { + public: + WithoutMutex() {} + + int get() { return field_1; } + + int set(std::mutex& mutex, int data) { + std::lock_guard lock(mutex); + field_1 = data; + } + + private: + int field_1; +}; +} // namespace without_mutex