[racerd] cleanups in reporting function

Summary:
There were several lists constructed unnecessarily -- replaced them with find_maps
and hopefully simplified the logic.

Reviewed By: mbouaziz, jvillard

Differential Revision: D12823559

fbshipit-source-id: 1f06b20f3
master
Nikos Gorogiannis 6 years ago committed by Facebook Github Bot
parent 5653dfac32
commit 77f7e70417

@ -1169,51 +1169,42 @@ let report_unsafe_accesses (aggregated_access_map : ReportMap.t) =
if is_duplicate_report snapshot.access pname reported_acc then reported_acc if is_duplicate_report snapshot.access pname reported_acc then reported_acc
else else
match TraceElem.kind snapshot.access with match TraceElem.kind snapshot.access with
| Access.InterfaceCall unannoted_call_pname -> | Access.InterfaceCall unannoted_call_pname
if when AccessSnapshot.is_unprotected snapshot
AccessSnapshot.is_unprotected snapshot
&& ThreadsDomain.is_any threads && ThreadsDomain.is_any threads
&& is_marked_thread_safe procdesc tenv && is_marked_thread_safe procdesc tenv ->
then (
(* un-annotated interface call + no lock in method marked thread-safe. warn *) (* un-annotated interface call + no lock in method marked thread-safe. warn *)
report_unannotated_interface_violation tenv procdesc snapshot.access threads report_unannotated_interface_violation tenv procdesc snapshot.access threads
unannoted_call_pname ; unannoted_call_pname ;
update_reported snapshot.access pname reported_acc ) update_reported snapshot.access pname reported_acc
else reported_acc | Access.InterfaceCall _ ->
| Access.Write _ | ContainerWrite _ -> ( reported_acc
match Procdesc.get_proc_name procdesc with | (Access.Write _ | ContainerWrite _) when Typ.Procname.is_java pname ->
| Java _ -> let conflict =
let writes_on_background_thread =
if ThreadsDomain.is_any threads then if ThreadsDomain.is_any threads then
(* unprotected write in method that may run in parallel with itself. warn *) (* unprotected write in method that may run in parallel with itself. warn *)
[] None
else else
(* unprotected write, but not on a method that may run in parallel with itself (* unprotected write, but not on a method that may run in parallel with itself
(i.e., not a self race). find accesses on a background thread this access might (i.e., not a self race). find accesses on a background thread this access might
conflict with and report them *) conflict with and report them *)
List.filter_map List.find_map accesses ~f:(fun {snapshot= other_snapshot; threads= other_threads} ->
~f:(fun {snapshot= other_snapshot; threads= other_threads} -> if TraceElem.is_write other_snapshot.access && ThreadsDomain.is_any other_threads
if
TraceElem.is_write other_snapshot.access
&& ThreadsDomain.is_any other_threads
then Some other_snapshot.access then Some other_snapshot.access
else None ) else None )
accesses
in in
if if
AccessSnapshot.is_unprotected snapshot AccessSnapshot.is_unprotected snapshot
&& ((not (List.is_empty writes_on_background_thread)) || ThreadsDomain.is_any threads) && (Option.is_some conflict || ThreadsDomain.is_any threads)
then ( then (
let conflict = List.hd writes_on_background_thread in
report_thread_safety_violation tenv procdesc report_thread_safety_violation tenv procdesc
~make_description:make_unprotected_write_description ~make_description:make_unprotected_write_description
~report_kind:(WriteWriteRace conflict) snapshot.access threads wobbly_paths ; ~report_kind:(WriteWriteRace conflict) snapshot.access threads wobbly_paths ;
update_reported snapshot.access pname reported_acc ) update_reported snapshot.access pname reported_acc )
else reported_acc else reported_acc
| _ -> | Access.Write _ | ContainerWrite _ ->
(* Do not report unprotected writes when an access can't run in parallel with itself, or (* Do not report unprotected writes for ObjC_Cpp *)
for ObjC_Cpp *) reported_acc
reported_acc )
| (Access.Read _ | ContainerRead _) when AccessSnapshot.is_unprotected snapshot -> | (Access.Read _ | ContainerRead _) when AccessSnapshot.is_unprotected snapshot ->
(* unprotected read. report all writes as conflicts for java. for c++ filter out (* unprotected read. report all writes as conflicts for java. for c++ filter out
unprotected writes *) unprotected writes *)
@ -1239,25 +1230,21 @@ let report_unsafe_accesses (aggregated_access_map : ReportMap.t) =
if snapshot1.lock && snapshot2.lock then false if snapshot1.lock && snapshot2.lock then false
else ThreadsDomain.can_conflict snapshot1.thread snapshot2.thread else ThreadsDomain.can_conflict snapshot1.thread snapshot2.thread
in in
let conflicting_writes = let is_conflict {snapshot= other_snapshot; threads= other_threads} =
List.filter
~f:(fun {snapshot= other_snapshot; threads= other_threads} ->
if AccessSnapshot.is_unprotected other_snapshot then if AccessSnapshot.is_unprotected other_snapshot then
TraceElem.is_write other_snapshot.access && ThreadsDomain.is_any other_threads TraceElem.is_write other_snapshot.access && ThreadsDomain.is_any other_threads
else else TraceElem.is_write other_snapshot.access && can_conflict snapshot other_snapshot
TraceElem.is_write other_snapshot.access && can_conflict snapshot other_snapshot
)
accesses
in in
if not (List.is_empty conflicting_writes) then ( List.find accesses ~f:is_conflict
let conflict = List.hd_exn conflicting_writes in |> Option.value_map ~default:reported_acc ~f:(fun conflict ->
(* protected read with conflicting unprotected write(s). warn. *) (* protected read with conflicting unprotected write(s). warn. *)
report_thread_safety_violation tenv procdesc let make_description =
~make_description:(make_read_write_race_description ~read_is_sync:true conflict) make_read_write_race_description ~read_is_sync:true conflict
~report_kind:(ReadWriteRace conflict.snapshot.access) snapshot.access threads in
wobbly_paths ; let report_kind = ReadWriteRace conflict.snapshot.access in
report_thread_safety_violation tenv procdesc ~make_description ~report_kind
snapshot.access threads wobbly_paths ;
update_reported snapshot.access pname reported_acc ) update_reported snapshot.access pname reported_acc )
else reported_acc
in in
let report_accesses_on_location (grouped_accesses : reported_access list) reported_acc = let report_accesses_on_location (grouped_accesses : reported_access list) reported_acc =
(* reset the reported reads and writes for each memory location *) (* reset the reported reads and writes for each memory location *)

Loading…
Cancel
Save