Skip to content
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

Ego-Lane-Derived-Digests for Privatizations: ProtectionBasedTIDPriv #1398

Merged
merged 14 commits into from
Mar 26, 2024

Conversation

michael-schwarz
Copy link
Member

@michael-schwarz michael-schwarz commented Mar 23, 2024

Introduces one of the new privatizations which will need to be evaluated for my thesis. This might actually be a lightweight interesting instance that might be worth evaluating it in practice.

This takes the protection based analysis and replaces [g] and [g]' by [g,A] and [g,A]' respectively, where A is an ego-lane derived digest.

Implementation-wise we once more use the Reduced-Cardinal-Powerset construction and change types of unknowns from ValD to partial maps Digest -> ValD with default value bot.

TODO:

  • Add new interesting tests
  • Modify TID.is_must_parent. Currently, this is reflexive, which is against intuition (I rarely think of myself as my own father)

@michael-schwarz
Copy link
Member Author

I was able to cast this as a generic lifter, which means we get no additional code duplication here 😄

@michael-schwarz michael-schwarz marked this pull request as ready for review March 25, 2024 12:15
@michael-schwarz michael-schwarz requested a review from sim642 March 25, 2024 12:15
Comment on lines +200 to +214
module ThreadNotStartedDigest:Digest =
struct
include ThreadIdDomain.ThreadLifted

module TID = ThreadIdDomain.Thread

let current (ask: Q.ask) =
ThreadId.get_current ask

let accounted_for (ask: Q.ask) ~(current: t) ~(other: t) =
match current, other with
| `Lifted current, `Lifted other ->
MHP.definitely_not_started (current, ask.f Q.CreatedThreads) other
| _ -> false
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does protection-tid use different digests than mutex-meet-tid?

Copy link
Member Author

@michael-schwarz michael-schwarz Mar 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The difference is that mutex-meet-tid is equipped with a L component that guarantees that all join-local contributions are already accounted for. protection-tid instead does not track such a component and thus needs to rely purely on the refinement provided by the ego-lane-derived digest. I'll add a comment to this effect.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But the digest modules don't have access to mutex-meet-tid's L, so how can it be coupled with that? Also in more-traces, the digest framework is introduced before L.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but things such as not reading from yourself and not reading from joined threads goes beyond what the digests can do, and relies on the analysis taking care of it in some way.

Consider e.g. reading from a thread that has been joined: It is possible that this thread held the mutex last before me, and thus the digests are compatible.
It is only because the analyses also read from some L that this contribution is accounted_for.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In more-traces this is the difference between
$$[[lock(a)]]^\sharp_\mathcal{A}(A_0,A_1) = \emptyset$$

and

$$\textsf{acc} A_0 A_1$$

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the explanation. I guess the coupling is kind of implicit here: this accounted_for assumes that the privatization actually accounts for things in its thread_join.

src/cdomain/value/cdomains/threadIdDomain.ml Outdated Show resolved Hide resolved
@michael-schwarz michael-schwarz requested a review from sim642 March 26, 2024 15:18
@sim642 sim642 added this to the v2.4.0 milestone Mar 26, 2024
@michael-schwarz michael-schwarz merged commit 423ecb6 into master Mar 26, 2024
20 of 21 checks passed
@michael-schwarz michael-schwarz deleted the protection-tid branch March 26, 2024 16:20
sim642 added a commit to sim642/opam-repository that referenced this pull request Aug 2, 2024
CHANGES:

* Remove unmaintained analyses: spec, file (goblint/analyzer#1281).
* Add linear two-variable equalities analysis (goblint/analyzer#1297, goblint/analyzer#1412, goblint/analyzer#1466).
* Add callstring, loopfree callstring and context gas analyses (goblint/analyzer#1038, goblint/analyzer#1340, goblint/analyzer#1379, goblint/analyzer#1427, goblint/analyzer#1439).
* Add non-relational thread-modular value analyses with thread IDs (goblint/analyzer#1366, goblint/analyzer#1398, goblint/analyzer#1399).
* Add NULL byte array domain (goblint/analyzer#1076).
* Fix spurious overflow warnings from internal evaluations (goblint/analyzer#1406, goblint/analyzer#1411, goblint/analyzer#1511).
* Refactor non-definite mutex handling to fix unsoundness (goblint/analyzer#1430, goblint/analyzer#1500, goblint/analyzer#1503, goblint/analyzer#1409).
* Fix non-relational thread-modular value analysis unsoundness with ambiguous points-to sets (goblint/analyzer#1457, goblint/analyzer#1458).
* Fix mutex type analysis unsoundness and enable it by default (goblint/analyzer#1414, goblint/analyzer#1416, goblint/analyzer#1510).
* Add points-to set refinement on mutex path splitting (goblint/analyzer#1287, goblint/analyzer#1343, goblint/analyzer#1374, goblint/analyzer#1396, goblint/analyzer#1407).
* Improve narrowing operators (goblint/analyzer#1502, goblint/analyzer#1540, goblint/analyzer#1543).
* Extract automatic configuration tuning for soundness (goblint/analyzer#1369).
* Fix many locations in witnesses (goblint/analyzer#1355, goblint/analyzer#1372, goblint/analyzer#1400, goblint/analyzer#1403).
* Improve output readability (goblint/analyzer#1294, goblint/analyzer#1312, goblint/analyzer#1405, goblint/analyzer#1497).
* Refactor logging (goblint/analyzer#1117).
* Modernize all library function specifications (goblint/analyzer#1029, goblint/analyzer#688, goblint/analyzer#1174, goblint/analyzer#1289, goblint/analyzer#1447, goblint/analyzer#1487).
* Remove OCaml 4.10, 4.11, 4.12 and 4.13 support (goblint/analyzer#1448).
avsm pushed a commit to avsm/opam-repository that referenced this pull request Sep 5, 2024
CHANGES:

* Remove unmaintained analyses: spec, file (goblint/analyzer#1281).
* Add linear two-variable equalities analysis (goblint/analyzer#1297, goblint/analyzer#1412, goblint/analyzer#1466).
* Add callstring, loopfree callstring and context gas analyses (goblint/analyzer#1038, goblint/analyzer#1340, goblint/analyzer#1379, goblint/analyzer#1427, goblint/analyzer#1439).
* Add non-relational thread-modular value analyses with thread IDs (goblint/analyzer#1366, goblint/analyzer#1398, goblint/analyzer#1399).
* Add NULL byte array domain (goblint/analyzer#1076).
* Fix spurious overflow warnings from internal evaluations (goblint/analyzer#1406, goblint/analyzer#1411, goblint/analyzer#1511).
* Refactor non-definite mutex handling to fix unsoundness (goblint/analyzer#1430, goblint/analyzer#1500, goblint/analyzer#1503, goblint/analyzer#1409).
* Fix non-relational thread-modular value analysis unsoundness with ambiguous points-to sets (goblint/analyzer#1457, goblint/analyzer#1458).
* Fix mutex type analysis unsoundness and enable it by default (goblint/analyzer#1414, goblint/analyzer#1416, goblint/analyzer#1510).
* Add points-to set refinement on mutex path splitting (goblint/analyzer#1287, goblint/analyzer#1343, goblint/analyzer#1374, goblint/analyzer#1396, goblint/analyzer#1407).
* Improve narrowing operators (goblint/analyzer#1502, goblint/analyzer#1540, goblint/analyzer#1543).
* Extract automatic configuration tuning for soundness (goblint/analyzer#1369).
* Fix many locations in witnesses (goblint/analyzer#1355, goblint/analyzer#1372, goblint/analyzer#1400, goblint/analyzer#1403).
* Improve output readability (goblint/analyzer#1294, goblint/analyzer#1312, goblint/analyzer#1405, goblint/analyzer#1497).
* Refactor logging (goblint/analyzer#1117).
* Modernize all library function specifications (goblint/analyzer#1029, goblint/analyzer#688, goblint/analyzer#1174, goblint/analyzer#1289, goblint/analyzer#1447, goblint/analyzer#1487).
* Remove OCaml 4.10, 4.11, 4.12 and 4.13 support (goblint/analyzer#1448).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants