Skip to content

Add support for assigns clauses on loops #6249

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion regression/contracts/assigns_enforce_20/test.desc
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ main.c
--enforce-contract foo
^EXIT=10$
^SIGNAL=0$
^\[foo.\d+\] line \d+ Check that \*y is assignable: FAILURE$
^\[foo.\d+\] line \d+ Check that POINTER_OBJECT\((\(.+\))?y\) is assignable: FAILURE$
^\[foo.\d+\] line \d+ Check that x is assignable: FAILURE$
^VERIFICATION FAILED$
--
Expand Down
2 changes: 1 addition & 1 deletion regression/contracts/assigns_enforce_address_of/test.desc
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ main.c
--enforce-contract foo
^EXIT=(1|64)$
^SIGNAL=0$
^.*error: illegal target in assigns clause$
^.*error: non-lvalue target in assigns clause$
^CONVERSION ERROR$
--
--
Expand Down
2 changes: 1 addition & 1 deletion regression/contracts/assigns_enforce_arrays_05/test.desc
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
CORE
main.c
--enforce-contract assigns_ptr --enforce-contract assigns_range
--enforce-contract assigns_range
^EXIT=10$
^SIGNAL=0$
^VERIFICATION FAILED$
Expand Down
2 changes: 1 addition & 1 deletion regression/contracts/assigns_enforce_elvis_4/test.desc
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ main.c
--enforce-contract foo
^EXIT=(1|64)$
^SIGNAL=0$
^.*error: void-typed targets not permitted$
^.*error: void-typed targets not permitted in assigns clause$
--
--
Check that Elvis operator expressions '*(cond ? if_true : if_false)' with different types in true and false branches are rejected.
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ main.c
--enforce-contract foo
^EXIT=(1|64)$
^SIGNAL=0$
^.*error: illegal target in assigns clause$
^.*error: non-lvalue target in assigns clause$
^CONVERSION ERROR$
--
--
Expand Down
2 changes: 1 addition & 1 deletion regression/contracts/assigns_enforce_literal/test.desc
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ main.c
--enforce-contract foo
^EXIT=(1|64)$
^SIGNAL=0$
^.*error: illegal target in assigns clause$
^.*error: non-lvalue target in assigns clause$
^CONVERSION ERROR$
--
--
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ main.c
--enforce-contract foo
^EXIT=(1|64)$
^SIGNAL=0$
^.*error: void-typed targets not permitted$
^.*error: void-typed targets not permitted in assigns clause$
^CONVERSION ERROR$
--
--
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ main.c
--enforce-contract foo
^EXIT=(1|64)$
^SIGNAL=0$
^.*error: illegal target in assigns clause$
^.*error: non-lvalue target in assigns clause$
^CONVERSION ERROR$
--
--
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ main.c
--enforce-contract foo
^EXIT=(1|64)$
^SIGNAL=0$
^.*error: illegal target in assigns clause$
^.*error: non-lvalue target in assigns clause$
^CONVERSION ERROR$
--
--
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@ main.c
^EXIT=(1|64)$
^SIGNAL=0$
^CONVERSION ERROR$
error: illegal target in assigns clause
error: non-lvalue target in assigns clause
--
Checks whether type checking reports illegal target in assigns clause.
14 changes: 0 additions & 14 deletions regression/contracts/invar_assigns_alias_analysis/test.desc

This file was deleted.

10 changes: 2 additions & 8 deletions regression/contracts/invar_assigns_empty/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,8 @@

int main()
{
int r;
__CPROVER_assume(r >= 0);
while(r > 0)
__CPROVER_assigns() __CPROVER_loop_invariant(r >= 0)
while(1 == 1)
__CPROVER_assigns() __CPROVER_loop_invariant(1 == 1)
{
r--;
}
assert(r == 0);

return 0;
}
1 change: 0 additions & 1 deletion regression/contracts/invar_assigns_empty/test.desc
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ main.c
^SIGNAL=0$
^\[main.1\] .* Check loop invariant before entry: SUCCESS$
^\[main.2\] .* Check that loop invariant is preserved: SUCCESS$
^\[main.assertion.1\] .* assertion r == 0: SUCCESS$
^VERIFICATION SUCCESSFUL$
--
--
Expand Down
18 changes: 10 additions & 8 deletions regression/contracts/invar_assigns_opt/test.desc
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,16 @@ main.c
--apply-loop-contracts
^EXIT=0$
^SIGNAL=0$
^\[main.1\] .* Check loop invariant before entry: SUCCESS$
^\[main.2\] .* Check that loop invariant is preserved: SUCCESS$
^\[main.3\] .* Check decreases clause on loop iteration: SUCCESS$
^\[main.assertion.1\] .* assertion r1 == 0: SUCCESS$
^\[main.4\] .* Check loop invariant before entry: SUCCESS$
^\[main.5\] .* Check that loop invariant is preserved: SUCCESS$
^\[main.6\] .* Check decreases clause on loop iteration: SUCCESS$
^\[main.assertion.2\] .* assertion r2 == 0: SUCCESS$
^\[main.\d+\] .* Check loop invariant before entry: SUCCESS$
^\[main.\d+\] .* Check that loop invariant is preserved: SUCCESS$
^\[main.\d+\] .* Check decreases clause on loop iteration: SUCCESS$
^\[main.assertion.\d+\] .* assertion r1 == 0: SUCCESS$
^\[main.\d+\] .* Check loop invariant before entry: SUCCESS$
^\[main.\d+\] .* Check that s2 is assignable: SUCCESS$
^\[main.\d+\] .* Check that r2 is assignable: SUCCESS$
^\[main.\d+\] .* Check that loop invariant is preserved: SUCCESS$
^\[main.\d+\] .* Check decreases clause on loop iteration: SUCCESS$
^\[main.assertion.\d+\] .* assertion r2 == 0: SUCCESS$
^VERIFICATION SUCCESSFUL$
--
--
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,9 @@ to pass -- we should not mistakenly havoc the allocations, just their values.
However, the `data[1][2][3] == 0` assertion is expected to fail since `data`
is havoced.

Blocked on issue #6021 -- alias analysis is imprecise and returns `unknown`.
Blocked on #6326:
The assigns clause in this case would have an entire 3D space,
and some of it must be re-established, to the original objects
but with different values at certain locations, by the loop invariant.
However, currently we cannot restrict same_object for pointers
within loop invariants.
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@ void main()
data[4][5][6] = 0;

for(unsigned i = 0; i < SIZE; i++)
// clang-format off
__CPROVER_assigns(i, data[4][5][6])
__CPROVER_loop_invariant(i <= SIZE)
// clang-format on
{
data[4][5][6] = 1;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
KNOWNBUG
CORE
main.c
--apply-loop-contracts
^EXIT=10$
^SIGNAL=0$
^\[main.1\] .* Check loop invariant before entry: SUCCESS$
^\[main.2\] .* Check that loop invariant is preserved: SUCCESS$
^\[main.assertion.1\] .* assertion data\[4\]\[5\]\[6\] == 0: FAILURE$
^\[main.assertion.2\] .* assertion data\[1\]\[2\]\[3\] == 0: SUCCESS$
^\[main.\d+\] .* Check loop invariant before entry: SUCCESS$
^\[main.\d+\] .* Check that i is assignable: SUCCESS$
^\[main.\d+\] .* Check that data\[(.*)4\]\[(.*)5\]\[(.*)6\] is assignable: SUCCESS$
^\[main.\d+\] .* Check that loop invariant is preserved: SUCCESS$
^\[main.assertion.\d+\] .* assertion data\[4\]\[5\]\[6\] == 0: FAILURE$
^\[main.assertion.\d+\] .* assertion data\[1\]\[2\]\[3\] == 0: SUCCESS$
^VERIFICATION FAILED$
--
--
Expand All @@ -17,5 +19,3 @@ The `data[4][5][6] == 0` assertion is expected to fail since `data[4][5][6]`
is havoced and the invariant does not reestablish its value.
However, the `data[1][2][3] == 0` assertion is expected to pass -- we should
not havoc the entire `data` array, if only a constant index if being modified.

Blocked on issue #6021 -- alias analysis is imprecise and returns `unknown`.
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,9 @@ We _could_ do a more precise analysis in future where we only havoc
`data[4][5][6]` but not `data[1][2][3]` since the latter clearly cannot be
modified in the given loop.

Blocked on issue #6021 -- alias analysis is imprecise and returns `unknown`.
Blocked on #6326:
The assigns clause in this case would have an entire 2D grid,
and some of it must be re-established, to the original objects
but with different values at certain locations, by the loop invariant.
However, currently we cannot restrict same_object for pointers
within loop invariants.
27 changes: 27 additions & 0 deletions regression/contracts/loop_assigns-01/main.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
#include <assert.h>
#include <stdlib.h>

#define SIZE 8

struct blob
{
char *data;
};

void main()
{
struct blob *b = malloc(sizeof(struct blob));
b->data = malloc(SIZE);

b->data[5] = 0;
for(unsigned i = 0; i < SIZE; i++)
// clang-format off
__CPROVER_assigns(i, __CPROVER_POINTER_OBJECT(b->data))
__CPROVER_loop_invariant(i <= SIZE)
// clang-format on
{
b->data[i] = 1;
}

assert(b->data[5] == 0);
}
16 changes: 16 additions & 0 deletions regression/contracts/loop_assigns-01/test.desc
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
CORE
main.c
--apply-loop-contracts
^EXIT=10$
^SIGNAL=0$
^\[main.\d+\] .* Check loop invariant before entry: SUCCESS$
^\[main.\d+\] .* Check that i is assignable: SUCCESS$
^\[main.\d+\] .* Check that b->data\[(.*)i\] is assignable: SUCCESS$
^\[main.\d+\] .* Check that loop invariant is preserved: SUCCESS$
^\[main.assertion.\d+\] .* assertion b->data\[5\] == 0: FAILURE$
^VERIFICATION FAILED$
--
--
This test (taken from #6021) shows the need for assigns clauses on loops.
The alias analysis in this case returns `unknown`,
and so we must manually annotate an assigns clause on the loop.
27 changes: 27 additions & 0 deletions regression/contracts/loop_assigns-02/main.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
#include <assert.h>
#include <stdlib.h>

#define SIZE 8

struct blob
{
char *data;
};

void main()
{
struct blob *b = malloc(sizeof(struct blob));
b->data = malloc(SIZE);

b->data[5] = 0;
for(unsigned i = 0; i < SIZE; i++)
// clang-format off
__CPROVER_assigns(i, b->data[i])
__CPROVER_loop_invariant(i <= SIZE)
// clang-format on
{
b->data[i] = 1;
}

assert(b->data[5] == 0);
}
20 changes: 20 additions & 0 deletions regression/contracts/loop_assigns-02/test.desc
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
CORE
main.c
--apply-loop-contracts
^EXIT=10$
^SIGNAL=0$
^\[main.\d+\] .* Check loop invariant before entry: SUCCESS$
^\[main.\d+\] .* Check that i is assignable: SUCCESS$
^\[main.\d+\] .* Check that b->data\[(.*)i\] is assignable: FAILURE$
^\[main.\d+\] .* Check that loop invariant is preserved: SUCCESS$
^\[main.assertion.\d+\] .* assertion b->data\[5\] == 0: FAILURE$
^VERIFICATION FAILED$
--
--
This test (taken from #6021) shows the need for assigns clauses on loops.
The alias analysis in this case returns `unknown`,
and so we must manually annotate an assigns clause on the loop.

Note that the annotated assigns clause in this case is an underapproximation,
per the current semantics of the assigns clause -- it must model ALL memory
being assigned to by the loop, not just a single symbolic iteration.
27 changes: 27 additions & 0 deletions regression/contracts/loop_assigns-03/main.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
#include <assert.h>
#include <stdlib.h>

#define SIZE 8

struct blob
{
char *data;
};

void main()
{
struct blob *b = malloc(sizeof(struct blob));
b->data = malloc(SIZE);

b->data[5] = 0;
for(unsigned i = 0; i < SIZE; i++)
// clang-format off
__CPROVER_assigns(__CPROVER_POINTER_OBJECT(b->data))
__CPROVER_loop_invariant(i <= SIZE)
// clang-format on
{
b->data[i] = 1;
}

assert(b->data[5] == 0);
}
17 changes: 17 additions & 0 deletions regression/contracts/loop_assigns-03/test.desc
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
CORE
main.c
--apply-loop-contracts
^EXIT=10$
^SIGNAL=0$
^\[main.\d+\] .* Check loop invariant before entry: SUCCESS$
^\[main.\d+\] .* Check that i is assignable: FAILURE$
^\[main.\d+\] .* Check that b->data\[(.*)i\] is assignable: SUCCESS$
^VERIFICATION FAILED$
--
--
This test (taken from #6021) shows the need for assigns clauses on loops.
The alias analysis in this case returns `unknown`,
and so we must manually annotate an assigns clause on the loop.

Note that the annotated assigns clause in this case is an underapproximation,
because `i` is also assigned in the loop and should be marked as assignable.
Comment on lines +16 to +17
Copy link
Collaborator

Choose a reason for hiding this comment

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

but again, isn't that part of the "local write set"?

Copy link
Contributor

@SaswatPadhi SaswatPadhi Nov 12, 2021

Choose a reason for hiding this comment

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

Note that the initialization of i is actually pulled "outside" of the loop. The loop body doesn't contain the definition of i, the enclosing scope does.

Copy link
Collaborator

Choose a reason for hiding this comment

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

moreover the information that i was declared for( <here>; ... ; ...) is destroyed by C-to-goto conversion

Copy link
Collaborator

Choose a reason for hiding this comment

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

moreover the information that i was declared for( <here>; ... ; ...) is destroyed by C-to-goto conversion

Oh that clarifies this a bit more. So a simple loop like this

  for(int i = 0; i < 2; i++)
    assert(i < 2);

is converted to something like this

        // 0 file main.c line 13 function main
        DECL main::1::1::i : signedbv[32]
        // 1 file main.c line 13 function main
        ASSIGN main::1::1::i := 0
        // 2 file main.c line 13 function main
     1: IF ¬(main::1::1::i < 2) THEN GOTO 2
        // 3 file main.c line 14 function main
        ASSERT main::1::1::i < 2 // assertion i < 2
        // 4 file main.c line 13 function main
        ASSIGN main::1::1::i := main::1::1::i + 1
        // 5 file main.c line 13 function main
        GOTO 1
        // 6 file main.c line 14 function main
     2: DEAD main::1::1::i

So the declaration of i in this context is outside of the loop body. If we have local variables in the loop body, do users must add them to the assign clause as well? It's a bit odd that we don't have access to these initial declarations since their scope is only the loop body.

Copy link
Contributor

@SaswatPadhi SaswatPadhi Nov 15, 2021

Choose a reason for hiding this comment

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

If we have local variables in the loop body, do users must add them to the assign clause as well?

Local variables (variables declared inside the loop / function body) are completely unrelated here. Loop-local variables need not be specified in the assigns clause -- they aren't even visible at the enclosing scope.

I think the main confusion here is that i is "a local variable in the loop body". It's not; it's outside of the loop body.

Before C99 standard, declarations within for loop initialization were not allowed. So the declaration for i had to be necessarily made outside of the loop. Starting with C99, i can be declared in the initialization part of the for loop and has the same scope as the loop (not the loop body).

cppreference states that:

{
  ...
  for ( init-statement ; condition ; iteration-expression ) statement
  ..
}

is equivalent to: (comments added by me)

{
  // This is the function-body scope
  ...
  {
    // This is NOT the loop-body scope
    init-statement
    while ( condition )
    // This is where the invariant goes
    {
      // This is the loop-body scope
      statement
      iteration-expression ;
    }
  }
  ...
} 

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thank you so much for adding a bit more context here! If there is a "loop scope" that encapsulates the entire loop and loop body, why are we focusing only in the loop body and not the most general scope? The only goal I'm trying to active with this discussion is: decrease the burden of annotation for users (this is not a blocker for this PR).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Your previous comment respond my previous question: #6249 (comment). If you could grab all declarations from the initialization clause of a for loop and add them to the assigns clause automatically, this would decrease the annotation burden to developers and it's perfectly sound for our approach. Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes exactly. I'll try to push that change in this PR if it works (still playing with it), or would make a separate PR for it later.

41 changes: 41 additions & 0 deletions regression/contracts/loop_assigns-04/main.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
#include <assert.h>
#include <stdlib.h>

#define SIZE 8

struct blob
{
char *data;
};

void main()
{
int y;
struct blob *b = malloc(sizeof(struct blob));
b->data = malloc(SIZE);

b->data[5] = 0;
for(unsigned i = 0; i < SIZE; i++)
// clang-format off
__CPROVER_assigns(i, __CPROVER_POINTER_OBJECT(b->data))
__CPROVER_loop_invariant(i <= SIZE)
// clang-format on
{
b->data[i] = 1;

int x;
for(unsigned j = 0; j < i; j++)
// clang-format off
// y is not assignable by outer loop, so this should be flagged
__CPROVER_assigns(j, y, x, b->data[i])
__CPROVER_loop_invariant(j <= i)
// clang-format on
{
x = b->data[j] * b->data[j];
b->data[i] += x;
y += j;
}
}

assert(b->data[5] == 0);
}
Loading