Skip to content

CONTRACTS: Add inductive checks for assigns clauses [depends-on: #7127] #7300

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

Closed
wants to merge 2 commits into from
Closed
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
4 changes: 2 additions & 2 deletions regression/contracts/loop_assigns-02/test.desc
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ main.c
^SIGNAL=0$
^\[main.\d+\] .* Check loop invariant before entry: SUCCESS$
^\[main.assigns.\d+\] .* Check that i is assignable: SUCCESS$
^\[main.assigns.\d+\] .* Check that b->data\[(.*)i\] is assignable: FAILURE$
^\[main.assigns.\d+\] .* Check that b->data\[(.*)i\] is inductively assignable: FAILURE$
^\[main.\d+\] .* Check that loop invariant is preserved: SUCCESS$
^\[main.assertion.\d+\] .* assertion b->data\[5\] == 0: FAILURE$
^\*\* 1 of 12 failed \(2 iterations\)
^VERIFICATION FAILED$
--
--
Expand Down
27 changes: 19 additions & 8 deletions src/goto-instrument/contracts/contracts.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,9 @@ void code_contractst::check_apply_loop_contracts(
//
// ... preamble ...
// ,- initialize loop_entry history vars;
// | entered_loop = false
// loop assigns check | initial_invariant_val = invariant_expr;
// - unchecked, temps | in_base_case = true;
// func assigns check | snapshot (write_set);
// loop assigns check | entered_loop = false
// - unchecked, temps | initial_invariant_val = invariant_expr;
// func assigns check | in_base_case = true;
// - disabled via pragma | goto HEAD;
// | STEP:
// --. | assert (initial_invariant_val);
Expand All @@ -105,10 +104,12 @@ void code_contractst::check_apply_loop_contracts(
// loop assigns check ,- ... eval guard ...
// + assertions added | if (!guard)
// func assigns check | goto EXIT;
// - disabled via pragma `- ... loop body ...
// - disabled via pragma | snapshot (write_set);
// `- ... loop body ...
// ,- entered_loop = true
// | if (in_base_case)
// | goto STEP;
// | assert CAR_begin in CAR_end
// loop assigns check | assert (invariant_expr);
// - unchecked, temps | new_variant_val = decreases_clause_expr;
// func assigns check | assert (new_variant_val < old_variant_val);
Expand Down Expand Up @@ -261,9 +262,8 @@ void code_contractst::check_apply_loop_contracts(

// Insert instrumentation
// This must be done before havocing the write set.
// FIXME: this is not true for write set targets that
// might depend on other write set targets.
pre_loop_head_instrs.destructive_append(snapshot_instructions);
goto_function.body.destructive_insert(
std::next(loop_head), snapshot_instructions);

// Insert a jump to the loop head
// (skipping over the step case initialization code below)
Expand Down Expand Up @@ -409,6 +409,17 @@ void code_contractst::check_apply_loop_contracts(
pre_loop_end_instrs.add(goto_programt::make_goto(
step_case_target, in_base_case, loop_head_location));

// Adding checks that demonstrating that the loop assigns clause is an
// inductive invariant.
// CAR_begin should be included in CAR_end
goto_programt assigns_inductive_check_instrs;
for(const auto &target : to_havoc)
{
instrument_spec_assigns.check_inclusion_induction(
target, assigns_inductive_check_instrs);
pre_loop_end_instrs.destructive_append(assigns_inductive_check_instrs);
}

// The following code is only reachable in the step case,
// i.e., when in_base_case == false,
// because of the unconditional jump above.
Expand Down
20 changes: 17 additions & 3 deletions src/goto-instrument/contracts/instrument_spec_assigns.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,17 @@ void instrument_spec_assignst::check_inclusion_assignment(
// create temporary car but do not track
const auto car = create_car_expr(true_exprt{}, lhs);
create_snapshot(car, dest);
inclusion_check_assertion(car, false, true, dest);
inclusion_check_assertion(car, false, true, false, dest);
}

void instrument_spec_assignst::check_inclusion_induction(
const exprt &lhs,
goto_programt &dest) const
{
// create temporary car but do not track
const auto car = create_car_expr(true_exprt{}, lhs);
create_snapshot(car, dest);
inclusion_check_assertion(car, false, true, true, dest);
}

void instrument_spec_assignst::track_static_locals(goto_programt &dest)
Expand Down Expand Up @@ -313,7 +323,7 @@ void instrument_spec_assignst::
create_snapshot(car, dest);

// check inclusion, allowing null and not allowing stack allocated locals
inclusion_check_assertion(car, true, false, dest);
inclusion_check_assertion(car, true, false, false, dest);

// invalidate aliases of the freed object
invalidate_heap_and_spec_aliases(car, dest);
Expand Down Expand Up @@ -680,6 +690,7 @@ void instrument_spec_assignst::inclusion_check_assertion(
const car_exprt &car,
bool allow_null_lhs,
bool include_stack_allocated,
bool is_inductive_check,
goto_programt &dest) const
{
source_locationt source_location(car.source_location());
Expand All @@ -705,7 +716,10 @@ void instrument_spec_assignst::inclusion_check_assertion(
else
comment += id2string(orig_comment);

comment += " is assignable";
if(is_inductive_check)
comment += " is inductively assignable";
else
comment += " is assignable";
source_location.set_comment(comment);

exprt inclusion_check =
Expand Down
8 changes: 8 additions & 0 deletions src/goto-instrument/contracts/instrument_spec_assigns.h
Original file line number Diff line number Diff line change
Expand Up @@ -438,6 +438,12 @@ class instrument_spec_assignst
const exprt &expr,
goto_programt &dest);

/// Generates inclusion check instructions for inductive checks of
/// the write set
/// \param lhs the assignment lhs or argument to havoc/havoc_object
/// \param dest destination program to append instructions to
void check_inclusion_induction(const exprt &lhs, goto_programt &dest) const;

/// Instruments a sequence of instructions with inclusion checks.
/// If `pred` is not provided,
/// then all instructions are instrumented.
Expand Down Expand Up @@ -543,11 +549,13 @@ class instrument_spec_assignst
/// \param allow_null_lhs if true, allow the lhs to be NULL
/// \param include_stack_allocated if true, include stack allocated targets
/// in the inclusion check.
/// \param is_inductive_check if true, assertions are for inductive check.
/// \param dest destination program to append instructions to
void inclusion_check_assertion(
const car_exprt &lhs,
bool allow_null_lhs,
bool include_stack_allocated,
bool is_inductive_check,
goto_programt &dest) const;

/// \brief Adds an assignment in dest to invalidate the tracked car if
Expand Down
9 changes: 0 additions & 9 deletions src/goto-instrument/havoc_utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,6 @@ void havoc_utilst::append_havoc_code_for_expr(
const exprt &expr,
goto_programt &dest) const
{
if(expr.id() == ID_index || expr.id() == ID_dereference)
{
address_of_exprt address_of_expr(expr);
if(!is_constant(address_of_expr))
{
append_object_havoc_code_for_expr(location, address_of_expr, dest);
return;
}
}
Comment on lines -34 to -42
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would appreciate if the commit message could explain why this change is needed here/is the right thing to do.

append_scalar_havoc_code_for_expr(location, expr, dest);
}

Expand Down
6 changes: 0 additions & 6 deletions src/goto-instrument/havoc_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,6 @@ class havoc_utilst

/// \brief Append goto instructions to havoc a single expression `expr`
///
/// If `expr` is an array index or object dereference expression,
/// with a non-constant offset, e.g. a[i] or *(b+i) with a non-constant `i`,
/// then instructions are generated to havoc the entire underlying object.
/// Otherwise, e.g. for a[0] or *(b+i) when `i` is a known constant,
/// the instructions are generated to only havoc the scalar value of `expr`.
///
/// \param location The source location to annotate on the havoc instruction
/// \param expr The expression to havoc
/// \param dest The destination goto program to append the instructions to
Expand Down