pghs975uc
/
infer_clone
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

[infer][thread-safety] Better error message for container writes

Browse Source 
Summary: distinguish writes via method calls (e.g., add) from writes via assignment in the error messages

Reviewed By: sblackshear

Differential Revision: D4611748

fbshipit-source-id: 7594d3b
master
Peter O'Hearn 8 years ago committed by Facebook Github Bot
parent d4af8f756a
commit 1580e5b3bb
2 changed files with 47 additions and 23 deletions
Show Stats Download Patch File Download Diff File

42
infer/src/checkers/ThreadSafety.ml
Unescape View File

@ -27,6 +27,20 @@ let is_owned access_path attribute_map =
ThreadSafetyDomain.AttributeMapDomain.has_attribute ThreadSafetyDomain.AttributeMapDomain.has_attribute
access_path ThreadSafetyDomain.Attribute.unconditionally_owned attribute_map access_path ThreadSafetyDomain.Attribute.unconditionally_owned attribute_map
let container_write_string = "__CONTAINERWRITE__"
let is_container_write_str str =
String.is_substring ~substring:container_write_string str
let strip_container_write str =
String.substr_replace_first str ~pattern:container_write_string ~with_:""
let is_container_write_sink (sink:ThreadSafetyDomain.TraceElem.t) =
let access_list = snd (ThreadSafetyDomain.TraceElem.kind sink) in
match List.rev access_list with
| FieldAccess (fn) :: _ -> is_container_write_str (Ident.fieldname_to_string fn)
| _ -> false
module TransferFunctions (CFG : ProcCfg.S) = struct module TransferFunctions (CFG : ProcCfg.S) = struct
module CFG = CFG module CFG = CFG
module Domain = ThreadSafetyDomain module Domain = ThreadSafetyDomain
@ -271,7 +285,9 @@ module TransferFunctions (CFG : ProcCfg.S) = struct
(* create a dummy write that represents mutating the contents of the container *) (* create a dummy write that represents mutating the contents of the container *)
let open Domain in let open Domain in
let dummy_fieldname = let dummy_fieldname =
Ident.create_fieldname (Mangled.from_string (Procname.get_method callee_pname)) 0 in Ident.create_fieldname
(Mangled.from_string
(container_write_string ^ (Procname.get_method callee_pname))) 0 in
let dummy_access_exp = Exp.Lfield (receiver_exp, dummy_fieldname, receiver_typ) in let dummy_access_exp = Exp.Lfield (receiver_exp, dummy_fieldname, receiver_typ) in
let callee_conditional_writes = let callee_conditional_writes =
match AccessPath.of_lhs_exp dummy_access_exp receiver_typ ~f_resolve_id with match AccessPath.of_lhs_exp dummy_access_exp receiver_typ ~f_resolve_id with
@ -839,9 +855,15 @@ let de_dup trace =
ThreadSafetyDomain.PathDomain.update_sinks trace de_duped_sinks ThreadSafetyDomain.PathDomain.update_sinks trace de_duped_sinks
(*A helper function used in the error reporting*) (*A helper function used in the error reporting*)
let pp_accesses_sink fmt sink = let pp_accesses_sink fmt ~is_write_access sink =
let _, accesses = ThreadSafetyDomain.PathDomain.Sink.kind sink in let access_path = ThreadSafetyDomain.PathDomain.Sink.kind sink in
AccessPath.pp_access_list fmt accesses let container_write = is_write_access && is_container_write_sink sink in
F.fprintf fmt
(if container_write then "container %a" else "%a")
AccessPath.pp_access_list (if container_write then
snd (AccessPath.Raw.truncate access_path)
else snd access_path
)
(* trace is really a set of accesses*) (* trace is really a set of accesses*)
@ -859,9 +881,10 @@ let report_thread_safety_violations ( _, tenv, pname, pdesc) make_description tr
let initial_sink_site = PathDomain.Sink.call_site initial_sink in let initial_sink_site = PathDomain.Sink.call_site initial_sink in
let final_sink_site = PathDomain.Sink.call_site final_sink in let final_sink_site = PathDomain.Sink.call_site final_sink in
let desc_of_sink sink = let desc_of_sink sink =
if CallSite.equal (PathDomain.Sink.call_site sink) final_sink_site if
CallSite.equal (PathDomain.Sink.call_site sink) final_sink_site
then then
Format.asprintf "access to %a" pp_accesses_sink sink Format.asprintf "access to %a" (pp_accesses_sink ~is_write_access:true) sink
else else
Format.asprintf Format.asprintf
"call to %a" Procname.pp (CallSite.pname (PathDomain.Sink.call_site sink)) in "call to %a" Procname.pp (CallSite.pname (PathDomain.Sink.call_site sink)) in
@ -881,10 +904,11 @@ let report_thread_safety_violations ( _, tenv, pname, pdesc) make_description tr
let make_unprotected_write_description let make_unprotected_write_description
tenv pname final_sink_site initial_sink_site final_sink _ = tenv pname final_sink_site initial_sink_site final_sink _ =
Format.asprintf Format.asprintf
"Unprotected write. Public method %a%s writes to field %a outside of synchronization.%s" "Unprotected write. Public method %a%s %s %a outside of synchronization.%s"
Procname.pp pname Procname.pp pname
(if CallSite.equal final_sink_site initial_sink_site then "" else " indirectly") (if CallSite.equal final_sink_site initial_sink_site then "" else " indirectly")
pp_accesses_sink final_sink (if is_container_write_sink final_sink then "mutates" else "writes to field")
(pp_accesses_sink ~is_write_access:true) final_sink
(calculate_addendum_message tenv pname) (calculate_addendum_message tenv pname)
let make_read_write_race_description tenv pname final_sink_site initial_sink_site final_sink tab = let make_read_write_race_description tenv pname final_sink_site initial_sink_site final_sink tab =
@ -904,7 +928,7 @@ let make_read_write_race_description tenv pname final_sink_site initial_sink_sit
Format.asprintf "Read/Write race. Public method %a%s reads from field %a. %s %s" Format.asprintf "Read/Write race. Public method %a%s reads from field %a. %s %s"
Procname.pp pname Procname.pp pname
(if CallSite.equal final_sink_site initial_sink_site then "" else " indirectly") (if CallSite.equal final_sink_site initial_sink_site then "" else " indirectly")
pp_accesses_sink final_sink (pp_accesses_sink ~is_write_access:false) final_sink
conflicts_description conflicts_description
(calculate_addendum_message tenv pname) (calculate_addendum_message tenv pname)

28
infer/tests/codetoanalyze/java/threadsafety/issues.exp
Unescape View File

@ -12,20 +12,20 @@ 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.<init>(Object),access to Constructors.staticField] codetoanalyze/java/threadsafety/Constructors.java, Constructors Constructors.singletonBad(), 2, THREAD_SAFETY_VIOLATION, [call to Constructors.<init>(Object),access to Constructors.staticField]
codetoanalyze/java/threadsafety/Constructors.java, Constructors.<init>(), 1, THREAD_SAFETY_VIOLATION, [access to Constructors.staticField] codetoanalyze/java/threadsafety/Constructors.java, Constructors.<init>(), 1, THREAD_SAFETY_VIOLATION, [access to Constructors.staticField]
codetoanalyze/java/threadsafety/Constructors.java, Constructors.<init>(Constructors), 1, THREAD_SAFETY_VIOLATION, [access to Constructors.field] codetoanalyze/java/threadsafety/Constructors.java, Constructors.<init>(Constructors), 1, THREAD_SAFETY_VIOLATION, [access to Constructors.field]
codetoanalyze/java/threadsafety/Containers.java, void Containers.containerWrapperUnownedWriteBad(Object), 1, THREAD_SAFETY_VIOLATION, [call to Object ContainerWrapper.write(Object),call to Object ContainerWrapper._write(Object),access to codetoanalyze.java.checkers.ContainerWrapper.children.add] codetoanalyze/java/threadsafety/Containers.java, void Containers.containerWrapperUnownedWriteBad(Object), 1, THREAD_SAFETY_VIOLATION, [call to Object ContainerWrapper.write(Object),call to Object ContainerWrapper._write(Object),access to container codetoanalyze.java.checkers.ContainerWrapper.children]
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.listAddAllBad(Collection), 1, THREAD_SAFETY_VIOLATION, [access to container codetoanalyze.java.checkers.Containers.mList]
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.listAddBad1(String), 1, THREAD_SAFETY_VIOLATION, [access to container codetoanalyze.java.checkers.Containers.mList]
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.listAddBad2(int,String), 1, THREAD_SAFETY_VIOLATION, [access to container codetoanalyze.java.checkers.Containers.mList]
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.listClearBad(), 1, THREAD_SAFETY_VIOLATION, [access to container codetoanalyze.java.checkers.Containers.mList]
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.listRemoveBad1(int), 1, THREAD_SAFETY_VIOLATION, [access to container codetoanalyze.java.checkers.Containers.mList]
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.listRemoveBad2(String), 1, THREAD_SAFETY_VIOLATION, [access to container codetoanalyze.java.checkers.Containers.mList]
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.listSetBad(int,String), 1, THREAD_SAFETY_VIOLATION, [access to container codetoanalyze.java.checkers.Containers.mList]
codetoanalyze/java/threadsafety/Containers.java, void Containers.listSubclassWriteBad(ArrayList,int), 1, THREAD_SAFETY_VIOLATION, [access to remove] codetoanalyze/java/threadsafety/Containers.java, void Containers.listSubclassWriteBad(ArrayList,int), 1, THREAD_SAFETY_VIOLATION, [access to container ]
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.mapClearBad(), 1, THREAD_SAFETY_VIOLATION, [access to container codetoanalyze.java.checkers.Containers.mMap]
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.mapPutAllBad(Map), 1, THREAD_SAFETY_VIOLATION, [access to container codetoanalyze.java.checkers.Containers.mMap]
codetoanalyze/java/threadsafety/Containers.java, void Containers.mapPutBad(String,String), 1, THREAD_SAFETY_VIOLATION, [access to codetoanalyze.java.checkers.Containers.mMap.put] codetoanalyze/java/threadsafety/Containers.java, void Containers.mapPutBad(String,String), 1, THREAD_SAFETY_VIOLATION, [access to container codetoanalyze.java.checkers.Containers.mMap]
codetoanalyze/java/threadsafety/Containers.java, void Containers.mapRemoveBad(String), 1, THREAD_SAFETY_VIOLATION, [access to codetoanalyze.java.checkers.Containers.mMap.remove] codetoanalyze/java/threadsafety/Containers.java, void Containers.mapRemoveBad(String), 1, THREAD_SAFETY_VIOLATION, [access to container codetoanalyze.java.checkers.Containers.mMap]
codetoanalyze/java/threadsafety/Containers.java, void Containers.mapSubclassWriteBad(HashMap,String), 1, THREAD_SAFETY_VIOLATION, [access to remove] codetoanalyze/java/threadsafety/Containers.java, void Containers.mapSubclassWriteBad(HashMap,String), 1, THREAD_SAFETY_VIOLATION, [access to container ]
codetoanalyze/java/threadsafety/DeDup.java, void DeDup.twoWritesOneInCaller(), 1, THREAD_SAFETY_VIOLATION, [call to void DeDup.writeToField(),access to codetoanalyze.java.checkers.DeDup.field] codetoanalyze/java/threadsafety/DeDup.java, void DeDup.twoWritesOneInCaller(), 1, THREAD_SAFETY_VIOLATION, [call to void DeDup.writeToField(),access to codetoanalyze.java.checkers.DeDup.field]
codetoanalyze/java/threadsafety/DeDup.java, void DeDup.two_reads(), 2, THREAD_SAFETY_VIOLATION, [access to codetoanalyze.java.checkers.DeDup.field] codetoanalyze/java/threadsafety/DeDup.java, void DeDup.two_reads(), 2, THREAD_SAFETY_VIOLATION, [access to codetoanalyze.java.checkers.DeDup.field]
codetoanalyze/java/threadsafety/DeDup.java, void DeDup.two_writes(), 1, THREAD_SAFETY_VIOLATION, [access to codetoanalyze.java.checkers.DeDup.field] codetoanalyze/java/threadsafety/DeDup.java, void DeDup.two_writes(), 1, THREAD_SAFETY_VIOLATION, [access to codetoanalyze.java.checkers.DeDup.field]

Write Preview
Loading…
Cancel
Save