Skip to content

Commit c943d84

Browse files
krfrickebenfred
andauthored
Fix native stack merging for Python 3.12/3.13 (#751)
* use last_is_shim information for merging python frames Signed-off-by: Kai Fricke <coding@kaifricke.com> * fix order for shim detection Signed-off-by: Kai Fricke <coding@kaifricke.com> * remove version check Signed-off-by: Kai Fricke <coding@kaifricke.com> * fix unittest compile --------- Signed-off-by: Kai Fricke <coding@kaifricke.com> Co-authored-by: Ben Frederickson <github@benfrederickson.com>
1 parent 96ad57c commit c943d84

4 files changed

Lines changed: 31 additions & 3 deletions

File tree

src/main.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -285,6 +285,7 @@ fn record_samples(pid: remoteprocess::Pid, config: &Config) -> Result<(), Error>
285285
line: 0,
286286
locals: None,
287287
is_entry: true,
288+
is_shim_entry: true,
288289
});
289290
}
290291

src/native_stack_trace.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,11 +94,14 @@ impl NativeStack {
9494
// merge it into the stack. (if we're out of bounds a later
9595
// check will pick up - and report overall totals mismatch)
9696

97-
// Merge all python frames until we hit one with `is_entry`.
97+
// Merge all python frames until we hit one with `is_entry` (py 3.11)
98+
// or `is_entry_shim` (py 3.12+)
9899
while python_frame_index < frames.len() {
99100
merged.push(frames[python_frame_index].clone());
100101

101-
if frames[python_frame_index].is_entry {
102+
if frames[python_frame_index].is_entry
103+
|| frames[python_frame_index].is_shim_entry
104+
{
102105
break;
103106
}
104107

@@ -150,6 +153,7 @@ impl NativeStack {
150153
module: None,
151154
locals: None,
152155
is_entry: true,
156+
is_shim_entry: true,
153157
});
154158
});
155159

@@ -291,6 +295,7 @@ impl NativeStack {
291295
module: Some(frame.module.clone()),
292296
locals: None,
293297
is_entry: true,
298+
is_shim_entry: true,
294299
})
295300
}
296301
None => Some(Frame {
@@ -301,6 +306,7 @@ impl NativeStack {
301306
short_filename: None,
302307
module: Some(frame.module.clone()),
303308
is_entry: true,
309+
is_shim_entry: true,
304310
}),
305311
}
306312
}

src/speedscope.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,7 @@ mod tests {
289289
line: 0,
290290
locals: None,
291291
is_entry: true,
292+
is_shim_entry: false,
292293
};
293294

294295
let trace = stack_trace::StackTrace {

src/stack_trace.rs

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,10 @@ pub struct Frame {
4747
pub line: i32,
4848
/// Local Variables associated with the frame
4949
pub locals: Option<Vec<LocalVariable>>,
50-
/// If this is an entry frame. Each entry frame corresponds to one native frame.
50+
/// If this is an entry frame. Each entry frame corresponds to one native frame (Python 3.11)
5151
pub is_entry: bool,
52+
/// If the last frame was a shim. This is used in Python 3.12+ to detect entry frames.
53+
pub is_shim_entry: bool,
5254
}
5355

5456
#[derive(Debug, Hash, Eq, PartialEq, Ord, PartialOrd, Clone, Serialize)]
@@ -124,6 +126,16 @@ where
124126
}
125127

126128
let mut frame_ptr = thread.frame(frame_address);
129+
130+
// We are iterating in reverse, i.e. from last call to first call.
131+
// Since Python 3.12, there are shim frames inserted before a block
132+
// of Python frames. When we encounter one, update the last frame.
133+
let set_last_frame_as_shim_entry = &mut |frames: &mut Vec<Frame>| {
134+
if let Some(frame) = frames.last_mut() {
135+
frame.is_shim_entry = true;
136+
}
137+
};
138+
127139
while !frame_ptr.is_null() {
128140
let frame = process
129141
.copy_pointer(frame_ptr)
@@ -143,8 +155,10 @@ where
143155
// would also have to figure out what the address of PyCode_Type is (which will be
144156
// easier if something like https://github.com/python/cpython/issues/100987#issuecomment-1487227139
145157
// is merged )
158+
// Unset file/function name in py3.13 means this is a shim.
146159
if filename.is_err() || name.is_err() {
147160
frame_ptr = frame.back();
161+
set_last_frame_as_shim_entry(&mut frames);
148162
continue;
149163
}
150164
let filename = filename?;
@@ -153,6 +167,7 @@ where
153167
// skip <shim> entries in python 3.12+
154168
if filename == "<shim>" {
155169
frame_ptr = frame.back();
170+
set_last_frame_as_shim_entry(&mut frames);
156171
continue;
157172
}
158173

@@ -191,6 +206,7 @@ where
191206
module: None,
192207
locals,
193208
is_entry,
209+
is_shim_entry: false,
194210
});
195211
if frames.len() > 4096 {
196212
return Err(format_err!("Max frame recursion depth reached"));
@@ -199,6 +215,9 @@ where
199215
frame_ptr = frame.back();
200216
}
201217

218+
// First frame is always a shim
219+
set_last_frame_as_shim_entry(&mut frames);
220+
202221
Ok(StackTrace {
203222
pid: 0,
204223
frames,
@@ -322,6 +341,7 @@ impl ProcessInfo {
322341
line: 0,
323342
locals: None,
324343
is_entry: true,
344+
is_shim_entry: true,
325345
}
326346
}
327347
}

0 commit comments

Comments
 (0)