[starvation] report using split events

Summary:
Use the component of the abstract state `events` to report. This set contains reachability facts about blocking calls and lock acquisitions.

The other component, `order`, contained pairs of a reachable event `e'` from an event option with the semantics that if the option is `None` then we have an element that now goes into `events`, and if the option is `Some e` then the element represents a lock acquired and a trace *from* `e` to `e'`

Now, `order` can be simplified to proper pairs of events, without the option.

Reviewed By: jvillard

Differential Revision: D8227665

fbshipit-source-id: e6f4dac
master
Nikos Gorogiannis 7 years ago committed by Facebook Github Bot
parent 8bda23fadc
commit 971cd84455

@ -132,20 +132,6 @@ let analyze_procedure {Callbacks.proc_desc; tenv; summary} =
|> Option.value_map ~default:summary ~f:(fun astate -> Payload.update_summary astate summary) |> Option.value_map ~default:summary ~f:(fun astate -> Payload.update_summary astate summary)
let make_trace_with_header ?(header= "") elem pname =
let trace = StarvationDomain.Order.make_loc_trace elem in
let trace_descr = Format.asprintf "%s %a" header (MF.wrap_monospaced Typ.Procname.pp) pname in
let start_loc =
List.hd trace |> Option.value_map ~default:Location.dummy ~f:(fun lt -> lt.Errlog.lt_loc)
in
Errlog.make_trace_element 0 start_loc trace_descr [] :: trace
let make_loc_trace pname trace_id elem =
let header = Printf.sprintf "[Trace %d] " trace_id in
make_trace_with_header ~header elem pname
let get_summaries_of_methods_in_class tenv clazz = let get_summaries_of_methods_in_class tenv clazz =
let tstruct_opt = Tenv.lookup tenv clazz in let tstruct_opt = Tenv.lookup tenv clazz in
let methods = let methods =
@ -218,21 +204,21 @@ end
let should_report_deadlock_on_current_proc current_elem endpoint_elem = let should_report_deadlock_on_current_proc current_elem endpoint_elem =
let open StarvationDomain in let open StarvationDomain in
match (current_elem.Order.first, current_elem.Order.eventually) with match (current_elem.Order.first, current_elem.Order.eventually) with
| None, _ | Some {Event.elem= MayBlock _}, _ | _, {Event.elem= MayBlock _} -> | {Event.elem= MayBlock _}, _ | _, {Event.elem= MayBlock _} ->
(* should never happen *) (* should never happen *)
L.die InternalError "Deadlock cannot occur without two lock events: %a" Order.pp current_elem L.die InternalError "Deadlock cannot occur without two lock events: %a" Order.pp current_elem
| Some {Event.elem= LockAcquire ((Var.LogicalVar _, _), [])}, _ -> | {Event.elem= LockAcquire ((Var.LogicalVar _, _), [])}, _ ->
(* first elem is a class object (see [lock_of_class]), so always report because the (* first elem is a class object (see [lock_of_class]), so always report because the
reverse ordering on the events will not occur *) reverse ordering on the events will not occur *)
true true
| Some {Event.elem= LockAcquire ((Var.LogicalVar _, _), _ :: _)}, _ | {Event.elem= LockAcquire ((Var.LogicalVar _, _), _ :: _)}, _
| _, {Event.elem= LockAcquire ((Var.LogicalVar _, _), _)} -> | _, {Event.elem= LockAcquire ((Var.LogicalVar _, _), _)} ->
(* first elem has an ident root, but has a non-empty access path, which means we are (* first elem has an ident root, but has a non-empty access path, which means we are
not filtering out local variables (see [exec_instr]), or, not filtering out local variables (see [exec_instr]), or,
second elem has an ident root, which should not happen if we are filtering locals *) second elem has an ident root, which should not happen if we are filtering locals *)
L.die InternalError "Deadlock cannot occur on these logical variables: %a @." Order.pp L.die InternalError "Deadlock cannot occur on these logical variables: %a @." Order.pp
current_elem current_elem
| Some {Event.elem= LockAcquire ((_, typ1), _)}, {Event.elem= LockAcquire ((_, typ2), _)} -> | {Event.elem= LockAcquire ((_, typ1), _)}, {Event.elem= LockAcquire ((_, typ2), _)} ->
(* use string comparison on types as a stable order to decide whether to report a deadlock *) (* use string comparison on types as a stable order to decide whether to report a deadlock *)
let c = String.compare (Typ.to_string typ1) (Typ.to_string typ2) in let c = String.compare (Typ.to_string typ1) (Typ.to_string typ2) in
c < 0 c < 0
@ -271,9 +257,9 @@ let report_deadlocks tenv current_pdesc {StarvationDomain.order; ui} report_map'
(MF.wrap_monospaced Typ.Procname.pp) (MF.wrap_monospaced Typ.Procname.pp)
endpoint_pname Order.pp elem endpoint_pname Order.pp elem
in in
let first_trace = List.rev (make_loc_trace current_pname 1 current_elem) in let first_trace = Order.make_trace ~header:"[Trace 1] " current_pname current_elem in
let second_trace = make_loc_trace endpoint_pname 2 elem in let second_trace = Order.make_trace ~header:"[Trace 2] " endpoint_pname elem in
let ltr = List.rev_append first_trace second_trace in let ltr = first_trace @ second_trace in
let loc = Order.get_loc current_elem in let loc = Order.get_loc current_elem in
ReportMap.add_deadlock current_pname loc ltr error_message report_map ReportMap.add_deadlock current_pname loc ltr error_message report_map
| _, _ -> | _, _ ->
@ -281,7 +267,7 @@ let report_deadlocks tenv current_pdesc {StarvationDomain.order; ui} report_map'
in in
let report_on_current_elem elem report_map = let report_on_current_elem elem report_map =
match elem with match elem with
| {Order.first= None} | {Order.eventually= {Event.elem= Event.MayBlock _}} -> | {Order.eventually= {Event.elem= Event.MayBlock _}} ->
report_map report_map
| {Order.eventually= {Event.elem= Event.LockAcquire endpoint_lock}} -> | {Order.eventually= {Event.elem= Event.LockAcquire endpoint_lock}} ->
Lock.owner_class endpoint_lock Lock.owner_class endpoint_lock
@ -301,14 +287,12 @@ let report_deadlocks tenv current_pdesc {StarvationDomain.order; ui} report_map'
OrderDomain.fold report_on_current_elem order report_map' OrderDomain.fold report_on_current_elem order report_map'
let report_starvation tenv current_pdesc {StarvationDomain.order; ui} report_map' = let report_starvation tenv current_pdesc {StarvationDomain.events; ui} report_map' =
let open StarvationDomain in let open StarvationDomain in
let current_pname = Procdesc.get_proc_name current_pdesc in let current_pname = Procdesc.get_proc_name current_pdesc in
let report_remote_block ui_explain current_elem current_lock endpoint_pname endpoint_elem let report_remote_block ui_explain event current_lock endpoint_pname endpoint_elem report_map =
report_map = match (endpoint_elem.Order.first, endpoint_elem.Order.eventually) with
match endpoint_elem with | {Event.elem= Event.LockAcquire lock}, {Event.elem= Event.MayBlock (block_descr, sev)}
| { Order.first= Some {Event.elem= Event.LockAcquire lock}
; eventually= {Event.elem= Event.MayBlock (block_descr, sev)} }
when Lock.equal current_lock lock -> when Lock.equal current_lock lock ->
let error_message = let error_message =
Format.asprintf Format.asprintf
@ -317,26 +301,26 @@ let report_starvation tenv current_pdesc {StarvationDomain.order; ui} report_map
(MF.wrap_monospaced Typ.Procname.pp) (MF.wrap_monospaced Typ.Procname.pp)
current_pname ui_explain Lock.pp lock block_descr current_pname ui_explain Lock.pp lock block_descr
in in
let first_trace = List.rev (make_loc_trace current_pname 1 current_elem) in let first_trace = Event.make_trace ~header:"[Trace 1] " current_pname event in
let second_trace = make_loc_trace endpoint_pname 2 endpoint_elem in let second_trace = Order.make_trace ~header:"[Trace 2] " endpoint_pname endpoint_elem in
let ltr = List.rev_append first_trace second_trace in let ltr = first_trace @ second_trace in
let loc = Order.get_loc current_elem in let loc = Event.get_loc event in
ReportMap.add_starvation sev current_pname loc ltr error_message report_map ReportMap.add_starvation sev current_pname loc ltr error_message report_map
| _ -> | _ ->
report_map report_map
in in
let report_on_current_elem ui_explain ({Order.eventually} as elem) report_map = let report_on_current_elem ui_explain event report_map =
match eventually with match event.Event.elem with
| {Event.elem= Event.MayBlock (_, sev)} -> | Event.MayBlock (_, sev) ->
let error_message = let error_message =
Format.asprintf "Method %a runs on UI thread (because %s), and may block; %a." Format.asprintf "Method %a runs on UI thread (because %s), and may block; %a."
(MF.wrap_monospaced Typ.Procname.pp) (MF.wrap_monospaced Typ.Procname.pp)
current_pname ui_explain Event.pp_event eventually.Event.elem current_pname ui_explain Event.pp_no_trace event
in in
let loc = Order.get_loc elem in let loc = Event.get_loc event in
let ltr = make_trace_with_header elem current_pname in let ltr = Event.make_trace current_pname event in
ReportMap.add_starvation sev current_pname loc ltr error_message report_map ReportMap.add_starvation sev current_pname loc ltr error_message report_map
| {Event.elem= Event.LockAcquire endpoint_lock} -> | Event.LockAcquire endpoint_lock ->
Lock.owner_class endpoint_lock Lock.owner_class endpoint_lock
|> Option.value_map ~default:report_map ~f:(fun endpoint_class -> |> Option.value_map ~default:report_map ~f:(fun endpoint_class ->
(* get the class of the root variable of the lock in the endpoint elem (* get the class of the root variable of the lock in the endpoint elem
@ -348,7 +332,7 @@ let report_starvation tenv current_pdesc {StarvationDomain.order; ui} report_map
(* skip methods known to run on ui thread, as they cannot run in parallel to us *) (* skip methods known to run on ui thread, as they cannot run in parallel to us *)
if UIThreadDomain.is_empty ui then if UIThreadDomain.is_empty ui then
OrderDomain.fold OrderDomain.fold
(report_remote_block ui_explain elem endpoint_lock endpoint_pname) (report_remote_block ui_explain event endpoint_lock endpoint_pname)
order acc order acc
else acc ) ) else acc ) )
in in
@ -356,7 +340,7 @@ let report_starvation tenv current_pdesc {StarvationDomain.order; ui} report_map
| AbstractDomain.Types.Bottom -> | AbstractDomain.Types.Bottom ->
report_map' report_map'
| AbstractDomain.Types.NonBottom ui_explain -> | AbstractDomain.Types.NonBottom ui_explain ->
OrderDomain.fold (report_on_current_elem ui_explain) order report_map' EventDomain.fold (report_on_current_elem ui_explain) events report_map'
let reporting {Callbacks.procedures; exe_env} = let reporting {Callbacks.procedures; exe_env} =

@ -57,13 +57,17 @@ module type TraceElem = sig
include PrettyPrintable.PrintableOrderedType with type t := t include PrettyPrintable.PrintableOrderedType with type t := t
val pp_no_trace : F.formatter -> t -> unit
val make : elem_t -> Location.t -> t val make : elem_t -> Location.t -> t
val get_loc : t -> Location.t val get_loc : t -> Location.t
val make_loc_trace : ?reverse:bool -> t -> Errlog.loc_trace val make_loc_trace : t -> Errlog.loc_trace
val with_callsite : t -> CallSite.t -> t val with_callsite : t -> CallSite.t -> t
val make_trace : ?header:string -> Typ.Procname.t -> t -> Errlog.loc_trace
end end
module MakeTraceElem (Elem : PrettyPrintable.PrintableOrderedType) : module MakeTraceElem (Elem : PrettyPrintable.PrintableOrderedType) :
@ -74,6 +78,8 @@ struct
type t = {elem: Elem.t; loc: Location.t; trace: CallSite.t list [@compare.ignore]} type t = {elem: Elem.t; loc: Location.t; trace: CallSite.t list [@compare.ignore]}
[@@deriving compare] [@@deriving compare]
let pp_no_trace fmt {elem; loc} = F.fprintf fmt "%a at %a" Elem.pp elem Location.pp loc
let pp fmt e = let pp fmt e =
let pp_trace fmt = function let pp_trace fmt = function
| [] -> | [] ->
@ -81,14 +87,14 @@ struct
| trace -> | trace ->
F.fprintf fmt " (trace: %a)" (Pp.semicolon_seq CallSite.pp) trace F.fprintf fmt " (trace: %a)" (Pp.semicolon_seq CallSite.pp) trace
in in
F.fprintf fmt "%a at %a%a" Elem.pp e.elem Location.pp e.loc pp_trace e.trace F.fprintf fmt "%a%a" pp_no_trace e pp_trace e.trace
let make elem loc = {elem; loc; trace= []} let make elem loc = {elem; loc; trace= []}
let get_loc {loc; trace} = List.hd trace |> Option.value_map ~default:loc ~f:CallSite.loc let get_loc {loc; trace} = List.hd trace |> Option.value_map ~default:loc ~f:CallSite.loc
let make_loc_trace ?(reverse= false) e = let make_loc_trace e =
let call_trace, nesting = let call_trace, nesting =
List.fold e.trace ~init:([], 0) ~f:(fun (tr, ns) callsite -> List.fold e.trace ~init:([], 0) ~f:(fun (tr, ns) callsite ->
let elem_descr = let elem_descr =
@ -101,11 +107,17 @@ struct
in in
let endpoint_descr = F.asprintf "%a" Elem.pp e.elem in let endpoint_descr = F.asprintf "%a" Elem.pp e.elem in
let endpoint = Errlog.make_trace_element nesting e.loc endpoint_descr [] in let endpoint = Errlog.make_trace_element nesting e.loc endpoint_descr [] in
let res = endpoint :: call_trace in List.rev (endpoint :: call_trace)
if reverse then res else List.rev res
let with_callsite elem callsite = {elem with trace= callsite :: elem.trace} let with_callsite elem callsite = {elem with trace= callsite :: elem.trace}
let make_trace ?(header= "") pname elem =
let trace = make_loc_trace elem in
let trace_descr = Format.asprintf "%s%a" header (MF.wrap_monospaced Typ.Procname.pp) pname in
let start_loc = get_loc elem in
let header_step = Errlog.make_trace_element 0 start_loc trace_descr [] in
header_step :: trace
end end
module Event = struct module Event = struct
@ -113,29 +125,18 @@ module Event = struct
type event_t = LockAcquire of Lock.t | MayBlock of (string * severity_t) [@@deriving compare] type event_t = LockAcquire of Lock.t | MayBlock of (string * severity_t) [@@deriving compare]
let pp_event fmt = function include MakeTraceElem (struct
type t = event_t [@@deriving compare]
let pp fmt = function
| LockAcquire lock -> | LockAcquire lock ->
Lock.pp fmt lock Lock.pp fmt lock
| MayBlock (msg, _) -> | MayBlock (msg, _) ->
F.pp_print_string fmt msg F.pp_print_string fmt msg
include MakeTraceElem (struct
type t = event_t [@@deriving compare]
let pp = pp_event
end) end)
let is_lock_event e = match e.elem with LockAcquire _ -> true | _ -> false let is_lock_event e = match e.elem with LockAcquire _ -> true | _ -> false
let locks_equal e e' =
match (e.elem, e'.elem) with
| LockAcquire lock, LockAcquire lock' ->
Lock.equal lock lock'
| _, _ ->
false
let locks_equal_modulo_base e e' = let locks_equal_modulo_base e e' =
match (e.elem, e'.elem) with match (e.elem, e'.elem) with
| LockAcquire lock, LockAcquire lock' -> | LockAcquire lock, LockAcquire lock' ->
@ -165,56 +166,45 @@ module EventDomain = struct
end end
module Order = struct module Order = struct
type t = {first: Event.t option; eventually: Event.t} [@@deriving compare] type t = {first: Event.t; eventually: Event.t} [@@deriving compare]
let pp fmt o = let pp fmt {first; eventually} =
match o.first with F.fprintf fmt "first %a, and before releasing it, %a" Event.pp first Event.pp eventually
| None ->
F.fprintf fmt "eventually %a" Event.pp o.eventually
| Some lock ->
F.fprintf fmt "first %a, and before releasing it, %a" Event.pp lock Event.pp o.eventually
let may_deadlock elem elem' = let may_deadlock {first; eventually} {first= first'; eventually= eventually'} =
match (elem.first, elem'.first) with Event.locks_equal_modulo_base first eventually'
| Some b, Some b' -> && Event.locks_equal_modulo_base first' eventually
Event.locks_equal_modulo_base b elem'.eventually
&& Event.locks_equal_modulo_base b' elem.eventually
| _, _ ->
false
let make_eventually eventually = {first= None; eventually} let make first eventually =
if not (Event.is_lock_event first) then L.(die InternalError) "Expected a lock elem first." ;
{first; eventually}
let make_first_and_eventually b eventually =
if not (Event.is_lock_event b) then L.(die InternalError) "Expected a lock elem first." ;
{first= Some b; eventually}
let with_callsite o callsite = {o with first= Event.with_callsite o.first callsite}
let with_callsite callsite o = {o with eventually= Event.with_callsite o.eventually callsite} let get_loc {first} = Event.get_loc first
let get_loc {first; eventually} = let make_loc_trace {first; eventually} =
match first with Some elem -> Event.get_loc elem | None -> Event.get_loc eventually let first_trace = Event.make_loc_trace first in
let eventually_trace = Event.make_loc_trace eventually in
first_trace @ eventually_trace
let make_loc_trace o = let make_trace ?(header= "") pname elem =
let first_trace = let trace = make_loc_trace elem in
Option.value_map o.first ~default:[] ~f:(Event.make_loc_trace ~reverse:true) let trace_descr = Format.asprintf "%s%a" header (MF.wrap_monospaced Typ.Procname.pp) pname in
in let start_loc = get_loc elem in
let eventually_trace = Event.make_loc_trace o.eventually in let header_step = Errlog.make_trace_element 0 start_loc trace_descr [] in
List.rev_append first_trace eventually_trace header_step :: trace
end end
module OrderDomain = struct module OrderDomain = struct
include AbstractDomain.FiniteSet (Order) include AbstractDomain.FiniteSet (Order)
let with_callsite callsite lo = let with_callsite lo callsite =
fold (fun o acc -> add (Order.with_callsite callsite o) acc) lo empty fold (fun o acc -> add (Order.with_callsite o callsite) acc) lo empty
let is_eventually_locked lock lo =
Event.is_lock_event lock
&& exists (fun pair -> Event.locks_equal pair.Order.eventually lock) lo
end end
module LockStack = AbstractDomain.StackDomain (Event) module LockStack = AbstractDomain.StackDomain (Event)
@ -302,30 +292,23 @@ let ( <= ) ~lhs ~rhs =
&& LockState.( <= ) ~lhs:lhs.lock_state ~rhs:rhs.lock_state && LockState.( <= ) ~lhs:lhs.lock_state ~rhs:rhs.lock_state
(* for every lock b held locally, add a pair (b, lock_event), plus (None, lock_event) *) (* for every lock b held locally, add a pair (b, event) *)
let add_order_pairs order lock_event acc = let add_order_pairs lock_state event acc =
(* add no pairs whatsoever if we already hold that lock *) (* add no pairs whatsoever if we already hold that lock *)
if LockState.is_taken lock_event order then acc if LockState.is_taken event lock_state then acc
else
let add_eventually acc =
(* don't add an eventually-locks pair if there is already another with same endpoint*)
if OrderDomain.is_eventually_locked lock_event acc then acc
else else
let elem = Order.make_eventually lock_event in
OrderDomain.add elem acc
in
let add_first_and_eventually acc first = let add_first_and_eventually acc first =
(* never add a pair of the form (a,a) -- should never happen due to the check above *) (* never add a pair of the form (a,a) -- should never happen due to the check above *)
let elem = Order.make_first_and_eventually first lock_event in let elem = Order.make first event in
OrderDomain.add elem acc OrderDomain.add elem acc
in in
LockState.fold_over_events add_first_and_eventually order acc |> add_eventually LockState.fold_over_events add_first_and_eventually lock_state acc
let acquire ({lock_state; events; order} as astate) loc lockid = let acquire ({lock_state; events; order} as astate) loc lock =
let new_event = Event.make_acquire lockid loc in let new_event = Event.make_acquire lock loc in
{ astate with { astate with
lock_state= LockState.acquire lockid new_event lock_state lock_state= LockState.acquire lock new_event lock_state
; events= EventDomain.add new_event events ; events= EventDomain.add new_event events
; order= add_order_pairs lock_state new_event order } ; order= add_order_pairs lock_state new_event order }
@ -336,27 +319,19 @@ let blocking_call ~caller ~callee sev loc ({lock_state; events; order} as astate
events= EventDomain.add new_event events; order= add_order_pairs lock_state new_event order } events= EventDomain.add new_event events; order= add_order_pairs lock_state new_event order }
let release ({lock_state} as astate) lockid = let release ({lock_state} as astate) lock =
{astate with lock_state= LockState.release lockid lock_state} {astate with lock_state= LockState.release lock lock_state}
let integrate_summary ({lock_state; events; order; ui} as astate) callee_pname loc callee_summary = let integrate_summary ({lock_state; events; order; ui} as astate) callee_pname loc callee_summary =
let callee_order = callee_summary.order in
let callee_ui = callee_summary.ui in
let callee_events = callee_summary.events in
(* for each pair (b,a) in the callee, add (l,b) and (l,a) to the current state, where
l is held locally *)
let do_elem elem acc =
Option.value_map elem.Order.first ~default:acc ~f:(fun b -> add_order_pairs lock_state b acc)
|> add_order_pairs lock_state elem.Order.eventually
in
let callsite = CallSite.make callee_pname loc in let callsite = CallSite.make callee_pname loc in
(* add callsite to the "eventually" trace *) let callee_order = OrderDomain.with_callsite callee_summary.order callsite in
let elems = OrderDomain.with_callsite callsite callee_order in let callee_events = EventDomain.with_callsite callee_summary.events callsite in
let order' = EventDomain.fold (add_order_pairs lock_state) callee_events callee_order in
{ astate with { astate with
events= EventDomain.join events (EventDomain.with_callsite callee_events callsite) events= EventDomain.join events callee_events
; order= OrderDomain.fold do_elem elems order ; order= OrderDomain.join order order'
; ui= UIThreadDomain.join ui callee_ui } ; ui= UIThreadDomain.join ui callee_summary.ui }
let set_on_ui_thread ({ui} as astate) explain = let set_on_ui_thread ({ui} as astate) explain =

@ -25,11 +25,15 @@ module Event : sig
type event_t = LockAcquire of Lock.t | MayBlock of (string * severity_t) [@@deriving compare] type event_t = LockAcquire of Lock.t | MayBlock of (string * severity_t) [@@deriving compare]
val pp_event : F.formatter -> event_t -> unit
type t = private {elem: event_t; loc: Location.t; trace: CallSite.t list} type t = private {elem: event_t; loc: Location.t; trace: CallSite.t list}
include PrettyPrintable.PrintableOrderedType with type t := t include PrettyPrintable.PrintableOrderedType with type t := t
val pp_no_trace : F.formatter -> t -> unit
val get_loc : t -> Location.t
val make_trace : ?header:string -> Typ.Procname.t -> t -> Errlog.loc_trace
end end
module EventDomain : module type of AbstractDomain.FiniteSet (Event) module EventDomain : module type of AbstractDomain.FiniteSet (Event)
@ -40,7 +44,7 @@ module EventDomain : module type of AbstractDomain.FiniteSet (Event)
- the "first" lock being taken *in the current method* and, before its release, the eventual - the "first" lock being taken *in the current method* and, before its release, the eventual
acquisition of "eventually" *) acquisition of "eventually" *)
module Order : sig module Order : sig
type t = private {first: Event.t option; eventually: Event.t} type t = private {first: Event.t; eventually: Event.t}
include PrettyPrintable.PrintableOrderedType with type t := t include PrettyPrintable.PrintableOrderedType with type t := t
@ -48,9 +52,9 @@ module Order : sig
(** check if two pairs are symmetric in terms of locks, where locks are compared modulo the (** check if two pairs are symmetric in terms of locks, where locks are compared modulo the
variable name at the root of each path. *) variable name at the root of each path. *)
val make_loc_trace : t -> Errlog.loc_trace
val get_loc : t -> Location.t val get_loc : t -> Location.t
val make_trace : ?header:string -> Typ.Procname.t -> t -> Errlog.loc_trace
end end
module OrderDomain : sig module OrderDomain : sig

Loading…
Cancel
Save