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

CN: Represent null pointers as an extra constructor #494

Merged
merged 1 commit into from
Aug 27, 2024

Conversation

dc-mak
Copy link
Contributor

@dc-mak dc-mak commented Aug 14, 2024

The current setup may be fine, but departs from the presentation of pointer in VIP and PNVI-ae-udi. It also leads to somewhat ugly is_null(p) || !addr_eq(p, NULL)) (see append.c) which I believe this change should be able to eliminate.

@dc-mak dc-mak requested a review from yav August 14, 2024 17:13
@dc-mak dc-mak changed the title CN: Represet null pointers as an extra constructor CN: Represent null pointers as an extra constructor Aug 14, 2024
@dc-mak

This comment was marked as resolved.

@dc-mak

This comment was marked as resolved.

@dc-mak

This comment was marked as resolved.

@dc-mak

This comment was marked as resolved.

@dc-mak

This comment was marked as resolved.

@dc-mak

This comment was marked as resolved.

dc-mak added a commit to dc-mak/cerberus that referenced this pull request Aug 16, 2024
Commit rems-project@2b30e70
introduced a new solver backend for CN. This code included a work-around
for Z3 (Z3Prover/z3#7268), which involved
calculating free variables for expressions. For `(let ((x term)) body)`,
the code correctly looked in `body` but did not look in `term`.

This commit fixes that, whilst tidying up some debugging and logging
code to make future debugging sessions easier.

It does not include a test because this bug was discovered during the
development of a different feature (with file `tests/cn/ptr_diff2.c`)
rems-project#494 which will be added in
separately, later.
dc-mak added a commit to dc-mak/cerberus that referenced this pull request Aug 16, 2024
Commit rems-project@2b30e70
introduced a new solver backend for CN. This code included a work-around
for Z3 (Z3Prover/z3#7268), which involved
calculating free variables for expressions. For `(let ((x term)) body)`,
the code correctly looked in `body` but did not look in `term`.

This commit fixes that, whilst tidying up some debugging and logging
code to make future debugging sessions easier.

It does not include a test because this bug was discovered during the
development of a different feature (with file `tests/cn/ptr_diff2.c`)
rems-project#494 which will be added in
separately, later.
dc-mak added a commit that referenced this pull request Aug 16, 2024
Commit 2b30e70
introduced a new solver backend for CN. This code included a work-around
for Z3 (Z3Prover/z3#7268), which involved
calculating free variables for expressions. For `(let ((x term)) body)`,
the code correctly looked in `body` but did not look in `term`.

This commit fixes that, whilst tidying up some debugging and logging
code to make future debugging sessions easier.

It does not include a test because this bug was discovered during the
development of a different feature (with file `tests/cn/ptr_diff2.c`)
#494 which will be added in
separately, later.
dc-mak added a commit to dc-mak/cerberus that referenced this pull request Aug 16, 2024
Commit
rems-project@7f4e8d6
added a test for commit rems-project@4e7fcb9
but it wasn't a test which passes once the error was fixed, so this
commit deletes the test (since there will be other ones sooon
rems-project#494
dc-mak added a commit that referenced this pull request Aug 16, 2024
Commit
7f4e8d6
added a test for commit 4e7fcb9
but it wasn't a test which passes once the error was fixed, so this
commit deletes the test (since there will be other ones sooon
#494
vzaliva pushed a commit that referenced this pull request Aug 18, 2024
Commit 2b30e70
introduced a new solver backend for CN. This code included a work-around
for Z3 (Z3Prover/z3#7268), which involved
calculating free variables for expressions. For `(let ((x term)) body)`,
the code correctly looked in `body` but did not look in `term`.

This commit fixes that, whilst tidying up some debugging and logging
code to make future debugging sessions easier.

It does not include a test because this bug was discovered during the
development of a different feature (with file `tests/cn/ptr_diff2.c`)
#494 which will be added in
separately, later.
vzaliva pushed a commit that referenced this pull request Aug 18, 2024
Commit
7f4e8d6
added a test for commit 4e7fcb9
but it wasn't a test which passes once the error was fixed, so this
commit deletes the test (since there will be other ones sooon
#494
@dc-mak dc-mak force-pushed the cn-vip-null branch 2 times, most recently from ffc47ca to 0ba0dd5 Compare August 20, 2024 10:43
@dc-mak dc-mak marked this pull request as ready for review August 20, 2024 12:11
@dc-mak dc-mak marked this pull request as draft August 20, 2024 12:12
dc-mak added a commit to dc-mak/cn-tutorial that referenced this pull request Aug 20, 2024
rems-project/cerberus#494 made this tests fail,
but it wasn't quite phrased correctly anyways. It may need further
changes for more VIP features later, but this should suffice for now.
dc-mak added a commit to rems-project/cn-tutorial that referenced this pull request Aug 20, 2024
rems-project/cerberus#494 made this tests fail,
but it wasn't quite phrased correctly anyways. It may need further
changes for more VIP features later, but this should suffice for now.
@dc-mak dc-mak force-pushed the cn-vip-null branch 3 times, most recently from f813240 to 5e144e8 Compare August 21, 2024 16:45
@dc-mak dc-mak marked this pull request as ready for review August 21, 2024 16:45
@dc-mak
Copy link
Contributor Author

dc-mak commented Aug 22, 2024

Seems to be some sort of issue where the CI gets stuck on
Checking /home/runner/work/cerberus/cerberus/cn-tutorial/src/examples/queue_pop.c ...

@dc-mak dc-mak marked this pull request as draft August 22, 2024 10:51
dc-mak added a commit to dc-mak/cn-tutorial that referenced this pull request Aug 26, 2024
rems-project/cerberus#494 exposed an issue in
the Z3 which is a bit difficult to work around in the implementation
itself and so we have this hacky work-around instead whilst it is fixed
upstream Z3Prover/z3#7352
dc-mak added a commit to rems-project/cn-tutorial that referenced this pull request Aug 26, 2024
rems-project/cerberus#494 exposed an issue in
the Z3 which is a bit difficult to work around in the implementation
itself and so we have this hacky work-around instead whilst it is fixed
upstream Z3Prover/z3#7352
@dc-mak dc-mak force-pushed the cn-vip-null branch 4 times, most recently from 61926c8 to 12b0c78 Compare August 27, 2024 11:54
This is to more closely match VIP and also probably remove the need to
specify `is_null(x) || !addr_eq(x, NULL` which is just silly.

This uses match (for future-proofing against the addition of a `funcptr`
constructor) and default<loc> for morally partial index terms. It
also rejects more programs as a result.

HasAllocId is a positive way to check that a pointer is in the `(@i, a)`
datatype case rather than not NULL. Whilst the two are equivalent right
now, this change is is mainly to future-proof against the addition of
`funcptr` constructor for function pointers as in VIP/PNVI-*.

The implementation relies on a few design choices:
1. Shifting on NULL is forbidden (implicitly) by the ISO C standard (it
   requires pointers which point to objects, hence cannot be NULL).
   Also, we believe real-world code only relies on this idiom for
   hand-implementing `offsetof`, so we forbid it. This is enforced in
   the typing rules.

2. It doesn't enforce that NULL is the same as/has address 0, although
   if need be, the implementation could use this info since no known
   optimisations/compilers etc. rely on NULL not being 0. Note that
   casting 0 to a pointer gives NULL, and casting a NULL pointer to an
   integer gives 0.

3. Shifting NULL in the specification language results in
   `default<Loc>`. Representing it as an provenance-address pair with an
   empty provenance would also be fine but would raise further questions
   on how to represent the empty provenance (an unconstrained variable,
   or a special value, the latter of which would require constraints on
   all Owned pointers to not have that special value as their
   provenance). For any choice here, one could construct a program in
   which proof and runtime testing diverge.

4. `each (u64 i; ..) { Owned(array_shift(p,i); }` is defined to imply
   `has_alloc_id(p)`.

5. Addresses of global variables are also constrained with
  `has_alloc_id(&global)`.

It uses SMT functions for a more readable SMT file output. Unfortunately
it also triggers: Z3Prover/z3#7352 which is
worked-around by using modifying the simplifier and calling it at
exactly the right moment (comments in the code). The test for this
is added in tests/cn/simplify_array_shift.c
@dc-mak dc-mak marked this pull request as ready for review August 27, 2024 13:13
@dc-mak dc-mak merged commit ca404db into rems-project:master Aug 27, 2024
2 checks passed
@dc-mak dc-mak deleted the cn-vip-null branch September 4, 2024 16:43
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.

1 participant