-
Notifications
You must be signed in to change notification settings - Fork 88
HB-relationship involving thread creations while mutexes are held #1913
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from 47 commits
4f611d5
d4189b4
4d5fee2
2cc4be9
470279f
1f883f2
bb16da6
8f46eb3
61cdb71
67a62bd
c8cdf89
6c68a6c
6020267
5bf536d
bd388a2
66ad3f1
d4aec37
743092a
b161fed
3461214
717b52f
ee18c9a
9f5b264
83d6f5a
6785efa
a71d29e
be913ba
624254b
30d7daf
9b40093
ba0cac3
9e8399e
6827d4d
ac2b66f
b0e6f4b
6dd1f3c
596cb68
7e4f3d8
f3ba129
2ab8e7f
f855eda
3d401db
0f52534
b395b79
e69cfd8
c6a43d7
e05ae7b
b0adaca
dfb2eba
5ec0f12
bc552ed
31f5f3f
76c1aa8
425d0d4
ec2b4f5
6d501eb
ab3b3c6
81d3ac9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,165 @@ | ||
| (** descendant lockset analysis [descendantLockset] | ||
| analyzes a happened-before relationship related to thread creations with mutexes held. | ||
|
|
||
| Enabling [creationLockset] may improve the precision of this analysis. | ||
|
|
||
| @see Bachelor's Thesis "Leveraging th Potential of Inter-Threaded Locksets in Abstract Interpretation". Available upon request. | ||
|
dabund24 marked this conversation as resolved.
Outdated
|
||
| *) | ||
|
dabund24 marked this conversation as resolved.
|
||
|
|
||
| open Analyses | ||
| module TID = ThreadIdDomain.Thread | ||
| module TIDs = ConcDomain.ThreadSet | ||
| module Lockset = LockDomain.MustLockset | ||
|
|
||
| module Spec = struct | ||
| include IdentityUnitContextsSpec | ||
|
|
||
| (** [{ t_d |-> L }] | ||
|
|
||
| [t_d] was transitively created with all members of [L] held. | ||
| Additionally, no member of [L] could have been unlocked after the creation of [t_d] | ||
| *) | ||
| module D = MapDomain.MapBot (TID) (Lockset) | ||
|
|
||
| (** [{ t_0 |-> { t_d |-> L } }] | ||
|
|
||
| [{ t_d |-> L }] is the descendant lockset valid for the [V] value, | ||
|
michael-schwarz marked this conversation as resolved.
|
||
| because [t_d] was created in [t_0] with the lockset being a superset of L. | ||
| *) | ||
| module G = MapDomain.MapBot (TID) (D) | ||
|
|
||
| module V = TIDV | ||
|
|
||
| let name () = "descendantLockset" | ||
| let startstate _ = D.empty () | ||
| let exitstate _ = D.empty () | ||
|
|
||
| let threadenter man ~multiple lval f args = [ D.empty () ] | ||
|
|
||
| let threadspawn_contribute_globals man tid must_ancestor_descendants = | ||
| let descendant_lockset = man.local in | ||
|
|
||
| (* intersect locksets, but return bot if any arg is bot *) | ||
| let lockset_inter_sticky_bot = function | ||
| | `Top, _ | _, `Top -> Lockset.bot () | ||
| | ls1, ls2 -> Lockset.inter ls1 ls2 | ||
|
Comment on lines
+42
to
+45
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a really interesting point. What I wanted to achieve here is explicitly checking which mutexes are included in both locksets (where besides In practice, though, this will never happen (at least with how the analyses work right now), as in both cases, Not sure how to proceed here. If you strongly lean towards using
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Those are must mutexes, right? So `Topwhich is the same as
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I phrased this in a somewhat sloppy way. In my analyses,
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The only issue I see these two representations to possibly clash is when reading out the must-lockset query. This would fortunately only be a precision problem, however, I admit that I have not given this a lot of thought
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think for the implementation this is fine for now, for the writeup we may have to think how to separate those things cleanly.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So I guess it is a bit similar to #1977. I think it would be good to have some of this explanation as a code comment. |
||
| in | ||
|
|
||
| let contribute_for_descendant t_d = | ||
| let creation_lockset = man.ask @@ Queries.CreationLockset t_d in | ||
| let to_contribute = | ||
| D.fold | ||
| (fun t_l l_dl acc -> | ||
| let l_cl = Queries.CL.find tid creation_lockset in | ||
| let l_inter = lockset_inter_sticky_bot (l_cl, l_dl) in | ||
| D.add t_l l_inter acc) | ||
| descendant_lockset | ||
| (D.empty ()) | ||
| in | ||
| man.sideg t_d (G.singleton tid to_contribute) | ||
| in | ||
| TIDs.iter contribute_for_descendant must_ancestor_descendants | ||
|
|
||
| let threadspawn_compute_local_contribution man tid must_ancestor_descendants = | ||
| let lockset = man.ask Queries.MustLockset in | ||
| TIDs.fold | ||
| (fun t_d -> D.join (D.singleton t_d lockset)) | ||
| must_ancestor_descendants | ||
| man.local | ||
|
|
||
| let threadspawn man ~multiple lval f args fman = | ||
| let tid_lifted = man.ask Queries.CurrentThreadId in | ||
| let child_tid_lifted = fman.ask Queries.CurrentThreadId in | ||
| match tid_lifted, child_tid_lifted with | ||
| | `Lifted tid, `Lifted child_tid when TID.must_be_ancestor tid child_tid -> | ||
|
michael-schwarz marked this conversation as resolved.
|
||
| let must_ancestor_descendants = | ||
| ThreadDescendants.must_ancestor_descendants_closure fman child_tid | ||
| in | ||
| threadspawn_contribute_globals man tid must_ancestor_descendants; | ||
| threadspawn_compute_local_contribution man tid must_ancestor_descendants | ||
| | _ -> man.local | ||
|
|
||
| let unlock man lock = | ||
| D.map (Lockset.remove lock) man.local | ||
|
|
||
| let unknown_unlock man = | ||
| D.map (fun _ -> Lockset.empty ()) man.local | ||
|
|
||
| let event man e _ = | ||
| match e with | ||
| | Events.Unlock addr -> | ||
| let tid_lifted = man.ask Queries.CurrentThreadId in | ||
| let lock_opt = LockDomain.MustLock.of_addr addr in | ||
| (match tid_lifted, lock_opt with | ||
| | `Lifted tid, Some lock -> unlock man lock | ||
| | `Lifted tid, None -> unknown_unlock man | ||
|
dabund24 marked this conversation as resolved.
Outdated
|
||
| | _ -> man.local) | ||
| | _ -> man.local | ||
|
|
||
| module A = struct | ||
| module DlLhProd = Printable.Prod3 (D) (G) (Queries.LH) | ||
|
|
||
| (** ego tid * (local descendant lockset * global descendant lockset * lock history) *) | ||
| include Printable.Prod (TID) (DlLhProd) | ||
|
|
||
| (** checks if program point 1 must happen before program point 2 | ||
| @param (t1,dl1) thread id and descendant lockset of program point 1 | ||
| @param (t2,lh2) thread id and mustlock history of program point 2 | ||
| @param M module of [dl1] | ||
| *) | ||
| let happens_before (t1, dl1) (t2, lh2) = | ||
| let locks_held_creating_t2 = D.find t2 dl1 in | ||
| if Lockset.is_bot locks_held_creating_t2 | ||
| then false | ||
| else | ||
| let relevant_lh2_threads = | ||
| Lockset.fold | ||
| (fun lock -> TIDs.union (Queries.LH.find lock lh2)) | ||
| locks_held_creating_t2 | ||
| (TIDs.empty ()) | ||
| in | ||
| TIDs.exists | ||
| (fun t_lh -> | ||
| TID.must_be_ancestor t1 t_lh | ||
| && (TID.equal t_lh t2 || TID.must_be_ancestor t_lh t2)) | ||
| relevant_lh2_threads | ||
|
dabund24 marked this conversation as resolved.
Outdated
|
||
|
|
||
| (** checks if the entire execution of a thread must happen before a program point | ||
| @param dlg1 glabal descendant lockset of the thread | ||
| @param (t2,lh2) thread id and mustlock history of the program point | ||
| *) | ||
| let happens_before_global dlg1 (t2, lh2) = | ||
| G.exists (fun t dl_map -> happens_before (t, dl_map) (t2, lh2)) dlg1 | ||
|
|
||
| let may_race (t1, (dl1, dlg1, lh1)) (t2, (dl2, dlg2, lh2)) = | ||
| not | ||
| (happens_before (t1, dl1) (t2, lh2) | ||
| || happens_before (t2, dl2) (t1, lh1) | ||
| || happens_before_global dlg1 (t2, lh2) | ||
| || happens_before_global dlg2 (t1, lh1)) | ||
|
|
||
| (* ego tid is already printed elsewhere *) | ||
| let pretty () (_, dl_dlg_lh) = DlLhProd.pretty () dl_dlg_lh | ||
| let show (_, dl_dlg_lh) = DlLhProd.show dl_dlg_lh | ||
| let to_yojson (_, dl_dlg_lh) = DlLhProd.to_yojson dl_dlg_lh | ||
| let printXml f (_, dl_dlg_lh) = DlLhProd.printXml f dl_dlg_lh | ||
|
|
||
| let should_print (_, (dl, dlg, lh)) = | ||
| let ls_not_empty _ ls = not @@ Lockset.is_empty ls in | ||
| D.exists ls_not_empty dl | ||
| || G.exists (fun _ -> D.exists ls_not_empty) dlg | ||
| || Queries.LH.exists (fun l tids -> not @@ TIDs.is_empty tids) lh | ||
| end | ||
|
|
||
| let access man _ = | ||
| let lh = man.ask Queries.MustlockHistory in | ||
| let tid_lifted = man.ask Queries.CurrentThreadId in | ||
| match tid_lifted with | ||
| | `Lifted tid -> tid, (man.local, man.global tid, lh) | ||
|
dabund24 marked this conversation as resolved.
Outdated
|
||
| | _ -> ThreadIdDomain.UnknownThread, (D.empty (), G.empty (), Queries.LH.empty ()) | ||
| end | ||
|
|
||
| let _ = | ||
| MCP.register_analysis | ||
| ~dep:[ "threadid"; "mutex"; "threadJoins"; "threadDescendants"; "mustlockHistory" ] | ||
|
michael-schwarz marked this conversation as resolved.
|
||
| (module Spec : MCPSpec) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| (** must-lock history analysis [mustlockHistory] | ||
| collects for locks, in which threads a lock operation must have happened | ||
| before reaching the current program point. | ||
|
|
||
| @see https://github.com/goblint/analyzer/pull/1923 | ||
| *) | ||
|
|
||
| open Analyses | ||
| module TIDs = SetDomain.Reverse (ConcDomain.ThreadSet) | ||
| module Lock = LockDomain.MustLock | ||
|
|
||
| module Spec = struct | ||
|
dabund24 marked this conversation as resolved.
|
||
| include IdentityUnitContextsSpec | ||
|
|
||
| (** [{ l |-> T }] | ||
|
|
||
| [l] must have been in all members of [T]. | ||
| *) | ||
| module D = Queries.LH | ||
|
|
||
| let name () = "mustlockHistory" | ||
| let startstate _ = D.empty () | ||
| let exitstate _ = D.empty () | ||
|
|
||
| let lock man tid lock = | ||
| let old_threadset = D.find lock man.local in | ||
| let new_threadset = TIDs.add tid old_threadset in | ||
| D.add lock new_threadset man.local | ||
|
|
||
| let event man e _ = | ||
| match e with | ||
| (* we only handle exclusive locks here *) | ||
| | Events.Lock (addr, true) -> | ||
| let tid_lifted = man.ask Queries.CurrentThreadId in | ||
| let lock_opt = Lock.of_addr addr in | ||
| (match tid_lifted, lock_opt with | ||
| | `Lifted tid, Some l -> lock man tid l | ||
| | _ -> man.local) | ||
| | _ -> man.local | ||
|
|
||
| let query man (type a) (x : a Queries.t) : a Queries.result = | ||
| match x with | ||
| | Queries.MustlockHistory -> (man.local : D.t) | ||
| | _ -> Queries.Result.top x | ||
| end | ||
|
|
||
| let _ = MCP.register_analysis ~dep:[ "threadid" ] (module Spec : MCPSpec) | ||
Uh oh!
There was an error while loading. Please reload this page.