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

Support YAML violation witnesses #680

Merged
merged 25 commits into from
Nov 19, 2024
Merged

Conversation

schuessf
Copy link
Contributor

@schuessf schuessf commented Sep 26, 2024

This PR adds support for violation witnesses in the YAML format. The main work (thanks to @HelenAnnaMeyer) was the following:

  • Add AST objects for the new witness format
  • Allow Parsing violation witnesses from YAML (extend YamlWitnessParser).
  • Generate AST object to represent violation witnesses from an IProgramExecution (see YamlViolationWitnessGenerator)
  • Validate violation witnesses (see YamlWitnessProductAutomaton): Create a product automaton of a program automaton and a witness to use this product as as the initial abstraction in the verification (TraceAbstraction / BüchiAutomizer).

In order to work properly, this required the following additional changes:

  • Add WitnessTransformer as an abstraction for violation witnesses in YAML and GraphML
  • Slightly adapt our C-translation (see CHandler) for loops to extract the correct branching condition for the witnesses
  • Add methods to get the required infos (line, column, function) for the witness to IBacktranslationValueProvider

There are still some issues / open questions:

  • Some of the added regression tests currently fail, because of bugs in the RCFGBuilder. They should however work properly, if the IcfgBuilder is used instead (i.e., once this branch is merged).
  • Currently we don't generate any assumptions from our counterexamples. We could get them from our program states (there is already the class ProgramStatePrinter for this), but we currently only get states at locations that are not legal for assumptions according to the format. This might also change with the usage of IcfgBuilder.
  • We currently cannot check the assumptions during validation (which happens in the trace abstraction), as this would require to translate them from C to SMT (which has to be done in the translation and not only in the trace abstraction). However, we currently try to validate at least the location of the assumptions. This might however be problematic if we use large block encoding (as no matching edge might exist for the C statement), therefore there is a boolean flag CHECK_ASSUMPTION_LOCATIONS in YamlWitnessProductAutomaton. We need to figure out the best "mode" for validation (combination of block encoding and this flag), especially for the SV-COMP.
  • Witness generation and validation is only properly tested for ReachSafety. However, I just tested generation and validation for simple examples for NoOverflow and MemSafety, and it seems to work in principle. This might require some further testing for SV-COMP.

For SV-COMP, we still need to add a benchdef for violation witnesses 2.0, once this PR is merged.

Copy link
Contributor

@maul-esel maul-esel left a comment

Choose a reason for hiding this comment

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

Looks good, thanks @HelenAnnaMeyer and @schuessf for adding this support!

I've left some smaller comments. I also remember in our discussion after the presentation, it was mentioned that some aspects of the semantics of violation witnesses were not entirely clear (though I forgot what it was). Should we take this up with the authors / at SV-COMP before merging this, or do you think it's better to work with the current implementation and, if necessary, make changes later on?

@schuessf schuessf force-pushed the wip/fs/yaml-violation-witnesses branch 2 times, most recently from 85a2f64 to e7b6541 Compare November 12, 2024 11:07
@schuessf schuessf force-pushed the wip/fs/yaml-violation-witnesses branch from e7b6541 to 521d0a2 Compare November 14, 2024 08:48
@schuessf schuessf force-pushed the wip/fs/yaml-violation-witnesses branch from 521d0a2 to 09c4861 Compare November 18, 2024 14:24
Copy link
Member

@bahnwaerter bahnwaerter left a comment

Choose a reason for hiding this comment

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

I quickly skimmed through the changes and they look good so far. I have also noted a few minor things. You could also consider removing personal data from the example witnesses. This affects in particular the encoding of the user name in the file path of the specified witness files in the metadata section of each witness file.

Comment on lines +92 to +95
logger.info(
"Constructing product of automaton with %d states and violation witness of the following lengths: %s",
abstraction.size(), witness.getEntries().stream().map(x -> ((ViolationSequence) x).getSegments().size())
.collect(Collectors.toList()));
Copy link
Member

Choose a reason for hiding this comment

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

There is some indentation issue. Is this the result of an unfavorable formatter setting?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, but this might have been fixed now, this was before the Java 21 migration, where we also slightly adapted the formatter. I will just keep this for now, we will run the formatter anyway on the dev branch.

Comment on lines +109 to +110
return Stream
.of(parseViolationSequence((List<Map<String, List<Map<String, Object>>>>) entry.get("content")));
Copy link
Member

Choose a reason for hiding this comment

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

Another example of an unfavorable formatter setting?

@schuessf schuessf requested a review from maul-esel November 18, 2024 19:03
@schuessf schuessf merged commit 2ad53c4 into dev Nov 19, 2024
3 of 4 checks passed
@schuessf schuessf deleted the wip/fs/yaml-violation-witnesses branch November 19, 2024 08:33
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.

4 participants