diff --git a/infer/src/checkers/ThreadSafety.ml b/infer/src/checkers/ThreadSafety.ml index 246a536c6..36157d090 100644 --- a/infer/src/checkers/ThreadSafety.ml +++ b/infer/src/checkers/ThreadSafety.ml @@ -128,11 +128,14 @@ module TransferFunctions (CFG : ProcCfg.S) = struct let typename = Typename.Java.from_string (Procname.java_get_class_name java_pname) in let is_container_write_ typename _ = match Typename.name typename, Procname.java_get_method java_pname with + | "java.util.List", ("add" | "addAll" | "clear" | "remove" | "set") -> true | "java.util.Map", ("clear" | "put" | "putAll" | "remove") -> true | _ -> false in let is_threadsafe_collection typename _ = match Typename.name typename with - | "java.util.concurrent.ConcurrentMap" -> true - | _ -> false in + | "java.util.concurrent.ConcurrentMap" | "java.util.concurrent.CopyOnWriteArrayList" -> + true + | _ -> + false in PatternMatch.supertype_exists tenv is_container_write_ typename && not (PatternMatch.supertype_exists tenv is_threadsafe_collection typename) | _ -> false in diff --git a/infer/tests/codetoanalyze/java/threadsafety/Containers.java b/infer/tests/codetoanalyze/java/threadsafety/Containers.java index 6e12cb69d..386ce88c9 100644 --- a/infer/tests/codetoanalyze/java/threadsafety/Containers.java +++ b/infer/tests/codetoanalyze/java/threadsafety/Containers.java @@ -9,18 +9,72 @@ package codetoanalyze.java.checkers; -import javax.annotation.concurrent.ThreadSafe; - +import java.util.ArrayList; +import java.util.Collection; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.CopyOnWriteArrayList; + +import javax.annotation.concurrent.ThreadSafe; @ThreadSafe class Containers { + List mList; Map mMap; + // lists + void listAddBad1(String s) { + mList.add(s); + } + + void listAddBad2(int index, String s) { + mList.add(index, s); + } + + void listAddAllBad(Collection c) { + mList.addAll(c); + } + + void listClearBad() { + mList.clear(); + } + + void listRemoveBad1(int index) { + mList.remove(index); + } + + void listRemoveBad2(String s) { + mList.remove(s); + } + + void listRemoveAllBad(Collection c) { + mList.removeAll(c); + } + + void listSetBad(int index, String s) { + mList.set(index, s); + } + + void listSubclassWriteBad(ArrayList list, int index) { + list.remove(index); + } + + void listReadOk(int index, String s) { + mList.contains(s); + mList.get(index); + mList.isEmpty(); + mList.size(); + } + + void accessSafeListOk(CopyOnWriteArrayList list, int index) { + list.remove(index); + } + + // maps void mapPutBad(String key, String value) { mMap.put(key, value); } @@ -53,7 +107,6 @@ class Containers { m.remove(key); } - synchronized void synchronizedWriteOk1(String key) { mMap.remove(key); } diff --git a/infer/tests/codetoanalyze/java/threadsafety/issues.exp b/infer/tests/codetoanalyze/java/threadsafety/issues.exp index f69441001..4a5f1da75 100644 --- a/infer/tests/codetoanalyze/java/threadsafety/issues.exp +++ b/infer/tests/codetoanalyze/java/threadsafety/issues.exp @@ -5,6 +5,14 @@ codetoanalyze/java/threadsafety/Builders.java, Builders$Obj Builders.mutateBad(B codetoanalyze/java/threadsafety/Constructors.java, Constructors Constructors.singletonBad(), 2, THREAD_SAFETY_VIOLATION, [call to Constructors.(Object),access to Constructors.staticField] codetoanalyze/java/threadsafety/Constructors.java, Constructors.(), 1, THREAD_SAFETY_VIOLATION, [access to Constructors.staticField] codetoanalyze/java/threadsafety/Constructors.java, Constructors.(Constructors), 1, THREAD_SAFETY_VIOLATION, [access to Constructors.field] +codetoanalyze/java/threadsafety/Containers.java, void Containers.listAddAllBad(Collection), 1, THREAD_SAFETY_VIOLATION, [access to codetoanalyze.java.checkers.Containers.mList.addAll] +codetoanalyze/java/threadsafety/Containers.java, void Containers.listAddBad1(String), 1, THREAD_SAFETY_VIOLATION, [access to codetoanalyze.java.checkers.Containers.mList.add] +codetoanalyze/java/threadsafety/Containers.java, void Containers.listAddBad2(int,String), 1, THREAD_SAFETY_VIOLATION, [access to codetoanalyze.java.checkers.Containers.mList.add] +codetoanalyze/java/threadsafety/Containers.java, void Containers.listClearBad(), 1, THREAD_SAFETY_VIOLATION, [access to codetoanalyze.java.checkers.Containers.mList.clear] +codetoanalyze/java/threadsafety/Containers.java, void Containers.listRemoveBad1(int), 1, THREAD_SAFETY_VIOLATION, [access to codetoanalyze.java.checkers.Containers.mList.remove] +codetoanalyze/java/threadsafety/Containers.java, void Containers.listRemoveBad2(String), 1, THREAD_SAFETY_VIOLATION, [access to codetoanalyze.java.checkers.Containers.mList.remove] +codetoanalyze/java/threadsafety/Containers.java, void Containers.listSetBad(int,String), 1, THREAD_SAFETY_VIOLATION, [access to codetoanalyze.java.checkers.Containers.mList.set] +codetoanalyze/java/threadsafety/Containers.java, void Containers.listSubclassWriteBad(ArrayList,int), 1, THREAD_SAFETY_VIOLATION, [access to remove] codetoanalyze/java/threadsafety/Containers.java, void Containers.mapClearBad(), 1, THREAD_SAFETY_VIOLATION, [access to codetoanalyze.java.checkers.Containers.mMap.clear] codetoanalyze/java/threadsafety/Containers.java, void Containers.mapPutAllBad(Map), 1, THREAD_SAFETY_VIOLATION, [access to codetoanalyze.java.checkers.Containers.mMap.putAll] codetoanalyze/java/threadsafety/Containers.java, void Containers.mapPutBad(String,String), 1, THREAD_SAFETY_VIOLATION, [access to codetoanalyze.java.checkers.Containers.mMap.put]