Skip to content

Cleanup of asserts and throws under the goto-symex directory #2902

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 2 commits into from
Sep 24, 2018
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
14 changes: 9 additions & 5 deletions src/goto-symex/build_goto_trace.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,10 @@ Author: Daniel Kroening

#include <cassert>

#include <util/threeval.h>
#include <util/simplify_expr.h>
#include <util/arith_tools.h>
#include <util/byte_operators.h>
#include <util/simplify_expr.h>
#include <util/threeval.h>

#include <solvers/prop/prop_conv.h>
#include <solvers/prop/prop.h>
Expand Down Expand Up @@ -92,8 +93,11 @@ exprt build_full_lhs_rec(
id==ID_byte_extract_big_endian)
{
exprt tmp=src_original;
assert(tmp.operands().size()==2);
tmp.op0()=build_full_lhs_rec(prop_conv, ns, tmp.op0(), src_ssa.op0());
tmp.op0() = build_full_lhs_rec(
prop_conv,
ns,
to_byte_extract_expr(tmp).op(),
to_byte_extract_expr(src_ssa).op());

// re-write into big case-split
}
Expand Down Expand Up @@ -229,7 +233,7 @@ void build_goto_trace(
else if(it->is_atomic_end() && current_time<0)
current_time*=-1;

assert(current_time>=0);
INVARIANT(current_time >= 0, "time keeping inconsistency");
// move any steps gathered in an atomic section

if(time_before<0)
Expand Down
26 changes: 10 additions & 16 deletions src/goto-symex/goto_symex_state.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,11 @@ Author: Daniel Kroening, [email protected]
#include <cstdlib>
#include <iostream>

#include <util/invariant.h>
#include <util/base_exceptions.h>
#include <util/std_expr.h>
#include <util/exception_utils.h>
#include <util/invariant.h>
#include <util/prefix.h>
#include <util/std_expr.h>

#include <analyses/dirty.h>

Expand Down Expand Up @@ -51,8 +52,7 @@ void goto_symex_statet::level0t::operator()(

const symbolt *s;
const bool found_l0 = !ns.lookup(obj_identifier, s);
DATA_INVARIANT(
found_l0, "level0: failed to find " + id2string(obj_identifier));
INVARIANT(found_l0, "level0: failed to find " + id2string(obj_identifier));

// don't rename shared variables or functions
if(s->type.id()==ID_code ||
Expand Down Expand Up @@ -187,10 +187,7 @@ bool goto_symex_statet::constant_propagation_reference(const exprt &expr) const
}
else if(expr.id()==ID_member)
{
if(expr.operands().size()!=1)
throw "member expects one operand";

return constant_propagation_reference(expr.op0());
return constant_propagation_reference(to_member_expr(expr).compound());
}
else if(expr.id()==ID_string_constant)
return true;
Expand Down Expand Up @@ -345,7 +342,8 @@ void goto_symex_statet::assignment(

// see #305 on GitHub for a simple example and possible discussion
if(is_shared && lhs.type().id() == ID_pointer && !allow_pointer_unsoundness)
throw "pointer handling for concurrency is unsound";
throw unsupported_operation_exceptiont(
"pointer handling for concurrency is unsound");

// for value propagation -- the RHS is L2

Expand Down Expand Up @@ -495,13 +493,9 @@ void goto_symex_statet::rename(
}
else if(expr.id()==ID_address_of)
{
DATA_INVARIANT(
expr.operands().size() == 1, "address_of should have a single operand");
rename_address(expr.op0(), ns, level);
DATA_INVARIANT(
expr.type().id() == ID_pointer,
"type of address_of should be ID_pointer");
expr.type().subtype()=expr.op0().type();
auto &address_of_expr = to_address_of_expr(expr);
rename_address(address_of_expr.object(), ns, level);
expr.type().subtype() = address_of_expr.object().type();
}
else
{
Expand Down
4 changes: 2 additions & 2 deletions src/goto-symex/memory_model_pso.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ bool memory_model_psot::program_order_is_relaxed(
partial_order_concurrencyt::event_it e1,
partial_order_concurrencyt::event_it e2) const
{
assert(e1->is_shared_read() || e1->is_shared_write());
assert(e2->is_shared_read() || e2->is_shared_write());
PRECONDITION(e1->is_shared_read() || e1->is_shared_write());
PRECONDITION(e2->is_shared_read() || e2->is_shared_write());

// no po relaxation within atomic sections
if(e1->atomic_section_id!=0 &&
Expand Down
4 changes: 2 additions & 2 deletions src/goto-symex/memory_model_sc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ bool memory_model_sct::program_order_is_relaxed(
partial_order_concurrencyt::event_it e1,
partial_order_concurrencyt::event_it e2) const
{
assert(e1->is_shared_read() || e1->is_shared_write());
assert(e2->is_shared_read() || e2->is_shared_write());
PRECONDITION(e1->is_shared_read() || e1->is_shared_write());
PRECONDITION(e2->is_shared_read() || e2->is_shared_write());

return false;
}
Expand Down
4 changes: 2 additions & 2 deletions src/goto-symex/memory_model_tso.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ bool memory_model_tsot::program_order_is_relaxed(
partial_order_concurrencyt::event_it e1,
partial_order_concurrencyt::event_it e2) const
{
assert(e1->is_shared_read() || e1->is_shared_write());
assert(e2->is_shared_read() || e2->is_shared_write());
PRECONDITION(e1->is_shared_read() || e1->is_shared_write());
PRECONDITION(e2->is_shared_read() || e2->is_shared_write());

// no po relaxation within atomic sections
if(e1->atomic_section_id!=0 &&
Expand Down
6 changes: 3 additions & 3 deletions src/goto-symex/partial_order_concurrency.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -142,8 +142,8 @@ symbol_exprt partial_order_concurrencyt::clock(
event_it event,
axiomt axiom)
{
PRECONDITION(!numbering.empty());
irep_idt identifier;
assert(!numbering.empty());

if(event->is_shared_write())
identifier=rw_clock_id(event, axiom);
Expand All @@ -163,7 +163,7 @@ symbol_exprt partial_order_concurrencyt::clock(

void partial_order_concurrencyt::build_clock_type()
{
assert(!numbering.empty());
PRECONDITION(!numbering.empty());

std::size_t width = address_bits(numbering.size());
clock_type = unsignedbv_typet(width);
Expand Down Expand Up @@ -198,7 +198,7 @@ exprt partial_order_concurrencyt::before(
binary_relation_exprt(clock(e1, ax), ID_lt, clock(e2, ax)));
}

assert(!ops.empty());
POSTCONDITION(!ops.empty());

return conjunction(ops);
}
Expand Down
22 changes: 6 additions & 16 deletions src/goto-symex/postcondition.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -76,21 +76,16 @@ bool postconditiont::is_used_address_of(
}
else if(expr.id()==ID_index)
{
assert(expr.operands().size()==2);

return
is_used_address_of(expr.op0(), identifier) ||
is_used(expr.op1(), identifier);
return is_used_address_of(to_index_expr(expr).array(), identifier) ||
is_used(to_index_expr(expr).index(), identifier);
}
else if(expr.id()==ID_member)
{
assert(expr.operands().size()==1);
return is_used_address_of(expr.op0(), identifier);
return is_used_address_of(to_member_expr(expr).compound(), identifier);
}
else if(expr.id()==ID_dereference)
{
assert(expr.operands().size()==1);
return is_used(expr.op0(), identifier);
return is_used(to_dereference_expr(expr).pointer(), identifier);
}

return false;
Expand Down Expand Up @@ -154,9 +149,7 @@ bool postconditiont::is_used(
{
if(expr.id()==ID_address_of)
{
// only do index!
assert(expr.operands().size()==1);
return is_used_address_of(expr.op0(), identifier);
return is_used_address_of(to_address_of_expr(expr).object(), identifier);
}
else if(expr.id()==ID_symbol &&
expr.get_bool(ID_C_SSA_symbol))
Expand All @@ -169,12 +162,9 @@ bool postconditiont::is_used(
}
else if(expr.id()==ID_dereference)
{
assert(expr.operands().size()==1);

// aliasing may happen here

value_setst::valuest expr_set;
value_set.get_value_set(expr.op0(), expr_set, ns);
value_set.get_value_set(to_dereference_expr(expr).pointer(), expr_set, ns);
std::unordered_set<irep_idt> symbols;

for(value_setst::valuest::const_iterator
Expand Down
27 changes: 12 additions & 15 deletions src/goto-symex/precondition.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,19 +78,17 @@ void preconditiont::compute_address_of(exprt &dest)
}
else if(dest.id()==ID_index)
{
assert(dest.operands().size()==2);
compute_address_of(dest.op0());
compute(dest.op1());
auto &index_expr = to_index_expr(dest);
compute_address_of(index_expr.array());
compute(index_expr.index());
}
else if(dest.id()==ID_member)
{
assert(dest.operands().size()==1);
compute_address_of(dest.op0());
compute_address_of(to_member_expr(dest).compound());
}
else if(dest.id()==ID_dereference)
{
assert(dest.operands().size()==1);
compute(dest.op0());
compute(to_dereference_expr(dest).pointer());
}
}

Expand All @@ -104,19 +102,18 @@ void preconditiont::compute_rec(exprt &dest)
if(dest.id()==ID_address_of)
{
// only do index!
assert(dest.operands().size()==1);
compute_address_of(dest.op0());
compute_address_of(to_address_of_expr(dest).object());
}
else if(dest.id()==ID_dereference)
{
assert(dest.operands().size()==1);
auto &deref_expr = to_dereference_expr(dest);

const irep_idt &lhs_identifier=SSA_step.ssa_lhs.get_object_name();

// aliasing may happen here

value_setst::valuest expr_set;
value_sets.get_values(target, dest.op0(), expr_set);
value_sets.get_values(target, deref_expr.pointer(), expr_set);
std::unordered_set<irep_idt> symbols;

for(value_setst::valuest::const_iterator
Expand All @@ -129,15 +126,15 @@ void preconditiont::compute_rec(exprt &dest)
{
// may alias!
exprt tmp;
tmp.swap(dest.op0());
tmp.swap(deref_expr.pointer());
dereference(target, tmp, ns, value_sets);
dest.swap(tmp);
compute_rec(dest);
deref_expr.swap(tmp);
compute_rec(deref_expr);
}
else
{
// nah, ok
compute_rec(dest.op0());
compute_rec(deref_expr.pointer());
}
}
else if(dest==SSA_step.ssa_lhs.get_original_expr())
Expand Down
4 changes: 3 additions & 1 deletion src/goto-symex/show_vcc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ Author: Daniel Kroening, [email protected]
#include <langapi/language_util.h>
#include <langapi/mode.h>

#include <util/exception_utils.h>
#include <util/json.h>
#include <util/json_expr.h>
#include <util/ui_message.h>
Expand Down Expand Up @@ -165,7 +166,8 @@ void show_vcc(
{
of.open(filename);
if(!of)
throw "failed to open file " + filename;
throw invalid_user_input_exceptiont(
"invalid file to read trace from: " + filename, "--outfile");
}

std::ostream &out = have_file ? of : std::cout;
Expand Down
5 changes: 2 additions & 3 deletions src/goto-symex/slice.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ void symex_slicet::slice(symex_target_equationt::SSA_stept &SSA_step)
void symex_slicet::slice_assignment(
symex_target_equationt::SSA_stept &SSA_step)
{
assert(SSA_step.ssa_lhs.id()==ID_symbol);
PRECONDITION(SSA_step.ssa_lhs.id() == ID_symbol);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe this should be true by design.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Still, isn't it a good idea to leave it there, even if only for documentation purposes?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Not a big deal, but I think if we were to go down that route we'd have, e.g., any use of an index_exprt prefixed by expr.id() == ID_index.

const irep_idt &id=SSA_step.ssa_lhs.get_identifier();

if(depends.find(id)==depends.end())
Expand All @@ -127,8 +127,7 @@ void symex_slicet::slice_assignment(
void symex_slicet::slice_decl(
symex_target_equationt::SSA_stept &SSA_step)
{
assert(SSA_step.ssa_lhs.id()==ID_symbol);
const irep_idt &id=SSA_step.ssa_lhs.get_identifier();
const irep_idt &id = to_symbol_expr(SSA_step.ssa_lhs).get_identifier();

if(depends.find(id)==depends.end())
{
Expand Down
28 changes: 16 additions & 12 deletions src/goto-symex/slice_by_trace.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,13 @@ Author: Daniel Kroening, [email protected]
#include <fstream>
#include <iostream>

#include <util/string2int.h>
#include <util/simplify_expr.h>
#include <util/arith_tools.h>
#include <util/std_expr.h>
#include <util/guard.h>
#include <util/exception_utils.h>
#include <util/format_expr.h>
#include <util/guard.h>
#include <util/simplify_expr.h>
#include <util/std_expr.h>
#include <util/string2int.h>

void symex_slice_by_tracet::slice_by_trace(
std::string trace_files,
Expand Down Expand Up @@ -79,6 +80,11 @@ void symex_slice_by_tracet::slice_by_trace(
{
exprt g_copy(*i);

DATA_INVARIANT(
g_copy.id() == ID_symbol || g_copy.id() == ID_not ||
g_copy.id() == ID_and || g_copy.id() == ID_constant,
"guards should only be and, symbol, constant, or `not'");

if(g_copy.id()==ID_symbol || g_copy.id() == ID_not)
{
g_copy.make_not();
Expand All @@ -92,10 +98,6 @@ void symex_slice_by_tracet::slice_by_trace(
simplify(copy_last, ns);
implications.insert(copy_last);
}
else if(!(g_copy.id()==ID_constant))
{
throw "guards should only be and, symbol, constant, or `not'";
}
}

slice_SSA_steps(equation, implications); // Slice based on implications
Expand All @@ -122,7 +124,8 @@ void symex_slice_by_tracet::read_trace(std::string filename)
std::cout << "Reading trace from file " << filename << '\n';
std::ifstream file(filename);
if(file.fail())
throw "failed to read from trace file";
throw invalid_user_input_exceptiont(
"invalid file to read trace from: " + filename, "");

// In case not the first trace read
alphabet.clear();
Expand Down Expand Up @@ -219,9 +222,10 @@ void symex_slice_by_tracet::parse_events(std::string read_line)
const std::string::size_type vnext=read_line.find(",", vidx);
std::string event=read_line.substr(vidx, vnext - vidx);
eis.insert(event);
if((!alphabet.empty()) &&
((alphabet.count(event)!=0)!=alphabet_parity))
throw "trace uses symbol not in alphabet: "+event;
PRECONDITION(!alphabet.empty());
INVARIANT(
(alphabet.count(event) != 0) == alphabet_parity,
"trace uses symbol not in alphabet: " + event);
if(vnext==std::string::npos)
break;
vidx=vnext;
Expand Down
Loading