[racerd] recognise wrappers that make containers thread-safe

Summary: Java has this pattern of wrapping non-thread-safe containers in factory methods producing identically-typed results, but wrapped in a synchronised shell.  This diff teaches RacerD about some common factory methods and uses the attribute domain to track the dynamic type of their results.

Reviewed By: ezgicicek

Differential Revision: D21155538

fbshipit-source-id: 42ebe6251
master
Nikos Gorogiannis 5 years ago committed by Facebook GitHub Bot
parent 8f351669bf
commit 7e34576bcc

@ -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

@ -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

@ -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? *)

@ -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<String> mWrappedList = Collections.synchronizedList(new ArrayList<String>());
void wrappedListAddOk(String value) {
mWrappedList.add(value);
}
Set<String> mGoogleSynchronizedSet = Sets.newConcurrentHashSet();
void googleSynchronizedSetAddOk(String value) {
mGoogleSynchronizedSet.add(value);
}
}

@ -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.<init>(Object),access to `Constructors.staticField`]
codetoanalyze/java/racerd/Constructors.java, Constructors.singleton2Bad():Constructors, 63, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [<Read trace>,access to `Constructors.sSingleton2`,<Write trace>,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 trace>,Read of container `this.mSomeOtherList` via call to `size`,<Write trace>,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 trace>,Read of container `this.mSomeMap` via call to `size`,<Write trace>,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 trace>,Read of container `this.mListSyncWrites` via call to `contains`,<Write trace>,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 trace>,Read of container `this.si_map` via call to `get`,<Write trace>,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 trace>,Read of container `this.mSomeOtherList` via call to `size`,<Write trace>,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 trace>,Read of container `this.mSomeMap` via call to `size`,<Write trace>,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 trace>,Read of container `this.mListSyncWrites` via call to `contains`,<Write trace>,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 trace>,Read of container `this.si_map` via call to `get`,<Write trace>,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()]

Loading…
Cancel
Save