[racerd] move de-duplication back to the backend

Reviewed By: jberdine

Differential Revision: D6778795

fbshipit-source-id: a871056
master
Sam Blackshear 7 years ago committed by Facebook Github Bot
parent 66ad5c3018
commit c98bc53ede

@ -9,22 +9,6 @@
open! IStd open! IStd
(** type for remembering what we have already reported to avoid duplicates. our policy is to report
each kind of access (read/write) to the same field reachable from the same procedure only once.
in addition, if a call to a procedure (transitively) accesses multiple fields, we will only
report one of each kind of access *)
type reported =
{ reported_sites: CallSite.Set.t
; reported_writes: Typ.Procname.Set.t
; reported_reads: Typ.Procname.Set.t }
let empty_reported =
let reported_sites = CallSite.Set.empty in
let reported_writes = Typ.Procname.Set.empty in
let reported_reads = Typ.Procname.Set.empty in
{reported_sites; reported_reads; reported_writes}
(** set of lists of locations for remembering what trace ends have been reported *) (** set of lists of locations for remembering what trace ends have been reported *)
module LocListSet = struct module LocListSet = struct
include Caml.Set.Make (struct include Caml.Set.Make (struct
@ -36,36 +20,8 @@ module LocListSet = struct
let add s xs = if List.is_empty xs then s else add (List.sort ~cmp:Location.compare xs) s let add s xs = if List.is_empty xs then s else add (List.sort ~cmp:Location.compare xs) s
end end
let is_duplicate_report pname access end_locs {reported_sites; reported_writes; reported_reads} let is_duplicate_report end_locs reported_ends =
reported_ends = Config.filtering && LocListSet.mem reported_ends end_locs
let open RacerDDomain in
Config.filtering
&& ( LocListSet.mem reported_ends end_locs
|| CallSite.Set.mem (TraceElem.call_site access) reported_sites
||
match TraceElem.kind access with
| Access.Write _ | Access.ContainerWrite _ ->
Typ.Procname.Set.mem pname reported_writes
| Access.Read _ | Access.ContainerRead _ ->
Typ.Procname.Set.mem pname reported_reads
| Access.InterfaceCall _ ->
false )
let update_reported pname access reported =
let open RacerDDomain in
if Config.filtering then
let reported_sites = CallSite.Set.add (TraceElem.call_site access) reported.reported_sites in
match TraceElem.kind access with
| Access.Write _ | Access.ContainerWrite _ ->
let reported_writes = Typ.Procname.Set.add pname reported.reported_writes in
{reported with reported_writes; reported_sites}
| Access.Read _ | Access.ContainerRead _ ->
let reported_reads = Typ.Procname.Set.add pname reported.reported_reads in
{reported with reported_reads; reported_sites}
| Access.InterfaceCall _ ->
reported
else reported
let sort_by_decreasing_preference_to_report issues = let sort_by_decreasing_preference_to_report issues =
@ -87,21 +43,16 @@ let sort_by_location issues =
let dedup (issues: Jsonbug_t.jsonbug list) = let dedup (issues: Jsonbug_t.jsonbug list) =
List.fold (sort_by_decreasing_preference_to_report issues) List.fold (sort_by_decreasing_preference_to_report issues) ~init:(LocListSet.empty, []) ~f:
~init:(empty_reported, LocListSet.empty, []) ~f: (fun (reported_ends, nondup_issues) (issue: Jsonbug_t.jsonbug) ->
(fun (reported, reported_ends, nondup_issues) (issue: Jsonbug_t.jsonbug) ->
match issue.access with match issue.access with
| Some encoded -> | Some encoded ->
let pname, access, end_locs = IssueAuxData.decode encoded in let _, _, end_locs = IssueAuxData.decode encoded in
if is_duplicate_report pname access end_locs reported reported_ends then if is_duplicate_report end_locs reported_ends then (reported_ends, nondup_issues)
(reported, reported_ends, nondup_issues) else (LocListSet.add reported_ends end_locs, {issue with access= None} :: nondup_issues)
else
( update_reported pname access reported
, LocListSet.add reported_ends end_locs
, {issue with access= None} :: nondup_issues )
| None -> | None ->
(reported, reported_ends, {issue with access= None} :: nondup_issues) ) (reported_ends, {issue with access= None} :: nondup_issues) )
|> trd3 |> sort_by_location |> snd |> sort_by_location
type t = {introduced: Jsonbug_t.report; fixed: Jsonbug_t.report; preexisting: Jsonbug_t.report} type t = {introduced: Jsonbug_t.report; fixed: Jsonbug_t.report; preexisting: Jsonbug_t.report}

@ -1278,6 +1278,22 @@ let make_read_write_race_description ~read_is_sync (conflict: reported_access) p
pp_access final_sink conflicts_description pp_access final_sink conflicts_description
(** type for remembering what we have already reported to avoid duplicates. our policy is to report
each kind of access (read/write) to the same field reachable from the same procedure only once.
in addition, if a call to a procedure (transitively) accesses multiple fields, we will only
report one of each kind of access *)
type reported =
{ reported_sites: CallSite.Set.t
; reported_writes: Typ.Procname.Set.t
; reported_reads: Typ.Procname.Set.t }
let empty_reported =
let reported_sites = CallSite.Set.empty in
let reported_writes = Typ.Procname.Set.empty in
let reported_reads = Typ.Procname.Set.empty in
{reported_sites; reported_reads; reported_writes}
(* return true if procedure is at an abstraction boundary or reporting has been explicitly (* return true if procedure is at an abstraction boundary or reporting has been explicitly
requested via @ThreadSafe *) requested via @ThreadSafe *)
let should_report_on_proc proc_desc tenv = let should_report_on_proc proc_desc tenv =
@ -1323,110 +1339,153 @@ let should_report_on_proc proc_desc tenv =
*) *)
let report_unsafe_accesses (aggregated_access_map: reported_access list AccessListMap.t) = let report_unsafe_accesses (aggregated_access_map: reported_access list AccessListMap.t) =
let open RacerDDomain in let open RacerDDomain in
let report_unsafe_access {access; precondition; threads; tenv; procdesc} accesses = let is_duplicate_report access pname {reported_sites; reported_writes; reported_reads} =
if Config.filtering then
CallSite.Set.mem (TraceElem.call_site access) reported_sites
||
match TraceElem.kind access with
| Access.Write _ | Access.ContainerWrite _ ->
Typ.Procname.Set.mem pname reported_writes
| Access.Read _ | Access.ContainerRead _ ->
Typ.Procname.Set.mem pname reported_reads
| Access.InterfaceCall _ ->
false
else false
in
let update_reported access pname reported =
if Config.filtering then
let reported_sites = CallSite.Set.add (TraceElem.call_site access) reported.reported_sites in
match TraceElem.kind access with
| Access.Write _ | Access.ContainerWrite _ ->
let reported_writes = Typ.Procname.Set.add pname reported.reported_writes in
{reported with reported_writes; reported_sites}
| Access.Read _ | Access.ContainerRead _ ->
let reported_reads = Typ.Procname.Set.add pname reported.reported_reads in
{reported with reported_reads; reported_sites}
| Access.InterfaceCall _ ->
reported
else reported
in
let report_unsafe_access {access; precondition; threads; tenv; procdesc} accesses reported_acc =
let pname = Procdesc.get_proc_name procdesc in let pname = Procdesc.get_proc_name procdesc in
match (TraceElem.kind access, precondition) with if is_duplicate_report access pname reported_acc then reported_acc
| ( Access.InterfaceCall unannoted_call_pname else
, (AccessPrecondition.Unprotected _ | AccessPrecondition.TotallyUnprotected) ) -> match (TraceElem.kind access, precondition) with
if ThreadsDomain.is_any threads && is_marked_thread_safe procdesc tenv then | ( Access.InterfaceCall unannoted_call_pname
(* un-annotated interface call + no lock in method marked thread-safe. warn *) , (AccessPrecondition.Unprotected _ | AccessPrecondition.TotallyUnprotected) ) ->
report_unannotated_interface_violation tenv procdesc access threads unannoted_call_pname if ThreadsDomain.is_any threads && is_marked_thread_safe procdesc tenv then (
| Access.InterfaceCall _, AccessPrecondition.Protected _ -> (* un-annotated interface call + no lock in method marked thread-safe. warn *)
(* un-annotated interface call, but it's protected by a lock/thread. don't report *) report_unannotated_interface_violation tenv procdesc access threads
() unannoted_call_pname ;
| ( (Access.Write _ | ContainerWrite _) update_reported access pname reported_acc )
, (AccessPrecondition.Unprotected _ | AccessPrecondition.TotallyUnprotected) ) -> ( else reported_acc
match Procdesc.get_proc_name procdesc with | Access.InterfaceCall _, AccessPrecondition.Protected _ ->
| Java _ -> (* un-annotated interface call, but it's protected by a lock/thread. don't report *)
let writes_on_background_thread = reported_acc
if ThreadsDomain.is_any threads then | ( (Access.Write _ | ContainerWrite _)
(* unprotected write in method that may run in parallel with itself. warn *) , (AccessPrecondition.Unprotected _ | AccessPrecondition.TotallyUnprotected) ) -> (
[] match Procdesc.get_proc_name procdesc with
else | Java _ ->
(* unprotected write, but not on a method that may run in parallel with itself let writes_on_background_thread =
if ThreadsDomain.is_any threads then
(* unprotected write in method that may run in parallel with itself. warn *)
[]
else
(* 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.filter_map
~f:(fun {access= other_access; threads= other_threads} -> ~f:(fun {access= other_access; threads= other_threads} ->
if TraceElem.is_write other_access && ThreadsDomain.is_any other_threads then if TraceElem.is_write other_access && ThreadsDomain.is_any other_threads then
Some other_access Some other_access
else None ) else None )
accesses accesses
in in
if not (List.is_empty writes_on_background_thread && not (ThreadsDomain.is_any threads)) if not (List.is_empty writes_on_background_thread && not (ThreadsDomain.is_any threads))
then then
let conflict = List.hd writes_on_background_thread in 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) access threads ~report_kind:(WriteWriteRace conflict) access threads ;
| _ -> update_reported access pname reported_acc
(* Do not report unprotected writes when an access can't run in parallel with itself, or else reported_acc
| _ ->
(* Do not report unprotected writes when an access can't run in parallel with itself, or
for ObjC_Cpp *) for ObjC_Cpp *)
() ) reported_acc )
| (Access.Write _ | ContainerWrite _), AccessPrecondition.Protected _ -> | (Access.Write _ | ContainerWrite _), AccessPrecondition.Protected _ ->
(* protected write, do nothing *) (* protected write, do nothing *)
() reported_acc
| ( (Access.Read _ | ContainerRead _) | ( (Access.Read _ | ContainerRead _)
, (AccessPrecondition.Unprotected _ | AccessPrecondition.TotallyUnprotected) ) -> , (AccessPrecondition.Unprotected _ | AccessPrecondition.TotallyUnprotected) ) ->
(* 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 *)
let is_cpp_protected_write pre = let is_cpp_protected_write pre =
match pre with match pre with
| AccessPrecondition.Unprotected _ | TotallyUnprotected -> | AccessPrecondition.Unprotected _ | TotallyUnprotected ->
Typ.Procname.is_java pname Typ.Procname.is_java pname
| AccessPrecondition.Protected _ -> | AccessPrecondition.Protected _ ->
true true
in in
let is_conflict other_access pre other_thread = let is_conflict other_access pre other_thread =
TraceElem.is_write other_access TraceElem.is_write other_access
&& &&
if Typ.Procname.is_java pname then if Typ.Procname.is_java pname then
ThreadsDomain.is_any threads || ThreadsDomain.is_any other_thread ThreadsDomain.is_any threads || ThreadsDomain.is_any other_thread
else is_cpp_protected_write pre else is_cpp_protected_write pre
in in
let all_writes = let all_writes =
List.filter List.filter
~f:(fun {access= other_access; precondition; threads= other_threads} -> ~f:(fun {access= other_access; precondition; threads= other_threads} ->
is_conflict other_access precondition other_threads ) is_conflict other_access precondition other_threads )
accesses accesses
in in
if not (List.is_empty all_writes) then if not (List.is_empty all_writes) then
let conflict = List.hd_exn all_writes in let conflict = List.hd_exn all_writes in
report_thread_safety_violation tenv procdesc report_thread_safety_violation tenv procdesc
~make_description:(make_read_write_race_description ~read_is_sync:false conflict) ~make_description:(make_read_write_race_description ~read_is_sync:false conflict)
~report_kind:(ReadWriteRace conflict.access) access threads ~report_kind:(ReadWriteRace conflict.access) access threads ;
| (Access.Read _ | ContainerRead _), AccessPrecondition.Protected excl -> update_reported access pname reported_acc
(* protected read. report unprotected writes and opposite protected writes as conflicts else reported_acc
| (Access.Read _ | ContainerRead _), AccessPrecondition.Protected excl ->
(* protected read. report unprotected writes and opposite protected writes as conflicts
Thread and Lock are opposites of one another, and Both has no opposite *) Thread and Lock are opposites of one another, and Both has no opposite *)
let is_opposite = function let is_opposite = function
| Excluder.Lock, Excluder.Thread -> | Excluder.Lock, Excluder.Thread ->
true true
| Excluder.Thread, Excluder.Lock -> | Excluder.Thread, Excluder.Lock ->
true true
| _, _ -> | _, _ ->
false false
in in
let conflicting_writes = let conflicting_writes =
List.filter List.filter
~f:(fun {access; precondition; threads= other_threads} -> ~f:(fun {access; precondition; threads= other_threads} ->
match precondition with match precondition with
| AccessPrecondition.Unprotected _ -> | AccessPrecondition.Unprotected _ ->
TraceElem.is_write access && ThreadsDomain.is_any other_threads TraceElem.is_write access && ThreadsDomain.is_any other_threads
| AccessPrecondition.Protected other_excl when is_opposite (excl, other_excl) -> | AccessPrecondition.Protected other_excl when is_opposite (excl, other_excl) ->
TraceElem.is_write access TraceElem.is_write access
| _ -> | _ ->
false ) false )
accesses accesses
in in
if not (List.is_empty conflicting_writes) then if not (List.is_empty conflicting_writes) then
let conflict = List.hd_exn conflicting_writes in let conflict = List.hd_exn conflicting_writes in
(* protected read with conflicting unprotected write(s). warn. *) (* protected read with conflicting unprotected write(s). warn. *)
report_thread_safety_violation tenv procdesc report_thread_safety_violation tenv procdesc
~make_description:(make_read_write_race_description ~read_is_sync:true conflict) ~make_description:(make_read_write_race_description ~read_is_sync:true conflict)
~report_kind:(ReadWriteRace conflict.access) access threads ~report_kind:(ReadWriteRace conflict.access) access threads ;
update_reported access pname reported_acc
else reported_acc
in in
AccessListMap.iter AccessListMap.fold
(fun _ (grouped_accesses: reported_access list) -> (fun _ (grouped_accesses: reported_access list) reported_acc ->
(* reset the reported reads and writes for each memory location *)
let reported =
{ reported_acc with
reported_writes= Typ.Procname.Set.empty; reported_reads= Typ.Procname.Set.empty }
in
let class_has_mutex_member objc_cpp tenv = let class_has_mutex_member objc_cpp tenv =
let class_name = Typ.Procname.objc_cpp_get_class_type_name objc_cpp in let class_name = Typ.Procname.objc_cpp_get_class_type_name objc_cpp in
let matcher = QualifiedCppName.Match.of_fuzzy_qual_names ["std::mutex"] in let matcher = QualifiedCppName.Match.of_fuzzy_qual_names ["std::mutex"] in
@ -1459,10 +1518,10 @@ let report_unsafe_accesses (aggregated_access_map: reported_access list AccessLi
let reportable_accesses = let reportable_accesses =
List.filter ~f:(fun {tenv; procdesc} -> should_report procdesc tenv) grouped_accesses List.filter ~f:(fun {tenv; procdesc} -> should_report procdesc tenv) grouped_accesses
in in
List.iter List.fold
~f:(fun access -> report_unsafe_access access reportable_accesses) ~f:(fun acc access -> report_unsafe_access access reportable_accesses acc)
reportable_accesses ) reportable_accesses ~init:reported )
aggregated_access_map aggregated_access_map empty_reported
|> ignore |> ignore

Loading…
Cancel
Save