Skip to content

Commit 291d541

Browse files
committed
Fix static lifetime pointer initialization dependency ordering
Add dependency tracking between static objects per C standard (C99/C11 6.7.9, 6.6) and use this for static-lifetime object initialisation order. Fixes: #8593
1 parent 4fe3ade commit 291d541

File tree

5 files changed

+259
-15
lines changed

5 files changed

+259
-15
lines changed
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
#include <assert.h>
2+
3+
// Test case for chained static initialization dependencies
4+
// Tests: A -> B -> C dependency chain
5+
//
6+
// According to C standard (C99/C11):
7+
// - Section 6.7.9 paragraph 4: All expressions in an initializer for an
8+
// object that has static storage duration shall be constant expressions.
9+
// - Section 6.6 paragraph 9: An address constant is a pointer to an lvalue
10+
// designating an object of static storage duration.
11+
//
12+
// This means c_int must be initialized before b_ptr (which points to it),
13+
// and b_ptr must be initialized before a_ptr_ptr (which points to it).
14+
15+
static int c_int = 42;
16+
static int *b_ptr = &c_int;
17+
static int **a_ptr_ptr = &b_ptr;
18+
19+
int main()
20+
{
21+
// Verify the entire chain is correctly initialized
22+
assert(a_ptr_ptr != 0);
23+
assert(*a_ptr_ptr != 0);
24+
assert(**a_ptr_ptr == 42);
25+
26+
// Verify pointer values are correct
27+
assert(*a_ptr_ptr == b_ptr);
28+
assert(**a_ptr_ptr == c_int);
29+
30+
return 0;
31+
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
CORE
2+
main.c
3+
4+
^EXIT=0$
5+
^SIGNAL=0$
6+
^VERIFICATION SUCCESSFUL$
7+
--
8+
^warning: ignoring
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
#include <assert.h>
2+
#include <stdint.h>
3+
#include <string.h>
4+
5+
#define PACKETBUF_SIZE 128
6+
7+
// Test case for GitHub issue #8593
8+
// When a pointer is initialized outside a function with a cast expression
9+
// pointing to another static storage duration object, CBMC should properly
10+
// handle the dependency and not mark the pointer as invalid.
11+
12+
static uint32_t packetbuf_aligned[(PACKETBUF_SIZE + 3) / 4];
13+
// This pointer initialization depends on packetbuf_aligned being initialized first
14+
uint8_t *packetbuf = (uint8_t *)packetbuf_aligned;
15+
16+
int main()
17+
{
18+
uint16_t channelId = 0x1234;
19+
uint8_t *data = packetbuf;
20+
21+
// This should not fail - data should point to valid memory
22+
assert(data != 0);
23+
24+
// Write some data
25+
memcpy(data, &channelId, 2);
26+
27+
// Verify the data was written correctly
28+
uint16_t result;
29+
memcpy(&result, data, 2);
30+
assert(result == channelId);
31+
32+
return 0;
33+
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
CORE
2+
main.c
3+
4+
^EXIT=0$
5+
^SIGNAL=0$
6+
^VERIFICATION SUCCESSFUL$
7+
--
8+
^warning: ignoring

src/linking/static_lifetime_init.cpp

Lines changed: 179 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ Author: Daniel Kroening, kroening@kroening.com
1111
#include <util/arith_tools.h>
1212
#include <util/c_types.h>
1313
#include <util/expr_initializer.h>
14+
#include <util/find_symbols.h>
1415
#include <util/namespace.h>
1516
#include <util/prefix.h>
1617
#include <util/std_code.h>
@@ -20,7 +21,135 @@ Author: Daniel Kroening, kroening@kroening.com
2021

2122
#include <ansi-c/goto-conversion/goto_convert_functions.h>
2223

24+
#include <algorithm>
25+
#include <functional>
26+
#include <map>
2327
#include <set>
28+
#include <vector>
29+
30+
/// Build a dependency graph for static lifetime objects.
31+
/// Returns a map from symbol identifier to the set of identifiers it depends
32+
/// on. According to C standard (C99/C11 Section 6.7.9 and 6.6 paragraph 9):
33+
/// - Objects with static storage duration can be initialized with constant
34+
/// expressions or string literals
35+
/// - An address constant is a pointer to an lvalue designating an object of
36+
/// static storage duration
37+
/// - When a static object's initializer references another static object,
38+
/// that referenced object must be initialized first to maintain proper
39+
/// initialization order
40+
static std::map<irep_idt, std::set<irep_idt>>
41+
build_static_initialization_dependencies(
42+
const symbol_table_baset &symbol_table,
43+
const std::set<std::string> &symbols)
44+
{
45+
std::map<irep_idt, std::set<irep_idt>> dependencies;
46+
const namespacet ns(symbol_table);
47+
48+
for(const std::string &id : symbols)
49+
{
50+
const symbolt &symbol = ns.lookup(id);
51+
52+
// Only track dependencies for objects with static lifetime that have
53+
// initializers
54+
if(
55+
!symbol.is_static_lifetime || symbol.is_type || symbol.is_macro ||
56+
symbol.type.id() == ID_code || symbol.type.id() == ID_empty)
57+
{
58+
continue;
59+
}
60+
61+
// Skip if no initializer or nondet initializer
62+
if(symbol.value.is_nil() || symbol.value.id() == ID_nondet)
63+
continue;
64+
65+
// Find all symbols referenced in the initializer
66+
find_symbols_sett referenced_symbols;
67+
find_symbols(symbol.value, referenced_symbols);
68+
69+
// Add dependencies on other static lifetime objects
70+
for(const irep_idt &ref_id : referenced_symbols)
71+
{
72+
// Skip self-references
73+
if(ref_id == symbol.name)
74+
continue;
75+
76+
// Check if the referenced symbol is in our set of symbols to initialize
77+
if(symbols.find(id2string(ref_id)) == symbols.end())
78+
continue;
79+
80+
// Verify it's a static lifetime object
81+
const symbolt *ref_symbol = symbol_table.lookup(ref_id);
82+
if(ref_symbol != nullptr && ref_symbol->is_static_lifetime)
83+
{
84+
dependencies[symbol.name].insert(ref_id);
85+
}
86+
}
87+
}
88+
89+
return dependencies;
90+
}
91+
92+
/// Perform a topological sort on symbols considering their initialization
93+
/// dependencies. Returns a vector of symbol identifiers in initialization
94+
/// order. Uses alphabetical ordering as a tiebreaker for reproducibility.
95+
static std::vector<std::string> topological_sort_with_dependencies(
96+
const std::set<std::string> &symbols,
97+
const std::map<irep_idt, std::set<irep_idt>> &dependencies)
98+
{
99+
std::vector<std::string> result;
100+
std::set<std::string> visited;
101+
std::set<std::string> in_progress; // For cycle detection
102+
103+
// Recursive helper function for depth-first search
104+
std::function<void(const std::string &)> visit = [&](const std::string &id)
105+
{
106+
// If already visited, nothing to do
107+
if(visited.find(id) != visited.end())
108+
return;
109+
110+
// Check for cycles (should not happen in valid C code)
111+
if(in_progress.find(id) != in_progress.end())
112+
{
113+
// Cycle detected - this indicates an error in the source code
114+
// For now, we'll just continue to avoid infinite recursion
115+
return;
116+
}
117+
118+
in_progress.insert(id);
119+
120+
// Visit all dependencies first
121+
auto dep_it = dependencies.find(id);
122+
if(dep_it != dependencies.end())
123+
{
124+
// Sort dependencies alphabetically for reproducibility
125+
std::vector<std::string> sorted_deps;
126+
for(const irep_idt &dep : dep_it->second)
127+
{
128+
sorted_deps.push_back(id2string(dep));
129+
}
130+
std::sort(sorted_deps.begin(), sorted_deps.end());
131+
132+
for(const std::string &dep : sorted_deps)
133+
{
134+
// Only visit if it's in our symbol set
135+
if(symbols.find(dep) != symbols.end())
136+
visit(dep);
137+
}
138+
}
139+
140+
in_progress.erase(id);
141+
visited.insert(id);
142+
result.push_back(id);
143+
};
144+
145+
// Process all symbols in alphabetical order for reproducibility
146+
for(const std::string &id : symbols)
147+
{
148+
visit(id);
149+
}
150+
151+
return result;
152+
}
24153

25154
static std::optional<codet> static_lifetime_init(
26155
const irep_idt &identifier,
@@ -116,31 +245,66 @@ void static_lifetime_init(
116245

117246
// do assignments based on "value"
118247

119-
// sort alphabetically for reproducible results
248+
// Build dependency graph for static lifetime initialization.
249+
// According to the C standard (C99/C11):
250+
// - Section 6.7.9 paragraph 4: All expressions in an initializer for an
251+
// object that has static storage duration shall be constant expressions
252+
// or string literals.
253+
// - Section 6.6 paragraph 9: An address constant is a null pointer, a pointer
254+
// to an lvalue designating an object of static storage duration, or a
255+
// pointer to a function designator.
256+
// - Section 6.2.4: Objects with static storage duration are initialized
257+
// before program startup.
258+
//
259+
// When one static object's initializer takes the address of another static
260+
// object, the referenced object must be initialized first to ensure the
261+
// address constant is properly available.
262+
263+
// First, collect all symbols and sort alphabetically for reproducible results
120264
std::set<std::string> symbols;
121265

122266
for(const auto &symbol_pair : symbol_table.symbols)
123267
{
124268
symbols.insert(id2string(symbol_pair.first));
125269
}
126270

127-
// first do framework variables
271+
// Build dependency graph
272+
auto dependencies =
273+
build_static_initialization_dependencies(symbol_table, symbols);
274+
275+
// Separate CPROVER framework variables from user variables
276+
std::set<std::string> cprover_symbols;
277+
std::set<std::string> user_symbols;
278+
128279
for(const std::string &id : symbols)
280+
{
129281
if(has_prefix(id, CPROVER_PREFIX))
130-
{
131-
auto code = static_lifetime_init(id, symbol_table);
132-
if(code.has_value())
133-
dest.add(std::move(*code));
134-
}
282+
cprover_symbols.insert(id);
283+
else
284+
user_symbols.insert(id);
285+
}
135286

136-
// now all other variables
137-
for(const std::string &id : symbols)
138-
if(!has_prefix(id, CPROVER_PREFIX))
139-
{
140-
auto code = static_lifetime_init(id, symbol_table);
141-
if(code.has_value())
142-
dest.add(std::move(*code));
143-
}
287+
// First initialize framework variables with topological sort
288+
std::vector<std::string> sorted_cprover =
289+
topological_sort_with_dependencies(cprover_symbols, dependencies);
290+
291+
for(const std::string &id : sorted_cprover)
292+
{
293+
auto code = static_lifetime_init(id, symbol_table);
294+
if(code.has_value())
295+
dest.add(std::move(*code));
296+
}
297+
298+
// Now initialize all other variables with topological sort
299+
std::vector<std::string> sorted_user =
300+
topological_sort_with_dependencies(user_symbols, dependencies);
301+
302+
for(const std::string &id : sorted_user)
303+
{
304+
auto code = static_lifetime_init(id, symbol_table);
305+
if(code.has_value())
306+
dest.add(std::move(*code));
307+
}
144308

145309
// now call designated "initialization" functions
146310

0 commit comments

Comments
 (0)