From ad334aef65d99f0f6fcb1603b0c72dd3c87532e5 Mon Sep 17 00:00:00 2001 From: Nikos Gorogiannis Date: Wed, 22 Apr 2020 04:26:45 -0700 Subject: [PATCH] [racerd] improve and complete container models Summary: Models were partial and/or simply missing (`Map` writes!). Now the modelled containers use inheritance for conciseness (`List` reads are only those not caught by the `Collection` matcher, etc). Also, add URLs to documentation sources. Reviewed By: ezgicicek Differential Revision: D21132069 fbshipit-source-id: fefb360f0 --- infer/src/concurrency/RacerDModels.ml | 95 ++++++++++++------- .../codetoanalyze/java/racerd/Containers.java | 16 ++++ .../codetoanalyze/java/racerd/issues.exp | 3 + 3 files changed, 80 insertions(+), 34 deletions(-) diff --git a/infer/src/concurrency/RacerDModels.ml b/infer/src/concurrency/RacerDModels.ml index bef302ad6..a9e009557 100644 --- a/infer/src/concurrency/RacerDModels.ml +++ b/infer/src/concurrency/RacerDModels.ml @@ -33,21 +33,34 @@ let is_java_container_write = let array_methods = ["append"; "clear"; "delete"; "put"; "remove"; "removeAt"; "removeAtRange"; "setValueAt"] in + (* https://developer.android.com/reference/androidx/core/util/Pools.SimplePool *) make_android_support_template "Pools$SimplePool" ["acquire"; "release"] + (* https://developer.android.com/reference/android/support/v4/util/SimpleArrayMap *) @ make_android_support_template "SimpleArrayMap" ["clear"; "ensureCapacity"; "put"; "putAll"; "remove"; "removeAt"; "setValueAt"] + (* https://developer.android.com/reference/android/support/v4/util/SparseArrayCompat *) @ make_android_support_template "SparseArrayCompat" array_methods - @ [ {default with classname= "android.util.SparseArray"; methods= array_methods} - ; { default with - classname= "java.util.List" - ; methods= ["add"; "addAll"; "clear"; "remove"; "set"] } - ; {default with classname= "java.util.Map"; methods= ["clear"; "put"; "putAll"; "remove"]} ] + @ (* https://developer.android.com/reference/android/util/SparseArray *) + [ {default with classname= "android.util.SparseArray"; methods= array_methods} + ; (* https://docs.oracle.com/javase/8/docs/api/java/util/List.html + Only methods not in parent interface [Collection] are listed *) + {default with classname= "java.util.List"; methods= ["replaceAll"; "retainAll"; "set"; "sort"]} + ; (* https://docs.oracle.com/javase/8/docs/api/java/util/Map.html *) + { default with + classname= "java.util.Map" + ; methods= ["clear"; "merge"; "put"; "putAll"; "putIfAbsent"; "remove"; "replace"; "replaceAll"] + } + ; (* https://docs.oracle.com/javase/8/docs/api/java/util/Collection.html *) + { default with + classname= "java.util.Collection" + ; methods= ["add"; "addAll"; "clear"; "remove"; "removeAll"; "removeIf"] } ] |> of_records let is_java_container_read = let open MethodMatcher in let array_methods = ["clone"; "get"; "indexOfKey"; "indexOfValue"; "keyAt"; "size"; "valueAt"] in + (* https://developer.android.com/reference/android/support/v4/util/SimpleArrayMap *) make_android_support_template "SimpleArrayMap" [ "containsKey" ; "containsValue" @@ -58,36 +71,50 @@ let is_java_container_read = ; "keyAt" ; "size" ; "valueAt" ] + (* https://developer.android.com/reference/android/support/v4/util/SparseArrayCompat *) @ make_android_support_template "SparseArrayCompat" array_methods - @ [ {default with classname= "android.util.SparseArray"; methods= array_methods} - ; { default with - classname= "java.util.List" - ; methods= - [ "contains" - ; "containsAll" - ; "equals" - ; "get" - ; "hashCode" - ; "indexOf" - ; "isEmpty" - ; "iterator" - ; "lastIndexOf" - ; "listIterator" - ; "size" - ; "toArray" ] } - ; { default with - classname= "java.util.Map" - ; methods= - [ "containsKey" - ; "containsValue" - ; "entrySet" - ; "equals" - ; "get" - ; "hashCode" - ; "isEmpty" - ; "keySet" - ; "size" - ; "values" ] } ] + @ (* https://developer.android.com/reference/android/util/SparseArray *) + [ {default with classname= "android.util.SparseArray"; methods= array_methods} + ; (* https://docs.oracle.com/javase/8/docs/api/java/util/List.html + Only methods not in parent interface [Collection] are listed *) + { default with + classname= "java.util.List" + ; methods= ["get"; "indexOf"; "isEmpty"; "lastIndexOf"; "listIterator"] } + ; (* https://docs.oracle.com/javase/8/docs/api/java/util/Map.html *) + { default with + classname= "java.util.Map" + ; methods= + [ "compute" + ; "computeIfAbsent" + ; "computeIfPresent" + ; "containsKey" + ; "containsValue" + ; "entrySet" + ; "equals" + ; "forEach" + ; "get" + ; "getOrDefault" + ; "hashCode" + ; "isEmpty" + ; "keySet" + ; "size" + ; "values" ] } + ; (* https://docs.oracle.com/javase/8/docs/api/java/util/Collection.html *) + { default with + classname= "java.util.Collection" + ; methods= + [ "contains" + ; "containsAll" + ; "equals" + ; "get" + ; "hashCode" + ; "isEmpty" + ; "iterator" + ; "parallelStream" + ; "size" + ; "spliterator" + ; "stream" + ; "toArray" ] } ] |> of_records diff --git a/infer/tests/codetoanalyze/java/racerd/Containers.java b/infer/tests/codetoanalyze/java/racerd/Containers.java index af7538b3b..ecd787213 100644 --- a/infer/tests/codetoanalyze/java/racerd/Containers.java +++ b/infer/tests/codetoanalyze/java/racerd/Containers.java @@ -314,4 +314,20 @@ class Containers { void dynamicallyTypedConcurrentMapPutOk(String key, String value) { mConcurrentMap.put(key, value); } + + List mSomeList = new ArrayList(); + + void addToUnsynchronizedListBad(String value) { + mSomeList.add(value); + } + + List mSomeOtherList = new ArrayList(); + + int getListSizeBad() { + return mSomeOtherList.size(); + } + + synchronized void raceWithSizeBad(String value) { + mSomeOtherList.remove(value); + } } diff --git a/infer/tests/codetoanalyze/java/racerd/issues.exp b/infer/tests/codetoanalyze/java/racerd/issues.exp index 632cc7f31..76a23a3e9 100644 --- a/infer/tests/codetoanalyze/java/racerd/issues.exp +++ b/infer/tests/codetoanalyze/java/racerd/issues.exp @@ -29,12 +29,15 @@ codetoanalyze/java/racerd/Constructors.java, Constructors.singleton2Bad():Constr codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.addToSimpleArrayMapBad(android.support.v4.util.SimpleArrayMap):void, 279, 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, 269, 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, 260, 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, 321, 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, 167, 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, 327, 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.listAddAllBad(java.util.Collection):void, 55, 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, 47, 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, 51, 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, 59, 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, 98, 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, 71, 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, 63, 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, 67, 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, 75, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.mList` via call to `set`]