diff --git a/infer/src/concurrency/RacerD.ml b/infer/src/concurrency/RacerD.ml index eead84ed0..8fd3c46d8 100644 --- a/infer/src/concurrency/RacerD.ml +++ b/infer/src/concurrency/RacerD.ml @@ -165,7 +165,19 @@ module TransferFunctions (CFG : ProcCfg.S) = struct || (* non-static [$Builder] methods with same return type as receiver type *) is_builder_passthrough callee_pname in - if is_box callee_pname then + if RacerDModels.is_synchronized_container_constructor tenv callee_pname actuals then + List.hd actuals |> Option.bind ~f:get_access_exp + |> Option.value_map ~default:astate ~f:(fun receiver -> + let attribute_map = + AttributeMapDomain.add receiver Synchronized astate.attribute_map + in + {astate with attribute_map} ) + else if RacerDModels.is_converter_to_synchronized_container tenv callee_pname actuals then + let attribute_map = + AttributeMapDomain.add (AccessExpression.base ret_base) Synchronized astate.attribute_map + in + {astate with attribute_map} + else if is_box callee_pname then match actuals with | HilExp.AccessExpression actual_access_expr :: _ when AttributeMapDomain.is_functional astate.attribute_map actual_access_expr -> @@ -191,13 +203,6 @@ module TransferFunctions (CFG : ProcCfg.S) = struct astate.ownership in {astate with ownership} - else if RacerDModels.is_synchronized_container_constructor tenv callee_pname actuals then - List.hd actuals |> Option.bind ~f:get_access_exp - |> Option.value_map ~default:astate ~f:(fun receiver -> - let attribute_map = - AttributeMapDomain.add receiver Synchronized astate.attribute_map - in - {astate with attribute_map} ) else astate diff --git a/infer/src/concurrency/RacerDModels.ml b/infer/src/concurrency/RacerDModels.ml index 6566e8387..8cb9a8c83 100644 --- a/infer/src/concurrency/RacerDModels.ml +++ b/infer/src/concurrency/RacerDModels.ml @@ -452,6 +452,18 @@ let synchronized_container_classes = ; "java.util.Hashtable" ] +let is_converter_to_synchronized_container = + let open MethodMatcher in + [ (* any method in [java.util.Collections] that starts with [synchronized] is a wrapper that produces a thread-safe container *) + {default with classname= "java.util.Collections"; method_prefix= true; methods= ["synchronized"]} + ; {default with classname= "com.google.common.collect.Maps"; methods= ["newConcurrentMap"]} + ; {default with classname= "com.google.common.collect.Sets"; methods= ["newConcurrentHashSet"]} + ; { default with + classname= "com.google.common.collect.Queues" + ; methods= ["newConcurrentLinkedQueue"] } ] + |> of_records + + let is_synchronized_container_constructor = let open MethodMatcher in let default = {default with methods= [Procname.Java.constructor_method_name]} in diff --git a/infer/src/concurrency/RacerDModels.mli b/infer/src/concurrency/RacerDModels.mli index cb203ac2b..7e76a2698 100644 --- a/infer/src/concurrency/RacerDModels.mli +++ b/infer/src/concurrency/RacerDModels.mli @@ -66,3 +66,6 @@ val is_initializer : Tenv.t -> Procname.t -> bool (** should the given procedure be treated as a constructor/initializer? *) val is_synchronized_container_constructor : Tenv.t -> Procname.t -> HilExp.t list -> bool + +val is_converter_to_synchronized_container : Tenv.t -> Procname.t -> HilExp.t list -> bool +(** is the given [procname] a method that wraps a container into a thread-safe wrapper? *) diff --git a/infer/tests/codetoanalyze/java/racerd/Containers.java b/infer/tests/codetoanalyze/java/racerd/Containers.java index 13dc9f5a6..0a813b146 100644 --- a/infer/tests/codetoanalyze/java/racerd/Containers.java +++ b/infer/tests/codetoanalyze/java/racerd/Containers.java @@ -13,8 +13,10 @@ import android.support.v4.util.Pools.SynchronizedPool; import android.support.v4.util.SimpleArrayMap; import android.support.v4.util.SparseArrayCompat; import android.util.SparseArray; +import com.google.common.collect.Sets; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.HashMap; import java.util.Hashtable; import java.util.List; @@ -355,4 +357,16 @@ class Containers { void dynamicallyTypedConcurrentSetAddOk(String value) { mConcurrentSet.add(value); } + + List mWrappedList = Collections.synchronizedList(new ArrayList()); + + void wrappedListAddOk(String value) { + mWrappedList.add(value); + } + + Set mGoogleSynchronizedSet = Sets.newConcurrentHashSet(); + + void googleSynchronizedSetAddOk(String value) { + mGoogleSynchronizedSet.add(value); + } } diff --git a/infer/tests/codetoanalyze/java/racerd/issues.exp b/infer/tests/codetoanalyze/java/racerd/issues.exp index 1d7bcaba9..a725ffe26 100644 --- a/infer/tests/codetoanalyze/java/racerd/issues.exp +++ b/infer/tests/codetoanalyze/java/racerd/issues.exp @@ -26,30 +26,30 @@ codetoanalyze/java/racerd/Constructors.java, Constructors.FP_singleton2Ok():Cons codetoanalyze/java/racerd/Constructors.java, Constructors.singleton1Bad():Constructors, 57, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [call to Constructors.(Object),access to `Constructors.staticField`] codetoanalyze/java/racerd/Constructors.java, Constructors.singleton2Bad():Constructors, 63, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [,access to `Constructors.sSingleton2`,,access to `Constructors.sSingleton2`] codetoanalyze/java/racerd/Constructors.java, Constructors.singleton2Bad():Constructors, 64, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [access to `Constructors.sSingleton2`] -codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.addToSimpleArrayMapBad(android.support.v4.util.SimpleArrayMap):void, 282, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `map` via call to `put`] -codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.addToSparseArrayBad(android.util.SparseArray):void, 272, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `sparseArray` via call to `put`] -codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.addToSparseArrayCompatBad(android.support.v4.util.SparseArrayCompat):void, 263, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `sparseArray` via call to `put`] -codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.addToUnsynchronizedListBad(java.lang.String):void, 324, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.mSomeList` via call to `add`] -codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.containerWrapperUnownedWriteBad(java.lang.Object):void, 170, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [call to Object ContainerWrapper.write(Object),call to Object ContainerWrapper._write(Object),Write to container `this.mContainerWrapper.children` via call to `add`] -codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.getListSizeBad():int, 330, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [,Read of container `this.mSomeOtherList` via call to `size`,,Write to container `this.mSomeOtherList` via call to `remove`] -codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.getMapSizeBad():int, 340, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [,Read of container `this.mSomeMap` via call to `size`,,Write to container `this.mSomeMap` via call to `remove`] -codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.listAddAllBad(java.util.Collection):void, 58, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.mList` via call to `addAll`] -codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.listAddBad1(java.lang.String):void, 50, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.mList` via call to `add`] -codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.listAddBad2(int,java.lang.String):void, 54, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.mList` via call to `add`] -codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.listClearBad():void, 62, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.mList` via call to `clear`] -codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.listReadBad(java.lang.String):boolean, 101, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [,Read of container `this.mListSyncWrites` via call to `contains`,,Write to container `this.mListSyncWrites` via call to `add`] -codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.listRemoveAllBad(java.util.Collection):void, 74, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.mList` via call to `removeAll`] -codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.listRemoveBad1(int):void, 66, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.mList` via call to `remove`] -codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.listRemoveBad2(java.lang.String):void, 70, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.mList` via call to `remove`] -codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.listSetBad(int,java.lang.String):void, 78, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.mList` via call to `set`] -codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.listSubclassWriteBad(java.util.ArrayList,int):void, 82, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `list` via call to `remove`] -codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.mapClearBad():void, 118, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.mMap` via call to `clear`] -codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.mapPutAllBad(java.util.Map):void, 122, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.mMap` via call to `putAll`] -codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.mapPutBad(java.lang.String,java.lang.String):void, 110, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.mMap` via call to `put`] -codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.mapRemoveBad(java.lang.String):void, 114, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.mMap` via call to `remove`] -codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.mapSubclassWriteBad(java.util.HashMap,java.lang.String):void, 140, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `m` via call to `remove`] -codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.poolBad():void, 301, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.simplePool` via call to `release`] -codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.readSimpleArrayMap():int, 287, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [,Read of container `this.si_map` via call to `get`,,Write to container `this.si_map` via call to `put`] +codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.addToSimpleArrayMapBad(android.support.v4.util.SimpleArrayMap):void, 284, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `map` via call to `put`] +codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.addToSparseArrayBad(android.util.SparseArray):void, 274, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `sparseArray` via call to `put`] +codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.addToSparseArrayCompatBad(android.support.v4.util.SparseArrayCompat):void, 265, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `sparseArray` via call to `put`] +codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.addToUnsynchronizedListBad(java.lang.String):void, 326, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.mSomeList` via call to `add`] +codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.containerWrapperUnownedWriteBad(java.lang.Object):void, 172, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [call to Object ContainerWrapper.write(Object),call to Object ContainerWrapper._write(Object),Write to container `this.mContainerWrapper.children` via call to `add`] +codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.getListSizeBad():int, 332, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [,Read of container `this.mSomeOtherList` via call to `size`,,Write to container `this.mSomeOtherList` via call to `remove`] +codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.getMapSizeBad():int, 342, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [,Read of container `this.mSomeMap` via call to `size`,,Write to container `this.mSomeMap` via call to `remove`] +codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.listAddAllBad(java.util.Collection):void, 60, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.mList` via call to `addAll`] +codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.listAddBad1(java.lang.String):void, 52, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.mList` via call to `add`] +codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.listAddBad2(int,java.lang.String):void, 56, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.mList` via call to `add`] +codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.listClearBad():void, 64, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.mList` via call to `clear`] +codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.listReadBad(java.lang.String):boolean, 103, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [,Read of container `this.mListSyncWrites` via call to `contains`,,Write to container `this.mListSyncWrites` via call to `add`] +codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.listRemoveAllBad(java.util.Collection):void, 76, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.mList` via call to `removeAll`] +codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.listRemoveBad1(int):void, 68, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.mList` via call to `remove`] +codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.listRemoveBad2(java.lang.String):void, 72, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.mList` via call to `remove`] +codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.listSetBad(int,java.lang.String):void, 80, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.mList` via call to `set`] +codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.listSubclassWriteBad(java.util.ArrayList,int):void, 84, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `list` via call to `remove`] +codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.mapClearBad():void, 120, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.mMap` via call to `clear`] +codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.mapPutAllBad(java.util.Map):void, 124, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.mMap` via call to `putAll`] +codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.mapPutBad(java.lang.String,java.lang.String):void, 112, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.mMap` via call to `put`] +codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.mapRemoveBad(java.lang.String):void, 116, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.mMap` via call to `remove`] +codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.mapSubclassWriteBad(java.util.HashMap,java.lang.String):void, 142, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `m` via call to `remove`] +codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.poolBad():void, 303, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.simplePool` via call to `release`] +codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.readSimpleArrayMap():int, 289, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [,Read of container `this.si_map` via call to `get`,,Write to container `this.si_map` via call to `put`] codetoanalyze/java/racerd/DeepOwnership.java, DeepOwnership.globalNotOwnedBad():void, 16, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [access to `DeepOwnership.global.next`] codetoanalyze/java/racerd/Dispatch.java, codetoanalyze.java.checkers.Dispatch.callUnannotatedInterfaceBad(codetoanalyze.java.checkers.UnannotatedInterface):void, 49, INTERFACE_NOT_THREAD_SAFE, no_bucket, WARNING, [Call to un-annotated interface method void UnannotatedInterface.foo()] codetoanalyze/java/racerd/Dispatch.java, codetoanalyze.java.checkers.Dispatch.callUnannotatedInterfaceIndirectBad(codetoanalyze.java.checkers.NotThreadSafe,codetoanalyze.java.checkers.UnannotatedInterface):void, 53, INTERFACE_NOT_THREAD_SAFE, no_bucket, WARNING, [call to void NotThreadSafe.notThreadSafeOk(UnannotatedInterface),Call to un-annotated interface method void UnannotatedInterface.foo()]