-
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
Account for allocated heap memory which is unreachable from globals #1241
Account for allocated heap memory which is unreachable from globals #1241
Conversation
Thank you for the PR!
Confusingly this is exactly the definition of |
I will start a run with the two suggestions from above applied to see what the results are. |
With these two suggestions on top of #1234, we succeed on an additional 17 tasks:
|
Tables: results.2023-11-08_10-02-03.zip |
I see, thanks for the clarification! It's a bit confusing, indeed. In this case I'll leave the reporting of this violation as is. |
Co-authored-by: Michael Schwarz <[email protected]>
Co-authored-by: Michael Schwarz <[email protected]>
… github.com:mrstanb/analyzer into improve-valid-memtrack-for-single-threaded-programs
I added some changes regarding the check of whether some memory is still transitively reachable via the points-to set of a global struct variable (both of pointer and non-pointer type), as well as its fields and the contents of the points-to sets. I also added two new regression test cases to check if the changes make sense (cf. |
Maybe we should add a check that if Iirc, we discussed with @jerhard that |
I don't think we use |
Also use `Queries.AD.fold` where applicable and prepend to accumulators
I may have missed this, but in what sense are global structs special? As in, why aren't we also considering the contents of global arrays and whatnot? |
I'm not really sure about arrays, because I haven't worked a lot with the array domain of Goblint and so I can't express an opinion on that. I've thought about global typedef union {
int i;
char *str;
} un;
un *glob_var;
int main() {
...
} What happens if the program first accesses |
Yeah, unions do not seem to be a good candidate here. We discussed the issue of arrays at our Gobcon yesterday, and decided that due to the abstraction we use for arrays, it is unlikely that things are must-reachable from arrays and have thus decided against implementing dedicated support for it for now. |
We then need to explicitly limit the filtering to structs though. CIL's |
I think we can now merge this into master as well? Wdyt @sim642? |
CHANGES: Functionally equivalent to Goblint in SV-COMP 2024. * Add termination analysis for loops (goblint/analyzer#1093). * Add memory out-of-bounds analysis (goblint/analyzer#1094, goblint/analyzer#1197). * Add memory leak analysis (goblint/analyzer#1127, goblint/analyzer#1241, goblint/analyzer#1246). * Add SV-COMP `termination`, `valid-memsafety` and `valid-memcleanup` properties support (goblint/analyzer#1220, goblint/analyzer#1228, goblint/analyzer#1201, goblint/analyzer#1199, goblint/analyzer#1259, goblint/analyzer#1262). * Add YAML witness version 2.0 support (goblint/analyzer#1238, goblint/analyzer#1240, goblint/analyzer#1217, goblint/analyzer#1226, goblint/analyzer#1225, goblint/analyzer#1248). * Add final warnings about unsound results (goblint/analyzer#1190, goblint/analyzer#1191). * Add many library function specifications (goblint/analyzer#1167, goblint/analyzer#1174, goblint/analyzer#1203, goblint/analyzer#1205, goblint/analyzer#1212, goblint/analyzer#1220, goblint/analyzer#1239, goblint/analyzer#1242, goblint/analyzer#1244, goblint/analyzer#1254, goblint/analyzer#1269). * Adapt automatic configuration tuning (goblint/analyzer#912, goblint/analyzer#921, goblint/analyzer#987, goblint/analyzer#1168, goblint/analyzer#1214, goblint/analyzer#1234).
CHANGES: Functionally equivalent to Goblint in SV-COMP 2024. * Add termination analysis for loops (goblint/analyzer#1093). * Add memory out-of-bounds analysis (goblint/analyzer#1094, goblint/analyzer#1197). * Add memory leak analysis (goblint/analyzer#1127, goblint/analyzer#1241, goblint/analyzer#1246). * Add SV-COMP `termination`, `valid-memsafety` and `valid-memcleanup` properties support (goblint/analyzer#1220, goblint/analyzer#1228, goblint/analyzer#1201, goblint/analyzer#1199, goblint/analyzer#1259, goblint/analyzer#1262). * Add YAML witness version 2.0 support (goblint/analyzer#1238, goblint/analyzer#1240, goblint/analyzer#1217, goblint/analyzer#1226, goblint/analyzer#1225, goblint/analyzer#1248). * Add final warnings about unsound results (goblint/analyzer#1190, goblint/analyzer#1191). * Add many library function specifications (goblint/analyzer#1167, goblint/analyzer#1174, goblint/analyzer#1203, goblint/analyzer#1205, goblint/analyzer#1212, goblint/analyzer#1220, goblint/analyzer#1239, goblint/analyzer#1242, goblint/analyzer#1244, goblint/analyzer#1254, goblint/analyzer#1269). * Adapt automatic configuration tuning (goblint/analyzer#912, goblint/analyzer#921, goblint/analyzer#987, goblint/analyzer#1168, goblint/analyzer#1214, goblint/analyzer#1234).
This PR:
valid-memtrack
Some notes:
valid-memtrack
states:All allocated memory is tracked, i.e., pointed to or deallocated
. Hence, for now I've kept the setting of the global SV-COMP flag forInvalidMemTrack
also for the case of not callingfree
on allocated heap memory (not necessarily only referenced by global vars). I'm open for a discussion here if you think we could improve on that as well76/09
):In the above example,
p
's memory is leaked (missingfree
), but it's also reported as unreachable (because it's not reachable via the globals, but only via a local var). Should we try to improve on that (maybe at least in the error reporting for the unreachable-from-globals-memory case) or are we ok with it as is?Sidenote:
76/08
and76/09
as new test casesTODOs:
earlyglobs
is activated, then abort the analysis