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

Location fixes for YAML witness generation/validation #1372

Merged
merged 31 commits into from
Apr 4, 2024

Conversation

sim642
Copy link
Member

@sim642 sim642 commented Feb 27, 2024

Closes #1355.

Changes

  1. Adapt compilation to location fixes from Location fixes for Goblint YAML witness generation/validation cil#167.
  2. Deduplicate witness node identification logic to WitnessUtil. This prevents various copies going out of sync and makes them easier to test.
  3. Extend the CFG cram test output with information about witness node identification. This makes it easy to see which CFG nodes can have which witness invariants.
  4. Fix loop invariant locations. Loop heads for witnesses are not identified by back edges in CFG, but by checking for the Loop statement in the corresponding CIL AST. Loop statements are not part of our CFG nodes, so this lookup happens through skippedByEdge. This has two benefits:
    1. Loop invariants are no longer emitted for goto-loops. This matches YAML witness specification.
    2. Locations for loop heads now correctly point to the beginning of the structural loop keyword. This also works for do-while loops because the Loop node contains the location for the do.
  5. Fix location for first initializer assignment. Previously initializer assignments in declarations used expression locations in the middle of the statement. Now the first assignment has its location before the entire declaration, which is a valid point before any initializers execute.
  6. Make precondition_loop_invariant match loop nodes not arbitrary location nodes. This is our thing and not really used for anything, but at least the name matches the behavior now. This is from the time when loop_invariant was confused with location invariants in YAML witness specification.
  7. Mark YAML witnesses incompatible with loop unrolling for now.

TODO

@sim642 sim642 added bug sv-comp SV-COMP (analyses, results), witnesses labels Feb 27, 2024
@sim642 sim642 self-assigned this Feb 27, 2024
src/transform/expressionEvaluation.ml Dismissed Show dismissed Hide dismissed
src/transform/expressionEvaluation.ml Dismissed Show dismissed Hide dismissed
@sim642 sim642 force-pushed the fix-locations-pred branch from c22c775 to 02bba93 Compare February 28, 2024 13:26
src/framework/control.ml Dismissed Show dismissed Hide dismissed
@sim642 sim642 marked this pull request as ready for review February 28, 2024 14:53
@sim642 sim642 added this to the SV-COMP 2025 milestone Feb 28, 2024
@sim642 sim642 mentioned this pull request Feb 29, 2024
7 tasks
Copy link
Member

@michael-schwarz michael-schwarz left a comment

Choose a reason for hiding this comment

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

I think it's a step forward, there are a few things that we should still discuss before merging though.
It might make sense to pull out the loop unrolling and sv-comp default config discussion and discuss that separately?

conf/svcomp.json Outdated Show resolved Hide resolved
src/analyses/unassumeAnalysis.ml Show resolved Hide resolved
src/common/framework/myCFG.ml Show resolved Hide resolved
src/framework/control.ml Show resolved Hide resolved
src/framework/control.ml Show resolved Hide resolved
src/witness/witnessUtil.ml Show resolved Hide resolved
src/witness/witnessUtil.ml Show resolved Hide resolved
src/witness/yamlWitness.ml Outdated Show resolved Hide resolved
@@ -340,11 +328,11 @@ struct
entries
in

(* Generate precondition invariants.
(* Generate precondition loop invariants.
Copy link
Member

Choose a reason for hiding this comment

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

@jerhard How does this change affect our planned usage for precondition invariants for the project with LMU? We need general precondition invariants there iirc?

Copy link
Member Author

Choose a reason for hiding this comment

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

Having precondition_location_invariant entries is just a matter of defining the corresponding types and doing what we already have for location_invariant vs loop_invariant: use a different predicate for filtering nodes.

tests/regression/cfg/util/dune Outdated Show resolved Hide resolved
@michael-schwarz michael-schwarz requested a review from jerhard March 18, 2024 10:06
sim642 added a commit that referenced this pull request Apr 2, 2024
@sim642
Copy link
Member Author

sim642 commented Apr 2, 2024

It might make sense to pull out the loop unrolling and sv-comp default config discussion and discuss that separately?

It's now extracted to #1402.

src/witness/yamlWitness.ml Show resolved Hide resolved
@sim642 sim642 merged commit 58ce3e6 into master Apr 4, 2024
21 checks passed
@sim642 sim642 deleted the fix-locations-pred branch April 4, 2024 12:43
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
Labels
bug sv-comp SV-COMP (analyses, results), witnesses
Projects
None yet
Development

Successfully merging this pull request may close these issues.

for loop invariant location in YAML witness
2 participants