Skip to content

Commit ccf40fd

Browse files
youknowoneyouknowone
committed
Fix faulthandler
1 parent 0c0e8d4 commit ccf40fd

File tree

5 files changed

+121
-99
lines changed

5 files changed

+121
-99
lines changed

‎Lib/test/test_faulthandler.py‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -701,18 +701,15 @@ def func(timeout, repeat, cancel, file, loops):
701701
self.assertEqual(trace, '')
702702
self.assertEqual(exitcode, 0)
703703

704-
@unittest.expectedFailure # TODO: RUSTPYTHON; AssertionError: Regex didn't match: '^Timeout \\(0:00:00.500000\\)!\\nThread 0x[0-9a-f]+( \\[.*\\])? \\(most recent call first\\):\\n File "<string>", line 17 in func\n File "<string>", line 26 in <module>$' not found in 'Traceback (most recent call last):\n File "<string>", line 26, in <module>\n File "<string>", line 14, in func\nAttributeError: \'NoneType\' object has no attribute \'fileno\''
705704
def test_dump_traceback_later(self):
706705
self.check_dump_traceback_later()
707706

708-
@unittest.expectedFailure # TODO: RUSTPYTHON; AssertionError: Regex didn't match: '^Timeout \\(0:00:00.500000\\)!\\nThread 0x[0-9a-f]+( \\[.*\\])? \\(most recent call first\\):\\n File "<string>", line 17 in func\n File "<string>", line 26 in <module>\nTimeout \\(0:00:00.500000\\)!\\nThread 0x[0-9a-f]+( \\[.*\\])? \\(most recent call first\\):\\n File "<string>", line 17 in func\n File "<string>", line 26 in <module>' not found in 'Traceback (most recent call last):\n File "<string>", line 26, in <module>\n File "<string>", line 14, in func\nAttributeError: \'NoneType\' object has no attribute \'fileno\''
709707
def test_dump_traceback_later_repeat(self):
710708
self.check_dump_traceback_later(repeat=True)
711709

712710
def test_dump_traceback_later_cancel(self):
713711
self.check_dump_traceback_later(cancel=True)
714712

715-
@unittest.expectedFailure # TODO: RUSTPYTHON; AssertionError: Regex didn't match: '^Timeout \\(0:00:00.500000\\)!\\nThread 0x[0-9a-f]+( \\[.*\\])? \\(most recent call first\\):\\n File "<string>", line 17 in func\n File "<string>", line 26 in <module>$' not found in 'Timeout (00:00:00.500000)!\n<timeout: cannot dump traceback from watchdog thread>'
716713
def test_dump_traceback_later_file(self):
717714
with temporary_filename() as filename:
718715
self.check_dump_traceback_later(filename=filename)
@@ -723,7 +720,6 @@ def test_dump_traceback_later_fd(self):
723720
with tempfile.TemporaryFile('wb+') as fp:
724721
self.check_dump_traceback_later(fd=fp.fileno())
725722

726-
@unittest.expectedFailure # TODO: RUSTPYTHON; AssertionError: Regex didn't match: '^Timeout \\(0:00:00.500000\\)!\\nThread 0x[0-9a-f]+( \\[.*\\])? \\(most recent call first\\):\\n File "<string>", line 17 in func\n File "<string>", line 26 in <module>\nTimeout \\(0:00:00.500000\\)!\\nThread 0x[0-9a-f]+( \\[.*\\])? \\(most recent call first\\):\\n File "<string>", line 17 in func\n File "<string>", line 26 in <module>' not found in 'Traceback (most recent call last):\n File "<string>", line 26, in <module>\n File "<string>", line 14, in func\nAttributeError: \'NoneType\' object has no attribute \'fileno\''
727723
@support.requires_resource('walltime')
728724
def test_dump_traceback_later_twice(self):
729725
self.check_dump_traceback_later(loops=2)

‎crates/codegen/src/compile.rs‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4624,7 +4624,7 @@ impl Compiler {
46244624
self.emit_load_const(ConstantData::Str { value: name.into() });
46254625

46264626
if let Some(arguments) = arguments {
4627-
self.codegen_call_helper(2, arguments)?;
4627+
self.codegen_call_helper(2, arguments, self.current_source_range)?;
46284628
} else {
46294629
emit!(self, Instruction::Call { nargs: 2 });
46304630
}
@@ -7065,6 +7065,10 @@ impl Compiler {
70657065
}
70667066

70677067
fn compile_call(&mut self, func: &ast::Expr, args: &ast::Arguments) -> CompileResult<()> {
7068+
// Save the call expression's source range so CALL instructions use the
7069+
// call start line, not the last argument's line.
7070+
let call_range = self.current_source_range;
7071+
70687072
// Method call: obj → LOAD_ATTR_METHOD → [method, self_or_null] → args → CALL
70697073
// Regular call: func → PUSH_NULL → args → CALL
70707074
if let ast::Expr::Attribute(ast::ExprAttribute { value, attr, .. }) = &func {
@@ -7082,21 +7086,21 @@ impl Compiler {
70827086
self.emit_load_zero_super_method(idx);
70837087
}
70847088
}
7085-
self.codegen_call_helper(0, args)?;
7089+
self.codegen_call_helper(0, args, call_range)?;
70867090
} else {
70877091
// Normal method call: compile object, then LOAD_ATTR with method flag
70887092
// LOAD_ATTR(method=1) pushes [method, self_or_null] on stack
70897093
self.compile_expression(value)?;
70907094
let idx = self.name(attr.as_str());
70917095
self.emit_load_attr_method(idx);
7092-
self.codegen_call_helper(0, args)?;
7096+
self.codegen_call_helper(0, args, call_range)?;
70937097
}
70947098
} else {
70957099
// Regular call: push func, then NULL for self_or_null slot
70967100
// Stack layout: [func, NULL, args...] - same as method call [func, self, args...]
70977101
self.compile_expression(func)?;
70987102
emit!(self, Instruction::PushNull);
7099-
self.codegen_call_helper(0, args)?;
7103+
self.codegen_call_helper(0, args, call_range)?;
71007104
}
71017105
Ok(())
71027106
}
@@ -7138,10 +7142,13 @@ impl Compiler {
71387142
}
71397143

71407144
/// Compile call arguments and emit the appropriate CALL instruction.
7145+
/// `call_range` is the source range of the call expression, used to set
7146+
/// the correct line number on the CALL instruction.
71417147
fn codegen_call_helper(
71427148
&mut self,
71437149
additional_positional: u32,
71447150
arguments: &ast::Arguments,
7151+
call_range: TextRange,
71457152
) -> CompileResult<()> {
71467153
let nelts = arguments.args.len();
71477154
let nkwelts = arguments.keywords.len();
@@ -7172,13 +7179,16 @@ impl Compiler {
71727179
self.compile_expression(&keyword.value)?;
71737180
}
71747181

7182+
// Restore call expression range for kwnames and CALL_KW
7183+
self.set_source_range(call_range);
71757184
self.emit_load_const(ConstantData::Tuple {
71767185
elements: kwarg_names,
71777186
});
71787187

71797188
let nargs = additional_positional + nelts.to_u32() + nkwelts.to_u32();
71807189
emit!(self, Instruction::CallKw { nargs });
71817190
} else {
7191+
self.set_source_range(call_range);
71827192
let nargs = additional_positional + nelts.to_u32();
71837193
emit!(self, Instruction::Call { nargs });
71847194
}
@@ -7270,6 +7280,7 @@ impl Compiler {
72707280
emit!(self, Instruction::PushNull);
72717281
}
72727282

7283+
self.set_source_range(call_range);
72737284
emit!(self, Instruction::CallFunctionEx);
72747285
}
72757286

‎crates/stdlib/src/faulthandler.rs‎

Lines changed: 78 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,7 @@ mod decl {
66
use crate::vm::{
77
PyObjectRef, PyResult, VirtualMachine,
88
frame::Frame,
9-
frame_snapshot::{
10-
self, FRAME_SNAPSHOTS, MAX_SNAPSHOT_FRAMES, SNAPSHOT_COUNT, SNAPSHOT_ENABLED,
11-
},
9+
frame_snapshot::{self, FRAME_SNAPSHOTS, SNAPSHOT_COUNT, SNAPSHOT_ENABLED},
1210
function::{ArgIntoFloat, OptionalArg},
1311
py_io::Write,
1412
};
@@ -283,8 +281,6 @@ mod decl {
283281
}
284282

285283
/// Snapshot current Python frames into signal-safe global storage.
286-
/// Used by _sigsegv, _read_null, etc. before triggering a fault.
287-
#[cfg(any(unix, windows))]
288284
fn snapshot_current_frames(vm: &VirtualMachine) {
289285
frame_snapshot::do_update_frame_snapshots(&vm.frames.borrow());
290286
}
@@ -301,9 +297,13 @@ mod decl {
301297

302298
fn collect_frame_info(frame: &crate::vm::PyRef<Frame>) -> String {
303299
let func_name = truncate_name(frame.code.obj_name.as_str());
304-
// If lasti is 0, execution hasn't started yet - use first line number or 1
305300
let line = if frame.lasti() == 0 {
306-
frame.code.first_line_number.map(|n| n.get()).unwrap_or(1)
301+
// Frame hasn't started executing yet. Use first instruction's line.
302+
if !frame.code.locations.is_empty() {
303+
frame.code.locations[0].0.line.get()
304+
} else {
305+
frame.code.first_line_number.map(|n| n.get()).unwrap_or(1)
306+
}
307307
} else {
308308
frame.current_location().line.get()
309309
};
@@ -334,35 +334,35 @@ mod decl {
334334
_ => None,
335335
};
336336

337-
if let Some(ref f) = file_obj {
338-
if let Ok(fd) = f.try_to_value::<i32>(vm) {
339-
// Write directly to file descriptor using libc::write
340-
#[cfg(not(target_arch = "wasm32"))]
341-
{
342-
let mut output = String::new();
343-
output.push_str("Stack (most recent call first):\n");
344-
for line in frame_lines.iter().rev() {
345-
output.push_str(line);
346-
output.push('\n');
337+
if let Some(ref f) = file_obj
338+
&& let Ok(fd) = f.try_to_value::<i32>(vm)
339+
{
340+
// Write directly to file descriptor using libc::write
341+
#[cfg(not(target_arch = "wasm32"))]
342+
{
343+
let mut output = String::new();
344+
output.push_str("Stack (most recent call first):\n");
345+
for line in frame_lines.iter().rev() {
346+
output.push_str(line);
347+
output.push('\n');
348+
}
349+
let bytes = output.as_bytes();
350+
unsafe {
351+
#[cfg(windows)]
352+
{
353+
libc::write(
354+
fd,
355+
bytes.as_ptr() as *const libc::c_void,
356+
bytes.len() as u32,
357+
);
347358
}
348-
let bytes = output.as_bytes();
349-
unsafe {
350-
#[cfg(windows)]
351-
{
352-
libc::write(
353-
fd,
354-
bytes.as_ptr() as *const libc::c_void,
355-
bytes.len() as u32,
356-
);
357-
}
358-
#[cfg(not(windows))]
359-
{
360-
libc::write(fd, bytes.as_ptr() as *const libc::c_void, bytes.len());
361-
}
359+
#[cfg(not(windows))]
360+
{
361+
libc::write(fd, bytes.as_ptr() as *const libc::c_void, bytes.len());
362362
}
363363
}
364-
return Ok(());
365364
}
365+
return Ok(());
366366
}
367367

368368
// Use PyWriter for file objects (or stderr)
@@ -406,8 +406,9 @@ mod decl {
406406
.all_threads
407407
.store(args.all_threads, Ordering::Relaxed);
408408

409-
// Enable frame snapshot updates
409+
// Enable frame snapshot updates and immediately snapshot existing frames
410410
SNAPSHOT_ENABLED.store(true, Ordering::Relaxed);
411+
snapshot_current_frames(vm);
411412

412413
// Install signal handlers
413414
if !faulthandler_enable_internal() {
@@ -765,17 +766,6 @@ mod decl {
765766
fn get_fd_from_file_opt(file: OptionalArg<PyObjectRef>, vm: &VirtualMachine) -> PyResult<i32> {
766767
match file {
767768
OptionalArg::Present(f) if !vm.is_none(&f) => {
768-
// Handle None - treat as missing (use stderr)
769-
if vm.is_none(&f) {
770-
let stderr = vm.sys_module.get_attr("stderr", vm)?;
771-
if vm.is_none(&stderr) {
772-
return Err(vm.new_runtime_error("sys.stderr is None".to_owned()));
773-
}
774-
let fileno = vm.call_method(&stderr, "fileno", ())?;
775-
let fd: i32 = fileno.try_to_value(vm)?;
776-
let _ = vm.call_method(&stderr, "flush", ());
777-
return Ok(fd);
778-
}
779769
// Check if it's an integer (file descriptor)
780770
if let Ok(fd) = f.try_to_value::<i32>(vm) {
781771
if fd < 0 {
@@ -889,6 +879,7 @@ mod decl {
889879

890880
// Enable frame snapshot updates so watchdog can read them
891881
SNAPSHOT_ENABLED.store(true, Ordering::Relaxed);
882+
snapshot_current_frames(vm);
892883

893884
// Cancel any previous watchdog
894885
cancel_dump_traceback_later();
@@ -943,13 +934,13 @@ mod decl {
943934

944935
const NSIG: usize = 64;
945936

946-
#[derive(Clone)]
937+
#[derive(Clone, Copy)]
947938
pub struct UserSignal {
948939
pub enabled: bool,
949940
pub fd: i32,
950941
pub all_threads: bool,
951942
pub chain: bool,
952-
pub previous: libc::sighandler_t,
943+
pub previous: libc::sigaction,
953944
}
954945

955946
impl Default for UserSignal {
@@ -959,7 +950,8 @@ mod decl {
959950
fd: 2, // stderr
960951
all_threads: true,
961952
chain: false,
962-
previous: libc::SIG_DFL,
953+
// SAFETY: sigaction is a C struct that can be zero-initialized
954+
previous: unsafe { core::mem::zeroed() },
963955
}
964956
}
965957
}
@@ -989,7 +981,7 @@ mod decl {
989981
&& signum < v.len()
990982
&& v[signum].enabled
991983
{
992-
let old = v[signum].clone();
984+
let old = v[signum];
993985
v[signum] = UserSignal::default();
994986
return Some(old);
995987
}
@@ -1023,16 +1015,20 @@ mod decl {
10231015
dump_snapshot_frames(user.fd);
10241016

10251017
// If chain is enabled, call the previous handler
1026-
if user.chain && user.previous != libc::SIG_DFL && user.previous != libc::SIG_IGN {
1027-
// Re-register the old handler and raise the signal
1028-
unsafe {
1029-
libc::signal(signum, user.previous);
1030-
libc::raise(signum);
1031-
// Re-register our handler
1032-
libc::signal(
1033-
signum,
1034-
faulthandler_user_signal as *const () as libc::sighandler_t,
1035-
);
1018+
if user.chain {
1019+
let prev_handler = user.previous.sa_sigaction;
1020+
if prev_handler != libc::SIG_DFL && prev_handler != libc::SIG_IGN {
1021+
// Restore previous handler, re-raise, then re-install ours
1022+
unsafe {
1023+
libc::sigaction(signum, &user.previous, core::ptr::null_mut());
1024+
libc::raise(signum);
1025+
// Re-install our handler
1026+
let mut action: libc::sigaction = core::mem::zeroed();
1027+
action.sa_sigaction =
1028+
faulthandler_user_signal as *const () as libc::sighandler_t;
1029+
action.sa_flags = libc::SA_NODEFER;
1030+
libc::sigaction(signum, &action, core::ptr::null_mut());
1031+
}
10361032
}
10371033
}
10381034
}
@@ -1079,30 +1075,32 @@ mod decl {
10791075

10801076
let signum = args.signum as usize;
10811077

1082-
// Enable frame snapshot updates
1078+
// Enable frame snapshot updates and immediately snapshot existing frames
10831079
SNAPSHOT_ENABLED.store(true, Ordering::Relaxed);
1080+
snapshot_current_frames(vm);
10841081

10851082
// Get current handler to save as previous
10861083
let previous = if !user_signals::is_enabled(signum) {
1087-
// Install signal handler
1088-
let prev = unsafe {
1089-
libc::signal(
1090-
args.signum,
1091-
faulthandler_user_signal as *const () as libc::sighandler_t,
1092-
)
1093-
};
1094-
if prev == libc::SIG_ERR {
1095-
return Err(vm.new_os_error(format!(
1096-
"Failed to register signal handler for signal {}",
1097-
args.signum
1098-
)));
1084+
// Install signal handler using sigaction with SA_NODEFER
1085+
unsafe {
1086+
let mut action: libc::sigaction = core::mem::zeroed();
1087+
action.sa_sigaction = faulthandler_user_signal as *const () as libc::sighandler_t;
1088+
action.sa_flags = libc::SA_NODEFER;
1089+
1090+
let mut prev: libc::sigaction = core::mem::zeroed();
1091+
if libc::sigaction(args.signum, &action, &mut prev) != 0 {
1092+
return Err(vm.new_os_error(format!(
1093+
"Failed to register signal handler for signal {}",
1094+
args.signum
1095+
)));
1096+
}
1097+
prev
10991098
}
1100-
prev
11011099
} else {
11021100
// Already registered, keep previous handler
11031101
user_signals::get_user_signal(signum)
11041102
.map(|u| u.previous)
1105-
.unwrap_or(libc::SIG_DFL)
1103+
.unwrap_or(unsafe { core::mem::zeroed() })
11061104
};
11071105

11081106
user_signals::set_user_signal(
@@ -1127,7 +1125,7 @@ mod decl {
11271125
if let Some(old) = user_signals::clear_user_signal(signum as usize) {
11281126
// Restore previous handler
11291127
unsafe {
1130-
libc::signal(signum, old.previous);
1128+
libc::sigaction(signum, &old.previous, core::ptr::null_mut());
11311129
}
11321130
Ok(true)
11331131
} else {
@@ -1164,19 +1162,13 @@ mod decl {
11641162
suppress_crash_report();
11651163
snapshot_current_frames(vm);
11661164

1167-
// On Windows, raise SIGSEGV in a loop: once for our handler,
1168-
// once more for the previous handler (faulthandler.c:1094-1107)
1169-
#[cfg(windows)]
1170-
{
1171-
loop {
1172-
unsafe {
1173-
libc::raise(libc::SIGSEGV);
1174-
}
1175-
}
1176-
}
1177-
#[cfg(not(windows))]
1165+
// Write to NULL pointer to trigger a real hardware SIGSEGV,
1166+
// matching CPython's *((volatile int *)NULL) = 0;
1167+
// Using raise(SIGSEGV) doesn't work reliably because Rust's runtime
1168+
// installs its own signal handler that may swallow software signals.
11781169
unsafe {
1179-
libc::raise(libc::SIGSEGV);
1170+
let ptr: *mut i32 = core::ptr::null_mut();
1171+
core::ptr::write_volatile(ptr, 0);
11801172
}
11811173
}
11821174
}

0 commit comments

Comments
 (0)