Skip to content

Bugfix/char array in string solver #3248

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
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
2 changes: 1 addition & 1 deletion jbmc/src/java_bytecode/java_string_library_preprocess.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class java_string_library_preprocesst:public messaget
java_string_library_preprocesst()
: char_type(java_char_type()),
index_type(java_int_type()),
refined_string_type(index_type, char_type)
refined_string_type(index_type, pointer_type(char_type))
{
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class tt
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⛏️ typo in commit message:

We The way it was used was inconsistent.

Copy link
Contributor

Choose a reason for hiding this comment

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

First sentence also has some problems.

refined_string_typet string_type() const
{
return refined_string_typet(length_type(), char_type());
return refined_string_typet(length_type(), pointer_type(char_type()));
}
array_typet witness_type() const
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ SCENARIO("dependency_graph", "[core][solvers][refinement][string_refinement]")
GIVEN("dependency graph")
{
string_dependenciest dependencies;
refined_string_typet string_type(java_char_type(), java_int_type());
const typet string_type =
refined_string_typet(java_int_type(), pointer_type(java_char_type()));
const exprt string1 = make_string_argument("string1");
const exprt string2 = make_string_argument("string2");
const exprt string3 = make_string_argument("string3");
Expand Down
45 changes: 27 additions & 18 deletions src/solvers/refinement/string_refinement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -270,12 +270,15 @@ static void make_char_array_pointer_associations(
}
}

void replace_symbols_in_equations(
const union_find_replacet &symbol_resolve,
std::vector<equal_exprt> &equations)
/// Substitute sub-expressions in equation by representative elements of
/// `symbol_resolve` whenever possible.
/// Similar to `symbol_resolve.replace_expr` but doesn't mutate the expression
/// and returns the transformed expression instead.
static exprt
replace_expr_copy(const union_find_replacet &symbol_resolve, exprt expr)
{
for(equal_exprt &eq : equations)
symbol_resolve.replace_expr(eq);
symbol_resolve.replace_expr(expr);
return expr;
}

/// Record the constraints to ensure that the expression is true when
Expand All @@ -301,20 +304,22 @@ void string_refinementt::set_to(const exprt &expr, bool value)
}

/// Add association for each char pointer in the equation
/// \param symbol_solver: a union_find_replacet object to keep track of
/// char pointer equations
/// \param equations: vector of equations
/// \param ns: namespace
/// \param stream: output stream
/// \return union_find_replacet where char pointer that have been set equal
/// by an equation are associated to the same element
static union_find_replacet generate_symbol_resolution_from_equations(
static void add_equations_for_symbol_resolution(
union_find_replacet &symbol_solver,
const std::vector<equal_exprt> &equations,
const namespacet &ns,
messaget::mstreamt &stream)
{
const auto eom = messaget::eom;
const std::string log_message =
"WARNING string_refinement.cpp generate_symbol_resolution_from_equations:";
union_find_replacet solver;
for(const equal_exprt &eq : equations)
{
const exprt &lhs = eq.lhs();
Expand All @@ -335,7 +340,7 @@ static union_find_replacet generate_symbol_resolution_from_equations(

if(is_char_pointer_type(rhs.type()))
{
solver.make_union(lhs, rhs);
symbol_solver.make_union(lhs, rhs);
}
else if(rhs.id() == ID_function_application)
{
Expand All @@ -355,7 +360,7 @@ static union_find_replacet generate_symbol_resolution_from_equations(
const member_exprt lhs_data(lhs, comp.get_name(), comp.type());
const exprt rhs_data = simplify_expr(
member_exprt(rhs, comp.get_name(), comp.type()), ns);
solver.make_union(lhs_data, rhs_data);
symbol_solver.make_union(lhs_data, rhs_data);
}
}
}
Expand All @@ -366,7 +371,6 @@ static union_find_replacet generate_symbol_resolution_from_equations(
}
}
}
return solver;
}

/// This is meant to be used on the lhs of an equation with string subtype.
Expand Down Expand Up @@ -613,9 +617,9 @@ decision_proceduret::resultt string_refinementt::dec_solve()
#endif

debug() << "dec_solve: Build symbol solver from equations" << eom;
// This is used by get, that's why we use a class member here
symbol_resolve =
generate_symbol_resolution_from_equations(equations, ns, debug());
// symbol_resolve is used by get and is kept between calls to dec_solve,
// that's why we use a class member here
add_equations_for_symbol_resolution(symbol_resolve, equations, ns, debug());
#ifdef DEBUG
debug() << "symbol resolve:" << eom;
for(const auto &pair : symbol_resolve.to_vector())
Expand All @@ -632,9 +636,6 @@ decision_proceduret::resultt string_refinementt::dec_solve()
}
#endif

debug() << "dec_solve: Replacing char pointer symbols" << eom;
replace_symbols_in_equations(symbol_resolve, equations);

debug() << "dec_solve: Replacing string ids in function applications" << eom;
for(equal_exprt &eq : equations)
{
Expand All @@ -653,10 +654,18 @@ decision_proceduret::resultt string_refinementt::dec_solve()

debug() << "dec_solve: compute dependency graph and remove function "
<< "applications captured by the dependencies:" << eom;
std::vector<exprt> local_equations;
std::vector<equal_exprt> local_equations;
for(const equal_exprt &eq : equations)
{
if(!add_node(dependencies, eq, generator.array_pool))
// 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(
dependencies,
eq_with_char_array_replaced_with_representative_elements,
generator.array_pool);
if(!node_added)
Copy link
Contributor

Choose a reason for hiding this comment

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

That's much clearer to me. Thanks!

local_equations.push_back(eq);
}
equations.clear();
Expand Down
6 changes: 3 additions & 3 deletions src/util/refined_string_type.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@ Author: Romain Brenguier, romain.brenguier@diffblue.com
#include "std_expr.h"

refined_string_typet::refined_string_typet(
const typet &index_type, const typet &char_type)
const typet &index_type,
const pointer_typet &content_type)
{
array_typet char_array(char_type, infinity_exprt(index_type));
components().emplace_back("length", index_type);
components().emplace_back("content", char_array);
components().emplace_back("content", content_type);
set_tag(CPROVER_PREFIX"refined_string_type");
}
4 changes: 3 additions & 1 deletion src/util/refined_string_type.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@ Author: Romain Brenguier, romain.brenguier@diffblue.com
class refined_string_typet: public struct_typet
{
public:
refined_string_typet(const typet &index_type, const typet &char_type);
refined_string_typet(
const typet &index_type,
const pointer_typet &content_type);

// Type for the content (list of characters) of a string
const array_typet &get_content_type() const
Expand Down
2 changes: 1 addition & 1 deletion src/util/string_expr.h
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ class refined_string_exprt : public struct_exprt,
: refined_string_exprt(
_length,
_content,
refined_string_typet(_length.type(), _content.type()))
refined_string_typet(_length.type(), to_pointer_type(_content.type())))
{
}

Expand Down