Skip to content

Commit 2324bcd

Browse files
Bernhard M. Wiedemannbmwiedemann
Bernhard M. Wiedemann
authored and
bmwiedemann
committed
Fix #23745 - inline: scan local symbols in a deterministic order
The symbol table iterates in bucket order, which follows the addresses of the Identifier keys and so varies between runs with ASLR. The order matters because inlining one nested function changes the inline cost of the others, so different runs inlined different functions, making builds unreproducible: compiling dmd's own ob.d gave a different .o almost every run. Sort the symbols by name before visiting them, as dtoh.d already does for module members. The dshell test compiles an order-sensitive module ten times and requires bit-identical objects; without the fix each run gives a different object file. Fixes #23745
1 parent 1906158 commit 2324bcd

3 files changed

Lines changed: 286 additions & 1 deletion

File tree

‎compiler/src/dmd/inline.d‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1822,10 +1822,21 @@ public:
18221822

18231823
if (fd.localsymtab)
18241824
{
1825+
// Sort by name: table order follows Identifier addresses,
1826+
// and inline decisions are order-dependent
1827+
Dsymbols symbols;
1828+
symbols.reserve(fd.localsymtab.length);
18251829
foreach (keyValue; fd.localsymtab.tab.asRange)
1830+
symbols.push(keyValue.value);
1831+
1832+
static int compare(const Dsymbol* a, const Dsymbol* b)
18261833
{
1827-
keyValue.value.accept(this);
1834+
return strcmp(a.ident.toChars(), b.ident.toChars());
18281835
}
1836+
symbols.sort!compare();
1837+
1838+
foreach (s; symbols)
1839+
s.accept(this);
18291840
}
18301841
}
18311842

‎compiler/test/dshell/extra-files/test23745.d‎

Lines changed: 254 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,254 @@
1+
// https://github.com/dlang/dmd/issues/23745
2+
// Nested functions whose inline verdicts depend on the order they are
3+
// scanned in: groups of near-threshold visitors calling shared helpers.
4+
// The varied identifier lengths give each symbol table its own layout.
5+
module test23745;
6+
struct Node { int val; int extra; Node* next; }
7+
8+
int processA(Node* root)
9+
{
10+
int count;
11+
Node*[8] stack;
12+
int sp;
13+
14+
void pushA(Node* n) { if (sp < 8) { stack[sp] = n; ++sp; ++count; } }
15+
Node* popA() { if (sp) { --sp; return stack[sp]; } return null; }
16+
void goNextA(Node* n) { if (n && n.next) { pushA(n.next); n.extra += count; } }
17+
void markA(Node* n) { n.extra = count + sp; ++count; }
18+
19+
void visitA(Node* n)
20+
{
21+
void vaA(Node* x) { pushA(x); goNextA(x); markA(x); if (x.val > 1) visitA(x.next); }
22+
void vbAxxx(Node* x) { goNextA(x); markA(x); pushA(x); if (x.val > 2) visitA(x.next); }
23+
void vcAxxxxxx(Node* x) { markA(x); goNextA(x); if (x.val > 3) { pushA(x); visitA(x.next); } }
24+
void vdAxxxxxxxxx(Node* x) { pushA(x); markA(x); if (x.val > 4) visitA(x.next); goNextA(x); }
25+
void veAx(Node* x) { pushA(x); goNextA(x); markA(x); if (x.val > 5) visitA(x.next); }
26+
void vfAxxxx(Node* x) { goNextA(x); markA(x); pushA(x); if (x.val > 6) visitA(x.next); }
27+
switch (n.val % 6)
28+
{
29+
case 0: vaA(n); break;
30+
case 1: vbAxxx(n); break;
31+
case 2: vcAxxxxxx(n); break;
32+
case 3: vdAxxxxxxxxx(n); break;
33+
case 4: veAx(n); break;
34+
case 5: vfAxxxx(n); break;
35+
default: vaA(n); vfAxxxx(n); break;
36+
}
37+
}
38+
for (Node* n = root; n; n = n.next)
39+
visitA(n);
40+
while (auto n = popA())
41+
count += n.extra;
42+
return count;
43+
}
44+
45+
int processB(Node* root)
46+
{
47+
int count;
48+
Node*[8] stack;
49+
int sp;
50+
51+
void pushBxxx(Node* n) { if (sp < 8) { stack[sp] = n; ++sp; ++count; } }
52+
Node* popBxxx() { if (sp) { --sp; return stack[sp]; } return null; }
53+
void goNextBxxx(Node* n) { if (n && n.next) { pushBxxx(n.next); n.extra += count; } }
54+
void markBxxx(Node* n) { n.extra = count + sp; ++count; }
55+
56+
void visitBxxx(Node* n)
57+
{
58+
void dummy0B(Node* x) { x.extra += 0; }
59+
void dummy1Bx(Node* x) { x.extra += 1; }
60+
void dummy2Bxx(Node* x) { x.extra += 2; }
61+
void vaBxxx(Node* x) { pushBxxx(x); goNextBxxx(x); markBxxx(x); if (x.val > 1) visitBxxx(x.next); }
62+
void vbBxxxxxx(Node* x) { goNextBxxx(x); markBxxx(x); pushBxxx(x); if (x.val > 2) visitBxxx(x.next); }
63+
void vcBxxxxxxxxx(Node* x) { markBxxx(x); goNextBxxx(x); if (x.val > 3) { pushBxxx(x); visitBxxx(x.next); } }
64+
void vdBx(Node* x) { pushBxxx(x); markBxxx(x); if (x.val > 4) visitBxxx(x.next); goNextBxxx(x); }
65+
void veBxxxx(Node* x) { pushBxxx(x); goNextBxxx(x); markBxxx(x); if (x.val > 5) visitBxxx(x.next); }
66+
void vfBxxxxxxx(Node* x) { goNextBxxx(x); markBxxx(x); pushBxxx(x); if (x.val > 6) visitBxxx(x.next); }
67+
void vgBxxxxxxxxxx(Node* x) { markBxxx(x); goNextBxxx(x); if (x.val > 7) { pushBxxx(x); visitBxxx(x.next); } }
68+
void vhBxx(Node* x) { pushBxxx(x); markBxxx(x); if (x.val > 8) visitBxxx(x.next); goNextBxxx(x); }
69+
switch (n.val % 8)
70+
{
71+
case 0: vaBxxx(n); break;
72+
case 1: vbBxxxxxx(n); break;
73+
case 2: vcBxxxxxxxxx(n); break;
74+
case 3: vdBx(n); break;
75+
case 4: veBxxxx(n); break;
76+
case 5: vfBxxxxxxx(n); break;
77+
case 6: vgBxxxxxxxxxx(n); break;
78+
case 7: vhBxx(n); break;
79+
default: vaBxxx(n); vhBxx(n); break;
80+
}
81+
}
82+
for (Node* n = root; n; n = n.next)
83+
visitBxxx(n);
84+
while (auto n = popBxxx())
85+
count += n.extra;
86+
return count;
87+
}
88+
89+
int processC(Node* root)
90+
{
91+
int count;
92+
Node*[8] stack;
93+
int sp;
94+
95+
void pushCxxxxxxx(Node* n) { if (sp < 8) { stack[sp] = n; ++sp; ++count; } }
96+
Node* popCxxxxxxx() { if (sp) { --sp; return stack[sp]; } return null; }
97+
void goNextCxxxxxxx(Node* n) { if (n && n.next) { pushCxxxxxxx(n.next); n.extra += count; } }
98+
void markCxxxxxxx(Node* n) { n.extra = count + sp; ++count; }
99+
100+
void visitCxxxxxxx(Node* n)
101+
{
102+
void dummy0C(Node* x) { x.extra += 0; }
103+
void dummy1Cx(Node* x) { x.extra += 1; }
104+
void dummy2Cxx(Node* x) { x.extra += 2; }
105+
void vaCxxxxxxx(Node* x) { pushCxxxxxxx(x); goNextCxxxxxxx(x); markCxxxxxxx(x); if (x.val > 1) visitCxxxxxxx(x.next); }
106+
void vbCxxxxxxxxxx(Node* x) { goNextCxxxxxxx(x); markCxxxxxxx(x); pushCxxxxxxx(x); if (x.val > 2) visitCxxxxxxx(x.next); }
107+
void vcCxx(Node* x) { markCxxxxxxx(x); goNextCxxxxxxx(x); if (x.val > 3) { pushCxxxxxxx(x); visitCxxxxxxx(x.next); } }
108+
void vdCxxxxx(Node* x) { pushCxxxxxxx(x); markCxxxxxxx(x); if (x.val > 4) visitCxxxxxxx(x.next); goNextCxxxxxxx(x); }
109+
void veCxxxxxxxx(Node* x) { pushCxxxxxxx(x); goNextCxxxxxxx(x); markCxxxxxxx(x); if (x.val > 5) visitCxxxxxxx(x.next); }
110+
switch (n.val % 5)
111+
{
112+
case 0: vaCxxxxxxx(n); break;
113+
case 1: vbCxxxxxxxxxx(n); break;
114+
case 2: vcCxx(n); break;
115+
case 3: vdCxxxxx(n); break;
116+
case 4: veCxxxxxxxx(n); break;
117+
default: vaCxxxxxxx(n); veCxxxxxxxx(n); break;
118+
}
119+
}
120+
for (Node* n = root; n; n = n.next)
121+
visitCxxxxxxx(n);
122+
while (auto n = popCxxxxxxx())
123+
count += n.extra;
124+
return count;
125+
}
126+
127+
int processD(Node* root)
128+
{
129+
int count;
130+
Node*[8] stack;
131+
int sp;
132+
133+
void pushDx(Node* n) { if (sp < 8) { stack[sp] = n; ++sp; ++count; } }
134+
Node* popDx() { if (sp) { --sp; return stack[sp]; } return null; }
135+
void goNextDx(Node* n) { if (n && n.next) { pushDx(n.next); n.extra += count; } }
136+
void markDx(Node* n) { n.extra = count + sp; ++count; }
137+
138+
void visitDx(Node* n)
139+
{
140+
void dummy0D(Node* x) { x.extra += 0; }
141+
void vaDx(Node* x) { pushDx(x); goNextDx(x); markDx(x); if (x.val > 1) visitDx(x.next); }
142+
void vbDxxxx(Node* x) { goNextDx(x); markDx(x); pushDx(x); if (x.val > 2) visitDx(x.next); }
143+
void vcDxxxxxxx(Node* x) { markDx(x); goNextDx(x); if (x.val > 3) { pushDx(x); visitDx(x.next); } }
144+
void vdDxxxxxxxxxx(Node* x) { pushDx(x); markDx(x); if (x.val > 4) visitDx(x.next); goNextDx(x); }
145+
void veDxx(Node* x) { pushDx(x); goNextDx(x); markDx(x); if (x.val > 5) visitDx(x.next); }
146+
void vfDxxxxx(Node* x) { goNextDx(x); markDx(x); pushDx(x); if (x.val > 6) visitDx(x.next); }
147+
void vgDxxxxxxxx(Node* x) { markDx(x); goNextDx(x); if (x.val > 7) { pushDx(x); visitDx(x.next); } }
148+
void vhD(Node* x) { pushDx(x); markDx(x); if (x.val > 8) visitDx(x.next); goNextDx(x); }
149+
void viDxxx(Node* x) { pushDx(x); goNextDx(x); markDx(x); if (x.val > 9) visitDx(x.next); }
150+
void vjDxxxxxx(Node* x) { goNextDx(x); markDx(x); pushDx(x); if (x.val > 10) visitDx(x.next); }
151+
switch (n.val % 10)
152+
{
153+
case 0: vaDx(n); break;
154+
case 1: vbDxxxx(n); break;
155+
case 2: vcDxxxxxxx(n); break;
156+
case 3: vdDxxxxxxxxxx(n); break;
157+
case 4: veDxx(n); break;
158+
case 5: vfDxxxxx(n); break;
159+
case 6: vgDxxxxxxxx(n); break;
160+
case 7: vhD(n); break;
161+
case 8: viDxxx(n); break;
162+
case 9: vjDxxxxxx(n); break;
163+
default: vaDx(n); vjDxxxxxx(n); break;
164+
}
165+
}
166+
for (Node* n = root; n; n = n.next)
167+
visitDx(n);
168+
while (auto n = popDx())
169+
count += n.extra;
170+
return count;
171+
}
172+
173+
int processE(Node* root)
174+
{
175+
int count;
176+
Node*[8] stack;
177+
int sp;
178+
179+
void pushExxxxxxxxxxxx(Node* n) { if (sp < 8) { stack[sp] = n; ++sp; ++count; } }
180+
Node* popExxxxxxxxxxxx() { if (sp) { --sp; return stack[sp]; } return null; }
181+
void goNextExxxxxxxxxxxx(Node* n) { if (n && n.next) { pushExxxxxxxxxxxx(n.next); n.extra += count; } }
182+
void markExxxxxxxxxxxx(Node* n) { n.extra = count + sp; ++count; }
183+
184+
void visitExxxxxxxxxxxx(Node* n)
185+
{
186+
void vaEx(Node* x) { pushExxxxxxxxxxxx(x); goNextExxxxxxxxxxxx(x); markExxxxxxxxxxxx(x); if (x.val > 1) visitExxxxxxxxxxxx(x.next); }
187+
void vbExxxx(Node* x) { goNextExxxxxxxxxxxx(x); markExxxxxxxxxxxx(x); pushExxxxxxxxxxxx(x); if (x.val > 2) visitExxxxxxxxxxxx(x.next); }
188+
void vcExxxxxxx(Node* x) { markExxxxxxxxxxxx(x); goNextExxxxxxxxxxxx(x); if (x.val > 3) { pushExxxxxxxxxxxx(x); visitExxxxxxxxxxxx(x.next); } }
189+
void vdExxxxxxxxxx(Node* x) { pushExxxxxxxxxxxx(x); markExxxxxxxxxxxx(x); if (x.val > 4) visitExxxxxxxxxxxx(x.next); goNextExxxxxxxxxxxx(x); }
190+
void veExx(Node* x) { pushExxxxxxxxxxxx(x); goNextExxxxxxxxxxxx(x); markExxxxxxxxxxxx(x); if (x.val > 5) visitExxxxxxxxxxxx(x.next); }
191+
void vfExxxxx(Node* x) { goNextExxxxxxxxxxxx(x); markExxxxxxxxxxxx(x); pushExxxxxxxxxxxx(x); if (x.val > 6) visitExxxxxxxxxxxx(x.next); }
192+
void vgExxxxxxxx(Node* x) { markExxxxxxxxxxxx(x); goNextExxxxxxxxxxxx(x); if (x.val > 7) { pushExxxxxxxxxxxx(x); visitExxxxxxxxxxxx(x.next); } }
193+
switch (n.val % 7)
194+
{
195+
case 0: vaEx(n); break;
196+
case 1: vbExxxx(n); break;
197+
case 2: vcExxxxxxx(n); break;
198+
case 3: vdExxxxxxxxxx(n); break;
199+
case 4: veExx(n); break;
200+
case 5: vfExxxxx(n); break;
201+
case 6: vgExxxxxxxx(n); break;
202+
default: vaEx(n); vgExxxxxxxx(n); break;
203+
}
204+
}
205+
for (Node* n = root; n; n = n.next)
206+
visitExxxxxxxxxxxx(n);
207+
while (auto n = popExxxxxxxxxxxx())
208+
count += n.extra;
209+
return count;
210+
}
211+
212+
int processF(Node* root)
213+
{
214+
int count;
215+
Node*[8] stack;
216+
int sp;
217+
218+
void pushFxxxxx(Node* n) { if (sp < 8) { stack[sp] = n; ++sp; ++count; } }
219+
Node* popFxxxxx() { if (sp) { --sp; return stack[sp]; } return null; }
220+
void goNextFxxxxx(Node* n) { if (n && n.next) { pushFxxxxx(n.next); n.extra += count; } }
221+
void markFxxxxx(Node* n) { n.extra = count + sp; ++count; }
222+
223+
void visitFxxxxx(Node* n)
224+
{
225+
void dummy0F(Node* x) { x.extra += 0; }
226+
void vaFxxxxx(Node* x) { pushFxxxxx(x); goNextFxxxxx(x); markFxxxxx(x); if (x.val > 1) visitFxxxxx(x.next); }
227+
void vbFxxxxxxxx(Node* x) { goNextFxxxxx(x); markFxxxxx(x); pushFxxxxx(x); if (x.val > 2) visitFxxxxx(x.next); }
228+
void vcF(Node* x) { markFxxxxx(x); goNextFxxxxx(x); if (x.val > 3) { pushFxxxxx(x); visitFxxxxx(x.next); } }
229+
void vdFxxx(Node* x) { pushFxxxxx(x); markFxxxxx(x); if (x.val > 4) visitFxxxxx(x.next); goNextFxxxxx(x); }
230+
void veFxxxxxx(Node* x) { pushFxxxxx(x); goNextFxxxxx(x); markFxxxxx(x); if (x.val > 5) visitFxxxxx(x.next); }
231+
void vfFxxxxxxxxx(Node* x) { goNextFxxxxx(x); markFxxxxx(x); pushFxxxxx(x); if (x.val > 6) visitFxxxxx(x.next); }
232+
void vgFx(Node* x) { markFxxxxx(x); goNextFxxxxx(x); if (x.val > 7) { pushFxxxxx(x); visitFxxxxx(x.next); } }
233+
void vhFxxxx(Node* x) { pushFxxxxx(x); markFxxxxx(x); if (x.val > 8) visitFxxxxx(x.next); goNextFxxxxx(x); }
234+
void viFxxxxxxx(Node* x) { pushFxxxxx(x); goNextFxxxxx(x); markFxxxxx(x); if (x.val > 9) visitFxxxxx(x.next); }
235+
switch (n.val % 9)
236+
{
237+
case 0: vaFxxxxx(n); break;
238+
case 1: vbFxxxxxxxx(n); break;
239+
case 2: vcF(n); break;
240+
case 3: vdFxxx(n); break;
241+
case 4: veFxxxxxx(n); break;
242+
case 5: vfFxxxxxxxxx(n); break;
243+
case 6: vgFx(n); break;
244+
case 7: vhFxxxx(n); break;
245+
case 8: viFxxxxxxx(n); break;
246+
default: vaFxxxxx(n); viFxxxxxxx(n); break;
247+
}
248+
}
249+
for (Node* n = root; n; n = n.next)
250+
visitFxxxxx(n);
251+
while (auto n = popFxxxxx())
252+
count += n.extra;
253+
return count;
254+
}

‎compiler/test/dshell/test23745.d‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
import dshell;
2+
3+
// https://github.com/dlang/dmd/issues/23745
4+
// Nested functions were inline-scanned in symbol-table bucket order, which
5+
// follows the Identifier addresses, so -inline codegen varied under ASLR.
6+
void main()
7+
{
8+
const obj = shellExpand("$OUTPUT_BASE/test23745$OBJ");
9+
const(ubyte)[] first;
10+
foreach (i; 0 .. 10)
11+
{
12+
run("$DMD -m$MODEL -c -inline -of$OUTPUT_BASE/test23745$OBJ"
13+
~ " $EXTRA_FILES/test23745.d");
14+
const bytes = cast(const(ubyte)[]) std.file.read(obj);
15+
if (i == 0)
16+
first = bytes;
17+
else
18+
enforce(bytes == first, "-inline codegen differs between identical runs");
19+
}
20+
}

0 commit comments

Comments
 (0)