From 4ee4ebb42aa3dced9e191cdf05742ec774ffce18 Mon Sep 17 00:00:00 2001 From: Sam Blackshear Date: Wed, 1 Nov 2017 09:26:25 -0700 Subject: [PATCH] [hil] don't move any accesses across lock acquisition/release Summary: More general version of the fix in D6138749. This diff moves RacerD's lock modeling into a separate module and uses the module in the HIL translation to check when a function has lock/unlock semantics. Reviewed By: jberdine, da319 Differential Revision: D6191886 fbshipit-source-id: 6e1fdc3 --- infer/src/absint/LowerHil.ml | 14 +- infer/src/concurrency/RacerD.ml | 194 +----------------- infer/src/concurrency/RacerDConfig.ml | 186 +++++++++++++++++ infer/src/concurrency/RacerDConfig.mli | 22 ++ .../java/racerd/Constructors.java | 23 ++- .../codetoanalyze/java/racerd/Locks.java | 18 ++ .../codetoanalyze/java/racerd/issues.exp | 6 +- 7 files changed, 269 insertions(+), 194 deletions(-) diff --git a/infer/src/absint/LowerHil.ml b/infer/src/absint/LowerHil.ml index 605d83a77..7a21854b2 100644 --- a/infer/src/absint/LowerHil.ml +++ b/infer/src/absint/LowerHil.ml @@ -39,6 +39,12 @@ struct NodePrinter.finish_session underyling_node + let is_java_unlock pname actuals = + (* would check is_java, but we want to include builtins too *) + not (Typ.Procname.is_c_method pname) + && match RacerDConfig.Models.get_lock pname actuals with Unlock -> true | _ -> false + + let exec_instr ((actual_state, id_map) as astate) extras node instr = let f_resolve_id id = try Some (IdAccessPathMapDomain.find id id_map) @@ -55,9 +61,10 @@ struct List.fold ~f:(fun acc id -> IdAccessPathMapDomain.remove id acc) ~init:id_map ids in if phys_equal id_map id_map' then astate else (actual_state, id_map') - | Instr (Call (_, Direct callee_pname, _, _, loc) as hil_instr) - when Typ.Procname.equal callee_pname BuiltinDecl.__delete_locked_attribute -> - (* need to be careful not to move reads/writes out of a critical section. to ensure this, + | Instr (Call (_, Direct callee_pname, actuals, _, loc) as hil_instr) + when is_java_unlock callee_pname actuals -> + (* need to be careful not to move reads/writes out of a critical section due to odd + temporaries introduced in our translation of try/synchronized in Java. to ensure this, "dump" all of the temporaries out of the id map, then execute the unlock instruction. *) let actual_state' = IdAccessPathMapDomain.fold @@ -91,4 +98,5 @@ struct let compute_post proc_data ~initial = let initial' = (initial, IdAccessPathMapDomain.empty) in Option.map ~f:fst (Interpreter.compute_post ~debug:false proc_data ~initial:initial') + end diff --git a/infer/src/concurrency/RacerD.ml b/infer/src/concurrency/RacerD.ml index 95191ca42..9578d51f3 100644 --- a/infer/src/concurrency/RacerD.ml +++ b/infer/src/concurrency/RacerD.ml @@ -28,190 +28,6 @@ module TransferFunctions (CFG : ProcCfg.S) = struct type extras = Typ.Procname.t -> Procdesc.t option - type lock_model = Lock | Unlock | LockedIfTrue | NoEffect - - type thread_model = BackgroundThread | MainThread | MainThreadIfTrue | UnknownThread - - type container_access_model = ContainerRead | ContainerWrite - - let is_thread_utils_type java_pname = - let pn = Typ.Procname.java_get_class_name java_pname in - String.is_suffix ~suffix:"ThreadUtils" pn || String.is_suffix ~suffix:"ThreadUtil" pn - - - let is_thread_utils_method method_name_str = function - | Typ.Procname.Java java_pname -> - is_thread_utils_type java_pname - && String.equal (Typ.Procname.java_get_method java_pname) method_name_str - | _ -> - false - - - let get_lock_model = - let is_cpp_lock = - let matcher_lock = - QualifiedCppName.Match.of_fuzzy_qual_names - [ "apache::thrift::concurrency::ReadWriteMutex::acquireRead" - ; "apache::thrift::concurrency::ReadWriteMutex::acquireWrite" - ; "folly::MicroSpinLock::lock" - ; "folly::RWSpinLock::lock" - ; "folly::RWSpinLock::lock_shared" - ; "folly::SharedMutexImpl::lockExclusiveImpl" - ; "folly::SharedMutexImpl::lockSharedImpl" - ; "std::mutex::lock" - ; "std::unique_lock::lock" ] - in - let matcher_lock_constructor = - QualifiedCppName.Match.of_fuzzy_qual_names - ["std::lock_guard::lock_guard"; "std::unique_lock::unique_lock"] - in - fun pname actuals -> - QualifiedCppName.Match.match_qualifiers matcher_lock (Typ.Procname.get_qualifiers pname) - || QualifiedCppName.Match.match_qualifiers matcher_lock_constructor - (Typ.Procname.get_qualifiers pname) - (* Passing additional parameter allows to defer the lock *) - && Int.equal 2 (List.length actuals) - and is_cpp_unlock = - let matcher = - QualifiedCppName.Match.of_fuzzy_qual_names - [ "apache::thrift::concurrency::ReadWriteMutex::release" - ; "folly::MicroSpinLock::unlock" - ; "folly::RWSpinLock::unlock" - ; "folly::RWSpinLock::unlock_shared" - ; "folly::SharedMutexImpl::unlock" - ; "folly::SharedMutexImpl::unlock_shared" - ; "std::lock_guard::~lock_guard" - ; "std::mutex::unlock" - ; "std::unique_lock::unlock" - ; "std::unique_lock::~unique_lock" ] - in - fun pname -> - QualifiedCppName.Match.match_qualifiers matcher (Typ.Procname.get_qualifiers pname) - and is_cpp_trylock = - let matcher = - QualifiedCppName.Match.of_fuzzy_qual_names - ["std::unique_lock::owns_lock"; "std::unique_lock::try_lock"] - in - fun pname -> - QualifiedCppName.Match.match_qualifiers matcher (Typ.Procname.get_qualifiers pname) - in - fun pname actuals -> - match pname with - | Typ.Procname.Java java_pname - -> ( - if is_thread_utils_method "assertHoldsLock" (Typ.Procname.Java java_pname) then Lock - else - match - (Typ.Procname.java_get_class_name java_pname, Typ.Procname.java_get_method java_pname) - with - | ( ( "java.util.concurrent.locks.Lock" | "java.util.concurrent.locks.ReentrantLock" - | "java.util.concurrent.locks.ReentrantReadWriteLock$ReadLock" - | "java.util.concurrent.locks.ReentrantReadWriteLock$WriteLock" ) - , ("lock" | "lockInterruptibly") ) -> - Lock - | ( ( "java.util.concurrent.locks.Lock" | "java.util.concurrent.locks.ReentrantLock" - | "java.util.concurrent.locks.ReentrantReadWriteLock$ReadLock" - | "java.util.concurrent.locks.ReentrantReadWriteLock$WriteLock" ) - , "unlock" ) -> - Unlock - | ( ( "java.util.concurrent.locks.Lock" | "java.util.concurrent.locks.ReentrantLock" - | "java.util.concurrent.locks.ReentrantReadWriteLock$ReadLock" - | "java.util.concurrent.locks.ReentrantReadWriteLock$WriteLock" ) - , "tryLock" ) -> - LockedIfTrue - | ( "com.facebook.buck.util.concurrent.AutoCloseableReadWriteUpdateLock" - , ("readLock" | "updateLock" | "writeLock") ) -> - Lock - | _ -> - NoEffect ) - | Typ.Procname.ObjC_Cpp _ as pname when is_cpp_lock pname actuals -> - Lock - | Typ.Procname.ObjC_Cpp _ as pname when is_cpp_unlock pname -> - Unlock - | Typ.Procname.ObjC_Cpp _ as pname when is_cpp_trylock pname -> - LockedIfTrue - | pname when Typ.Procname.equal pname BuiltinDecl.__set_locked_attribute -> - Lock - | pname when Typ.Procname.equal pname BuiltinDecl.__delete_locked_attribute -> - Unlock - | _ -> - NoEffect - - - let get_thread_model = function - | Typ.Procname.Java java_pname when is_thread_utils_type java_pname -> ( - match Typ.Procname.java_get_method java_pname with - | "assertMainThread" | "assertOnUiThread" | "checkOnMainThread" -> - MainThread - | "isMainThread" | "isUiThread" -> - MainThreadIfTrue - | "assertOnBackgroundThread" | "assertOnNonUiThread" | "checkOnNonUiThread" -> - BackgroundThread - | _ -> - UnknownThread ) - | _ -> - UnknownThread - - - let get_container_access = - let is_cpp_container_read = - let matcher = QualifiedCppName.Match.of_fuzzy_qual_names ["std::map::find"] in - fun pname -> - QualifiedCppName.Match.match_qualifiers matcher (Typ.Procname.get_qualifiers pname) - and is_cpp_container_write = - let matcher = - QualifiedCppName.Match.of_fuzzy_qual_names ["std::map::operator[]"; "std::map::erase"] - in - fun pname -> - QualifiedCppName.Match.match_qualifiers matcher (Typ.Procname.get_qualifiers pname) - in - fun pn tenv -> - match pn with - | Typ.Procname.Java java_pname -> - let typename = Typ.Name.Java.from_string (Typ.Procname.java_get_class_name java_pname) in - let get_container_access_ typename = - match (Typ.Name.name typename, Typ.Procname.java_get_method java_pname) with - | ( ("android.util.SparseArray" | "android.support.v4.util.SparseArrayCompat") - , ( "append" | "clear" | "delete" | "put" | "remove" | "removeAt" | "removeAtRange" - | "setValueAt" ) ) -> - Some ContainerWrite - | ( ("android.util.SparseArray" | "android.support.v4.util.SparseArrayCompat") - , ("clone" | "get" | "indexOfKey" | "indexOfValue" | "keyAt" | "size" | "valueAt") ) -> - Some ContainerRead - | ( "android.support.v4.util.SimpleArrayMap" - , ( "clear" | "ensureCapacity" | "put" | "putAll" | "remove" | "removeAt" - | "setValueAt" ) ) -> - Some ContainerWrite - | ( "android.support.v4.util.SimpleArrayMap" - , ( "containsKey" | "containsValue" | "get" | "hashCode" | "indexOfKey" | "isEmpty" - | "keyAt" | "size" | "valueAt" ) ) -> - Some ContainerRead - | "android.support.v4.util.Pools$SimplePool", ("acquire" | "release") -> - Some ContainerWrite - | "java.util.List", ("add" | "addAll" | "clear" | "remove" | "set") -> - Some ContainerWrite - | ( "java.util.List" - , ( "contains" | "containsAll" | "equals" | "get" | "hashCode" | "indexOf" - | "isEmpty" | "iterator" | "lastIndexOf" | "listIterator" | "size" | "toArray" ) ) -> - Some ContainerRead - | "java.util.Map", ("clear" | "put" | "putAll" | "remove") -> - Some ContainerWrite - | ( "java.util.Map" - , ( "containsKey" | "containsValue" | "entrySet" | "equals" | "get" | "hashCode" - | "isEmpty" | "keySet" | "size" | "values" ) ) -> - Some ContainerRead - | _ -> - None - in - PatternMatch.supertype_find_map_opt tenv get_container_access_ typename - | Typ.Procname.ObjC_Cpp _ as pname when is_cpp_container_read pname -> - Some ContainerRead - | Typ.Procname.ObjC_Cpp _ as pname when is_cpp_container_write pname -> - Some ContainerWrite - | _ -> - None - - (* propagate attributes from the leaves to the root of an RHS Hil expression *) let rec attributes_of_expr attribute_map e = let open HilExp in @@ -567,6 +383,7 @@ module TransferFunctions (CFG : ProcCfg.S) = struct let get_summary caller_pdesc callee_pname actuals callee_loc tenv = + let open RacerDConfig in let get_receiver_ap actuals = match List.hd actuals with | Some HilExp.AccessPath receiver_ap -> @@ -576,7 +393,7 @@ module TransferFunctions (CFG : ProcCfg.S) = struct "Call to %a is marked as a container write, but has no receiver" Typ.Procname.pp callee_pname in - match (get_container_access callee_pname tenv, callee_pname) with + match (Models.get_container_access callee_pname tenv, callee_pname) with | Some ContainerWrite, _ -> make_container_access callee_pname ~is_write:true (get_receiver_ap actuals) callee_loc tenv | Some ContainerRead, _ -> @@ -655,6 +472,7 @@ module TransferFunctions (CFG : ProcCfg.S) = struct let exec_instr (astate: Domain.astate) ({ProcData.tenv; extras; pdesc} as proc_data) _ (instr: HilInstr.t) = let open Domain in + let open RacerDConfig in match instr with | Call (Some ret_base, Direct procname, actuals, _, loc) when acquires_ownership procname tenv -> let accesses = @@ -677,7 +495,7 @@ module TransferFunctions (CFG : ProcCfg.S) = struct in let astate = {astate with accesses} in let astate = - match get_thread_model callee_pname with + match Models.get_thread callee_pname with | BackgroundThread -> {astate with threads= ThreadsDomain.AnyThread} | MainThread -> @@ -699,7 +517,7 @@ module TransferFunctions (CFG : ProcCfg.S) = struct in let astate_callee = (* assuming that modeled procedures do not have useful summaries *) - if is_thread_utils_method "assertMainThread" callee_pname then + if Models.is_thread_utils_method "assertMainThread" callee_pname then {astate with threads= ThreadsDomain.AnyThreadButSelf} else (* if we don't have any evidence about whether the current function can run in parallel @@ -711,7 +529,7 @@ module TransferFunctions (CFG : ProcCfg.S) = struct | _ -> ThreadsDomain.AnyThread in - match get_lock_model callee_pname actuals with + match Models.get_lock callee_pname actuals with | Lock -> {astate with locks= true; threads= update_for_lock_use astate.threads} | Unlock -> diff --git a/infer/src/concurrency/RacerDConfig.ml b/infer/src/concurrency/RacerDConfig.ml index 039e959ae..3cd5629e2 100644 --- a/infer/src/concurrency/RacerDConfig.ml +++ b/infer/src/concurrency/RacerDConfig.ml @@ -20,3 +20,189 @@ module AnnotationAliases = struct "Couldn't parse thread-safety annotation aliases; expected list of strings" end + +module Models = struct + type lock = Lock | Unlock | LockedIfTrue | NoEffect + + type thread = BackgroundThread | MainThread | MainThreadIfTrue | UnknownThread + + type container_access = ContainerRead | ContainerWrite + + let is_thread_utils_type java_pname = + let pn = Typ.Procname.java_get_class_name java_pname in + String.is_suffix ~suffix:"ThreadUtils" pn || String.is_suffix ~suffix:"ThreadUtil" pn + + + let is_thread_utils_method method_name_str = function + | Typ.Procname.Java java_pname -> + is_thread_utils_type java_pname + && String.equal (Typ.Procname.java_get_method java_pname) method_name_str + | _ -> + false + + + let get_thread = function + | Typ.Procname.Java java_pname when is_thread_utils_type java_pname -> ( + match Typ.Procname.java_get_method java_pname with + | "assertMainThread" | "assertOnUiThread" | "checkOnMainThread" -> + MainThread + | "isMainThread" | "isUiThread" -> + MainThreadIfTrue + | "assertOnBackgroundThread" | "assertOnNonUiThread" | "checkOnNonUiThread" -> + BackgroundThread + | _ -> + UnknownThread ) + | _ -> + UnknownThread + + + let get_lock = + let is_cpp_lock = + let matcher_lock = + QualifiedCppName.Match.of_fuzzy_qual_names + [ "apache::thrift::concurrency::ReadWriteMutex::acquireRead" + ; "apache::thrift::concurrency::ReadWriteMutex::acquireWrite" + ; "folly::MicroSpinLock::lock" + ; "folly::RWSpinLock::lock" + ; "folly::RWSpinLock::lock_shared" + ; "folly::SharedMutexImpl::lockExclusiveImpl" + ; "folly::SharedMutexImpl::lockSharedImpl" + ; "std::mutex::lock" + ; "std::unique_lock::lock" ] + in + let matcher_lock_constructor = + QualifiedCppName.Match.of_fuzzy_qual_names + ["std::lock_guard::lock_guard"; "std::unique_lock::unique_lock"] + in + fun pname actuals -> + QualifiedCppName.Match.match_qualifiers matcher_lock (Typ.Procname.get_qualifiers pname) + || QualifiedCppName.Match.match_qualifiers matcher_lock_constructor + (Typ.Procname.get_qualifiers pname) + (* Passing additional parameter allows to defer the lock *) + && Int.equal 2 (List.length actuals) + and is_cpp_unlock = + let matcher = + QualifiedCppName.Match.of_fuzzy_qual_names + [ "apache::thrift::concurrency::ReadWriteMutex::release" + ; "folly::MicroSpinLock::unlock" + ; "folly::RWSpinLock::unlock" + ; "folly::RWSpinLock::unlock_shared" + ; "folly::SharedMutexImpl::unlock" + ; "folly::SharedMutexImpl::unlock_shared" + ; "std::lock_guard::~lock_guard" + ; "std::mutex::unlock" + ; "std::unique_lock::unlock" + ; "std::unique_lock::~unique_lock" ] + in + fun pname -> + QualifiedCppName.Match.match_qualifiers matcher (Typ.Procname.get_qualifiers pname) + and is_cpp_trylock = + let matcher = + QualifiedCppName.Match.of_fuzzy_qual_names + ["std::unique_lock::owns_lock"; "std::unique_lock::try_lock"] + in + fun pname -> + QualifiedCppName.Match.match_qualifiers matcher (Typ.Procname.get_qualifiers pname) + in + fun pname actuals -> + match pname with + | Typ.Procname.Java java_pname + -> ( + if is_thread_utils_method "assertHoldsLock" (Typ.Procname.Java java_pname) then Lock + else + match + (Typ.Procname.java_get_class_name java_pname, Typ.Procname.java_get_method java_pname) + with + | ( ( "java.util.concurrent.locks.Lock" | "java.util.concurrent.locks.ReentrantLock" + | "java.util.concurrent.locks.ReentrantReadWriteLock$ReadLock" + | "java.util.concurrent.locks.ReentrantReadWriteLock$WriteLock" ) + , ("lock" | "lockInterruptibly") ) -> + Lock + | ( ( "java.util.concurrent.locks.Lock" | "java.util.concurrent.locks.ReentrantLock" + | "java.util.concurrent.locks.ReentrantReadWriteLock$ReadLock" + | "java.util.concurrent.locks.ReentrantReadWriteLock$WriteLock" ) + , "unlock" ) -> + Unlock + | ( ( "java.util.concurrent.locks.Lock" | "java.util.concurrent.locks.ReentrantLock" + | "java.util.concurrent.locks.ReentrantReadWriteLock$ReadLock" + | "java.util.concurrent.locks.ReentrantReadWriteLock$WriteLock" ) + , "tryLock" ) -> + LockedIfTrue + | ( "com.facebook.buck.util.concurrent.AutoCloseableReadWriteUpdateLock" + , ("readLock" | "updateLock" | "writeLock") ) -> + Lock + | _ -> + NoEffect ) + | Typ.Procname.ObjC_Cpp _ as pname when is_cpp_lock pname actuals -> + Lock + | Typ.Procname.ObjC_Cpp _ as pname when is_cpp_unlock pname -> + Unlock + | Typ.Procname.ObjC_Cpp _ as pname when is_cpp_trylock pname -> + LockedIfTrue + | pname when Typ.Procname.equal pname BuiltinDecl.__set_locked_attribute -> + Lock + | pname when Typ.Procname.equal pname BuiltinDecl.__delete_locked_attribute -> + Unlock + | _ -> + NoEffect + + + let get_container_access = + let is_cpp_container_read = + let matcher = QualifiedCppName.Match.of_fuzzy_qual_names ["std::map::find"] in + fun pname -> + QualifiedCppName.Match.match_qualifiers matcher (Typ.Procname.get_qualifiers pname) + and is_cpp_container_write = + let matcher = + QualifiedCppName.Match.of_fuzzy_qual_names ["std::map::operator[]"; "std::map::erase"] + in + fun pname -> + QualifiedCppName.Match.match_qualifiers matcher (Typ.Procname.get_qualifiers pname) + in + fun pn tenv -> + match pn with + | Typ.Procname.Java java_pname -> + let typename = Typ.Name.Java.from_string (Typ.Procname.java_get_class_name java_pname) in + let get_container_access_ typename = + match (Typ.Name.name typename, Typ.Procname.java_get_method java_pname) with + | ( ("android.util.SparseArray" | "android.support.v4.util.SparseArrayCompat") + , ( "append" | "clear" | "delete" | "put" | "remove" | "removeAt" | "removeAtRange" + | "setValueAt" ) ) -> + Some ContainerWrite + | ( ("android.util.SparseArray" | "android.support.v4.util.SparseArrayCompat") + , ("clone" | "get" | "indexOfKey" | "indexOfValue" | "keyAt" | "size" | "valueAt") ) -> + Some ContainerRead + | ( "android.support.v4.util.SimpleArrayMap" + , ( "clear" | "ensureCapacity" | "put" | "putAll" | "remove" | "removeAt" + | "setValueAt" ) ) -> + Some ContainerWrite + | ( "android.support.v4.util.SimpleArrayMap" + , ( "containsKey" | "containsValue" | "get" | "hashCode" | "indexOfKey" | "isEmpty" + | "keyAt" | "size" | "valueAt" ) ) -> + Some ContainerRead + | "android.support.v4.util.Pools$SimplePool", ("acquire" | "release") -> + Some ContainerWrite + | "java.util.List", ("add" | "addAll" | "clear" | "remove" | "set") -> + Some ContainerWrite + | ( "java.util.List" + , ( "contains" | "containsAll" | "equals" | "get" | "hashCode" | "indexOf" + | "isEmpty" | "iterator" | "lastIndexOf" | "listIterator" | "size" | "toArray" ) ) -> + Some ContainerRead + | "java.util.Map", ("clear" | "put" | "putAll" | "remove") -> + Some ContainerWrite + | ( "java.util.Map" + , ( "containsKey" | "containsValue" | "entrySet" | "equals" | "get" | "hashCode" + | "isEmpty" | "keySet" | "size" | "values" ) ) -> + Some ContainerRead + | _ -> + None + in + PatternMatch.supertype_find_map_opt tenv get_container_access_ typename + | Typ.Procname.ObjC_Cpp _ as pname when is_cpp_container_read pname -> + Some ContainerRead + | Typ.Procname.ObjC_Cpp _ as pname when is_cpp_container_write pname -> + Some ContainerWrite + | _ -> + None + +end diff --git a/infer/src/concurrency/RacerDConfig.mli b/infer/src/concurrency/RacerDConfig.mli index 8555005b3..8abceb27c 100644 --- a/infer/src/concurrency/RacerDConfig.mli +++ b/infer/src/concurrency/RacerDConfig.mli @@ -14,3 +14,25 @@ module F = Format module AnnotationAliases : sig val of_json : Yojson.Basic.json -> string list end + +module Models : sig + type lock = Lock | Unlock | LockedIfTrue | NoEffect + + type thread = BackgroundThread | MainThread | MainThreadIfTrue | UnknownThread + + type container_access = ContainerRead | ContainerWrite + + (* TODO: clean this up so it takes only a procname *) + + val is_thread_utils_method : string -> Typ.Procname.t -> bool + (** return true if the given method name is a utility class for checking what thread we're on *) + + val get_lock : Typ.Procname.t -> HilExp.t list -> lock + (** describe how this procedure behaves with respect to locking *) + + val get_thread : Typ.Procname.t -> thread + (** describe how this procedure behaves with respect to thread access *) + + val get_container_access : Typ.Procname.t -> Tenv.t -> container_access option + (** return Some (access) if this procedure accesses the contents of a container (e.g., Map.get) *) +end diff --git a/infer/tests/codetoanalyze/java/racerd/Constructors.java b/infer/tests/codetoanalyze/java/racerd/Constructors.java index a0cb55129..e3876b656 100644 --- a/infer/tests/codetoanalyze/java/racerd/Constructors.java +++ b/infer/tests/codetoanalyze/java/racerd/Constructors.java @@ -38,15 +38,34 @@ public class Constructors { this.field = 7; } - public static synchronized Constructors singletonOk() { + public static synchronized Constructors singleton1Ok() { // ok because lock is held during write to static field in constructor return new Constructors(new Object()); } - public static Constructors singletonBad() { + private static Constructors sSingleton1; + + public static Constructors FP_singleton2Ok() { + synchronized (Constructors.class) { + if (sSingleton1 != null) { + sSingleton1 = new Constructors(0); + } + } + return sSingleton1; // not currently smart enough to understand that this read is ok + } + + public static Constructors singleton1Bad() { // not ok because no lock is held return new Constructors(new Object()); } + private static Constructors sSingleton2; + + public static Constructors singleton2Bad() { + if (sSingleton2 == null) { + sSingleton2 = new Constructors(0); + } + return sSingleton2; + } } diff --git a/infer/tests/codetoanalyze/java/racerd/Locks.java b/infer/tests/codetoanalyze/java/racerd/Locks.java index dfa5b6960..43edc0e5f 100644 --- a/infer/tests/codetoanalyze/java/racerd/Locks.java +++ b/infer/tests/codetoanalyze/java/racerd/Locks.java @@ -215,4 +215,22 @@ public class Locks { return tmp; } + public boolean readInTryCatchWithLockOk() { + mLock.lock(); + try { + return mField; + } finally { + mLock.unlock(); + } + } + + public void writeInsideTryCatchWithLockOk() { + mLock.lock(); + try { + mField = true; + } finally { + mLock.unlock(); + } + } + } diff --git a/infer/tests/codetoanalyze/java/racerd/issues.exp b/infer/tests/codetoanalyze/java/racerd/issues.exp index 80ad72e0b..4415c50b6 100644 --- a/infer/tests/codetoanalyze/java/racerd/issues.exp +++ b/infer/tests/codetoanalyze/java/racerd/issues.exp @@ -21,7 +21,11 @@ codetoanalyze/java/racerd/Builders.java, Builders$Obj Builders.buildThenMutateBa codetoanalyze/java/racerd/Builders.java, Builders$Obj Builders.buildThenMutateBad(Builders$Obj), 3, THREAD_SAFETY_VIOLATION, [access to `&input.codetoanalyze.java.checkers.Builders$Obj.g`] codetoanalyze/java/racerd/Builders.java, Builders$Obj Builders.mutateBad(Builders$Obj), 1, THREAD_SAFETY_VIOLATION, [access to `&o.codetoanalyze.java.checkers.Builders$Obj.g`] codetoanalyze/java/racerd/Builders.java, void TopLevelBuilder.setG(String), 1, THREAD_SAFETY_VIOLATION, [access to `&this.codetoanalyze.java.checkers.TopLevelBuilder.g`] -codetoanalyze/java/racerd/Constructors.java, Constructors Constructors.singletonBad(), 2, THREAD_SAFETY_VIOLATION, [call to Constructors.(Object),access to `&#GB$Constructors.Constructors.staticField`] +codetoanalyze/java/racerd/Constructors.java, Constructors Constructors.FP_singleton2Ok(), 6, THREAD_SAFETY_VIOLATION, [,access to `&#GB$Constructors.Constructors.sSingleton1`,,access to `&#GB$Constructors.Constructors.sSingleton1`] +codetoanalyze/java/racerd/Constructors.java, Constructors Constructors.singleton1Bad(), 2, THREAD_SAFETY_VIOLATION, [call to Constructors.(Object),access to `&#GB$Constructors.Constructors.staticField`] +codetoanalyze/java/racerd/Constructors.java, Constructors Constructors.singleton2Bad(), 1, THREAD_SAFETY_VIOLATION, [,access to `&#GB$Constructors.Constructors.sSingleton2`,,access to `&#GB$Constructors.Constructors.sSingleton2`] +codetoanalyze/java/racerd/Constructors.java, Constructors Constructors.singleton2Bad(), 2, THREAD_SAFETY_VIOLATION, [access to `&#GB$Constructors.Constructors.sSingleton2`] +codetoanalyze/java/racerd/Constructors.java, Constructors Constructors.singleton2Bad(), 4, THREAD_SAFETY_VIOLATION, [,access to `&#GB$Constructors.Constructors.sSingleton2`,,access to `&#GB$Constructors.Constructors.sSingleton2`] codetoanalyze/java/racerd/Constructors.java, Constructors.(), 1, THREAD_SAFETY_VIOLATION, [access to `&#GB$Constructors.Constructors.staticField`] codetoanalyze/java/racerd/Constructors.java, Constructors.(Constructors), 1, THREAD_SAFETY_VIOLATION, [access to `&o.Constructors.field`] codetoanalyze/java/racerd/Containers.java, boolean Containers.listReadBad(String), 1, THREAD_SAFETY_VIOLATION, [,Read of container `&this.codetoanalyze.java.checkers.Containers.mList` via call to `contains`,,Write to container `&this.codetoanalyze.java.checkers.Containers.mList` via call to `set`]