Skip to content

Expanding __CPROVER_{r,w,rw}_ok must not introduce overflow [depends-on: #6616] #6656

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

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open
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
10 changes: 10 additions & 0 deletions regression/cbmc/gcc_builtin_add_overflow/conversion-check.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
#ifndef __GNUC__
_Bool __builtin_add_overflow();
#endif

int main()
{
unsigned a, b, c;
if(__builtin_add_overflow(a, b, &c))
return 0;
}
10 changes: 10 additions & 0 deletions regression/cbmc/gcc_builtin_add_overflow/conversion-check.desc
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
CORE
conversion-check.c
--conversion-check --unsigned-overflow-check --signed-overflow-check
^VERIFICATION SUCCESSFUL$
^EXIT=0$
^SIGNAL=0$
--
^Generated [1-9]\d* VCC\(s\)
--
Zero VCCs should be generated.
22 changes: 22 additions & 0 deletions regression/cbmc/pragma_cprover4/main.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
#include <assert.h>

int nondet_int();

void main()
{
int a = nondet_int();
int b = nondet_int();
int c = a +
#pragma CPROVER check disable "signed-overflow"
(a + b);
#pragma CPROVER check pop

#pragma CPROVER check disable "signed-overflow"
for(int i = 0; i < 10; ++i)
{
int temp = a + b;
#pragma CPROVER check pop
int foo = temp + a;
assert(foo > 2);
}
}
13 changes: 13 additions & 0 deletions regression/cbmc/pragma_cprover4/test.desc
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
CORE
main.c
--signed-overflow-check
^\[main.overflow.1\] line 9 arithmetic overflow on signed \+ in a \+ a \+ b: FAILURE$
^\[main.overflow.2\] line 19 arithmetic overflow on signed \+ in temp \+ a: FAILURE$
^\[main.assertion.1\] line 20 assertion foo > 2: FAILURE$
^\*\* 3 of 3 failed
^VERIFICATION FAILED$
^EXIT=10$
^SIGNAL=0$
--
--
No assertions other than the above three are expected.
9 changes: 9 additions & 0 deletions regression/contracts/simplify-overflow/main.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
int main()
{
char buf[10];
__CPROVER_size_t max = 9;
__CPROVER_size_t start = 1;

__CPROVER_assert(
__CPROVER_r_ok(buf + start, max - start), "array is readable");
}
13 changes: 13 additions & 0 deletions regression/contracts/simplify-overflow/test.desc
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
CORE
main.c
--nan-check _ --signed-overflow-check --unsigned-overflow-check
^VERIFICATION SUCCESSFUL$
^EXIT=0$
^SIGNAL=0$
--
^warning: ignoring
--
This test doesn't actually use contracts, but rather demonstrates the workflow
of having goto-instrument expand __CPROVER_r_ok (but really doing nothing else)
before CBMC when may add (overflow) checks. Doing so must not result in failing
assertions that would not have failed on the original input.
31 changes: 31 additions & 0 deletions src/analyses/goto_check_c.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1895,6 +1895,33 @@ void goto_check_ct::check_rec_arithmetic_op(

void goto_check_ct::check_rec(const exprt &expr, const guardt &guard)
{
flag_overridet resetter(expr.source_location());
const auto &pragmas = expr.source_location().get_pragmas();
for(const auto &d : pragmas)
{
// match named-check related pragmas
auto matched = match_named_check(d.first);
if(matched.has_value())
{
auto named_check = matched.value();
auto name = named_check.first;
auto status = named_check.second;
bool *flag = name_to_flag.find(name)->second;
switch(status)
{
case check_statust::ENABLE:
resetter.set_flag(*flag, true, name);
break;
case check_statust::DISABLE:
resetter.set_flag(*flag, false, name);
break;
case check_statust::CHECKED:
resetter.disable_flag(*flag, name);
break;
}
}
}

if(expr.id() == ID_exists || expr.id() == ID_forall)
{
// the scoped variables may be used in the assertion
Expand Down Expand Up @@ -2020,6 +2047,10 @@ optionalt<exprt> goto_check_ct::rw_ok_check(exprt expr)
exprt c = conjunction(conjuncts);
if(enable_simplify)
simplify(c, ns);

// this is a logic expression, C language checks no longer apply

Choose a reason for hiding this comment

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

Possible worth rewording the comment here to be super explicit?

Suggested change
// this is a logic expression, C language checks no longer apply
// this is an internal logic expression to support contracts, not a C language expression.
// C language checks should not be applied to it

add_all_disable_named_check_pragmas(c.add_source_location());

return c;
}
else if(modified)
Expand Down
3 changes: 3 additions & 0 deletions src/ansi-c/ansi_c_parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,9 @@ void ansi_c_parsert::pragma_cprover_add_check(

bool ansi_c_parsert::pragma_cprover_clash(const irep_idt &name, bool enabled)
{
if(pragma_cprover_stack.empty())
return false;

auto top = pragma_cprover_stack.back();
auto found = top.find(name);
return found != top.end() && found->second != enabled;
Expand Down
9 changes: 6 additions & 3 deletions src/goto-programs/goto_convert_side_effect.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -656,9 +656,12 @@ void goto_convertt::remove_overflow(
if(result.type().id() == ID_pointer)
{
result_type = to_pointer_type(result.type()).base_type();
code_assignt result_assignment{dereference_exprt{result},
typecast_exprt{operation, *result_type},
expr.source_location()};
source_locationt source_location_annotated = expr.source_location();
source_location_annotated.add_pragma("disable:conversion-check");
code_assignt result_assignment{
dereference_exprt{result},
typecast_exprt{operation, *result_type},
source_location_annotated};
convert_assign(result_assignment, dest, mode);
}
else
Expand Down