-
Notifications
You must be signed in to change notification settings - Fork 76
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
Implement delayed widening #1483
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is indeed an interesting approach, thanks for the PR @RonaldJudin!
@jerhard and I considered it for a while, and made the following (perhaps obvious to you) observations:
- The gas is reset at every transfer function (which should be highlighted in the comment so this is more obvious).
- This, as we had discussed in Munich previously, avoids propagating high gas values outside of loops, which might lead to precision loss.
- Widening is only applied after the solver detects things as wpoints and thereby relies on the solver performing a widening between the previous value and the new one, rather than, e.g., performing widening between contributions from incoming edges.
As such, we presume the effect of this to be largely similar to #1442, but of course experiments are needed to confirm this suspicion.
What irks us a little is that this somehow mashes together the question of specifying the constraint system of interest with the process of computing (post-)solutions, which is evidenced by the fact that the gas components are not checked when performing leq
checks. This information thus strikes us more to be a self-observation of the solver than something that ought to be reflected in the specification. To go to extremes to make the point clear, putting stable
or point
into the constraint system, would - while technically possible - be a questionable choice.
With our student, we are considering submitting a paper on widening, narrowing and related issues for side-effecting constraint systems, which will likely include #1442. As this may be quite similar, we might align at GobCon on how to proceed.
Was the precision loss about high gas values outside of loops or their avoidance (which is the opposite)?
What do you mean by widening between incoming edges? #1442 is in the solver and doesn't even know about different incoming edges, it can only see the joined values, so what's the difference?
It's not the specification though. The lifted abstract values behave exactly as the unlifted ones, which is why
To go to the other extreme, putting I have to emphasize, this is the standard way to implement widening delays. It's precisely what Astree does:
In an earlier paper, they explicitly mention switching to this as opposed to some more global counting strategy:
This is also what Frama-C does: https://git.frama-c.com/pub/frama-c/-/blob/fe41a5490c3933fe18b9dbd4075f2f03f926714a/src/plugins/eva/partitioning/trace_partitioning.ml#L63. This and #1442 aren't mutually exclusive. This just adds to Goblint what everyone else already has and has it completely in a separate file, not bothering any existing solver, domain or analysis code to do so. This implementation is very much in alignment how various things in Goblint already are:
|
The consideration was that not resetting the counter would lead to precision loss at the code fragments located after a widening point.
This observation was not about a difference between #1442 and this. Rather it was an observation about what the solvers need to be fulfill for this to work (which TD3 does, but not necessarily all solvers must do). #1442 is integrated into one specific solver, so the question about what a solver must be like for it to work doesn't pose itself there. |
@Red-Panda64 has some benchmarks here, where this seems to be worse when it comes to performance then #1442. Those might be of interest. |
Well, yes, widening delay seems to be a bit slower. It times out 141 times where #1442 does not. Conversely, #1442 times out 10 times where widening delay does not. The benchmarks ran on the same machine with specs: Anyway, it is good to see that both implementations agree on everything and produce the same new true results. |
@jerhard: You promised a review on this until 2024-11-26 in the Gobcon. Do you have an estimate when you will be able to provide a review? |
This implements standard widening delay in-domain as suggested by #1442 (comment) and #1442 (comment).
The two separate options and analysis lifters allow different delays to be used for locally and globally. Moreover, local widening delays are applied per-path, which is not possible at the solver level at all.
The domain functor
WideningDelay.Dom
is generic and could be used as fine-grained as desired. For example, a specific analysis could only apply it to its own local or global abstract values, not for all analyses simultaneously. It could even be applied to only some parts of composed abstract domains.It lives in a completely separate file, not to clutter any existing solvers, domains or analysis lifters.
TODO
master
.D
andG
?