Skip to content

Commit ef6005d

Browse files
smowtonsmowton
committed
Symex: restore guards on function exit
Executing a function may have a cumulative effect on the state guard. For example, if the callee contained ASSUME statements that rendered one or more control-flow options unviable then the guard might still embody that restriction (i.e. for if(x) ASSUME(false) the guard might still be `!x`). However, on function return we know that all control-flow paths have converged or been shown to be unviable, so we can restore the simpler guard as it was when we entered the callee function. Exceptions: (a) if the guard is false it would be correct but inefficient to restore it; keep it false until we find a convergeance with another viable path (b) if we're doing path-sensitive symex then we do tail duplication, and there are no control-flow convergeances. Keep the guard as-was. (c) if we're executing a multi-threaded program then symex_assume_l2 uses the guard to accumulate assumptions, which we must not discard. In truth this optimisation is applicable whenever a block postdominates another, but function structure gives us a cheap way to establish postdominance without analysis (in the absence of setjmp/longjmp at least)
1 parent 719e9a9 commit ef6005d

18 files changed

+216
-10
lines changed

‎regression/cbmc/residual-guards-1/test.c

+19
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
void g()
2+
{
3+
g();
4+
}
5+
6+
void f(int y)
7+
{
8+
if(y == 5)
9+
{
10+
g();
11+
y = 10;
12+
}
13+
}
14+
15+
int main(int argc, char **argv)
16+
{
17+
f(argc);
18+
assert(argc == 1);
19+
}

‎regression/cbmc/residual-guards-1/test.desc

+14
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
CORE paths-lifo-expected-failure
2+
test.c
3+
--show-vcc --unwind 10
4+
^\{1\} main::argc!0@1#1 = 1$
5+
^EXIT=0$
6+
^SIGNAL=0$
7+
--
8+
^\{-[0-9]+\} f::y!0@1#[0-9]+ = 10$
9+
--
10+
This checks that the assertion, argc == 1, is unguarded (i.e. is not of the form
11+
'guard => condition'). Such a guard is redundant, but could be added before goto-symex
12+
made sure to restore guards to their state at function entry.
13+
We also check that no VCC is created for the unreachable code 'y = 10;'
14+
--paths mode is excluded as it currently always accrues large guards as it proceeds through execution

‎regression/cbmc/residual-guards-1/test_execution.desc

+11
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
CORE
2+
test.c
3+
--unwind 10
4+
^\[main\.assertion\.[0-9]+\] line [0-9]+ assertion argc == 1: FAILURE$
5+
^VERIFICATION FAILED$
6+
^EXIT=10$
7+
^SIGNAL=0$
8+
--
9+
--
10+
The main test is in test.desc; this just checks that the test is executable and
11+
the assertion fails as expected.

‎regression/cbmc/residual-guards-2/test.c

+16
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
void f(int y)
2+
{
3+
if(y == 5)
4+
{
5+
for(int i = 0; i < 20; ++i)
6+
{
7+
}
8+
y = 10;
9+
}
10+
}
11+
12+
int main(int argc, char **argv)
13+
{
14+
f(argc);
15+
assert(argc == 1);
16+
}

‎regression/cbmc/residual-guards-2/test.desc

+12
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
CORE paths-lifo-expected-failure
2+
test.c
3+
--show-vcc --unwind 10
4+
^\{1\} main::argc!0@1#1 = 1$
5+
^EXIT=0$
6+
^SIGNAL=0$
7+
--
8+
--
9+
This checks that the assertion, argc == 1, is unguarded (i.e. is not of the form
10+
'guard => condition'). Such a guard is redundant, but could be added before goto-symex
11+
made sure to restore guards to their state at function entry.
12+
--paths mode is excluded as it currently always accrues large guards as it proceeds through execution

‎regression/cbmc/residual-guards-2/test_execution.desc

+11
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
CORE
2+
test.c
3+
--unwind 10
4+
^\[main\.assertion\.[0-9]+\] line [0-9]+ assertion argc == 1: FAILURE$
5+
^VERIFICATION FAILED$
6+
^EXIT=10$
7+
^SIGNAL=0$
8+
--
9+
--
10+
The main test is in test.desc; this just checks that the test is executable and
11+
the assertion fails as expected.

‎regression/cbmc/residual-guards-2/unreachable-code.desc

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
FUTURE
2+
test.c
3+
--show-vcc --unwind 10
4+
^EXIT=0$
5+
^SIGNAL=0$
6+
--
7+
^\{-[0-9]+\} f::y!0@1#[0-9]+ = 10$
8+
--
9+
We also check that no VCC is created for the unreachable code 'y = 10;'

‎regression/cbmc/residual-guards-3/test.c

+19
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
void g(int x)
2+
{
3+
return g(x + 1);
4+
}
5+
6+
void f(int y)
7+
{
8+
if(y == 5)
9+
{
10+
g(1);
11+
y = 10;
12+
}
13+
}
14+
15+
int main(int argc, char **argv)
16+
{
17+
f(argc);
18+
assert(argc == 1);
19+
}

‎regression/cbmc/residual-guards-3/test.desc

+14
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
CORE paths-lifo-expected-failure
2+
test.c
3+
--show-vcc --depth 1000
4+
^\{1\} main::argc!0@1#1 = 1$
5+
^EXIT=0$
6+
^SIGNAL=0$
7+
--
8+
^\{-[0-9]+\} f::y!0@1#[0-9]+ = 10$
9+
--
10+
This checks that the assertion, argc == 1, is unguarded (i.e. is not of the form
11+
'guard => condition'). Such a guard is redundant, but could be added before goto-symex
12+
made sure to restore guards to their state at function entry.
13+
We also check that no VCC is created for the unreachable code 'y = 10;'
14+
--paths mode is excluded as it currently always accrues large guards as it proceeds through execution

‎regression/cbmc/residual-guards-3/test_execution.desc

+11
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
CORE
2+
test.c
3+
--depth 1000
4+
^\[main\.assertion\.[0-9]+\] line [0-9]+ assertion argc == 1: FAILURE$
5+
^VERIFICATION FAILED$
6+
^EXIT=10$
7+
^SIGNAL=0$
8+
--
9+
--
10+
The main test is in test.desc; this just checks that the test is executable and
11+
the assertion fails as expected.

‎regression/cbmc/residual-guards-4/test.c

+14
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
void f(int y)
2+
{
3+
if(y == 5)
4+
{
5+
__CPROVER_assume(0);
6+
y = 10;
7+
}
8+
}
9+
10+
int main(int argc, char **argv)
11+
{
12+
f(argc);
13+
assert(argc == 1);
14+
}

‎regression/cbmc/residual-guards-4/test.desc

+12
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
CORE paths-lifo-expected-failure
2+
test.c
3+
--show-vcc
4+
^\{1\} main::argc!0@1#1 = 1$
5+
^EXIT=0$
6+
^SIGNAL=0$
7+
--
8+
--
9+
This checks that the assertion, argc == 1, is unguarded (i.e. is not of the form
10+
'guard => condition'). Such a guard is redundant, but could be added before goto-symex
11+
made sure to restore guards to their state at function entry.
12+
--paths mode is excluded as it currently always accrues large guards as it proceeds through execution

‎regression/cbmc/residual-guards-4/test_execution.desc

+11
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
CORE
2+
test.c
3+
4+
^\[main\.assertion\.[0-9]+\] line [0-9]+ assertion argc == 1: FAILURE$
5+
^VERIFICATION FAILED$
6+
^EXIT=10$
7+
^SIGNAL=0$
8+
--
9+
--
10+
The main test is in test.desc; this just checks that the test is executable and
11+
the assertion fails as expected.

‎regression/cbmc/residual-guards-4/unreachable-code.desc

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
FUTURE
2+
test.c
3+
--show-vcc
4+
^EXIT=0$
5+
^SIGNAL=0$
6+
--
7+
^\{-[0-9]+\} f::y!0@1#[0-9]+ = 10$
8+
--
9+
We also check that no VCC is created for the unreachable code 'y = 10;'

‎src/goto-symex/call_stack.h

+3-2
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,10 @@ class call_stackt : public std::vector<framet>
2626
return back();
2727
}
2828

29-
framet &new_frame(symex_targett::sourcet calling_location)
29+
framet &
30+
new_frame(symex_targett::sourcet calling_location, const guardt &guard)
3031
{
31-
emplace_back(calling_location);
32+
emplace_back(calling_location, guard);
3233
return back();
3334
}
3435

‎src/goto-symex/frame.h

+4-3
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ struct framet
2727
std::map<goto_programt::const_targett, goto_state_listt> goto_state_map;
2828
symex_targett::sourcet calling_location;
2929
std::vector<irep_idt> parameter_names;
30-
30+
guardt guard_at_function_start;
3131
goto_programt::const_targett end_of_function;
3232
exprt return_value = nil_exprt();
3333
bool hidden_function = false;
@@ -48,8 +48,9 @@ struct framet
4848

4949
std::unordered_map<irep_idt, loop_infot> loop_iterations;
5050

51-
explicit framet(symex_targett::sourcet _calling_location)
52-
: calling_location(std::move(_calling_location))
51+
framet(symex_targett::sourcet _calling_location, const guardt &state_guard)
52+
: calling_location(std::move(_calling_location)),
53+
guard_at_function_start(state_guard)
5354
{
5455
}
5556
};

‎src/goto-symex/goto_symex_state.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ goto_symex_statet::goto_symex_statet(
4444
fresh_l2_name_provider(fresh_l2_name_provider)
4545
{
4646
threads.emplace_back(guard_manager);
47-
call_stack().new_frame(source);
47+
call_stack().new_frame(source, guardt(true_exprt(), manager));
4848
}
4949

5050
goto_symex_statet::~goto_symex_statet()=default;

‎src/goto-symex/symex_function_call.cpp

+26-4
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,7 @@ void goto_symext::symex_function_call_code(
303303

304304
// produce a new frame
305305
PRECONDITION(!state.call_stack().empty());
306-
framet &frame = state.call_stack().new_frame(state.source);
306+
framet &frame = state.call_stack().new_frame(state.source, state.guard);
307307

308308
// preserve locality of local variables
309309
locality(identifier, state, path_storage, goto_function, ns);
@@ -332,8 +332,10 @@ void goto_symext::symex_function_call_code(
332332
}
333333

334334
/// pop one call frame
335-
static void
336-
pop_frame(goto_symext::statet &state, const path_storaget &path_storage)
335+
static void pop_frame(
336+
goto_symext::statet &state,
337+
const path_storaget &path_storage,
338+
bool doing_path_exploration)
337339
{
338340
PRECONDITION(!state.call_stack().empty());
339341

@@ -347,6 +349,26 @@ pop_frame(goto_symext::statet &state, const path_storaget &path_storage)
347349
// restore L1 renaming
348350
state.level1.restore_from(frame.old_level1);
349351

352+
// If the program is multi-threaded then the state guard is used to
353+
// accumulate assumptions (in symex_assume_l2) and must be left alone.
354+
// If however it is single-threaded then we should restore the guard, as the
355+
// guard coming out of the function may be more complex (e.g. if the callee
356+
// was { if(x) __CPROVER_assume(false); } then the guard may still be `!x`),
357+
// but at this point all control-flow paths have either converged or been
358+
// proven unviable, so we can stop specifying the callee's constraints when
359+
// we generate an assumption or VCC.
360+
361+
// If the guard is false, *this* path is unviable and we shouldn't discard
362+
// that knowledge. If we're doing path exploration then we do
363+
// tail-duplication, and we actually *are* in a more-restricted context
364+
// than we were when the function began.
365+
if(
366+
state.threads.size() == 1 && !state.guard.is_false() &&
367+
!doing_path_exploration)
368+
{
369+
state.guard = frame.guard_at_function_start;
370+
}
371+
350372
symex_renaming_levelt::viewt view;
351373
state.get_level2().current_names.get_view(view);
352374

@@ -387,7 +409,7 @@ void goto_symext::symex_end_of_function(statet &state)
387409
state.guard.as_expr(), state.source.function_id, state.source, hidden);
388410

389411
// then get rid of the frame
390-
pop_frame(state, path_storage);
412+
pop_frame(state, path_storage, symex_config.doing_path_exploration);
391413
}
392414

393415
/// Preserves locality of parameters of a given function by applying L1

0 commit comments

Comments
 (0)