Skip to content

Commit 3633628

Browse files
youknowoneyouknowone
committed
Fix faulthandler
1 parent b95fbd7 commit 3633628

File tree

5 files changed

+121
-97
lines changed

5 files changed

+121
-97
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 & 84 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
};
@@ -301,9 +299,13 @@ mod decl {
301299

302300
fn collect_frame_info(frame: &crate::vm::PyRef<Frame>) -> String {
303301
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
305302
let line = if frame.lasti() == 0 {
306-
frame.code.first_line_number.map(|n| n.get()).unwrap_or(1)
303+
// Frame hasn't started executing yet. Use first instruction's line.
304+
if !frame.code.locations.is_empty() {
305+
frame.code.locations[0].0.line.get()
306+
} else {
307+
frame.code.first_line_number.map(|n| n.get()).unwrap_or(1)
308+
}
307309
} else {
308310
frame.current_location().line.get()
309311
};
@@ -334,35 +336,35 @@ mod decl {
334336
_ => None,
335337
};
336338

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

368370
// Use PyWriter for file objects (or stderr)
@@ -406,8 +408,9 @@ mod decl {
406408
.all_threads
407409
.store(args.all_threads, Ordering::Relaxed);
408410

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

412415
// Install signal handlers
413416
if !faulthandler_enable_internal() {
@@ -765,17 +768,6 @@ mod decl {
765768
fn get_fd_from_file_opt(file: OptionalArg<PyObjectRef>, vm: &VirtualMachine) -> PyResult<i32> {
766769
match file {
767770
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-
}
779771
// Check if it's an integer (file descriptor)
780772
if let Ok(fd) = f.try_to_value::<i32>(vm) {
781773
if fd < 0 {
@@ -889,6 +881,7 @@ mod decl {
889881

890882
// Enable frame snapshot updates so watchdog can read them
891883
SNAPSHOT_ENABLED.store(true, Ordering::Relaxed);
884+
snapshot_current_frames(vm);
892885

893886
// Cancel any previous watchdog
894887
cancel_dump_traceback_later();
@@ -943,13 +936,13 @@ mod decl {
943936

944937
const NSIG: usize = 64;
945938

946-
#[derive(Clone)]
939+
#[derive(Clone, Copy)]
947940
pub struct UserSignal {
948941
pub enabled: bool,
949942
pub fd: i32,
950943
pub all_threads: bool,
951944
pub chain: bool,
952-
pub previous: libc::sighandler_t,
945+
pub previous: libc::sigaction,
953946
}
954947

955948
impl Default for UserSignal {
@@ -959,7 +952,8 @@ mod decl {
959952
fd: 2, // stderr
960953
all_threads: true,
961954
chain: false,
962-
previous: libc::SIG_DFL,
955+
// SAFETY: sigaction is a C struct that can be zero-initialized
956+
previous: unsafe { core::mem::zeroed() },
963957
}
964958
}
965959
}
@@ -989,7 +983,7 @@ mod decl {
989983
&& signum < v.len()
990984
&& v[signum].enabled
991985
{
992-
let old = v[signum].clone();
986+
let old = v[signum];
993987
v[signum] = UserSignal::default();
994988
return Some(old);
995989
}
@@ -1023,16 +1017,20 @@ mod decl {
10231017
dump_snapshot_frames(user.fd);
10241018

10251019
// 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-
);
1020+
if user.chain {
1021+
let prev_handler = user.previous.sa_sigaction;
1022+
if prev_handler != libc::SIG_DFL && prev_handler != libc::SIG_IGN {
1023+
// Restore previous handler, re-raise, then re-install ours
1024+
unsafe {
1025+
libc::sigaction(signum, &user.previous, core::ptr::null_mut());
1026+
libc::raise(signum);
1027+
// Re-install our handler
1028+
let mut action: libc::sigaction = core::mem::zeroed();
1029+
action.sa_sigaction =
1030+
faulthandler_user_signal as *const () as libc::sighandler_t;
1031+
action.sa_flags = libc::SA_NODEFER;
1032+
libc::sigaction(signum, &action, core::ptr::null_mut());
1033+
}
10361034
}
10371035
}
10381036
}
@@ -1079,30 +1077,32 @@ mod decl {
10791077

10801078
let signum = args.signum as usize;
10811079

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

10851084
// Get current handler to save as previous
10861085
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-
)));
1086+
// Install signal handler using sigaction with SA_NODEFER
1087+
unsafe {
1088+
let mut action: libc::sigaction = core::mem::zeroed();
1089+
action.sa_sigaction = faulthandler_user_signal as *const () as libc::sighandler_t;
1090+
action.sa_flags = libc::SA_NODEFER;
1091+
1092+
let mut prev: libc::sigaction = core::mem::zeroed();
1093+
if libc::sigaction(args.signum, &action, &mut prev) != 0 {
1094+
return Err(vm.new_os_error(format!(
1095+
"Failed to register signal handler for signal {}",
1096+
args.signum
1097+
)));
1098+
}
1099+
prev
10991100
}
1100-
prev
11011101
} else {
11021102
// Already registered, keep previous handler
11031103
user_signals::get_user_signal(signum)
11041104
.map(|u| u.previous)
1105-
.unwrap_or(libc::SIG_DFL)
1105+
.unwrap_or(unsafe { core::mem::zeroed() })
11061106
};
11071107

11081108
user_signals::set_user_signal(
@@ -1127,7 +1127,7 @@ mod decl {
11271127
if let Some(old) = user_signals::clear_user_signal(signum as usize) {
11281128
// Restore previous handler
11291129
unsafe {
1130-
libc::signal(signum, old.previous);
1130+
libc::sigaction(signum, &old.previous, core::ptr::null_mut());
11311131
}
11321132
Ok(true)
11331133
} else {
@@ -1164,19 +1164,13 @@ mod decl {
11641164
suppress_crash_report();
11651165
snapshot_current_frames(vm);
11661166

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))]
1167+
// Write to NULL pointer to trigger a real hardware SIGSEGV,
1168+
// matching CPython's *((volatile int *)NULL) = 0;
1169+
// Using raise(SIGSEGV) doesn't work reliably because Rust's runtime
1170+
// installs its own signal handler that may swallow software signals.
11781171
unsafe {
1179-
libc::raise(libc::SIGSEGV);
1172+
let ptr: *mut i32 = core::ptr::null_mut();
1173+
core::ptr::write_volatile(ptr, 0);
11801174
}
11811175
}
11821176
}

‎crates/vm/src/frame.rs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -398,6 +398,12 @@ impl ExecutingFrame<'_> {
398398
let mut arg_state = bytecode::OpArgState::default();
399399
loop {
400400
let idx = self.lasti() as usize;
401+
// Update frame snapshot line number for signal handlers (faulthandler)
402+
if idx < self.code.locations.len() {
403+
crate::frame_snapshot::update_current_lineno(
404+
self.code.locations[idx].0.line.get() as u32
405+
);
406+
}
401407
self.update_lasti(|i| *i += 1);
402408
let bytecode::CodeUnit { op, arg } = instructions[idx];
403409
let arg = arg_state.extend(arg);

0 commit comments

Comments
 (0)