Skip to content

Commit 7884d7e

Browse files
committed
Fix segfault in k-induction step case with nested loops
The original code assumed `loop_head->condition()` always contained the loop guard, but in nested loop scenarios, the loop structure can vary. The backedge might contain the condition instead, or the loop might be unconditional. Also, when processing nested loops, modifying the outer loop's goto-program could invalidate iterators pointing to the inner loop, causing memory access violations. Co-authored-by: Kiro autonomous agent Fixes: #5357
1 parent 76dd20f commit 7884d7e

File tree

3 files changed

+110
-6
lines changed

3 files changed

+110
-6
lines changed
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// Test case for nested loops in k-induction instrumentation
2+
// This used to cause segmentation faults in CBMC versions 5.6 and 5.12
3+
// when using --step-case because the code incorrectly assumed loop heads
4+
// always have conditions and didn't handle nested loop iterator invalidation
5+
int nondet_condition();
6+
int main()
7+
{
8+
int i = 0;
9+
int j = 0;
10+
while(i < 10)
11+
{
12+
while(nondet_condition() == 0)
13+
{
14+
j++;
15+
if(j == 10)
16+
j = 0;
17+
}
18+
__CPROVER_assert(j < 11, "should hold");
19+
i++;
20+
}
21+
}
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
CORE
2+
main.c
3+
2
4+
^EXIT=0$
5+
^SIGNAL=0$
6+
^Instrumenting k-induction for k=2, base case$
7+
^Instrumenting k-induction for k=2, step case$
8+
--
9+
^warning: ignoring

src/goto-instrument/k_induction.cpp

Lines changed: 80 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,14 @@ Author: Daniel Kroening, [email protected]
1111

1212
#include "k_induction.h"
1313

14-
#include <analyses/natural_loops.h>
15-
#include <analyses/local_may_alias.h>
14+
#include <util/expr_util.h>
15+
#include <util/std_expr.h>
1616

1717
#include <goto-programs/remove_skip.h>
1818

19+
#include <analyses/local_may_alias.h>
20+
#include <analyses/natural_loops.h>
21+
1922
#include "havoc_utils.h"
2023
#include "loop_utils.h"
2124
#include "unwind.h"
@@ -66,13 +69,44 @@ void k_inductiont::process_loop(
6669
{
6770
PRECONDITION(!loop.empty());
6871

69-
// save the loop guard
70-
const exprt loop_guard = loop_head->condition();
71-
7272
// compute the loop exit
7373
goto_programt::targett loop_exit=
7474
get_loop_exit(loop);
7575

76+
// Find the loop guard. The loop condition can be in one of two places:
77+
// 1. In the backwards goto at the end of the loop (the backedge)
78+
// 2. In a forward goto at the loop head that targets the loop exit
79+
80+
// Find the backwards goto (backedge) for this loop
81+
goto_programt::targett backedge = loop_exit;
82+
for(loopt::const_iterator it = loop.begin(); it != loop.end(); ++it)
83+
{
84+
if((*it)->is_backwards_goto() && (*it)->get_target() == loop_head)
85+
{
86+
backedge = *it;
87+
break;
88+
}
89+
}
90+
91+
exprt loop_guard;
92+
93+
if(backedge != loop_exit && !backedge->condition().is_true())
94+
{
95+
// condition in backedge
96+
loop_guard = backedge->condition();
97+
}
98+
else if(loop_head->is_goto())
99+
{
100+
if(loop_head->get_target() == loop_exit) // condition in forward edge
101+
loop_guard = boolean_negate(loop_head->condition());
102+
else
103+
loop_guard = true_exprt(); // unconditional loop
104+
}
105+
else
106+
{
107+
loop_guard = true_exprt(); // unconditional loop
108+
}
109+
76110
if(base_case)
77111
{
78112
// now unwind k times
@@ -157,12 +191,52 @@ void k_inductiont::process_loop(
157191
void k_inductiont::k_induction()
158192
{
159193
// iterate over the (natural) loops in the function
194+
// We need to be careful with nested loops, as processing the outer loop
195+
// might invalidate iterators for the inner loop. So we collect all loops
196+
// first, then process only outermost loops.
197+
198+
std::vector<std::pair<goto_programt::targett, loopt>> loops_to_process;
160199

161200
for(natural_loops_mutablet::loop_mapt::const_iterator
162201
l_it=natural_loops.loop_map.begin();
163202
l_it!=natural_loops.loop_map.end();
164203
l_it++)
165-
process_loop(l_it->first, l_it->second);
204+
{
205+
// Check if this loop is nested within another loop
206+
bool is_nested = false;
207+
goto_programt::targett loop_head = l_it->first;
208+
209+
for(natural_loops_mutablet::loop_mapt::const_iterator other_it =
210+
natural_loops.loop_map.begin();
211+
other_it != natural_loops.loop_map.end();
212+
other_it++)
213+
{
214+
if(other_it == l_it)
215+
continue; // skip self
216+
217+
// Check if loop_head is contained within other_it's loop
218+
const loopt &other_loop = other_it->second;
219+
if(
220+
std::find(other_loop.begin(), other_loop.end(), loop_head) !=
221+
other_loop.end())
222+
{
223+
is_nested = true;
224+
break;
225+
}
226+
}
227+
228+
// Only process outermost loops to avoid iterator invalidation
229+
if(!is_nested)
230+
{
231+
loops_to_process.push_back(std::make_pair(l_it->first, l_it->second));
232+
}
233+
}
234+
235+
// Now process the outermost loops
236+
for(auto &loop_pair : loops_to_process)
237+
{
238+
process_loop(loop_pair.first, loop_pair.second);
239+
}
166240
}
167241

168242
void k_induction(

0 commit comments

Comments
 (0)