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

Incremental: Reset ckeys for anonymous structs and unions #942

Merged
merged 5 commits into from
Dec 7, 2022

Conversation

jerhard
Copy link
Member

@jerhard jerhard commented Dec 5, 2022

This PR adds code that resets the ckey for unchanged anonymous structs and unions. Previously, the cname for these structs and unions was already reset, but the ckey was not. This led to the issue in #678.

Background

Anonymous structs and unions (compounds) are provided with rather fragile names by Cil: The names of these compounds tend to change between program versions even when there is not semantic change relating to them. Therefore, there is special handling in CompareCIL that does not require that the names of anonymous compounds are the same as in the previous program version, as long as there is structural equality and they once appear in the same usage as in the previous program version. The cname of these anonymous compounds was reset to the one in the previous version.
This updating is done in CompareCIL, as opposed to updateCIL, because it is the easiest to also implement the requirement that the compound was used once in the same usage as in the previous program version.

However, the ckey, which is computed by Cil from the cname and the information whether the compinfo is referring to a struct or union, was not updated.

Fixes #678

The cnames of anonymous structs and unions are fragile between program versions. The cnames were already reset, this also resets the ckeys.
Copy link
Member

@sim642 sim642 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think another concern at the time I wrote #678 was how ckeys are generated non-sequentially in the first place.
With MaxIdUtil, we find maximum sequential IDs in order to renumber the new program's IDs to come after the previous maximums, ensuring no overlap. With ckeys this doesn't happen, so it's theoretically possible that due to a hash collision different structs in the old and new program by coincidence have the same ckey, causing issues down the line.

Although that is quite unlikely, so it might be fine to leave that for now.

src/incremental/compareAST.ml Outdated Show resolved Hide resolved
If they are already the same, the assignment would have no (negative) effect; so one can avoid the branching.
@jerhard jerhard merged commit f26292d into master Dec 7, 2022
@jerhard jerhard deleted the issue_678_minimal branch December 7, 2022 15:55
@sim642 sim642 added this to the v2.2.0 milestone Apr 5, 2023
sim642 added a commit to sim642/opam-repository that referenced this pull request Sep 13, 2023
CHANGES:

* Add `setjmp`/`longjmp` analysis (goblint/analyzer#887, goblint/analyzer#970, goblint/analyzer#1015, goblint/analyzer#1019).
* Refactor race analysis to lazy distribution (goblint/analyzer#1084, goblint/analyzer#1089, goblint/analyzer#1136, goblint/analyzer#1016).
* Add thread-unsafe library function call analysis (goblint/analyzer#723, goblint/analyzer#1082).
* Add mutex type analysis and mutex API analysis (goblint/analyzer#800, goblint/analyzer#839, goblint/analyzer#1073).
* Add interval set domain and string literals domain (goblint/analyzer#901, goblint/analyzer#966, goblint/analyzer#994, goblint/analyzer#1048).
* Add affine equalities analysis (goblint/analyzer#592).
* Add use-after-free analysis (goblint/analyzer#1050, goblint/analyzer#1114).
* Add dead code elimination transformation (goblint/analyzer#850, goblint/analyzer#979).
* Add taint analysis for partial contexts (goblint/analyzer#553, goblint/analyzer#952).
* Add YAML witness validation via unassume (goblint/analyzer#796, goblint/analyzer#977, goblint/analyzer#1044, goblint/analyzer#1045, goblint/analyzer#1124).
* Add incremental analysis rename detection (goblint/analyzer#774, goblint/analyzer#777).
* Fix address sets unsoundness (goblint/analyzer#822, goblint/analyzer#967, goblint/analyzer#564, goblint/analyzer#1032, goblint/analyzer#998, goblint/analyzer#1031).
* Fix thread escape analysis unsoundness (goblint/analyzer#939, goblint/analyzer#984, goblint/analyzer#1074, goblint/analyzer#1078).
* Fix many incremental analysis issues (goblint/analyzer#627, goblint/analyzer#836, goblint/analyzer#835, goblint/analyzer#841, goblint/analyzer#932, goblint/analyzer#678, goblint/analyzer#942, goblint/analyzer#949, goblint/analyzer#950, goblint/analyzer#957, goblint/analyzer#955, goblint/analyzer#954, goblint/analyzer#960, goblint/analyzer#959, goblint/analyzer#1004, goblint/analyzer#558, goblint/analyzer#1010, goblint/analyzer#1091).
* Fix server mode for abstract debugging (goblint/analyzer#983, goblint/analyzer#990, goblint/analyzer#997, goblint/analyzer#1000, goblint/analyzer#1001, goblint/analyzer#1013, goblint/analyzer#1018, goblint/analyzer#1017, goblint/analyzer#1026, goblint/analyzer#1027).
* Add documentation for configuration JSON schema and OCaml API (goblint/analyzer#999, goblint/analyzer#1054, goblint/analyzer#1055, goblint/analyzer#1053).
* Add many library function specifications (goblint/analyzer#962, goblint/analyzer#996, goblint/analyzer#1028, goblint/analyzer#1079, goblint/analyzer#1121, goblint/analyzer#1135, goblint/analyzer#1138).
* Add OCaml 5.0 support (goblint/analyzer#1003, goblint/analyzer#945, goblint/analyzer#1162).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

compinfo keys not updated incrementally
2 participants