Skip to content

Commit 91153e5

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 8640fc9 commit 91153e5

File tree

5 files changed

+254
-15
lines changed

5 files changed

+254
-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: 174 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>
@@ -22,6 +23,130 @@ Author: Daniel Kroening, kroening@kroening.com
2223

2324
#include <set>
2425

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

117242
// do assignments based on "value"
118243

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

122262
for(const auto &symbol_pair : symbol_table.symbols)
123263
{
124264
symbols.insert(id2string(symbol_pair.first));
125265
}
126266

127-
// first do framework variables
267+
// Build dependency graph
268+
auto dependencies = build_static_initialization_dependencies(symbols, ns);
269+
270+
// Separate CPROVER framework variables from user variables
271+
std::set<std::string> cprover_symbols;
272+
std::set<std::string> user_symbols;
273+
128274
for(const std::string &id : symbols)
275+
{
129276
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-
}
277+
cprover_symbols.insert(id);
278+
else
279+
user_symbols.insert(id);
280+
}
135281

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

145304
// now call designated "initialization" functions
146305

0 commit comments

Comments
 (0)