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

Add set struct domains #419

Merged
merged 35 commits into from
Nov 29, 2021
Merged

Conversation

jakoberzar
Copy link
Contributor

The existing struct abstract domain in Goblint (Simple) is a mapping from struct fields to their abstract values. It does not keep any information about relations between the fields.
We have tried to improve the struct domains in Goblint by adding two new domains (Sets and KeyedSets) that keep this information by using a set of mappings from fields to abstract values.
We have also added a third domain that can be picked within FlagConfiguredStructDomain that selects the struct domain dynamically depending on the struct field types.

I have squashed all of my commits into one.
Feel free to suggest improvements to the code. Currently, some code is kind of duplicated between Sets & KeyedSets (and some with HoareSet), I don't know how to solve that best.

Copy link
Member

@michael-schwarz michael-schwarz left a comment

Choose a reason for hiding this comment

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

I suggest we merge this and then do any further required changes by need.

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'm kind of bothered about the amount of copy-paste code from HoareDomain as well, because that adds two more Hoare domain implementations to the list in #101 (comment).

src/cdomains/valueDomain.ml Show resolved Hide resolved
src/util/defaults.ml Outdated Show resolved Hide resolved
src/cdomains/structDomain.ml Show resolved Hide resolved
src/cdomains/structDomain.ml Show resolved Hide resolved
src/cdomains/structDomain.ml Show resolved Hide resolved
src/cdomains/structDomain.ml Outdated Show resolved Hide resolved
src/analyses/base.ml Show resolved Hide resolved
@michael-schwarz
Copy link
Member

All x could indeed be redefined via let x = x_with_fct x but I'm not sure if that makes the code nicer to understand overall.

@michael-schwarz
Copy link
Member

The copy-paste from the HoareDomain in some sense seems inevitable to me, unless we are willing to equip it with those x_with_fct functions (which are needed for structs that contain arrays), but feel like they are leaking abstraction.

Even if we are willing to do that, there will still be some code to handle the particular weirdness of these partitioned arrays, that we will not be able to abandon.

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 guess the partial duplication of Hoare domain can stay, otherwise this will never get merged.

For the cases where I pointed out op being different from op_with_fct, maybe just add comments to acknowledge this mismatch.

tests/regression/43-struct-domain/01-single-thread.c Outdated Show resolved Hide resolved
@michael-schwarz
Copy link
Member

I redefined all operations x in terms of x_with_fct now (where appropriate).

@michael-schwarz michael-schwarz merged commit d32d7af into goblint:master Nov 29, 2021
@sim642 sim642 added this to the v2.0.0 milestone Aug 12, 2022
sim642 added a commit to sim642/opam-repository that referenced this pull request Aug 18, 2022
CHANGES:

Goblint "lean" release after a lot of cleanup.

* Remove unmaintained analyses: OSEK, ARINC, shapes, containment, deadlocksByRaces (goblint/analyzer#460, goblint/analyzer#736, goblint/analyzer#812, goblint/analyzer#474).
* Add interactive analysis (goblint/analyzer#391).
* Add server mode (goblint/analyzer#522).
* Add Compilation Database support (goblint/analyzer#406, goblint/analyzer#448).
* Add floating point domain, unrolled array domain and improved struct domains (goblint/analyzer#734, goblint/analyzer#761, goblint/analyzer#577, goblint/analyzer#419).
* Add static loop unrolling and heap variable unrolling (goblint/analyzer#563, goblint/analyzer#722).
* Improve race detection with may-happen-in-parallel analysis (goblint/analyzer#529, goblint/analyzer#518, goblint/analyzer#595).
* Reimplement lockset and deadlock analyses (goblint/analyzer#659, goblint/analyzer#662, goblint/analyzer#650, goblint/analyzer#655).
* Add pthread extraction to Promela (goblint/analyzer#220).
* Add location spans to output (goblint/analyzer#428, goblint/analyzer#449).
* Improve race reporting (goblint/analyzer#550, goblint/analyzer#551).
* Improve dead code reporting (goblint/analyzer#94, goblint/analyzer#353, goblint/analyzer#785).
* Refactor warnings (goblint/analyzer#55, goblint/analyzer#783).
* Add JSON schema for configuration (goblint/analyzer#476, goblint/analyzer#499).
* Refactor option names (goblint/analyzer#28, goblint/analyzer#192, goblint/analyzer#516, goblint/analyzer#675).
* Add bash completion (goblint/analyzer#669).
* Add OCaml 4.13 and 4.14 support, remove OCaml 4.09 support (goblint/analyzer#503, goblint/analyzer#672).
sim642 added a commit to sim642/opam-repository that referenced this pull request Aug 18, 2022
CHANGES:

Goblint "lean" release after a lot of cleanup.

* Remove unmaintained analyses: OSEK, ARINC, shapes, containment, deadlocksByRaces (goblint/analyzer#460, goblint/analyzer#736, goblint/analyzer#812, goblint/analyzer#474).
* Add interactive analysis (goblint/analyzer#391).
* Add server mode (goblint/analyzer#522).
* Add Compilation Database support (goblint/analyzer#406, goblint/analyzer#448).
* Add floating point domain, unrolled array domain and improved struct domains (goblint/analyzer#734, goblint/analyzer#761, goblint/analyzer#577, goblint/analyzer#419).
* Add static loop unrolling and heap variable unrolling (goblint/analyzer#563, goblint/analyzer#722).
* Improve race detection with may-happen-in-parallel analysis (goblint/analyzer#529, goblint/analyzer#518, goblint/analyzer#595).
* Reimplement lockset and deadlock analyses (goblint/analyzer#659, goblint/analyzer#662, goblint/analyzer#650, goblint/analyzer#655).
* Add pthread extraction to Promela (goblint/analyzer#220).
* Add location spans to output (goblint/analyzer#428, goblint/analyzer#449).
* Improve race reporting (goblint/analyzer#550, goblint/analyzer#551).
* Improve dead code reporting (goblint/analyzer#94, goblint/analyzer#353, goblint/analyzer#785).
* Refactor warnings (goblint/analyzer#55, goblint/analyzer#783).
* Add JSON schema for configuration (goblint/analyzer#476, goblint/analyzer#499).
* Refactor option names (goblint/analyzer#28, goblint/analyzer#192, goblint/analyzer#516, goblint/analyzer#675).
* Add bash completion (goblint/analyzer#669).
* Add OCaml 4.13 and 4.14 support, remove OCaml 4.09 support (goblint/analyzer#503, goblint/analyzer#672).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants