[starvation][concurrency] split guard lock treatment and add support for non-recursive locks (per language)

Reviewed By: jeremydubreil

Differential Revision: D13320542

fbshipit-source-id: feb5d58a8
master
Nikos Gorogiannis 6 years ago committed by Facebook Github Bot
parent a37d85dddc
commit 27d8a65906

@ -8,11 +8,17 @@
open! IStd
module F = Format
module MF = MarkupFormatter
module L = Logging
type lock_effect =
| Lock of HilExp.t list
| Unlock of HilExp.t list
| LockedIfTrue of HilExp.t list
| GuardConstruct of {guard: HilExp.t; lock: HilExp.t; acquire_now: bool}
| GuardLock of HilExp.t
| GuardLockedIfTrue of HilExp.t
| GuardUnlock of HilExp.t
| GuardDestroy of HilExp.t
| NoEffect
type thread = BackgroundThread | MainThread | MainThreadIfTrue | UnknownThread
@ -77,96 +83,142 @@ end = struct
; {shd with cls= "folly::SharedMutex"}
; {shd with cls= "folly::SharedMutexImpl"}
; {def with cls= "folly::SpinLock"}
; {def with cls= "std::shared_lock"}
; {def with cls= "std::mutex"}
; shd
; {def with cls= "std::unique_lock"; tlk= "owns_lock" :: def.tlk} ]
; shd ]
let mk_model_matcher ~f =
let lock_methods =
List.concat_map lock_models ~f:(fun mdl ->
List.map (f mdl) ~f:(fun mtd -> mdl.cls ^ "::" ^ mtd) )
in
let matcher = QualifiedCppName.Match.of_fuzzy_qual_names lock_methods in
let mk_matcher methods =
let matcher = QualifiedCppName.Match.of_fuzzy_qual_names methods in
fun pname ->
QualifiedCppName.Match.match_qualifiers matcher (Typ.Procname.get_qualifiers pname)
let is_lock = mk_model_matcher ~f:(fun mdl -> mdl.lck)
let is_unlock = mk_model_matcher ~f:(fun mdl -> mdl.unl)
let is_lock, is_unlock, is_trylock, is_std_lock =
(* TODO std::try_lock *)
let mk_model_matcher ~f =
let lock_methods =
List.concat_map lock_models ~f:(fun mdl ->
List.map (f mdl) ~f:(fun mtd -> mdl.cls ^ "::" ^ mtd) )
in
mk_matcher lock_methods
in
( mk_model_matcher ~f:(fun mdl -> mdl.lck)
, mk_model_matcher ~f:(fun mdl -> mdl.unl)
, mk_model_matcher ~f:(fun mdl -> mdl.tlk)
, mk_matcher ["std::lock"] )
let is_trylock = mk_model_matcher ~f:(fun mdl -> mdl.tlk)
let lock_types_matcher =
let class_names = List.map lock_models ~f:(fun mdl -> mdl.cls) in
QualifiedCppName.Match.of_fuzzy_qual_names class_names
(* TODO std::scoped_lock *)
(** C++ guard classes used for scope-based lock management.
NB we pretend all classes below implement the mutex interface even though only
[shared_lock] and [unique_lock] do, for simplicity. The comments summarise which
methods are implemented. *)
let guards =
[ "apache::thrift::concurrency::Guard"
; "apache::thrift::concurrency::RWGuard"
; "folly::SharedMutex::ReadHolder"
; "folly::SharedMutex::WriteHolder"
; "folly::LockedPtr"
; "folly::SpinLockGuard"
; "std::lock_guard"
; "std::shared_lock"
; "std::unique_lock" ]
let get_guard_constructor, get_guard_destructor =
(* TODO std::scoped_lock *)
[ (* no lock/unlock *)
"apache::thrift::concurrency::Guard"
; (* no lock/unlock *)
"apache::thrift::concurrency::RWGuard"
; (* only unlock *)
"folly::SharedMutex::ReadHolder"
; (* only unlock *)
"folly::SharedMutex::WriteHolder"
; (* read/write locks under operator() etc *)
"folly::LockedPtr"
; (* no lock/unlock *)
"folly::SpinLockGuard"
; (* no lock/unlock *)
"std::lock_guard"
; (* everything *)
"std::shared_lock"
; (* everything *)
"std::unique_lock" ]
let ( get_guard_constructor
, get_guard_destructor
, get_guard_lock
, get_guard_unlock
, get_guard_trylock ) =
let get_class_and_qual_name guard =
let qual_name = QualifiedCppName.of_qual_string guard in
let class_name, _ = Option.value_exn (QualifiedCppName.extract_last qual_name) in
(class_name, qual_name)
in
( (fun guard ->
let class_name, qual_name = get_class_and_qual_name guard in
let qual_constructor = QualifiedCppName.append_qualifier qual_name ~qual:class_name in
QualifiedCppName.to_qual_string qual_constructor )
, fun guard ->
let class_name, qual_name = get_class_and_qual_name guard in
let qual_destructor =
QualifiedCppName.append_qualifier qual_name ~qual:("~" ^ class_name)
in
QualifiedCppName.to_qual_string qual_destructor )
let is_guard_lock =
let constructors = List.map guards ~f:get_guard_constructor in
let matcher = QualifiedCppName.Match.of_fuzzy_qual_names constructors in
fun pname actuals ->
QualifiedCppName.Match.match_qualifiers matcher (Typ.Procname.get_qualifiers pname)
(* Passing additional parameter allows to defer the lock *)
&& Int.equal 2 (List.length actuals)
let is_guard_unlock =
let destructors = List.map guards ~f:get_guard_destructor in
let matcher = QualifiedCppName.Match.of_fuzzy_qual_names destructors in
fun pname ->
QualifiedCppName.Match.match_qualifiers matcher (Typ.Procname.get_qualifiers pname)
let make_with_classname ~f guard =
let class_name, qual_name = get_class_and_qual_name guard in
let qual = f class_name in
let qual_constructor = QualifiedCppName.append_qualifier qual_name ~qual in
QualifiedCppName.to_qual_string qual_constructor
in
let make_lock_unlock ~mthd guard =
let qual_name = QualifiedCppName.of_qual_string guard in
let qual_mthd = QualifiedCppName.append_qualifier qual_name ~qual:mthd in
QualifiedCppName.to_qual_string qual_mthd
in
let make_trylock ~mthds guard =
let qual_name = QualifiedCppName.of_qual_string guard in
List.map mthds ~f:(fun qual ->
QualifiedCppName.append_qualifier qual_name ~qual |> QualifiedCppName.to_qual_string )
in
( make_with_classname ~f:(fun class_name -> class_name)
, make_with_classname ~f:(fun class_name -> "~" ^ class_name)
, make_lock_unlock ~mthd:"lock"
, make_lock_unlock ~mthd:"unlock"
, make_trylock ~mthds:["try_lock"; "owns_lock"] )
let is_std_lock =
let matcher = QualifiedCppName.Match.of_fuzzy_qual_names ["std::lock"] in
fun pname ->
QualifiedCppName.Match.match_qualifiers matcher (Typ.Procname.get_qualifiers pname)
let is_guard_constructor, is_guard_destructor, is_guard_unlock, is_guard_lock, is_guard_trylock =
let make ~f =
let constructors = List.map guards ~f in
mk_matcher constructors
in
let make_trylock ~f =
let methods = List.concat_map guards ~f in
mk_matcher methods
in
( make ~f:get_guard_constructor
, make ~f:get_guard_destructor
, make ~f:get_guard_unlock
, make ~f:get_guard_lock
, make_trylock ~f:get_guard_trylock )
let get_lock_effect pname actuals =
let log_parse_error error =
L.internal_error "%s pname:%a actuals:%a@." error Typ.Procname.pp pname
(PrettyPrintable.pp_collection ~pp_item:HilExp.pp)
actuals
in
let guard_action ~f ~error =
match actuals with [guard] -> f guard | _ -> log_parse_error error ; NoEffect
in
let fst_arg = match actuals with x :: _ -> [x] | _ -> [] in
let snd_arg = match actuals with _ :: x :: _ -> [x] | _ -> [] in
if is_std_lock pname then Lock actuals
else if is_lock pname then Lock fst_arg
else if is_guard_lock pname actuals then Lock snd_arg
else if is_unlock pname then Unlock fst_arg
else if is_guard_unlock pname then Unlock snd_arg
else if is_trylock pname then LockedIfTrue fst_arg
else if is_guard_constructor pname then (
match actuals with
| [guard; lock] ->
GuardConstruct {guard; lock; acquire_now= true}
| [guard; lock; _defer_lock] ->
GuardConstruct {guard; lock; acquire_now= false}
| _ ->
log_parse_error "Cannot parse guard constructor call" ;
NoEffect )
else if is_guard_lock pname then
guard_action ~f:(fun guard -> GuardLock guard) ~error:"Can't parse guard lock"
else if is_guard_unlock pname then
guard_action ~f:(fun guard -> GuardUnlock guard) ~error:"Can't parse guard unlock"
else if is_guard_destructor pname then
guard_action ~f:(fun guard -> GuardDestroy guard) ~error:"Can't parse guard destructor"
else if is_guard_trylock pname then
guard_action ~f:(fun guard -> GuardLockedIfTrue guard) ~error:"Can't parse guard trylock"
else NoEffect
end

@ -10,10 +10,16 @@ module F = Format
(** effect of call plus Hil expressions being un/locked, if known *)
type lock_effect =
| Lock of HilExp.t list
| Unlock of HilExp.t list
| LockedIfTrue of HilExp.t list
| NoEffect
| Lock of HilExp.t list (** simultaneously acquire a list of locks *)
| Unlock of HilExp.t list (** simultaneously release a list of locks *)
| LockedIfTrue of HilExp.t list (** simultaneously attempt to acquire a list of locks *)
| GuardConstruct of {guard: HilExp.t; lock: HilExp.t; acquire_now: bool}
(** mutex guard construction - clang only *)
| GuardLock of HilExp.t (** lock underlying mutex via guard - clang only *)
| GuardLockedIfTrue of HilExp.t (** lock underlying mutex if true via guard - clang only *)
| GuardUnlock of HilExp.t (** unlock underlying mutex via guard - clang only *)
| GuardDestroy of HilExp.t (** destroy guard and unlock underlying mutex - clang only *)
| NoEffect (** function call has no lock-relevant effect *)
type thread = BackgroundThread | MainThread | MainThreadIfTrue | UnknownThread

@ -413,20 +413,22 @@ module TransferFunctions (CFG : ProcCfg.S) = struct
ThreadsDomain.AnyThread
in
match get_lock_effect callee_pname actuals with
| Lock _ ->
| Lock _ | GuardLock _ | GuardConstruct {acquire_now= true} ->
{ astate with
locks= LocksDomain.acquire_lock astate.locks
; threads= update_for_lock_use astate.threads }
| Unlock _ ->
| Unlock _ | GuardDestroy _ | GuardUnlock _ ->
{ astate with
locks= LocksDomain.release_lock astate.locks
; threads= update_for_lock_use astate.threads }
| LockedIfTrue _ ->
| LockedIfTrue _ | GuardLockedIfTrue _ ->
let attribute_map =
AttributeMapDomain.add_attribute ret_access_path (Choice Choice.LockHeld)
astate.attribute_map
in
{astate with attribute_map; threads= update_for_lock_use astate.threads}
| GuardConstruct {acquire_now= false} ->
astate
| NoEffect -> (
let summary_opt = get_summary pdesc callee_pname actuals loc tenv astate in
let callee_pdesc = Ondemand.get_proc_desc callee_pname in

@ -58,6 +58,11 @@ module TransferFunctions (CFG : ProcCfg.S) = struct
let exec_instr (astate : Domain.t) {ProcData.pdesc; tenv; extras} _ (instr : HilInstr.t) =
let open ConcurrencyModels in
let open StarvationModels in
let log_parse_error error pname actuals =
L.internal_error "%s pname:%a actuals:%a@." error Typ.Procname.pp pname
(PrettyPrintable.pp_collection ~pp_item:HilExp.pp)
actuals
in
let get_lock_path = function
| HilExp.AccessExpression access_exp -> (
match AccessExpression.to_access_path access_exp with
@ -73,25 +78,41 @@ module TransferFunctions (CFG : ProcCfg.S) = struct
| _ ->
None
in
let is_java = Procdesc.get_proc_name pdesc |> Typ.Procname.is_java in
let do_lock locks loc astate =
List.filter_map ~f:get_lock_path locks |> Domain.acquire astate loc
List.filter_map ~f:get_lock_path locks |> Domain.acquire ~recursive_locks:is_java astate loc
in
let do_unlock locks astate = List.filter_map ~f:get_lock_path locks |> Domain.release astate in
let do_call callee loc =
let do_call callee loc astate =
Payload.read pdesc callee
|> Option.value_map ~default:astate ~f:(Domain.integrate_summary astate callee loc)
|> Option.value_map ~default:astate
~f:(Domain.integrate_summary ~recursive_locks:is_java astate callee loc)
in
let is_java = Procdesc.get_proc_name pdesc |> Typ.Procname.is_java in
match instr with
| Assign _ | Assume _ | Call (_, Indirect _, _, _, _) | ExitScope _ ->
astate
| Call (_, Direct callee, actuals, _, _) when should_skip_analysis tenv callee actuals ->
astate
| Call (_, Direct callee, actuals, _, loc) -> (
match get_lock_effect callee actuals with
| Lock locks ->
do_lock locks loc astate
| GuardLock guard ->
Domain.lock_guard ~recursive_locks:is_java astate guard loc
| GuardConstruct {guard; lock; acquire_now} -> (
match get_lock_path lock with
| Some lock_path ->
Domain.add_guard astate guard lock_path ~acquire_now ~recursive_locks:false loc
| None ->
log_parse_error "Couldn't parse lock in guard constructor" callee actuals ;
astate )
| Unlock locks ->
do_unlock locks astate
| LockedIfTrue _ ->
astate
| NoEffect when should_skip_analysis tenv callee actuals ->
| GuardUnlock guard ->
Domain.unlock_guard astate guard
| GuardDestroy guard ->
Domain.remove_guard astate guard
| LockedIfTrue _ | GuardLockedIfTrue _ ->
astate
| NoEffect when is_synchronized_library_call tenv callee ->
(* model a synchronized call without visible internal behaviour *)
@ -102,17 +123,14 @@ module TransferFunctions (CFG : ProcCfg.S) = struct
Domain.set_on_ui_thread astate loc explanation
| NoEffect when is_java && StarvationModels.is_strict_mode_violation tenv callee actuals ->
Domain.strict_mode_call callee loc astate
| NoEffect when is_java -> (
match may_block tenv callee actuals with
| Some sev ->
Domain.blocking_call callee sev loc astate
| None ->
do_call callee loc )
| NoEffect ->
(* in C++/Obj C we only care about deadlocks, not starvation errors *)
do_call callee loc )
| _ ->
astate
if is_java then
may_block tenv callee actuals
|> Option.map ~f:(fun sev -> Domain.blocking_call callee sev loc astate)
|> IOption.value_default_f ~f:(fun () -> do_call callee loc astate)
else
(* in C++/Obj C we only care about deadlocks, not starvation errors *)
do_call callee loc astate )
let pp_session_name _node fmt = F.pp_print_string fmt "starvation"
@ -126,6 +144,7 @@ let analyze_procedure {Callbacks.proc_desc; tenv; summary} =
let formals = FormalMap.make proc_desc in
let proc_data = ProcData.make proc_desc tenv formals in
let loc = Procdesc.get_loc proc_desc in
let recursive_locks = Procdesc.get_proc_name proc_desc |> Typ.Procname.is_java in
let initial =
if not (Procdesc.is_java_synchronized proc_desc) then StarvationDomain.empty
else
@ -138,7 +157,7 @@ let analyze_procedure {Callbacks.proc_desc; tenv; summary} =
| _ ->
FormalMap.get_formal_base 0 formals |> Option.map ~f:(fun base -> (base, []))
in
StarvationDomain.acquire StarvationDomain.empty loc (Option.to_list lock)
StarvationDomain.acquire ~recursive_locks StarvationDomain.empty loc (Option.to_list lock)
in
let initial =
ConcurrencyModels.runs_on_ui_thread tenv proc_desc
@ -379,6 +398,14 @@ let report_deadlocks env {StarvationDomain.order; ui} report_map' =
match elem.Order.elem.eventually.elem with
| MayBlock _ | StrictModeCall _ ->
report_map
| LockAcquire endpoint_lock when Lock.equal endpoint_lock elem.Order.elem.first ->
let error_message =
Format.asprintf "Potential self deadlock. %a %a twice." pname_pp current_pname Lock.pp
endpoint_lock
in
let ltr = Order.make_trace ~header:"In method" current_pname elem in
let loc = Order.get_loc elem in
ReportMap.add_deadlock current_pname loc ltr error_message report_map
| LockAcquire endpoint_lock ->
Lock.owner_class endpoint_lock
|> Option.value_map ~default:report_map ~f:(fun endpoint_class ->

@ -11,6 +11,7 @@ module MF = MarkupFormatter
let pname_pp = MF.wrap_monospaced Typ.Procname.pp
module Lock = struct
(* TODO (T37174859): change to [HilExp.t] *)
type t = AccessPath.t
(* compare type, base variable modulo this and access list *)
@ -198,29 +199,46 @@ module UIThreadDomain = struct
(UIThreadExplanationDomain.with_callsite ui_explain callsite)
end
module FlatLock = AbstractDomain.Flat (Lock)
module GuardToLockMap = struct
include AbstractDomain.InvertedMap (HilExp) (FlatLock)
let remove_guard astate guard = remove guard astate
let add_guard astate ~guard ~lock = add guard (FlatLock.v lock) astate
end
type t =
{lock_state: LockState.t; events: EventDomain.t; order: OrderDomain.t; ui: UIThreadDomain.t}
{ events: EventDomain.t
; guard_map: GuardToLockMap.t
; lock_state: LockState.t
; order: OrderDomain.t
; ui: UIThreadDomain.t }
let empty =
{ lock_state= LockState.empty
; events= EventDomain.empty
{ events= EventDomain.empty
; guard_map= GuardToLockMap.empty
; lock_state= LockState.empty
; order= OrderDomain.empty
; ui= UIThreadDomain.empty }
let is_empty {lock_state; events; order; ui} =
UIThreadDomain.is_empty ui && EventDomain.is_empty events && OrderDomain.is_empty order
&& LockState.is_empty lock_state
let is_empty {events; guard_map; lock_state; order; ui} =
EventDomain.is_empty events && GuardToLockMap.is_empty guard_map && OrderDomain.is_empty order
&& LockState.is_empty lock_state && UIThreadDomain.is_empty ui
let pp fmt {lock_state; events; order; ui} =
F.fprintf fmt "{lock_state= %a; events= %a; order= %a; ui= %a}" LockState.pp lock_state
EventDomain.pp events OrderDomain.pp order UIThreadDomain.pp ui
let pp fmt {events; guard_map; lock_state; order; ui} =
F.fprintf fmt "{events= %a; guard_map= %a; lock_state= %a; order= %a; ui= %a}" EventDomain.pp
events GuardToLockMap.pp guard_map LockState.pp lock_state OrderDomain.pp order
UIThreadDomain.pp ui
let join lhs rhs =
{ lock_state= LockState.join lhs.lock_state rhs.lock_state
; events= EventDomain.join lhs.events rhs.events
{ events= EventDomain.join lhs.events rhs.events
; guard_map= GuardToLockMap.join lhs.guard_map rhs.guard_map
; lock_state= LockState.join lhs.lock_state rhs.lock_state
; order= OrderDomain.join lhs.order rhs.order
; ui= UIThreadDomain.join lhs.ui rhs.ui }
@ -228,16 +246,17 @@ let join lhs rhs =
let widen ~prev ~next ~num_iters:_ = join prev next
let ( <= ) ~lhs ~rhs =
UIThreadDomain.( <= ) ~lhs:lhs.ui ~rhs:rhs.ui
&& EventDomain.( <= ) ~lhs:lhs.events ~rhs:rhs.events
EventDomain.( <= ) ~lhs:lhs.events ~rhs:rhs.events
&& GuardToLockMap.( <= ) ~lhs:lhs.guard_map ~rhs:rhs.guard_map
&& OrderDomain.( <= ) ~lhs:lhs.order ~rhs:rhs.order
&& LockState.( <= ) ~lhs:lhs.lock_state ~rhs:rhs.lock_state
&& UIThreadDomain.( <= ) ~lhs:lhs.ui ~rhs:rhs.ui
(* for every lock b held locally, add a pair (b, event) *)
let add_order_pairs lock_state event acc =
let add_order_pairs ~recursive_locks lock_state event acc =
(* add no pairs whatsoever if we already hold that lock *)
if LockState.is_taken event lock_state then acc
if recursive_locks && LockState.is_taken event lock_state then acc
else
let add_first_and_eventually acc f =
match f.Event.elem with
@ -250,28 +269,32 @@ let add_order_pairs lock_state event acc =
LockState.fold_over_events add_first_and_eventually lock_state acc
let acquire ({lock_state; events; order} as astate) loc locks =
let acquire ~recursive_locks ({lock_state; events; order} as astate) loc locks =
let new_events = List.map locks ~f:(fun lock -> Event.make_acquire lock loc) in
{ astate with
events= List.fold new_events ~init:events ~f:(fun acc e -> EventDomain.add e acc)
; order=
List.fold new_events ~init:order ~f:(fun acc e ->
OrderDomain.union acc (add_order_pairs lock_state e order) )
OrderDomain.union acc (add_order_pairs ~recursive_locks lock_state e order) )
; lock_state=
List.fold2_exn locks new_events ~init:lock_state ~f:(fun acc lock e ->
LockState.acquire lock e acc ) }
let blocking_call callee sev loc ({lock_state; events; order} as astate) =
let new_event = Event.make_blocking_call callee sev loc in
let make_call_with_event new_event astate =
{ astate with
events= EventDomain.add new_event events; order= add_order_pairs lock_state new_event order }
events= EventDomain.add new_event astate.events
; order= add_order_pairs ~recursive_locks:false astate.lock_state new_event astate.order }
let blocking_call callee sev loc astate =
let new_event = Event.make_blocking_call callee sev loc in
make_call_with_event new_event astate
let strict_mode_call callee loc ({lock_state; events; order} as astate) =
let strict_mode_call callee loc astate =
let new_event = Event.make_strict_mode_call callee loc in
{ astate with
events= EventDomain.add new_event events; order= add_order_pairs lock_state new_event order }
make_call_with_event new_event astate
let release ({lock_state} as astate) locks =
@ -279,20 +302,27 @@ let release ({lock_state} as astate) locks =
lock_state= List.fold locks ~init:lock_state ~f:(fun acc l -> LockState.release l acc) }
let integrate_summary ({lock_state; events; order; ui} as astate) callee_pname loc callee_summary =
let integrate_summary ~recursive_locks ({lock_state; events; order; ui} as astate) callee_pname loc
callee_summary =
let callsite = CallSite.make callee_pname loc in
let callee_order = OrderDomain.with_callsite callee_summary.order callsite in
let callee_ui = UIThreadDomain.with_callsite callee_summary.ui callsite in
let filtered_order =
OrderDomain.filter
(fun {elem= {eventually}} -> LockState.is_taken eventually lock_state |> not)
callee_order
if recursive_locks then
OrderDomain.filter
(fun {elem= {eventually}} -> LockState.is_taken eventually lock_state |> not)
callee_order
else callee_order
in
let callee_events = EventDomain.with_callsite callee_summary.events callsite in
let filtered_events =
EventDomain.filter (fun e -> LockState.is_taken e lock_state |> not) callee_events
if recursive_locks then
EventDomain.filter (fun e -> LockState.is_taken e lock_state |> not) callee_events
else callee_events
in
let order' =
EventDomain.fold (add_order_pairs ~recursive_locks lock_state) filtered_events filtered_order
in
let order' = EventDomain.fold (add_order_pairs lock_state) filtered_events filtered_order in
{ astate with
events= EventDomain.join events filtered_events
; order= OrderDomain.join order order'
@ -307,6 +337,31 @@ let set_on_ui_thread ({ui} as astate) loc explain =
{astate with ui}
let add_guard astate guard lock ~acquire_now ~recursive_locks loc =
let astate = {astate with guard_map= GuardToLockMap.add_guard ~guard ~lock astate.guard_map} in
if acquire_now then acquire ~recursive_locks astate loc [lock] else astate
let remove_guard astate guard =
GuardToLockMap.find_opt guard astate.guard_map
|> Option.value_map ~default:astate ~f:(fun lock_opt ->
let locks = FlatLock.get lock_opt |> Option.to_list in
let astate = release astate locks in
{astate with guard_map= GuardToLockMap.remove_guard astate.guard_map guard} )
let unlock_guard astate guard =
GuardToLockMap.find_opt guard astate.guard_map
|> Option.value_map ~default:astate ~f:(fun lock_opt ->
FlatLock.get lock_opt |> Option.to_list |> release astate )
let lock_guard ~recursive_locks astate guard loc =
GuardToLockMap.find_opt guard astate.guard_map
|> Option.value_map ~default:astate ~f:(fun lock_opt ->
FlatLock.get lock_opt |> Option.to_list |> acquire astate ~recursive_locks loc )
type summary = t
let pp_summary = pp

@ -69,12 +69,18 @@ module UIThreadDomain :
AbstractDomain.WithBottom
with type t = UIThreadExplanationDomain.t AbstractDomain.Types.bottom_lifted
module GuardToLockMap : AbstractDomain.WithBottom
type t =
{lock_state: LockState.t; events: EventDomain.t; order: OrderDomain.t; ui: UIThreadDomain.t}
{ events: EventDomain.t
; guard_map: GuardToLockMap.t
; lock_state: LockState.t
; order: OrderDomain.t
; ui: UIThreadDomain.t }
include AbstractDomain.WithBottom with type t := t
val acquire : t -> Location.t -> Lock.t list -> t
val acquire : recursive_locks:bool -> t -> Location.t -> Lock.t list -> t
(** simultaneously acquire a number of locks, no-op if list is empty *)
val release : t -> Lock.t list -> t
@ -88,8 +94,21 @@ val set_on_ui_thread : t -> Location.t -> string -> t
(** set the property "runs on UI thread" to true by attaching the given explanation string as to
why this method is thought to do so *)
val add_guard :
t -> HilExp.t -> Lock.t -> acquire_now:bool -> recursive_locks:bool -> Location.t -> t
(** Install a mapping from the guard expression to the lock provided, and optionally lock it. *)
val lock_guard : recursive_locks:bool -> t -> HilExp.t -> Location.t -> t
(** Acquire the lock the guard was constructed with. *)
val remove_guard : t -> HilExp.t -> t
(** Destroy the guard and release its lock. *)
val unlock_guard : t -> HilExp.t -> t
(** Release the lock the guard was constructed with. *)
type summary = t
val pp_summary : F.formatter -> summary -> unit
val integrate_summary : t -> Typ.Procname.t -> Location.t -> summary -> t
val integrate_summary : recursive_locks:bool -> t -> Typ.Procname.t -> Location.t -> summary -> t

@ -13,7 +13,8 @@ class Basic {
public:
Basic() {}
void thread1() {
// deadlock between thread1_bad() and thread2_bad()
void thread1_bad() {
mutex_1.lock();
mutex_2.lock();
@ -21,7 +22,7 @@ class Basic {
mutex_1.unlock();
}
void thread2() {
void thread2_bad() {
mutex_2.lock();
mutex_1.lock();
@ -38,12 +39,33 @@ class WithGuard {
public:
WithGuard() {}
void thread1() {
// deadlock between thread1_bad() and thread2_bad()
void thread1_bad() {
std::lock_guard<std::mutex> lock1(mutex_1);
std::lock_guard<std::mutex> lock2(mutex_2);
}
void thread2() {
void thread2_bad() {
std::lock_guard<std::mutex> lock2(mutex_2);
std::lock_guard<std::mutex> lock1(mutex_1);
}
private:
std::mutex mutex_1;
std::mutex mutex_2;
};
class DeferredGuard {
public:
DeferredGuard() {}
// NO deadlock between thread1_bad() and thread2_bad()
void thread1_ok() {
std::unique_lock<std::mutex> lock1(mutex_1, std::defer_lock);
std::unique_lock<std::mutex> lock2(mutex_2);
}
void thread2_ok() {
std::lock_guard<std::mutex> lock2(mutex_2);
std::lock_guard<std::mutex> lock1(mutex_1);
}
@ -58,13 +80,13 @@ class StdLock {
StdLock() {}
// no reports, std::lock magically avoids deadlocks
void thread1() {
void foo_ok() {
std::lock<std::mutex>(mutex_1, mutex_2);
mutex_1.unlock();
mutex_2.unlock();
}
void thread2() {
void bar_ok() {
std::lock<std::mutex>(mutex_2, mutex_1);
mutex_2.unlock();
mutex_1.unlock();
@ -74,4 +96,44 @@ class StdLock {
std::mutex mutex_1;
std::mutex mutex_2;
};
class SelfDeadlock {
public:
SelfDeadlock() {}
void thread_bad() {
mutex_.lock();
mutex_.lock();
mutex_.unlock();
mutex_.unlock();
}
void interproc2_bad() { std::lock_guard<std::mutex> lock(mutex_); }
void interproc1_bad() {
std::lock_guard<std::mutex> lock(mutex_);
interproc2_bad();
}
void foo_ok() {
{ std::lock_guard<std::mutex> lock(mutex_); }
int i = 0;
{ std::lock_guard<std::mutex> lock(mutex_); }
}
void bar_ok() {
std::unique_lock<std::mutex> lock1(mutex_, std::defer_lock);
std::lock_guard<std::mutex> lock2(mutex_);
}
void complicated_bad() {
std::unique_lock<std::mutex> lock1(mutex_, std::defer_lock);
std::lock_guard<std::mutex> lock2(mutex_);
lock1.lock();
}
private:
std::mutex mutex_;
};
} // namespace basics

@ -1,2 +1,8 @@
codetoanalyze/cpp/starvation/basics.cpp, basics::Basic_thread1, 17, DEADLOCK, no_bucket, ERROR, [[Trace 1] `basics::Basic_thread1`,locks `this.mutex_1` in class `basics::Basic*`,locks `this.mutex_2` in class `basics::Basic*`,[Trace 2] `basics::Basic_thread2`,locks `this.mutex_2` in class `basics::Basic*`,locks `this.mutex_1` in class `basics::Basic*`]
codetoanalyze/cpp/starvation/basics.cpp, basics::WithGuard_thread1, 42, DEADLOCK, no_bucket, ERROR, [[Trace 1] `basics::WithGuard_thread1`,locks `this.mutex_1` in class `basics::WithGuard*`,locks `this.mutex_2` in class `basics::WithGuard*`,[Trace 2] `basics::WithGuard_thread2`,locks `this.mutex_2` in class `basics::WithGuard*`,locks `this.mutex_1` in class `basics::WithGuard*`]
codetoanalyze/cpp/starvation/basics.cpp, basics::Basic_thread1_bad, 18, DEADLOCK, no_bucket, ERROR, [[Trace 1] `basics::Basic_thread1_bad`,locks `this.mutex_1` in class `basics::Basic*`,locks `this.mutex_2` in class `basics::Basic*`,[Trace 2] `basics::Basic_thread2_bad`,locks `this.mutex_2` in class `basics::Basic*`,locks `this.mutex_1` in class `basics::Basic*`]
codetoanalyze/cpp/starvation/basics.cpp, basics::SelfDeadlock_complicated_bad, 131, DEADLOCK, no_bucket, ERROR, [In method`basics::SelfDeadlock_complicated_bad`,locks `this.mutex_` in class `basics::SelfDeadlock*`,locks `this.mutex_` in class `basics::SelfDeadlock*`]
codetoanalyze/cpp/starvation/basics.cpp, basics::SelfDeadlock_interproc1_bad, 114, DEADLOCK, no_bucket, ERROR, [In method`basics::SelfDeadlock_interproc1_bad`,locks `this.mutex_` in class `basics::SelfDeadlock*`,Method call: `basics::SelfDeadlock_interproc2_bad`,locks `this.mutex_` in class `basics::SelfDeadlock*`]
codetoanalyze/cpp/starvation/basics.cpp, basics::SelfDeadlock_thread_bad, 105, DEADLOCK, no_bucket, ERROR, [In method`basics::SelfDeadlock_thread_bad`,locks `this.mutex_` in class `basics::SelfDeadlock*`,locks `this.mutex_` in class `basics::SelfDeadlock*`]
codetoanalyze/cpp/starvation/basics.cpp, basics::WithGuard_thread1_bad, 44, DEADLOCK, no_bucket, ERROR, [[Trace 1] `basics::WithGuard_thread1_bad`,locks `this.mutex_1` in class `basics::WithGuard*`,locks `this.mutex_2` in class `basics::WithGuard*`,[Trace 2] `basics::WithGuard_thread2_bad`,locks `this.mutex_2` in class `basics::WithGuard*`,locks `this.mutex_1` in class `basics::WithGuard*`]
codetoanalyze/cpp/starvation/recursive.cpp, Recursive_FP_multi_ok, 26, DEADLOCK, no_bucket, ERROR, [In method`Recursive_FP_multi_ok`,locks `this.recursive_mutex_` in class `Recursive*`,locks `this.recursive_mutex_` in class `Recursive*`]
codetoanalyze/cpp/starvation/recursive.cpp, Recursive_FP_path_sensitive_ok, 36, DEADLOCK, no_bucket, ERROR, [In method`Recursive_FP_path_sensitive_ok`,locks `this.mutex_` in class `Recursive*`,locks `this.mutex_` in class `Recursive*`]
codetoanalyze/cpp/starvation/recursive.cpp, Recursive_FP_unknown_ok, 31, DEADLOCK, no_bucket, ERROR, [In method`Recursive_FP_unknown_ok`,locks `this.umutex_` in class `Recursive*`,locks `this.umutex_` in class `Recursive*`]

@ -0,0 +1,48 @@
/*
* Copyright (c) 2018-present, Facebook, Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/
#include <mutex>
struct UnknownMutex {
UnknownMutex() {}
void lock() {}
void unlock() {}
UnknownMutex(const UnknownMutex&) = delete;
UnknownMutex& operator=(const UnknownMutex&) = delete;
};
class Recursive {
public:
Recursive() {}
void FP_multi_ok() {
std::lock_guard<std::recursive_mutex> l(recursive_mutex_);
{ std::lock_guard<std::recursive_mutex> l(recursive_mutex_); }
}
void FP_unknown_ok() {
std::lock_guard<UnknownMutex> l(umutex_);
{ std::lock_guard<UnknownMutex> l(umutex_); }
}
void FP_path_sensitive_ok() {
std::lock_guard<std::mutex> l(mutex_);
bool flag = false;
if (flag) {
std::lock_guard<std::mutex> l(mutex_);
}
}
private:
std::recursive_mutex recursive_mutex_;
std::mutex mutex_;
UnknownMutex umutex_;
};
Loading…
Cancel
Save