Skip to content

Commit bdd16b3

Browse files
committed
Restore member-of-union expressions in trace output
byte_extract operations precisely capture the semantics, but aren't meaningful to users.
1 parent 416b744 commit bdd16b3

File tree

5 files changed

+74
-9
lines changed

5 files changed

+74
-9
lines changed

regression/cbmc/xml-trace/test.desc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ VERIFICATION FAILED
77
<assignment assignment_type="state" base_name="u" display_name="test::u" hidden="false" identifier="test::u" mode="C" step_nr="\d+" thread="0">
88
<location file=".*" function="test" line="\d+" working-directory=".*"/>
99
<type>union myunion</type>
10-
<full_lhs>byte_extract_little_endian\(u, 0ll?, .*int.*\)</full_lhs>
10+
<full_lhs>u</full_lhs>
1111
<full_lhs_value binary="[01]+">\d+ll?</full_lhs_value>
1212
<value>\{ \.i=\d+ll? \}</value>
1313
<value_expression>

src/goto-programs/goto_trace.cpp

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ void goto_trace_stept::output(
126126
if(!comment.empty())
127127
out << " " << comment << '\n';
128128

129-
out << " " << format(pc->get_condition()) << '\n';
129+
out << " " << format(cond_expr) << '\n';
130130
out << '\n';
131131
}
132132
}
@@ -397,8 +397,7 @@ void show_compact_goto_trace(
397397

398398
if(step.pc->is_assert())
399399
{
400-
out << " "
401-
<< from_expr(ns, step.function_id, step.pc->get_condition())
400+
out << " " << from_expr(ns, step.function_id, step.cond_expr)
402401
<< '\n';
403402
}
404403

@@ -524,8 +523,7 @@ void show_full_goto_trace(
524523

525524
if(step.pc->is_assert())
526525
{
527-
out << " "
528-
<< from_expr(ns, step.function_id, step.pc->get_condition())
526+
out << " " << from_expr(ns, step.function_id, step.cond_expr)
529527
<< '\n';
530528
}
531529

src/goto-programs/rewrite_union.cpp

Lines changed: 63 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,10 @@ Author: Daniel Kroening, [email protected]
1212
#include "rewrite_union.h"
1313

1414
#include <util/arith_tools.h>
15-
#include <util/std_expr.h>
16-
#include <util/std_code.h>
1715
#include <util/byte_operators.h>
16+
#include <util/pointer_offset_size.h>
17+
#include <util/std_code.h>
18+
#include <util/std_expr.h>
1819

1920
#include <goto-programs/goto_model.h>
2021

@@ -124,3 +125,63 @@ void rewrite_union(goto_modelt &goto_model)
124125
{
125126
rewrite_union(goto_model.goto_functions);
126127
}
128+
129+
/// Undo the union access -> byte_extract replacement that rewrite_union did for
130+
/// the purpose of displaying expressions to users.
131+
/// \param expr: expression to inspect and possibly transform
132+
/// \param ns: namespace
133+
/// \return True if, and only if, the expression is unmodified
134+
static bool restore_union_rec(exprt &expr, const namespacet &ns)
135+
{
136+
bool unmodified = true;
137+
138+
Forall_operands(it, expr)
139+
unmodified &= restore_union_rec(*it, ns);
140+
141+
if(expr.id() == byte_extract_id())
142+
{
143+
byte_extract_exprt &be = to_byte_extract_expr(expr);
144+
if(be.op().type().id() == ID_union || be.op().type().id() == ID_union_tag)
145+
{
146+
const union_typet &union_type = to_union_type(ns.follow(be.op().type()));
147+
148+
for(const auto &comp : union_type.components())
149+
{
150+
if(be.offset().is_zero() && be.type() == comp.type())
151+
{
152+
expr = member_exprt{be.op(), comp.get_name(), be.type()};
153+
return false;
154+
}
155+
else if(
156+
comp.type().id() == ID_array || comp.type().id() == ID_struct ||
157+
comp.type().id() == ID_struct_tag)
158+
{
159+
optionalt<exprt> result = get_subexpression_at_offset(
160+
member_exprt{be.op(), comp.get_name(), comp.type()},
161+
be.offset(),
162+
be.type(),
163+
ns);
164+
if(result.has_value())
165+
{
166+
expr = *result;
167+
return false;
168+
}
169+
}
170+
}
171+
}
172+
}
173+
174+
return unmodified;
175+
}
176+
177+
/// Undo the union access -> byte_extract replacement that rewrite_union did for
178+
/// the purpose of displaying expressions to users.
179+
/// \param expr: expression to inspect and possibly transform
180+
/// \param ns: namespace
181+
void restore_union(exprt &expr, const namespacet &ns)
182+
{
183+
exprt tmp = expr;
184+
185+
if(!restore_union_rec(tmp, ns))
186+
expr.swap(tmp);
187+
}

src/goto-programs/rewrite_union.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,4 +24,6 @@ void rewrite_union(goto_functionst::goto_functiont &);
2424
void rewrite_union(goto_functionst &);
2525
void rewrite_union(goto_modelt &);
2626

27+
void restore_union(exprt &, const namespacet &);
28+
2729
#endif // CPROVER_GOTO_PROGRAMS_REWRITE_UNION_H

src/goto-symex/build_goto_trace.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ Author: Daniel Kroening
2222

2323
#include <solvers/decision_procedure.h>
2424

25+
#include <goto-programs/rewrite_union.h>
26+
2527
#include "partial_order_concurrency.h"
2628

2729
static exprt build_full_lhs_rec(
@@ -378,6 +380,7 @@ void build_goto_trace(
378380
SSA_step.ssa_full_lhs),
379381
ns);
380382
replace_nondet_in_type(goto_trace_step.full_lhs, decision_procedure);
383+
restore_union(goto_trace_step.full_lhs, ns);
381384
}
382385

383386
if(SSA_step.ssa_full_lhs.is_not_nil())
@@ -404,7 +407,8 @@ void build_goto_trace(
404407

405408
if(SSA_step.is_assert() || SSA_step.is_assume() || SSA_step.is_goto())
406409
{
407-
goto_trace_step.cond_expr = SSA_step.cond_expr;
410+
goto_trace_step.cond_expr = SSA_step.source.pc->get_condition();
411+
restore_union(goto_trace_step.cond_expr, ns);
408412

409413
goto_trace_step.cond_value =
410414
decision_procedure.get(SSA_step.cond_handle).is_true();

0 commit comments

Comments
 (0)