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

VR: extend to verify some cstr benchmarks #66

Open
wants to merge 52 commits into
base: master
Choose a base branch
from

Conversation

GiraffeReversed
Copy link
Contributor

Tested on SV-COMP21_valid-memsafety category, changed no answers except for newly verified benchmarks.

Depends on VR: use fixed and refactored version #65.

Copy link
Contributor

@trtikm trtikm left a comment

Choose a reason for hiding this comment

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

This is quite big PR. I will need some assistance to fully understand the changes.

@@ -0,0 +1,17959 @@
/*
* Catch v2.13.7
* Generated: 2021-07-28 20:29:27.753164
Copy link
Contributor

Choose a reason for hiding this comment

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

This file is generated. Should it really be versioned in Git?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe I wanted to add it properly as a dependency, but I think the problem was that the CMake version does not support FetchContent (and my knowledge of CMake is too limited to be able to do it otherwise), so I added the one necessary file manually. I believe it is done in the same way in tests for dg.

@mchalupa
Copy link
Member

This are the commits from svcomp22 branch right? Don't know how much they are polished, but they have been quite well tested as they were used in SV-COMP 22. Also, I do not know how much @GiraffeReversed will react on any request for changes, so maybe they are good to be merged as they are? (maybe with some cosmetic changes).

Copy link
Member

@mchalupa mchalupa left a comment

Choose a reason for hiding this comment

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

I haven't went through the commits, but I have tested them repeatedly (and they ran in SV-COMP'22), so once all cosmetic issues are solved, I think they are good to be merged. Sorry that they are not in the svcomp22 branch, but apparently I haven't pushed the final state of the branch to github...

@GiraffeReversed
Copy link
Contributor Author

Hi :) I don't really know what the requested cosmetic changes are... (but hopefully I am able to perform them after the exam period).
Also, I will gladly try to explain how the code works, once I am back from Erasmus.

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