Skip to content

Commit caff2bd

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 caff2bd

File tree

5 files changed

+258
-15
lines changed

5 files changed

+258
-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: 178 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,134 @@ 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 on.
32+
/// 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 order.
94+
/// 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+
// If already visited, nothing to do
106+
if(visited.find(id) != visited.end())
107+
return;
108+
109+
// Check for cycles (should not happen in valid C code)
110+
if(in_progress.find(id) != in_progress.end())
111+
{
112+
// Cycle detected - this indicates an error in the source code
113+
// For now, we'll just continue to avoid infinite recursion
114+
return;
115+
}
116+
117+
in_progress.insert(id);
118+
119+
// Visit all dependencies first
120+
auto dep_it = dependencies.find(id);
121+
if(dep_it != dependencies.end())
122+
{
123+
// Sort dependencies alphabetically for reproducibility
124+
std::vector<std::string> sorted_deps;
125+
for(const irep_idt &dep : dep_it->second)
126+
{
127+
sorted_deps.push_back(id2string(dep));
128+
}
129+
std::sort(sorted_deps.begin(), sorted_deps.end());
130+
131+
for(const std::string &dep : sorted_deps)
132+
{
133+
// Only visit if it's in our symbol set
134+
if(symbols.find(dep) != symbols.end())
135+
visit(dep);
136+
}
137+
}
138+
139+
in_progress.erase(id);
140+
visited.insert(id);
141+
result.push_back(id);
142+
};
143+
144+
// Process all symbols in alphabetical order for reproducibility
145+
for(const std::string &id : symbols)
146+
{
147+
visit(id);
148+
}
149+
150+
return result;
151+
}
24152

25153
static std::optional<codet> static_lifetime_init(
26154
const irep_idt &identifier,
@@ -116,31 +244,66 @@ void static_lifetime_init(
116244

117245
// do assignments based on "value"
118246

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

122265
for(const auto &symbol_pair : symbol_table.symbols)
123266
{
124267
symbols.insert(id2string(symbol_pair.first));
125268
}
126269

127-
// first do framework variables
270+
// Build dependency graph
271+
auto dependencies =
272+
build_static_initialization_dependencies(symbol_table, symbols);
273+
274+
// Separate CPROVER framework variables from user variables
275+
std::set<std::string> cprover_symbols;
276+
std::set<std::string> user_symbols;
277+
128278
for(const std::string &id : symbols)
279+
{
129280
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-
}
281+
cprover_symbols.insert(id);
282+
else
283+
user_symbols.insert(id);
284+
}
135285

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-
}
286+
// First initialize framework variables with topological sort
287+
std::vector<std::string> sorted_cprover =
288+
topological_sort_with_dependencies(cprover_symbols, dependencies);
289+
290+
for(const std::string &id : sorted_cprover)
291+
{
292+
auto code = static_lifetime_init(id, symbol_table);
293+
if(code.has_value())
294+
dest.add(std::move(*code));
295+
}
296+
297+
// Now initialize all other variables with topological sort
298+
std::vector<std::string> sorted_user =
299+
topological_sort_with_dependencies(user_symbols, dependencies);
300+
301+
for(const std::string &id : sorted_user)
302+
{
303+
auto code = static_lifetime_init(id, symbol_table);
304+
if(code.has_value())
305+
dest.add(std::move(*code));
306+
}
144307

145308
// now call designated "initialization" functions
146309

0 commit comments

Comments
 (0)