Skip to content

Fix local static variables detection in assigns clauses #6447

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
31 changes: 31 additions & 0 deletions regression/contracts/assigns_enforce_detect_local_statics/main.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
#include <assert.h>
#include <stdlib.h>

static int x;
static int xx;

void foo()
{
int *y = &x;
int *yy = &xx;

static int x;
// must pass (modifies local static)
x++;

// must pass (modifies assignable global static )
(*y)++;

// must fail (modifies non-assignable global static)
(*yy)++;
}

void bar() __CPROVER_assigns(x)
{
foo();
}

int main()
{
bar();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
CORE
main.c
--enforce-contract bar
^EXIT=10$
^SIGNAL=0$
^\[foo.\d+\] line \d+ Check that y is assignable: SUCCESS$
^\[foo.\d+\] line \d+ Check that foo\$\$1\$\$x is assignable: SUCCESS$
^\[foo.\d+\] line \d+ Check that \*y is assignable: SUCCESS$
^\[foo.\d+\] line \d+ Check that \*yy is assignable: FAILURE$
^VERIFICATION FAILED$
--
--
Checks whether static local and global variables are correctly tracked
in assigns clause verification, accross subfunction calls.
10 changes: 5 additions & 5 deletions regression/contracts/assigns_enforce_statics/main.c
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
#include <assert.h>
#include <stdlib.h>

static int x;
static int x = 0;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Random place for comment: the commit message ...

  1. Could make clear that this is about code contracts;
  2. Should likely have the "A symbol is ..." all moved into the body of the message;
  3. Should likely talk about "called functions" instead of "subfunctions."


void foo() __CPROVER_assigns(x)
{
int *y = &x;

static int x;
static int x = 0;

// should pass (assigns local x)
x++;

// should fail (assigns global x)
(*y)++;
}

Expand Down
20 changes: 6 additions & 14 deletions regression/contracts/function-calls-03-1/test-enf-f1.desc
Original file line number Diff line number Diff line change
@@ -1,20 +1,12 @@
CORE
main.c
--enforce-contract f1 _ --unwind 20 --unwinding-assertions
^EXIT=10$
^file main\.c line 13 function f2: recursion is ignored on call to 'f2'$
^Invariant check failed$
^Condition: decorated\.get_recursive_function_warnings_count\(\) == 0$
^Reason: Recursive functions found during inlining$
^EXIT=(127|134|137)$
^SIGNAL=0$
^file main.c line \d+ function f2: recursion is ignored on call to \'f2\'$
^\[postcondition.\d+\] file main.c line \d+ Check ensures clause: FAILURE$
^VERIFICATION FAILED$
--
--
Verification:
function | pre-cond | post-cond
---------|----------|----------
f1 | assumed | asserted

Test should fail:
The postcondition of f2 is incorrect, considering the recursion particularity.

Recursion:
The base case for the recursive call to f2 provides different behavior than the common case (given the pre-conditions).
Test should fail because a recursive function is found during inlining.
21 changes: 6 additions & 15 deletions regression/contracts/function-calls-03-1/test-enf-f2.desc
Original file line number Diff line number Diff line change
@@ -1,21 +1,12 @@
CORE
main.c
--enforce-contract f2 _ --unwind 20 --unwinding-assertions
^EXIT=10$
^file main\.c line 13 function f2: recursion is ignored on call to 'f2'$
^Invariant check failed$
^Condition: decorated\.get_recursive_function_warnings_count\(\) == 0$
^Reason: Recursive functions found during inlining$
^EXIT=(127|134|137)$
^SIGNAL=0$
^file main.c line \d+ function f2: recursion is ignored on call to \'f2\'$
^\[postcondition.\d+\] file main.c line \d+ Check ensures clause: FAILURE$
^\[f2.\d+\] line \d+ Check that loc is assignable: SUCCESS$
^VERIFICATION FAILED$
--
--
Verification:
function | pre-cond | post-cond
---------|----------|----------
f2 | assumed | asserted

Test should fail:
The postcondition of f2 is incorrect, considering the recursion particularity.

Recursion:
The base case for the recursive call to f2 provides different behavior than the common case (given the pre-conditions).
Test should fail because a recursive function is found during inlining.
21 changes: 6 additions & 15 deletions regression/contracts/function-calls-04-1/test-enf-f1.desc
Original file line number Diff line number Diff line change
@@ -1,21 +1,12 @@
CORE
main.c
--enforce-contract f1 _ --unwind 20 --unwinding-assertions
^EXIT=10$
^file main\.c line 19 function f2_in: recursion is ignored on call to 'f2_out'$
^Invariant check failed$
^Condition: decorated\.get_recursive_function_warnings_count\(\) == 0$
^Reason: Recursive functions found during inlining$
^EXIT=(127|134|137)$
^SIGNAL=0$
^file main.c line \d+ function f2\_in: recursion is ignored on call to \'f2\_out\'$
^\[postcondition.\d+\] file main.c line \d+ Check ensures clause: FAILURE$
^VERIFICATION FAILED$
--
--
Verification:
function | pre-cond | post-cond
---------|----------|----------
f1 | assumed | asserted

Test should fail:
The postcondition of f2_out is incorrect, considering the recursion particularity.

Recursion:
The base case for the recursive call to f2_out provides different behavior
than the general case (given the pre-conditions).
Test should fail because a recursive function is found during inlining.
22 changes: 6 additions & 16 deletions regression/contracts/function-calls-04-1/test-enf-f2_in.desc
Original file line number Diff line number Diff line change
@@ -1,22 +1,12 @@
CORE
main.c
--enforce-contract f2_in _ --unwind 20 --unwinding-assertions
^EXIT=10$
^file main\.c line 13 function f2_out: recursion is ignored on call to 'f2_in'$
^Invariant check failed$
^Condition: decorated\.get_recursive_function_warnings_count\(\) == 0$
^Reason: Recursive functions found during inlining$
^EXIT=(127|134|137)$
^SIGNAL=0$
^file main.c line \d+ function f2\_out: recursion is ignored on call to \'f2\_in\'$
^\[postcondition.\d+\] file main.c line \d+ Check ensures clause: FAILURE$
^\[f2\_out.\d+\] line \d+ Check that loc2 is assignable: SUCCESS$
^VERIFICATION FAILED$
--
--
Verification:
function | pre-cond | post-cond
---------|----------|----------
f2_in | assumed | asserted

Test should fail:
The postcondition of f2_out is incorrect, considering the recursion particularity.

Recursion:
The base case for the recursive call to f2_out provides different behavior
than the general case (given the pre-conditions).
Test should fail because a recursive function is found during inlining.
22 changes: 6 additions & 16 deletions regression/contracts/function-calls-04-1/test-enf-f2_out.desc
Original file line number Diff line number Diff line change
@@ -1,22 +1,12 @@
CORE
main.c
--enforce-contract f2_out _ --unwind 20 --unwinding-assertions
^EXIT=10$
^file main\.c line 19 function f2_in: recursion is ignored on call to 'f2_out'$
^Invariant check failed$
^Condition: decorated\.get_recursive_function_warnings_count\(\) == 0$
^Reason: Recursive functions found during inlining$
^EXIT=(127|134|137)$
^SIGNAL=0$
^file main.c line \d+ function f2\_in: recursion is ignored on call to \'f2\_out\'$
^\[postcondition.\d+\] file main.c line \d+ Check ensures clause: FAILURE$
^\[f2\_out.\d+\] line \d+ Check that loc2 is assignable: SUCCESS$
^VERIFICATION FAILED$
--
--
Verification:
function | pre-cond | post-cond
---------|----------|----------
f2_out | assumed | asserted

Test should fail:
The postcondition of f2 is incorrect, considering the recursion particularity.

Recursion:
The base case for the recursive call to f2_out provides different behavior
than the general case (given the pre-conditions).
Test should fail because a recursive function is found during inlining.
23 changes: 23 additions & 0 deletions regression/contracts/function-calls-recursive-01/main.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
#include <stdlib.h>

int sum_rec(int i, int acc)
{
if(i >= 0)
return sum_rec(i - 1, acc + i);
return acc;
}

int sum(int i) __CPROVER_requires(0 <= i && i <= 50)
__CPROVER_ensures(__CPROVER_return_value >= 0) __CPROVER_assigns()
{
int j = i;
int res = sum_rec(j, 0);
return res;
}

int main()
{
int result = sum(10);
__CPROVER_assert(result == 55, "result == 55");
return 0;
}
12 changes: 12 additions & 0 deletions regression/contracts/function-calls-recursive-01/test.desc
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
CORE
main.c
--enforce-contract sum _ --trace
^file main\.c line 6 function sum_rec: recursion is ignored on call to 'sum_rec'$
^Invariant check failed$
^Condition: decorated\.get_recursive_function_warnings_count\(\) == 0$
^Reason: Recursive functions found during inlining$
^EXIT=(127|134|137)$
^SIGNAL=0$
--
--
Test should fail because a recursive function is found during inlining.
10 changes: 5 additions & 5 deletions src/analyses/call_graph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,11 @@ static void forall_callsites(
if(i_it->is_function_call())
{
const exprt &function_expr = i_it->call_function();
if(function_expr.id()==ID_symbol)
{
const irep_idt &callee=to_symbol_expr(function_expr).get_identifier();
call_task(i_it, callee);
}
PRECONDITION_WITH_DIAGNOSTICS(
function_expr.id() == ID_symbol,
"call graph computation requires function pointer removal");
const irep_idt &callee = to_symbol_expr(function_expr).get_identifier();
call_task(i_it, callee);
}
}
}
Expand Down
30 changes: 30 additions & 0 deletions src/goto-instrument/contracts/assigns.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ Date: July 2021

#include "assigns.h"

#include <analyses/call_graph.h>
Copy link
Contributor

Choose a reason for hiding this comment

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

[nit] this should go above langapi include(s).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed


#include <langapi/language_util.h>

#include <util/arith_tools.h>
Expand Down Expand Up @@ -196,3 +198,31 @@ void havoc_assigns_targetst::append_havoc_code_for_expr(

havoc_utilst::append_havoc_code_for_expr(location, expr, dest);
}

void assigns_clauset::add_static_locals_to_write_set(
const goto_functionst &functions,
const irep_idt &root_function)
{
auto call_graph =
call_grapht::create_from_root_function(functions, root_function, true)
.get_directed_graph();

for(const auto &sym_pair : symbol_table)
{
const auto &sym = sym_pair.second;
if(sym.is_static_lifetime)
{
auto fname = sym.location.get_function();
if(
!fname.empty() && (fname == root_function ||
call_graph.get_node_index(fname).has_value()))
{
// We found a symbol with
// - a static lifetime
// - non empty location function
// - this function appears in the call graph of the function
add_to_write_set(sym.symbol_expr());
}
}
}
}
15 changes: 15 additions & 0 deletions src/goto-instrument/contracts/assigns.h
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,21 @@ class assigns_clauset
return write_set;
}

/// \brief Finds symbols declared with a static lifetime in the given
/// `root_function` or one of the functions it calls,
/// and adds them to the write set of this assigns clause.
///
/// @param functions all functions of the goto_model
/// @param root_function the root function under which to search statics
///
/// A symbol is considered a static local symbol iff:
/// - it has a static lifetime annotation
/// - its source location has a non-empty function attribute
/// - this function attribute is found in the call graph of the root_function
void add_static_locals_to_write_set(
const goto_functionst &functions,
const irep_idt &root_function);

const messaget &log;
const namespacet &ns;
const irep_idt &function_name;
Expand Down
Loading