diff --git a/infer/src/checkers/ThreadSafety.ml b/infer/src/checkers/ThreadSafety.ml index 91db797b8..01d70a5f0 100644 --- a/infer/src/checkers/ThreadSafety.ml +++ b/infer/src/checkers/ThreadSafety.ml @@ -1223,17 +1223,29 @@ let report_unsafe_accesses ~is_file_threadsafe aggregated_access_map = List.exists ~f:(fun (_, _, _, tenv, pdesc) -> is_thread_safe_method pdesc tenv) grouped_accesses in - (* report if - - the method/class of the access is thread-safe (or an override or superclass is), or - - any access is in a field marked thread-safe (or an override) *) + let class_has_mutex_member objc_cpp tenv = + let class_name = Typ.Procname.objc_cpp_get_class_type_name objc_cpp in + let matcher = QualifiedCppName.Match.of_fuzzy_qual_names ["std::mutex"] in + Option.exists (Tenv.lookup tenv class_name) ~f:(fun class_str -> + (* check if the class contains a member of type std::mutex *) + List.exists (class_str.Typ.Struct.fields) ~f:(fun (_, ft, _) -> + Option.exists (Typ.name ft) ~f:(fun name -> + QualifiedCppName.Match.match_qualifiers matcher (Typ.Name.qual_name name)) + ) + ) in let should_report pdesc tenv = match Procdesc.get_proc_name pdesc with | Java _ -> + (* report if + - the method/class of the access is thread-safe + (or an override or superclass is), or + - any access is in a field marked thread-safe (or an override) *) ((is_file_threadsafe || accessed_by_threadsafe_method) && should_report_on_proc pdesc tenv) || is_thread_safe_method pdesc tenv - | ObjC_Cpp _ -> - true + | ObjC_Cpp objc_cpp -> + (* report if the class has a mutex member *) + class_has_mutex_member objc_cpp tenv | _ -> false in diff --git a/infer/tests/codetoanalyze/cpp/threadsafety/basics_no_mutex.cpp b/infer/tests/codetoanalyze/cpp/threadsafety/basics_no_mutex.cpp new file mode 100644 index 000000000..5e3192847 --- /dev/null +++ b/infer/tests/codetoanalyze/cpp/threadsafety/basics_no_mutex.cpp @@ -0,0 +1,31 @@ +/* + * Copyright (c) 2017 - present Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + */ + +namespace basics { + +class BasicsNoMutex { + public: + BasicsNoMutex() {} + + void set(int new_value) { + field_1 = new_value; + field_2 = new_value; + field_3 = new_value; + } + + int get1() { return field_1; } + + int get2() { return field_2; } + + private: + int field_1; + int field_2; + int field_3; +}; +} diff --git a/infer/tests/codetoanalyze/cpp/threadsafety/basics_with_mutex.cpp b/infer/tests/codetoanalyze/cpp/threadsafety/basics_with_mutex.cpp new file mode 100644 index 000000000..8ac513e97 --- /dev/null +++ b/infer/tests/codetoanalyze/cpp/threadsafety/basics_with_mutex.cpp @@ -0,0 +1,32 @@ +/* + * Copyright (c) 2017 - present Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + */ + +#include + +namespace basics { + +class BasicsWithHeader { + public: + BasicsWithHeader(){}; + + void set(int new_value) { + field_1 = new_value; + field_2 = new_value; + field_3 = new_value; + }; + int get1() { return field_1; }; + int get2() { return field_2; }; + + private: + int field_1; + int field_2; + int field_3; + std::mutex mutex_; +}; +} diff --git a/infer/tests/codetoanalyze/cpp/threadsafety/issues.exp b/infer/tests/codetoanalyze/cpp/threadsafety/issues.exp index 82060295f..3fb48c686 100644 --- a/infer/tests/codetoanalyze/cpp/threadsafety/issues.exp +++ b/infer/tests/codetoanalyze/cpp/threadsafety/issues.exp @@ -1,3 +1,5 @@ codetoanalyze/cpp/threadsafety/basics.cpp, basics::Basic_get2, 3, THREAD_SAFETY_VIOLATION, [access to `suspiciously_written`] codetoanalyze/cpp/threadsafety/basics.cpp, basics::Basic_get3, 0, THREAD_SAFETY_VIOLATION, [,access to `not_guarded`,,access to `not_guarded`] codetoanalyze/cpp/threadsafety/basics.cpp, basics::Basic_get4, 0, THREAD_SAFETY_VIOLATION, [,access to `suspiciously_read`,,access to `suspiciously_read`] +codetoanalyze/cpp/threadsafety/basics_with_mutex.cpp, basics::BasicsWithHeader_get1, 0, THREAD_SAFETY_VIOLATION, [,access to `field_1`,,access to `field_1`] +codetoanalyze/cpp/threadsafety/basics_with_mutex.cpp, basics::BasicsWithHeader_get2, 0, THREAD_SAFETY_VIOLATION, [,access to `field_2`,,access to `field_2`]