Skip to content

Cast ints on read instead of write in base analysis#128

Merged
sim642 merged 5 commits intomasterfrom
base/cast-on-read
Nov 10, 2020
Merged

Cast ints on read instead of write in base analysis#128
sim642 merged 5 commits intomasterfrom
base/cast-on-read

Conversation

@sim642
Copy link
Copy Markdown
Member

@sim642 sim642 commented Nov 5, 2020

This is an experimental attempt towards fixing #119.

The only reason types are required in VD.update_offset is to cast ints to appropriate types when written to through pointers which have been cast to a different type. This moves that cast from write time to later read time. Thus eliminating the need for having (problematic) type info available when writing.

This cast in the Var case is actually consistent with the already existing cast in the Mem case.

The bigger impact of this simple change is unknown. It could also be problematic for #121.


TODO

  • Actually remove type argument from VD.update_offset.
  • Run SV-COMP SoftwareSystems benchmarks to see if that fixes all of the type calculation errors.

This compensates for the removed int cast on write.
It is also consistent with existing Mem behavior which does this cast.
Without this the implicit function declaration for malloc has int return type, causing an unintended imprecise cast.
@sim642
Copy link
Copy Markdown
Member Author

sim642 commented Nov 9, 2020

I did another SV-COMP SoftwareSystems run on the commit exactly before this PR for comparison. The differences are:

  • It fixes all the Field on a non-compound errors. One of them turns into a correct true answer, the rest become unknown or time out after 60 seconds.
  • Understandably, now blindly casting even more things just to be sure adds a tiny performance penalty. Luckily this is relatively minor: only three benchmarks turn from a correct true answer to a 60 second timeout.

So SV-COMP-wise this doesn't change much, a few benchmarks this way or that way. Therefore I don't feel too strongly about merging this for SV-COMP but it's related to issues we'd still want to have fixed.

@sim642 sim642 marked this pull request as ready for review November 9, 2020 09:29
@sim642 sim642 added the bug label Nov 9, 2020
@michael-schwarz
Copy link
Copy Markdown
Member

My intuition would be merging this for svcomp, and then working on our more long-term, less ad-hoc solution after that. Let's discuss this tomorrow at the GobCon.

@sim642 sim642 merged commit e7f1922 into master Nov 10, 2020
@sim642 sim642 deleted the base/cast-on-read branch November 10, 2020 14:44
@sim642 sim642 mentioned this pull request Nov 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants