Skip to content

Commit d23a8e5

Browse files
committed
fix: free()/malloc() calls misattributed as calling into the next code on arm64
Root-caused a real production bug: `free() -> analyze_fractal_tree` (and similar edges out of malloc/free/alloc shims) showing up in aarch64 call graphs, stealing cost from wherever the allocator actually returned to. Two compounding defects in callstack.c/bbcc.c's handling of emulated calls (a Boring/Jump jump promoted to jk_Call for cost attribution, e.g. a tail call crossing an ELF object boundary -- `__rdl_dealloc` tail-calling `free@plt`, `__rdl_alloc` tail-calling `malloc@plt`): 1. push_call_stack only computed ret_addr for jumps statically classified jk_Call, so an emulated call's frame got ret_addr==0 (silently, this was apparently expected/documented behavior) and could never match on return. Fixed by inheriting the emulating caller's own ret_addr, since a tail call never owns its own return slot. 2. That inheritance means a chain of 2+ emulated calls now shares one ret_addr across multiple stacked frames. The return-matching loops stopped at the *first* frame matching that address, leaving deeper aliased frames stale -- so the real callee's return still misattributed the next jump as a fresh call. Fixed by continuing to pop through further equal-SP frames that independently match the same target. Verified against the actual production binary (codspeed-integrations-e2e-tests) via `codspeed run --skip-upload`: zero bogus edges out of any alloc/dealloc function post-fix, versus several before. Added a real heap-allocating Rust fixture (fractal_alloc.rs, adapted from the production benchmark) and a minimal C reproduction (arm64_wrapped_alloc_chain.c) as permanent regression coverage; bisection-confirmed both fail without fix #2 applied.
1 parent a8af7e6 commit d23a8e5

9 files changed

Lines changed: 1233 additions & 5 deletions

File tree

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
// AArch64 reproducer for the "aliased emulated frames" bug: a chain of
2+
// THREE plain-`b` tail calls (mirroring Rust's real
3+
// `__rust_alloc -> __rdl_alloc -> malloc@plt` / `__rust_dealloc ->
4+
// __rdl_dealloc -> free@plt` shims) into a real external allocator
5+
// function. Each tail-called wrapper's call-stack entry inherits its
6+
// ret_addr from the frame that emulated it (bbcc.c's push_call_stack), so
7+
// all three stacked entries end up sharing the exact same ret_addr. When
8+
// the real allocator function finally does its own `ret`, only the
9+
// topmost of these aliased entries gets popped unless the return-matching
10+
// loop keeps consuming deeper equal-SP frames that independently match
11+
// the same target (bbcc.c's extend_popcount_through_aliases) -- otherwise
12+
// the two stale wrapper entries misattribute the NEXT jump as a fresh
13+
// call into whatever code runs after the allocation, instead of a plain
14+
// continuation of the real caller.
15+
//
16+
// A single-hop wrapper (see arm64_tail_call.c/arm64_free_during_recursion.c)
17+
// is not enough to exercise this: with only one emulated frame between the
18+
// real caller and the real external function, "descend until a match is
19+
// found" (the pre-existing loop) walks past the lone zero/aliased entry
20+
// and lands correctly on the real caller's own (non-aliased) entry by
21+
// coincidence. Three hops stacks enough aliased frames that under-counting
22+
// by even one leaves a stale entry behind.
23+
#include <callgrind.h>
24+
#include <stdlib.h>
25+
#include <string.h>
26+
27+
#define MAX_DEPTH 5
28+
#define MAX_NODES 256
29+
30+
typedef struct Node {
31+
int value;
32+
struct Node *left;
33+
struct Node *right;
34+
} Node;
35+
36+
static Node pool[MAX_NODES];
37+
static int used;
38+
39+
__attribute__((noinline)) static Node *pool_alloc(int value) {
40+
Node *node = &pool[used++];
41+
node->value = value;
42+
node->left = 0;
43+
node->right = 0;
44+
return node;
45+
}
46+
47+
__attribute__((noinline)) static int child_value(int parent, int side, int depth) {
48+
return parent * 3 + side + depth;
49+
}
50+
51+
// Three-hop tail-call chains into the real allocator, matching the real
52+
// Rust shim depth (`__rust_alloc -> __rdl_alloc -> malloc@plt`).
53+
__attribute__((noinline)) static void *alloc_hop3(size_t n) {
54+
return malloc(n);
55+
}
56+
__attribute__((noinline)) static void *alloc_hop2(size_t n) {
57+
return alloc_hop3(n);
58+
}
59+
__attribute__((noinline)) static void *alloc_hop1(size_t n) {
60+
return alloc_hop2(n);
61+
}
62+
63+
__attribute__((noinline)) static void dealloc_hop3(void *ptr) {
64+
free(ptr);
65+
}
66+
__attribute__((noinline)) static void dealloc_hop2(void *ptr) {
67+
dealloc_hop3(ptr);
68+
}
69+
__attribute__((noinline)) static void dealloc_hop1(void *ptr) {
70+
dealloc_hop2(ptr);
71+
}
72+
73+
__attribute__((noinline)) static void collect_leaf(const Node *node, int *buf, int *count) {
74+
if (!node) return;
75+
if (!node->left && !node->right) {
76+
buf[(*count)++] = node->value;
77+
return;
78+
}
79+
collect_leaf(node->left, buf, count);
80+
collect_leaf(node->right, buf, count);
81+
}
82+
83+
__attribute__((noinline)) static int compute_stat(const Node *root) {
84+
// Real call (`bl`) into the 3-hop alloc chain -- the frame that
85+
// eventually emulates the tail-called wrappers.
86+
int *buf = alloc_hop1(sizeof(int) * MAX_NODES);
87+
int count = 0;
88+
collect_leaf(root, buf, &count);
89+
90+
int sum = 0;
91+
for (int i = 0; i < count; i++) sum += buf[i];
92+
93+
// Real call (`bl`) into the 3-hop dealloc chain.
94+
dealloc_hop1(buf);
95+
96+
// Post-free work in this same frame -- exactly the code that gets
97+
// stolen and re-parented under the allocator if the aliased frames
98+
// above aren't all correctly popped.
99+
return sum % 1000;
100+
}
101+
102+
__attribute__((noinline)) static Node *build_tree(int depth, int seed) {
103+
Node *node = pool_alloc(seed);
104+
if (depth < MAX_DEPTH) {
105+
node->left = build_tree(depth + 1, child_value(seed, 1, depth));
106+
node->right = build_tree(depth + 1, child_value(seed, 2, depth));
107+
}
108+
return node;
109+
}
110+
111+
__attribute__((noinline)) static int complex_benchmark(void) {
112+
used = 0;
113+
Node *root = build_tree(0, 1);
114+
return compute_stat(root);
115+
}
116+
117+
__attribute__((noinline)) static int run_measured(void) {
118+
CALLGRIND_START_INSTRUMENTATION;
119+
CALLGRIND_ZERO_STATS;
120+
121+
int result = complex_benchmark();
122+
123+
CALLGRIND_STOP_INSTRUMENTATION;
124+
return result;
125+
}
126+
127+
__attribute__((noinline)) static int warmup(void) {
128+
volatile int acc = 0;
129+
for (int i = 0; i < 2; i++) {
130+
acc += complex_benchmark();
131+
}
132+
(void)acc;
133+
return run_measured();
134+
}
135+
136+
__attribute__((noinline)) static int run_benchmark(void) {
137+
return warmup();
138+
}
139+
140+
int main(void) {
141+
volatile int result = run_benchmark();
142+
(void)result;
143+
return 0;
144+
}

0 commit comments

Comments
 (0)