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

__CPROVER_forall fails when dealing with flattening of nested structure #8570

Open
hanno-becker opened this issue Jan 16, 2025 · 2 comments
Open

Comments

@hanno-becker
Copy link

hanno-becker commented Jan 16, 2025

I'm observing unexpected behaviour when trying to use __CPROVER_forall to access the fields of a nested array structure when cast as a flat array of cells. cc @tautschnig @remi-delmas-3000

Minimal example:

/* instructions

foo:
	goto-cc harness.c --function foo_harness -o a.out
	goto-instrument --dfcc foo_harness --enforce-contract foo a.out b.out
	cbmc b.out --bitwuzla
*/

#include <stdint.h>

typedef struct __attribute__((packed)) {
    int data[2];
} arr;

typedef struct __attribute__((packed)) {
    arr vec[2];
} arrvec;

void foo(arrvec *x)
  __CPROVER_requires(__CPROVER_is_fresh(x, sizeof(arrvec)))
  __CPROVER_requires(x->vec[1].data[0] < 42)
{
    // OK:
    __CPROVER_assert(((int*)x)[2] < 42, "");
    // NOT OK:
    __CPROVER_assert(__CPROVER_forall {unsigned k; k == 2 ==> ((int*)x)[k] < 42}, "");
    // OK:
    __CPROVER_assert(__CPROVER_forall {unsigned k; k == 2 ==> ((int (*)[2])x)[k/2][k % 2] < 42}, "");
}

void foo_harness(void) {
    arr *x;
    foo(x);
}
@tautschnig
Copy link
Collaborator

Taking a look. Here is a simplified version that equally fails and neither uses contracts nor requires the use of an SMT solver - cbmc harness.c will do:

typedef struct __attribute__((packed)) {
    int data[2];
} arr;

typedef struct __attribute__((packed)) {
    arr vec[2];
} arrvec;

int main() {
    arrvec A;
    arrvec *x = &A;
    __CPROVER_assume(x->vec[1].data[0] < 42);
    // OK:
    __CPROVER_assert(((int*)x)[2] < 42, "");
    // NOT OK:
    __CPROVER_assert(__CPROVER_forall {unsigned k; k == 2 ==> ((int*)x)[k] < 42}, "");
    // OK:
    __CPROVER_assert(__CPROVER_forall {unsigned k; k == 2 ==> ((int (*)[2])x)[k/2][k % 2] < 42}, "");
}

@tautschnig
Copy link
Collaborator

Turns out this isn't a problem related to quantifiers. The following further simplification (still equivalent to the original one) also fails:

typedef struct __attribute__((packed)) {
    int data[2];
} arr;

typedef struct __attribute__((packed)) {
    arr vec[2];
} arrvec;

int main() {
    arrvec A;
    arrvec *x = &A;
    __CPROVER_assume(x->vec[1].data[0] < 42);
    unsigned k;
    __CPROVER_assert(k != 2 || ((int*)x)[k] < 42, "");
}

It seems the problem is with field sensitivity during symbolic execution, where we appear to (spuriously) turn ((int*)x)k effectively into A.vec[0].data[k].

hanno-becker added a commit to pq-code-package/mlkem-native that referenced this issue Jan 16, 2025
Previously, runtime assertions via debug_assert_xxx and CBMC
assertions via cassert(...) were separate.

This commit modifies the implementation of the debug assertion
macros so that when CBMC is used, debug assertions are intepreted
as proof obligations.

This removes some redundancy and non-uniformity in the code,
and also reduces the likelihood that debug assertions and CBMC
contracts get out of sync. In some case, this actually happened,
and the commit fixes this. The commit also adds further bounds
assertions in alignment with pre/post conditions.

A slight nuisance is that the debug assertions cannot flatten
nested structures like polyvec for the bounds check, running
into issue diffblue/cbmc#8570. We
work around this by introducing a new `xxx_2d` (for 2-dimensional)
macro which takes two dimensions and uses a two-step array
access, circumventing the above CBMC issue.

Signed-off-by: Hanno Becker <[email protected]>
hanno-becker added a commit to pq-code-package/mlkem-native that referenced this issue Jan 16, 2025
Previously, runtime assertions via debug_assert_xxx and CBMC
assertions via cassert(...) were separate.

This commit modifies the implementation of the debug assertion
macros so that when CBMC is used, debug assertions are intepreted
as proof obligations.

This removes some redundancy and non-uniformity in the code,
and also reduces the likelihood that debug assertions and CBMC
contracts get out of sync. In some case, this actually happened,
and the commit fixes this. The commit also adds further bounds
assertions in alignment with pre/post conditions.

A slight nuisance is that the debug assertions cannot flatten
nested structures like polyvec for the bounds check, running
into issue diffblue/cbmc#8570. We
work around this by introducing a new `xxx_2d` (for 2-dimensional)
macro which takes two dimensions and uses a two-step array
access, circumventing the above CBMC issue.

Signed-off-by: Hanno Becker <[email protected]>
hanno-becker added a commit to pq-code-package/mlkem-native that referenced this issue Jan 17, 2025
Previously, runtime assertions via debug_assert_xxx and CBMC
assertions via cassert(...) were separate.

This commit modifies the implementation of the debug assertion
macros so that when CBMC is used, debug assertions are intepreted
as proof obligations.

This removes some redundancy and non-uniformity in the code,
and also reduces the likelihood that debug assertions and CBMC
contracts get out of sync. In some case, this actually happened,
and the commit fixes this. The commit also adds further bounds
assertions in alignment with pre/post conditions.

A slight nuisance is that the debug assertions cannot flatten
nested structures like polyvec for the bounds check, running
into issue diffblue/cbmc#8570. We
work around this by introducing a new `xxx_2d` (for 2-dimensional)
macro which takes two dimensions and uses a two-step array
access, circumventing the above CBMC issue.

Signed-off-by: Hanno Becker <[email protected]>
hanno-becker added a commit to pq-code-package/mlkem-native that referenced this issue Jan 17, 2025
Previously, runtime assertions via debug_assert_xxx and CBMC
assertions via cassert(...) were separate.

This commit modifies the implementation of the debug assertion
macros so that when CBMC is used, debug assertions are intepreted
as proof obligations.

This removes some redundancy and non-uniformity in the code,
and also reduces the likelihood that debug assertions and CBMC
contracts get out of sync. In some case, this actually happened,
and the commit fixes this. The commit also adds further bounds
assertions in alignment with pre/post conditions.

A slight nuisance is that the debug assertions cannot flatten
nested structures like polyvec for the bounds check, running
into issue diffblue/cbmc#8570. We
work around this by introducing a new `xxx_2d` (for 2-dimensional)
macro which takes two dimensions and uses a two-step array
access, circumventing the above CBMC issue.

Signed-off-by: Hanno Becker <[email protected]>
hanno-becker added a commit to pq-code-package/mlkem-native that referenced this issue Jan 17, 2025
Previously, runtime assertions via debug_assert_xxx and CBMC
assertions via cassert(...) were separate.

This commit modifies the implementation of the debug assertion
macros so that when CBMC is used, debug assertions are intepreted
as proof obligations.

This removes some redundancy and non-uniformity in the code,
and also reduces the likelihood that debug assertions and CBMC
contracts get out of sync. In some case, this actually happened,
and the commit fixes this. The commit also adds further bounds
assertions in alignment with pre/post conditions.

A slight nuisance is that the debug assertions cannot flatten
nested structures like polyvec for the bounds check, running
into issue diffblue/cbmc#8570. We
work around this by introducing a new `xxx_2d` (for 2-dimensional)
macro which takes two dimensions and uses a two-step array
access, circumventing the above CBMC issue.

Signed-off-by: Hanno Becker <[email protected]>
hanno-becker added a commit to pq-code-package/mlkem-native that referenced this issue Jan 17, 2025
Previously, runtime assertions via debug_assert_xxx and CBMC
assertions via cassert(...) were separate.

This commit modifies the implementation of the debug assertion
macros so that when CBMC is used, debug assertions are intepreted
as proof obligations.

This removes some redundancy and non-uniformity in the code,
and also reduces the likelihood that debug assertions and CBMC
contracts get out of sync. In some case, this actually happened,
and the commit fixes this. The commit also adds further bounds
assertions in alignment with pre/post conditions.

A slight nuisance is that the debug assertions cannot flatten
nested structures like polyvec for the bounds check, running
into issue diffblue/cbmc#8570. We
work around this by introducing a new `xxx_2d` (for 2-dimensional)
macro which takes two dimensions and uses a two-step array
access, circumventing the above CBMC issue.

Signed-off-by: Hanno Becker <[email protected]>
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

No branches or pull requests

2 participants