Skip to content

Commit ebbbccb

Browse files
committed
Fix support for with_exprt with more than 3 operands
A `with_exprt` needs to have at least 3 operands, but can encode multiple updates when using more operands (where any further operands need to come in pairs of two: an index/member and a new value). We already support this in several places, but were still missing support in others. This led to wrong verification results in Kani as recent changes in CBMC make increasing use of the value set (which is among those fixed in this commit).
1 parent 5d1438a commit ebbbccb

File tree

6 files changed

+123
-77
lines changed

6 files changed

+123
-77
lines changed

src/goto-programs/interpreter_evaluate.cpp

Lines changed: 21 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -948,30 +948,34 @@ interpretert::mp_vectort interpretert::evaluate(const exprt &expr)
948948
const auto &wexpr=to_with_expr(expr);
949949

950950
mp_vectort dest = evaluate(wexpr.old());
951-
mp_vectort where = evaluate(wexpr.where());
952-
mp_vectort new_value = evaluate(wexpr.new_value());
953951

954-
const auto &subtype = to_array_type(expr.type()).element_type();
955-
956-
if(!new_value.empty() && where.size()==1 && !unbounded_size(subtype))
952+
for(std::size_t i = 1; i < wexpr.operands().size(); i += 2)
957953
{
958-
// Ignore indices < 0, which the string solver sometimes produces
959-
if(where[0]<0)
960-
return {};
954+
mp_vectort where = evaluate(wexpr.operands()[i]);
955+
mp_vectort new_value = evaluate(wexpr.operands()[i + 1]);
956+
957+
const auto &subtype = to_array_type(expr.type()).element_type();
961958

962-
mp_integer where_idx=where[0];
963-
mp_integer subtype_size=get_size(subtype);
964-
mp_integer need_size=(where_idx+1)*subtype_size;
959+
if(!new_value.empty() && where.size() == 1 && !unbounded_size(subtype))
960+
{
961+
// Ignore indices < 0, which the string solver sometimes produces
962+
if(where[0] < 0)
963+
return {};
965964

966-
if(dest.size()<need_size)
967-
dest.resize(numeric_cast_v<std::size_t>(need_size), 0);
965+
mp_integer where_idx = where[0];
966+
mp_integer subtype_size = get_size(subtype);
967+
mp_integer need_size = (where_idx + 1) * subtype_size;
968968

969-
for(std::size_t i=0; i<new_value.size(); ++i)
970-
dest[numeric_cast_v<std::size_t>((where_idx * subtype_size) + i)] =
971-
new_value[i];
969+
if(dest.size() < need_size)
970+
dest.resize(numeric_cast_v<std::size_t>(need_size), 0);
972971

973-
return {};
972+
for(std::size_t i = 0; i < new_value.size(); ++i)
973+
dest[numeric_cast_v<std::size_t>((where_idx * subtype_size) + i)] =
974+
new_value[i];
975+
}
974976
}
977+
978+
return dest;
975979
}
976980
else if(expr.id()==ID_nil)
977981
{

src/pointer-analysis/value_set.cpp

Lines changed: 68 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -973,26 +973,38 @@ void value_sett::get_value_set_rec(
973973
(expr.type().id() == ID_struct_tag || expr.type().id() == ID_struct) &&
974974
!suffix.empty())
975975
{
976-
irep_idt component_name = with_expr.where().get(ID_component_name);
977-
if(suffix_starts_with_field(suffix, id2string(component_name)))
976+
bool any_matching_suffix = false;
977+
bool all_matching_component_names = true;
978+
for(std::size_t i = 1; i < with_expr.operands().size(); i += 2)
978979
{
979-
// Looking for the member overwritten by this WITH expression
980-
std::string remaining_suffix =
981-
strip_first_field_from_suffix(suffix, id2string(component_name));
982-
get_value_set_rec(
983-
with_expr.new_value(),
984-
dest,
985-
includes_nondet_pointer,
986-
remaining_suffix,
987-
original_type,
988-
ns);
980+
irep_idt component_name =
981+
with_expr.operands()[i].get(ID_component_name);
982+
if(suffix_starts_with_field(suffix, id2string(component_name)))
983+
{
984+
// Looking for the member overwritten by this WITH expression
985+
any_matching_suffix = true;
986+
std::string remaining_suffix =
987+
strip_first_field_from_suffix(suffix, id2string(component_name));
988+
get_value_set_rec(
989+
with_expr.operands()[i + 1],
990+
dest,
991+
includes_nondet_pointer,
992+
remaining_suffix,
993+
original_type,
994+
ns);
995+
}
996+
else if(
997+
all_matching_component_names &&
998+
(expr.type().id() != ID_struct ||
999+
!to_struct_type(expr.type()).has_component(component_name)) &&
1000+
(expr.type().id() != ID_struct_tag ||
1001+
!ns.follow_tag(to_struct_tag_type(expr.type()))
1002+
.has_component(component_name)))
1003+
{
1004+
all_matching_component_names = false;
1005+
}
9891006
}
990-
else if(
991-
(expr.type().id() == ID_struct &&
992-
to_struct_type(expr.type()).has_component(component_name)) ||
993-
(expr.type().id() == ID_struct_tag &&
994-
ns.follow_tag(to_struct_tag_type(expr.type()))
995-
.has_component(component_name)))
1007+
if(!any_matching_suffix && all_matching_component_names)
9961008
{
9971009
// Looking for a non-overwritten member, look through this expression
9981010
get_value_set_rec(
@@ -1003,7 +1015,7 @@ void value_sett::get_value_set_rec(
10031015
original_type,
10041016
ns);
10051017
}
1006-
else
1018+
else if(!any_matching_suffix)
10071019
{
10081020
// Member we're looking for is not defined in this struct -- this
10091021
// must be a reinterpret cast of some sort. Default to conservatively
@@ -1015,13 +1027,16 @@ void value_sett::get_value_set_rec(
10151027
suffix,
10161028
original_type,
10171029
ns);
1018-
get_value_set_rec(
1019-
with_expr.new_value(),
1020-
dest,
1021-
includes_nondet_pointer,
1022-
"",
1023-
original_type,
1024-
ns);
1030+
for(std::size_t i = 1; i < with_expr.operands().size(); i += 2)
1031+
{
1032+
get_value_set_rec(
1033+
with_expr.operands()[i + 1],
1034+
dest,
1035+
includes_nondet_pointer,
1036+
"",
1037+
original_type,
1038+
ns);
1039+
}
10251040
}
10261041
}
10271042
else if(expr.type().id() == ID_array && !suffix.empty())
@@ -1040,13 +1055,16 @@ void value_sett::get_value_set_rec(
10401055
suffix,
10411056
original_type,
10421057
ns);
1043-
get_value_set_rec(
1044-
with_expr.new_value(),
1045-
dest,
1046-
includes_nondet_pointer,
1047-
new_value_suffix,
1048-
original_type,
1049-
ns);
1058+
for(std::size_t i = 1; i < with_expr.operands().size(); i += 2)
1059+
{
1060+
get_value_set_rec(
1061+
with_expr.operands()[i + 1],
1062+
dest,
1063+
includes_nondet_pointer,
1064+
new_value_suffix,
1065+
original_type,
1066+
ns);
1067+
}
10501068
}
10511069
else
10521070
{
@@ -1059,13 +1077,16 @@ void value_sett::get_value_set_rec(
10591077
suffix,
10601078
original_type,
10611079
ns);
1062-
get_value_set_rec(
1063-
with_expr.new_value(),
1064-
dest,
1065-
includes_nondet_pointer,
1066-
"",
1067-
original_type,
1068-
ns);
1080+
for(std::size_t i = 1; i < with_expr.operands().size(); i += 2)
1081+
{
1082+
get_value_set_rec(
1083+
with_expr.operands()[i + 1],
1084+
dest,
1085+
includes_nondet_pointer,
1086+
"",
1087+
original_type,
1088+
ns);
1089+
}
10691090
}
10701091
}
10711092
else if(expr.id()==ID_array)
@@ -1624,14 +1645,18 @@ void value_sett::assign(
16241645
}
16251646
else if(rhs.id()==ID_with)
16261647
{
1648+
const with_exprt &with_expr = to_with_expr(rhs);
16271649
const index_exprt op0_index(
1628-
to_with_expr(rhs).old(),
1650+
with_expr.old(),
16291651
exprt(ID_unknown, c_index_type()),
16301652
to_array_type(lhs.type()).element_type());
16311653

16321654
assign(lhs_index, op0_index, ns, is_simplified, add_to_sets);
1633-
assign(
1634-
lhs_index, to_with_expr(rhs).new_value(), ns, is_simplified, true);
1655+
for(std::size_t i = 1; i < with_expr.operands().size(); i += 2)
1656+
{
1657+
assign(
1658+
lhs_index, with_expr.operands()[i + 1], ns, is_simplified, true);
1659+
}
16351660
}
16361661
else
16371662
{

src/pointer-analysis/value_set_fi.cpp

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1058,17 +1058,21 @@ void value_set_fit::assign(
10581058
{
10591059
// see if this is the member we want
10601060
const auto &rhs_with = to_with_expr(rhs);
1061-
const exprt &member_operand = rhs_with.where();
1061+
bool member_found = false;
1062+
for(std::size_t i = 1; i < rhs_with.operands().size(); i += 2)
1063+
{
1064+
const exprt &member_operand = rhs_with.operands()[i];
10621065

1063-
const irep_idt &component_name=
1064-
member_operand.get(ID_component_name);
1066+
const irep_idt &component_name =
1067+
member_operand.get(ID_component_name);
10651068

1066-
if(component_name==name)
1067-
{
1068-
// yes! just take op2
1069-
rhs_member = rhs_with.new_value();
1069+
if(component_name == name)
1070+
{
1071+
rhs_member = rhs_with.operands()[i + 1];
1072+
member_found = true;
1073+
}
10701074
}
1071-
else
1075+
if(!member_found)
10721076
{
10731077
// no! do op0
10741078
rhs_member=exprt(ID_member, subtype);
@@ -1126,13 +1130,17 @@ void value_set_fit::assign(
11261130
}
11271131
else if(rhs.id()==ID_with)
11281132
{
1133+
const with_exprt &with_expr = to_with_expr(rhs);
11291134
const index_exprt op0_index(
1130-
to_with_expr(rhs).old(),
1135+
with_expr.old(),
11311136
exprt(ID_unknown, c_index_type()),
11321137
to_array_type(lhs.type()).element_type());
11331138

11341139
assign(lhs_index, op0_index, ns);
1135-
assign(lhs_index, to_with_expr(rhs).new_value(), ns);
1140+
for(std::size_t i = 1; i < with_expr.operands().size(); i += 2)
1141+
{
1142+
assign(lhs_index, with_expr.operands()[i + 1], ns);
1143+
}
11361144
}
11371145
else
11381146
{

src/solvers/smt2/smt2_format.cpp

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -139,9 +139,14 @@ std::ostream &smt2_format_rec(std::ostream &out, const exprt &expr)
139139
else if(expr.id() == ID_with && expr.type().id() == ID_array)
140140
{
141141
const auto &with_expr = to_with_expr(expr);
142-
out << "(store " << smt2_format(with_expr.old()) << ' '
143-
<< smt2_format(with_expr.where()) << ' '
144-
<< smt2_format(with_expr.new_value()) << ')';
142+
for(std::size_t i = 1; i < with_expr.operands().size(); i += 2)
143+
out << "(store ";
144+
out << smt2_format(with_expr.old()) << ' ';
145+
for(std::size_t i = 1; i < with_expr.operands().size(); i += 2)
146+
{
147+
out << smt2_format(with_expr.operands()[i]) << ' '
148+
<< smt2_format(with_expr.operands()[i + 1]) << ')';
149+
}
145150
}
146151
else if(expr.id() == ID_array_list)
147152
{

src/solvers/strings/string_refinement_util.cpp

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,9 +70,12 @@ sparse_arrayt::sparse_arrayt(const with_exprt &expr)
7070
while(can_cast_expr<with_exprt>(ref.get()))
7171
{
7272
const auto &with_expr = expr_dynamic_cast<with_exprt>(ref.get());
73-
const auto current_index =
74-
numeric_cast_v<std::size_t>(to_constant_expr(with_expr.where()));
75-
entries[current_index] = with_expr.new_value();
73+
for(std::size_t i = 1; i < with_expr.operands().size(); i += 2)
74+
{
75+
const auto current_index =
76+
numeric_cast_v<std::size_t>(to_constant_expr(with_expr.operands()[i]));
77+
entries[current_index] = with_expr.operands()[i + 1];
78+
}
7679
ref = with_expr.old();
7780
}
7881

src/util/simplify_expr.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2157,7 +2157,7 @@ simplify_exprt::simplify_byte_update(const byte_update_exprt &expr)
21572157
* value)
21582158
*/
21592159

2160-
if(value.id()==ID_with)
2160+
if(value.id() == ID_with && value.operands().size() == 3)
21612161
{
21622162
const with_exprt &with=to_with_expr(value);
21632163

@@ -2297,6 +2297,7 @@ simplify_exprt::simplify_byte_update(const byte_update_exprt &expr)
22972297

22982298
if(
22992299
expr_at_offset_C.id() == ID_with &&
2300+
expr_at_offset_C.operands().size() == 3 &&
23002301
to_with_expr(expr_at_offset_C).where().is_zero())
23012302
{
23022303
tmp.set_op(to_with_expr(expr_at_offset_C).old());

0 commit comments

Comments
 (0)