-
Notifications
You must be signed in to change notification settings - Fork 88
Apron: Track relational information for variables that have their address taken #742
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
Conversation
|
Looks like this is not a simple local change but will require extensive changes, as after considering escaping, it no longer is true that a variable is either a global or a local, but instead it may be both depending on whether it escaped. 😮💨 |
But at any particular program point, it's either must-local or may-escaped, no? |
86821fc to
739ef9c
Compare
|
Something I just remembered related to this: previously analyzer/src/analyses/apron/apronAnalysis.apron.ml Lines 372 to 388 in 739ef9c
With these changes-to-be, this is no longer true and it will have to invalidate written arguments, similarly to how base analysis does it. In doing so, #720 might be very useful to have such that some library function doesn't completely top-ify everything. Because pre-#720, |
Co-authored-by: Simmo Saan <[email protected]>
ad280be to
ea41437
Compare
| let vname = Var.to_string var in | ||
| match List.find_opt (fun v -> v.vname = vname) (fundec.sformals @ fundec.slocals) with | ||
| | None -> true | ||
| | Some v -> Queries.LS.exists (fun (v',_) -> v'.vid = v.vid) reachable_from_args |
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.
I guess it's fine for now as-is so we can quickly move to see if this helps us find any more relational invariants.
Totally off-topic, but exists on a topped set being true is actually weird, since it claims that exists (fun _ -> false) is also true 🙃
|
I just tried running the relational traces bench on this branch and they all seem to immediately crash with: |
|
Is it only this branch or also already on master? There should be no changes to the handling of non-apron things in here, and apron does not do anything with |
|
Only on this branch, the initial problem is that dereferencing doesn't exclude the unknown pointer. This patch fixes that: diff --git a/src/analyses/apron/apronAnalysis.apron.ml b/src/analyses/apron/apronAnalysis.apron.ml
index 3ac185d7a..5c5b494ef 100644
--- a/src/analyses/apron/apronAnalysis.apron.ml
+++ b/src/analyses/apron/apronAnalysis.apron.ml
@@ -158,8 +158,8 @@ struct
| Lval (Var v, off) -> Lval (Var v, off)
| Lval (Mem e, NoOffset) ->
(match ask (Queries.MayPointTo e) with
- | (`Lifted _) as r when (Queries.LS.cardinal r) = 1 ->
- let lval = Lval.CilLval.to_lval (Queries.LS.choose r) in
+ | a when not (Queries.LS.is_top a || Queries.LS.mem (dummyFunDec.svar, `NoOffset) a) && (Queries.LS.cardinal a) = 1 ->
+ let lval = Lval.CilLval.to_lval (Queries.LS.choose a) in
Lval lval
(* It would be possible to do better here, exploiting e.g. that the things pointed to are known to be equal *)
(* see: https://github.com/goblint/analyzer/pull/742#discussion_r879099745 *)After fixing that, a |
|
Ok, this goes deep, but I think I've managed to fix all the following crashes. I'll just go ahead and commit my fixes to this branch. |
All three lookups crashed on pfscan.
TODOs:
The strange numbering in
apron2is for consistency with #391Closes #660