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

Change Machdep based on SV-COMP architecture #1574

Merged
merged 9 commits into from
Oct 2, 2024
Merged

Change Machdep based on SV-COMP architecture #1574

merged 9 commits into from
Oct 2, 2024

Conversation

sim642
Copy link
Member

@sim642 sim642 commented Sep 24, 2024

Closes #54, at least for SV-COMP usage.

TODO

initCIL already depends on theMachine, but it overwrites it. Must use envMachine to select it based on exp.architecture beforehand.
@sim642 sim642 added feature unsound sv-comp SV-COMP (analyses, results), witnesses labels Sep 24, 2024
@sim642 sim642 added this to the SV-COMP 2025 milestone Sep 24, 2024
@sim642 sim642 self-assigned this Sep 24, 2024
@sim642 sim642 marked this pull request as ready for review September 25, 2024 12:52
@sim642
Copy link
Member Author

sim642 commented Sep 27, 2024

The sv-benchmarks results (with 60s timeout) indicate some changes to verdicts:

  • 32 tasks go from unknown to true thanks to the more precise bounds we use for ILP32.
  • 56 tasks go from true to unknown which are all cases of previous unsoundness: we assumed larger type bounds for some no-overflow tasks than they actually have and were simply lucky to still have sound verdicts.
  • 55 tasks go from true to EXCEPTION (IntDomain.IncompatibleIKinds) which for some reason all are valid-memsafety tasks.

@michael-schwarz
Copy link
Member

I think we should investigate the 55 new EXCEPTION (IntDomain.IncompatibleIKinds exceptions before merging, while we still have an idea of what could be the case.

Also, when this is addressed, I think it makes sense to do a full ressource run the state before this and after it on server01 here before merging. I can start it if you ping me.

@sim642
Copy link
Member Author

sim642 commented Sep 27, 2024

I just made a fix ea2f616 which fixes one of those. I'm doing another quick run to see if that covers all of them.
EDIT: It fixes all those cases, so you can try a full resources run @michael-schwarz.

@michael-schwarz
Copy link
Member

The runs for the context gas values are now finished, I just started the comparison run on the current master (approx 24h), and will then start the run with this after.

@michael-schwarz
Copy link
Member

In the meantime, there seems to be some problems with OS X left here.

@michael-schwarz
Copy link
Member

The comparison run I started yesterday has finished this morning, I Just started https://github.com/goblint/analyzer/tree/staging_machdep-arch now.

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.

Good to go from my view as soon as the OS X issue is fixed.

@sim642
Copy link
Member Author

sim642 commented Oct 2, 2024

For now I disabled the test on MacOS. But I also added error messages about the missing machine definition. At least it's still possible to do SV-COMP runs on MacOS for ILP32 tasks but there will be a clear message about the possible unsoundness. The alternative would be to just crash when the machdep is missing, but that might be too harsh and inconvenient (because -m64 probably also doesn't work).

In the future we might want to provide pre-generated machdeps in CIL (at least for type sizes if not all machdep components). That should be enough for SV-COMP where preprocessing isn't needed.

@sim642 sim642 merged commit a65b86e into master Oct 2, 2024
21 checks passed
@sim642 sim642 deleted the machdep-arch branch October 2, 2024 10:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature sv-comp SV-COMP (analyses, results), witnesses unsound
Projects
None yet
2 participants