Skip to content

Handle more formulas than just equations in string solver #5016

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
15 changes: 9 additions & 6 deletions jbmc/unit/solvers/strings/string_refinement/dependency_graph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -89,12 +89,15 @@ SCENARIO("dependency_graph", "[core][solvers][refinement][string_refinement]")
symbol_generatort generator;
array_poolt array_pool(generator);

bool success = add_node(dependencies, equation1, array_pool);
REQUIRE(success);
success = add_node(dependencies, equation2, array_pool);
REQUIRE(success);
success = add_node(dependencies, equation3, array_pool);
REQUIRE(success);
optionalt<exprt> new_equation1 =
add_node(dependencies, equation1, array_pool, generator);
REQUIRE(new_equation1.has_value());
optionalt<exprt> new_equation2 =
add_node(dependencies, equation2, array_pool, generator);
REQUIRE(new_equation2.has_value());
optionalt<exprt> new_equation3 =
add_node(dependencies, equation3, array_pool, generator);
REQUIRE(new_equation3.has_value());

#ifdef DEBUG // useful output for visualizing the graph
{
Expand Down
39 changes: 28 additions & 11 deletions src/solvers/strings/string_dependencies.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ string_dependenciest::node_at(const array_string_exprt &e) const
}

string_dependenciest::builtin_function_nodet &string_dependenciest::make_node(
std::unique_ptr<string_builtin_functiont> &builtin_function)
std::unique_ptr<string_builtin_functiont> builtin_function)
{
builtin_function_nodes.emplace_back(
std::move(builtin_function), builtin_function_nodes.size());
Expand Down Expand Up @@ -195,22 +195,39 @@ void string_dependenciest::clean_cache()
e.reset();
}

bool add_node(
optionalt<exprt> add_node(
string_dependenciest &dependencies,
const equal_exprt &equation,
array_poolt &array_pool)
const exprt &expr,
array_poolt &array_pool,
symbol_generatort &fresh_symbol)
{
const auto fun_app =
expr_try_dynamic_cast<function_application_exprt>(equation.rhs());
const auto fun_app = expr_try_dynamic_cast<function_application_exprt>(expr);
if(!fun_app)
return false;
{
exprt copy = expr;
bool copy_differs_from_expr = false;
for(std::size_t i = 0; i < expr.operands().size(); ++i)
{
auto new_op =
add_node(dependencies, expr.operands()[i], array_pool, fresh_symbol);
if(new_op)
{
copy.operands()[i] = *new_op;
copy_differs_from_expr = true;
}
}
if(copy_differs_from_expr)
return copy;
return {};
}

const exprt return_code = fresh_symbol("string_builtin_return", expr.type());
auto builtin_function =
to_string_builtin_function(*fun_app, equation.lhs(), array_pool);
to_string_builtin_function(*fun_app, return_code, array_pool);

CHECK_RETURN(builtin_function != nullptr);
const auto &builtin_function_node = dependencies.make_node(builtin_function);
// Warning: `builtin_function` has been emptied and should not be used anymore
const auto &builtin_function_node =
dependencies.make_node(std::move(builtin_function));

if(
const auto &string_result =
Expand All @@ -233,7 +250,7 @@ bool add_node(
add_dependency_to_string_subexprs(
dependencies, *fun_app, builtin_function_node, array_pool);

return true;
return return_code;
}

void string_dependenciest::for_each_dependency(
Expand Down
22 changes: 13 additions & 9 deletions src/solvers/strings/string_dependencies.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,8 @@ class string_dependenciest
std::unique_ptr<const string_nodet>
node_at(const array_string_exprt &e) const;

/// `builtin_function` is reset to an empty pointer after the node is created
builtin_function_nodet &
make_node(std::unique_ptr<string_builtin_functiont> &builtin_function);
make_node(std::unique_ptr<string_builtin_functiont> builtin_function);
const string_builtin_functiont &
get_builtin_function(const builtin_function_nodet &node) const;

Expand Down Expand Up @@ -183,22 +182,27 @@ class string_dependenciest
const std::function<void(const nodet &)> &f) const;
};

/// When right hand side of equation is a builtin_function add
/// When a sub-expression of \p expr is a builtin_function, add
/// a "string_builtin_function" node to the graph and connect it to the strings
/// on which it depends and which depends on it.
/// If the string builtin_function is not a supported one, mark all the string
/// arguments as depending on an unknown builtin_function.
/// \param dependencies: graph to which we add the node
/// \param equation: an equation whose right hand side is possibly a call to a
Copy link
Contributor

Choose a reason for hiding this comment

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

The function description needs updating. It still mentions the rhs.

/// \param expr: an expression which may contain a call to a
/// string builtin_function.
/// \param array_pool: array pool containing arrays corresponding to the string
/// exprt arguments of the builtin_function call
/// \return true if a node was added, if false is returned it either means that
/// the right hand side is not a function application
/// \param fresh_symbol: used to create new symbols for the return values of
/// builtin functions
/// \return An expression in which function applications have been replaced
/// by symbols corresponding to the `return_value` field of the associated
/// builtin function. Or an empty optional when no function applications has
/// been encountered
/// \todo there should be a class with just the three functions we require here
bool add_node(
optionalt<exprt> add_node(
string_dependenciest &dependencies,
const equal_exprt &equation,
array_poolt &array_pool);
const exprt &expr,
array_poolt &array_pool,
symbol_generatort &fresh_symbol);

#endif // CPROVER_SOLVERS_STRINGS_STRING_DEPENDENCIES_H
108 changes: 65 additions & 43 deletions src/solvers/strings/string_refinement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ Author: Alberto Griggio, [email protected]
#include <util/expr_iterator.h>
#include <util/expr_util.h>
#include <util/magic.h>
#include <util/range.h>
#include <util/simplify_expr.h>

#include "equation_symbol_mapping.h"
Expand Down Expand Up @@ -239,23 +240,24 @@ static std::vector<exprt> generate_instantiations(
return lemmas;
}

/// Fill the array_pointer correspondence and replace the right hand sides of
/// the corresponding equations
/// If \p expr is an equation whose right-hand-side is a
/// associate_array_to_pointer call, add the correspondence and replace the call
/// by an expression representing its result.
static void make_char_array_pointer_associations(
string_constraint_generatort &generator,
std::vector<equal_exprt> &equations)
exprt &expr)
{
for(equal_exprt &eq : equations)
if(const auto equal_expr = expr_try_dynamic_cast<equal_exprt>(expr))
{
if(
const auto fun_app =
expr_try_dynamic_cast<function_application_exprt>(eq.rhs()))
const auto fun_app = expr_try_dynamic_cast<function_application_exprt>(
as_const(equal_expr->rhs())))
{
const auto new_equation =
generator.make_array_pointer_association(eq.lhs(), *fun_app);
generator.make_array_pointer_association(equal_expr->lhs(), *fun_app);
if(new_equation)
{
eq =
expr =
equal_exprt{from_integer(true, new_equation->type()), *new_equation};
}
}
Expand All @@ -281,18 +283,10 @@ void string_refinementt::set_to(const exprt &expr, bool value)
{
PRECONDITION(expr.type().id() == ID_bool);
PRECONDITION(equality_propagation);

if(expr.id() == ID_equal && value)
{
const equal_exprt &eq_expr = to_equal_expr(expr);
equations.push_back(eq_expr);
}
if(!value)
equations.push_back(not_exprt{expr});
Copy link
Contributor

Choose a reason for hiding this comment

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

Is that to compensate for 🐎 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not really, it's more like the opposite: because we introduce things that may not be equal_exprt in equations they need to be filtered out in the previous commit

else
{
INVARIANT(
!has_char_array_subexpr(expr, ns), "char array only appear in equations");
supert::set_to(expr, value);
}
equations.push_back(expr);
}

/// Add association for each char pointer in the equation
Expand All @@ -305,16 +299,19 @@ void string_refinementt::set_to(const exprt &expr, bool value)
/// by an equation are associated to the same element
static void add_equations_for_symbol_resolution(
union_find_replacet &symbol_solver,
const std::vector<equal_exprt> &equations,
const std::vector<exprt> &equations,
const namespacet &ns,
messaget::mstreamt &stream)
{
const std::string log_message =
"WARNING string_refinement.cpp generate_symbol_resolution_from_equations:";
for(const equal_exprt &eq : equations)
auto equalities = make_range(equations).filter(
[&](const exprt &e) { return can_cast_expr<equal_exprt>(e); });
for(const exprt &e : equalities)
{
const exprt &lhs = eq.lhs();
const exprt &rhs = eq.rhs();
const equal_exprt &eq = to_equal_expr(e);
const exprt &lhs = to_equal_expr(eq).lhs();
const exprt &rhs = to_equal_expr(eq).rhs();
if(lhs.id() != ID_symbol)
{
stream << log_message << "non symbol lhs: " << format(lhs)
Expand Down Expand Up @@ -463,7 +460,7 @@ static void add_string_equation_to_symbol_resolution(
/// \param stream: output stream
/// \return union_find_replacet structure containing the correspondences.
union_find_replacet string_identifiers_resolution_from_equations(
std::vector<equal_exprt> &equations,
const std::vector<equal_exprt> &equations,
const namespacet &ns,
messaget::mstreamt &stream)
{
Expand Down Expand Up @@ -534,13 +531,11 @@ union_find_replacet string_identifiers_resolution_from_equations(

#ifdef DEBUG
/// Output a vector of equations to the given stream, used for debugging.
static void output_equations(
std::ostream &output,
const std::vector<equal_exprt> &equations)
static void
output_equations(std::ostream &output, const std::vector<exprt> &equations)
{
for(std::size_t i = 0; i < equations.size(); ++i)
output << " [" << i << "] " << format(equations[i].lhs())
<< " == " << format(equations[i].rhs()) << std::endl;
output << " [" << i << "] " << format(equations[i]) << std::endl;
}
#endif

Expand Down Expand Up @@ -629,7 +624,18 @@ decision_proceduret::resultt string_refinementt::dec_solve()
#endif

const union_find_replacet string_id_symbol_resolve =
string_identifiers_resolution_from_equations(equations, ns, log.debug());
string_identifiers_resolution_from_equations(
[&] {
std::vector<equal_exprt> equalities;
for(const auto &eq : equations)
{
if(auto equal_expr = expr_try_dynamic_cast<equal_exprt>(eq))
equalities.push_back(*equal_expr);
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure I understand. Why do equalities that don't appear as top-level equal_exprts not need to be added to the vector of equalities? 🐎

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If they don't appear at the top then we can't be sure they always hold, for instance if we have g1 => (s1 == s2), we cannot be sure s1 and s2 will be equal in the end and we have to keep them as separate symbols (instead of replacing occurences of s1 by s2).

}
return equalities;
}(),
ns,
log.debug());
#ifdef DEBUG
log.debug() << "symbol resolve string:" << messaget::eom;
for(const auto &pair : string_id_symbol_resolve.to_vector())
Expand All @@ -639,39 +645,55 @@ decision_proceduret::resultt string_refinementt::dec_solve()
}
#endif

log.debug() << "dec_solve: Replacing string ids in function applications"
log.debug() << "dec_solve: Replacing string ids and simplifying arguments"
" in function applications"
<< messaget::eom;
for(equal_exprt &eq : equations)
for(exprt &expr : equations)
{
if(can_cast_expr<function_application_exprt>(eq.rhs()))
auto it = expr.depth_begin();
while(it != expr.depth_end())
{
simplify(eq.rhs(), ns);
string_id_symbol_resolve.replace_expr(eq.rhs());
if(can_cast_expr<function_application_exprt>(*it))
{
// Simplification is required because the array pool may not realize
// that an expression like
// `(unsignedbv[16]*)((signedbv[8]*)&constarray[0] + 0)` is the
// same pointer as `&constarray[0]
simplify(it.mutate(), ns);
string_id_symbol_resolve.replace_expr(it.mutate());
it.next_sibling_or_parent();
}
else
++it;
}
}

// Constraints start clear at each `dec_solve` call.
string_constraintst constraints;
make_char_array_pointer_associations(generator, equations);
for(auto &expr : equations)
make_char_array_pointer_associations(generator, expr);

#ifdef DEBUG
output_equations(log.debug(), equations);
#endif

log.debug() << "dec_solve: compute dependency graph and remove function "
<< "applications captured by the dependencies:" << messaget::eom;
std::vector<equal_exprt> local_equations;
for(const equal_exprt &eq : equations)
std::vector<exprt> local_equations;
for(const exprt &eq : equations)
{
// Ensures that arrays that are equal, are associated to the same nodes
// in the graph.
const equal_exprt eq_with_char_array_replaced_with_representative_elements =
to_equal_expr(replace_expr_copy(symbol_resolve, eq));
const bool node_added = add_node(
const exprt eq_with_char_array_replaced_with_representative_elements =
replace_expr_copy(symbol_resolve, eq);
const optionalt<exprt> new_equation = add_node(
dependencies,
eq_with_char_array_replaced_with_representative_elements,
generator.array_pool);
if(!node_added)
generator.array_pool,
generator.fresh_symbol);
if(new_equation)
local_equations.push_back(*new_equation);
else
local_equations.push_back(eq);
}
equations.clear();
Expand Down
4 changes: 2 additions & 2 deletions src/solvers/strings/string_refinement.h
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ class string_refinementt final : public bv_refinementt
index_set_pairt index_sets;
union_find_replacet symbol_resolve;

std::vector<equal_exprt> equations;
std::vector<exprt> equations;

string_dependenciest dependencies;

Expand All @@ -122,7 +122,7 @@ exprt substitute_array_lists(exprt expr, std::size_t string_max_length);

// Declaration required for unit-test:
union_find_replacet string_identifiers_resolution_from_equations(
std::vector<equal_exprt> &equations,
const std::vector<equal_exprt> &equations,
const namespacet &ns,
messaget::mstreamt &stream);

Expand Down
1 change: 1 addition & 0 deletions unit/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ SRC += analyses/ai/ai.cpp \
solvers/strings/string_format_builtin_function/length_of_decimal_int.cpp \
solvers/strings/string_refinement/concretize_array.cpp \
solvers/strings/string_refinement/sparse_array.cpp \
solvers/strings/string_refinement/string_refinement.cpp \
solvers/strings/string_refinement/substitute_array_list.cpp \
solvers/strings/string_refinement/union_find_replace.cpp \
util/allocate_objects.cpp \
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
solvers/refinement
solvers/sat
solvers/strings
string_refinement
testing-utils
Expand Down
Loading