Skip to content

Print thread ID in panic message #115746

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion library/std/src/panicking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,7 @@ fn default_hook(info: &PanicHookInfo<'_>) {

thread::with_current_name(|name| {
let name = name.unwrap_or("<unnamed>");
let tid = thread::current_os_id();

// Try to write the panic message to a buffer first to prevent other concurrent outputs
// interleaving with it.
Expand All @@ -277,7 +278,7 @@ fn default_hook(info: &PanicHookInfo<'_>) {

let write_msg = |dst: &mut dyn crate::io::Write| {
// We add a newline to ensure the panic message appears at the start of a line.
writeln!(dst, "\nthread '{name}' panicked at {location}:\n{msg}")
writeln!(dst, "\nthread '{name}' ({tid}) panicked at {location}:\n{msg}")
};

if write_msg(&mut cursor).is_ok() {
Expand Down
4 changes: 4 additions & 0 deletions library/std/src/sys/pal/hermit/thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,10 @@ impl Thread {
}
}

pub(crate) fn current_os_id() -> Option<u64> {
None
}

pub fn available_parallelism() -> io::Result<NonZero<usize>> {
unsafe { Ok(NonZero::new_unchecked(hermit_abi::available_parallelism())) }
}
4 changes: 4 additions & 0 deletions library/std/src/sys/pal/itron/thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,10 @@ unsafe fn terminate_and_delete_current_task() -> ! {
unsafe { crate::hint::unreachable_unchecked() };
}

pub(crate) fn current_os_id() -> Option<u64> {
None
}

pub fn available_parallelism() -> io::Result<NonZero<usize>> {
super::unsupported()
}
6 changes: 5 additions & 1 deletion library/std/src/sys/pal/sgx/thread.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#![cfg_attr(test, allow(dead_code))] // why is this necessary?

use super::abi::usercalls;
use super::abi::{thread, usercalls};
use super::unsupported;
use crate::ffi::CStr;
use crate::io;
Expand Down Expand Up @@ -137,6 +137,10 @@ impl Thread {
}
}

pub(crate) fn current_os_id() -> Option<u64> {
Some(thread::current().addr().get() as u64)
}

pub fn available_parallelism() -> io::Result<NonZero<usize>> {
unsupported()
}
4 changes: 4 additions & 0 deletions library/std/src/sys/pal/teeos/thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,10 @@ impl Drop for Thread {
}
}

pub(crate) fn current_os_id() -> Option<u64> {
None
}

// Note: Both `sched_getaffinity` and `sysconf` are available but not functional on
// teeos, so this function always returns an Error!
pub fn available_parallelism() -> io::Result<NonZero<usize>> {
Expand Down
4 changes: 4 additions & 0 deletions library/std/src/sys/pal/uefi/thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@ impl Thread {
}
}

pub(crate) fn current_os_id() -> Option<u64> {
None
}

pub fn available_parallelism() -> io::Result<NonZero<usize>> {
// UEFI is single threaded
Ok(NonZero::new(1).unwrap())
Expand Down
6 changes: 4 additions & 2 deletions library/std/src/sys/pal/unix/stack_overflow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,8 @@ mod imp {
&& thread_info.guard_page_range.contains(&fault_addr)
{
let name = thread_info.thread_name.as_deref().unwrap_or("<unknown>");
rtprintpanic!("\nthread '{name}' has overflowed its stack\n");
let tid = crate::thread::current_os_id();
rtprintpanic!("\nthread '{name}' ({tid}) has overflowed its stack\n");
rtabort!("stack overflow");
}
})
Expand Down Expand Up @@ -696,7 +697,8 @@ mod imp {
if code == c::EXCEPTION_STACK_OVERFLOW {
crate::thread::with_current_name(|name| {
let name = name.unwrap_or("<unknown>");
rtprintpanic!("\nthread '{name}' has overflowed its stack\n");
let tid = crate::thread::current_os_id();
rtprintpanic!("\nthread '{name}' ({tid}) has overflowed its stack\n");
});
}
c::EXCEPTION_CONTINUE_SEARCH
Expand Down
56 changes: 56 additions & 0 deletions library/std/src/sys/pal/unix/thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,62 @@ impl Drop for Thread {
}
}

pub(crate) fn current_os_id() -> Option<u64> {
// Most Unix platforms have a way to query an integer ID of the current thread, all with
// slightly different spellings.
//
// The OS thread ID is used rather than `pthread_self` so as to match what will be displayed
// for process inspection (debuggers, trace, `top`, etc.).
cfg_if::cfg_if! {
// Most platforms have a function returning a `pid_t` or int, which is an `i32`.
if #[cfg(any(target_os = "android", target_os = "linux"))] {
use crate::sys::weak::syscall;

// `libc::gettid` is only available on glibc 2.30+, but the syscall is available
// since Linux 2.4.11.
syscall!(fn gettid() -> libc::pid_t;);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated, can we modify the macro in another PR to accept "safe extern" functions?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possibly, but there should still be an unsafe somewhere to promise that the FFI signature is correct.

We don't even really need "safe extern" though, because it's wrapped anyway -- e.g. we could just declare this safe if we wanted:

unsafe fn $name($($param: $t),*) -> $ret {
weak!(fn $name($($param: $t),*) -> $ret;);
// Use a weak symbol from libc when possible, allowing `LD_PRELOAD`
// interposition, but if it's not found just use a raw syscall.
if let Some(fun) = $name.get() {
unsafe { fun($($param),*) }
} else {
unsafe { libc::syscall(libc::${concat(SYS_, $name)}, $($param),*) as $ret }
}
}


// SAFETY: FFI call with no preconditions.
let id: libc::pid_t = unsafe { gettid() };
Some(id as u64)
} else if #[cfg(target_os = "nto")] {
// SAFETY: FFI call with no preconditions.
let id: libc::pid_t = unsafe { libc::gettid() };
Some(id as u64)
} else if #[cfg(target_os = "openbsd")] {
// SAFETY: FFI call with no preconditions.
let id: libc::pid_t = unsafe { libc::getthrid() };
Some(id as u64)
} else if #[cfg(target_os = "freebsd")] {
// SAFETY: FFI call with no preconditions.
let id: libc::c_int = unsafe { libc::pthread_getthreadid_np() };
Some(id as u64)
} else if #[cfg(target_os = "netbsd")] {
// SAFETY: FFI call with no preconditions.
let id: libc::lwpid_t = unsafe { libc::_lwp_self() };
Some(id as u64)
} else if #[cfg(any(target_os = "illumos", target_os = "solaris"))] {
// On Illumos and Solaris, the `pthread_t` is the same as the OS thread ID.
// SAFETY: FFI call with no preconditions.
let id: libc::pthread_t = unsafe { libc::pthread_self() };
Some(id as u64)
} else if #[cfg(target_vendor = "apple")] {
// Apple allows querying arbitrary thread IDs, `thread=NULL` queries the current thread.
let mut id = 0u64;
// SAFETY: `thread_id` is a valid pointer, no other preconditions.
let status: libc::c_int = unsafe { libc::pthread_threadid_np(0, &mut id) };
if status == 0 {
Some(id)
} else {
None
}
} else {
// Other platforms don't have an OS thread ID or don't have a way to access it.
None
}
}
}

#[cfg(any(
target_os = "linux",
target_os = "nto",
Expand Down
4 changes: 4 additions & 0 deletions library/std/src/sys/pal/unsupported/thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ impl Thread {
}
}

pub(crate) fn current_os_id() -> Option<u64> {
None
}

pub fn available_parallelism() -> io::Result<NonZero<usize>> {
unsupported()
}
4 changes: 4 additions & 0 deletions library/std/src/sys/pal/wasi/thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,10 @@ impl Thread {
}
}

pub(crate) fn current_os_id() -> Option<u64> {
None
}

pub fn available_parallelism() -> io::Result<NonZero<usize>> {
cfg_if::cfg_if! {
if #[cfg(target_feature = "atomics")] {
Expand Down
1 change: 1 addition & 0 deletions library/std/src/sys/pal/windows/c/bindings.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2185,6 +2185,7 @@ GetSystemInfo
GetSystemTimeAsFileTime
GetSystemTimePreciseAsFileTime
GetTempPathW
GetThreadId
GetUserProfileDirectoryW
GetWindowsDirectoryW
HANDLE
Expand Down
1 change: 1 addition & 0 deletions library/std/src/sys/pal/windows/c/windows_sys.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ windows_targets::link!("kernel32.dll" "system" fn GetSystemInfo(lpsysteminfo : *
windows_targets::link!("kernel32.dll" "system" fn GetSystemTimeAsFileTime(lpsystemtimeasfiletime : *mut FILETIME));
windows_targets::link!("kernel32.dll" "system" fn GetSystemTimePreciseAsFileTime(lpsystemtimeasfiletime : *mut FILETIME));
windows_targets::link!("kernel32.dll" "system" fn GetTempPathW(nbufferlength : u32, lpbuffer : PWSTR) -> u32);
windows_targets::link!("kernel32.dll" "system" fn GetThreadId(thread : HANDLE) -> u32);
windows_targets::link!("userenv.dll" "system" fn GetUserProfileDirectoryW(htoken : HANDLE, lpprofiledir : PWSTR, lpcchsize : *mut u32) -> BOOL);
windows_targets::link!("kernel32.dll" "system" fn GetWindowsDirectoryW(lpbuffer : PWSTR, usize : u32) -> u32);
windows_targets::link!("kernel32.dll" "system" fn InitOnceBeginInitialize(lpinitonce : *mut INIT_ONCE, dwflags : u32, fpending : *mut BOOL, lpcontext : *mut *mut core::ffi::c_void) -> BOOL);
Expand Down
3 changes: 2 additions & 1 deletion library/std/src/sys/pal/windows/stack_overflow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ unsafe extern "system" fn vectored_handler(ExceptionInfo: *mut c::EXCEPTION_POIN
if code == c::EXCEPTION_STACK_OVERFLOW {
thread::with_current_name(|name| {
let name = name.unwrap_or("<unknown>");
rtprintpanic!("\nthread '{name}' has overflowed its stack\n");
let tid = thread::current_os_id();
rtprintpanic!("\nthread '{name}' ({tid}) has overflowed its stack\n");
});
}
c::EXCEPTION_CONTINUE_SEARCH
Expand Down
8 changes: 8 additions & 0 deletions library/std/src/sys/pal/windows/thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,14 @@ impl Thread {
}
}

pub(crate) fn current_os_id() -> Option<u64> {
// SAFETY: FFI call with no preconditions.
let id: u32 = unsafe { c::GetThreadId(c::GetCurrentThread()) };
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am going to change this to GetCurrentThreadId that does both steps, but not until the Miri shims merge so I can stop manually syncing changes.


// A return value of 0 indicates failed lookup.
if id == 0 { None } else { Some(id.into()) }
}

pub fn available_parallelism() -> io::Result<NonZero<usize>> {
let res = unsafe {
let mut sysinfo: c::SYSTEM_INFO = crate::mem::zeroed();
Expand Down
4 changes: 4 additions & 0 deletions library/std/src/sys/pal/xous/thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,10 @@ impl Thread {
}
}

pub(crate) fn current_os_id() -> Option<u64> {
None
}

pub fn available_parallelism() -> io::Result<NonZero<usize>> {
// We're unicore right now.
Ok(unsafe { NonZero::new_unchecked(1) })
Expand Down
13 changes: 12 additions & 1 deletion library/std/src/thread/current.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use super::{Thread, ThreadId};
use super::{Thread, ThreadId, imp};
use crate::mem::ManuallyDrop;
use crate::ptr;
use crate::sys::thread_local::local_pointer;
Expand Down Expand Up @@ -148,6 +148,17 @@ pub(crate) fn current_id() -> ThreadId {
id::get_or_init()
}

/// Gets the OS thread ID of the thread that invokes it, if available. If not, return the Rust
/// thread ID.
///
/// We use a `u64` to all possible platform IDs without excess `cfg`; most use `int`, some use a
/// pointer, and Apple uses `uint64_t`. This is a "best effort" approach for diagnostics and is
/// allowed to fall back to a non-OS ID (such as the Rust thread ID) or a non-unique ID (such as a
/// PID) if the thread ID cannot be retrieved.
pub(crate) fn current_os_id() -> u64 {
imp::current_os_id().unwrap_or_else(|| current_id().as_u64().get())
}

/// Gets a reference to the handle of the thread that invokes it, if the handle
/// has been initialized.
pub(super) fn try_with_current<F, R>(f: F) -> R
Expand Down
2 changes: 1 addition & 1 deletion library/std/src/thread/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ mod current;

#[stable(feature = "rust1", since = "1.0.0")]
pub use current::current;
pub(crate) use current::{current_id, current_or_unnamed, drop_current};
pub(crate) use current::{current_id, current_or_unnamed, current_os_id, drop_current};
use current::{set_current, try_with_current};

mod spawnhook;
Expand Down
7 changes: 7 additions & 0 deletions library/std/src/thread/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,13 @@ fn test_thread_id_not_equal() {
assert!(thread::current().id() != spawned_id);
}

#[test]
fn test_thread_os_id_not_equal() {
let spawned_id = thread::spawn(|| thread::current_os_id()).join().unwrap();
let current_id = thread::current_os_id();
assert!(current_id != spawned_id);
}

#[test]
fn test_scoped_threads_drop_result_before_join() {
let actually_finished = &AtomicBool::new(false);
Expand Down
5 changes: 5 additions & 0 deletions src/tools/compiletest/src/runtest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2524,6 +2524,11 @@ impl<'test> TestCx<'test> {
})
.into_owned();

// Normalize thread IDs in panic messages
normalized = static_regex!(r"thread '(?P<name>.*?)' \((rtid )?\d+\) panicked")
.replace_all(&normalized, "thread '$name' ($$TID) panicked")
.into_owned();

normalized = normalized.replace("\t", "\\t"); // makes tabs visible

// Remove test annotations like `//~ ERROR text` from the output,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@

thread 'main' panicked at tests/fail/function_calls/exported_symbol_bad_unwind1.rs:LL:CC:
thread 'main' ($TID) panicked at tests/fail/function_calls/exported_symbol_bad_unwind1.rs:LL:CC:
explicit panic
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
note: in Miri, you may have to set `MIRIFLAGS=-Zmiri-env-forward=RUST_BACKTRACE` for the environment variable to have an effect
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@

thread 'main' panicked at tests/fail/function_calls/exported_symbol_bad_unwind2.rs:LL:CC:
thread 'main' ($TID) panicked at tests/fail/function_calls/exported_symbol_bad_unwind2.rs:LL:CC:
explicit panic
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
note: in Miri, you may have to set `MIRIFLAGS=-Zmiri-env-forward=RUST_BACKTRACE` for the environment variable to have an effect

thread 'main' panicked at RUSTLIB/core/src/panicking.rs:LL:CC:
thread 'main' ($TID) panicked at RUSTLIB/core/src/panicking.rs:LL:CC:
panic in a function that cannot unwind
stack backtrace:
thread caused non-unwinding panic. aborting.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@

thread 'main' panicked at tests/fail/function_calls/exported_symbol_bad_unwind2.rs:LL:CC:
thread 'main' ($TID) panicked at tests/fail/function_calls/exported_symbol_bad_unwind2.rs:LL:CC:
explicit panic
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
note: in Miri, you may have to set `MIRIFLAGS=-Zmiri-env-forward=RUST_BACKTRACE` for the environment variable to have an effect

thread 'main' panicked at RUSTLIB/core/src/panicking.rs:LL:CC:
thread 'main' ($TID) panicked at RUSTLIB/core/src/panicking.rs:LL:CC:
panic in a function that cannot unwind
stack backtrace:
thread caused non-unwinding panic. aborting.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@

thread 'main' panicked at tests/fail/function_calls/exported_symbol_bad_unwind2.rs:LL:CC:
thread 'main' ($TID) panicked at tests/fail/function_calls/exported_symbol_bad_unwind2.rs:LL:CC:
explicit panic
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
note: in Miri, you may have to set `MIRIFLAGS=-Zmiri-env-forward=RUST_BACKTRACE` for the environment variable to have an effect
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@

thread 'main' panicked at tests/fail/function_calls/return_pointer_on_unwind.rs:LL:CC:
thread 'main' ($TID) panicked at tests/fail/function_calls/return_pointer_on_unwind.rs:LL:CC:
explicit panic
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
note: in Miri, you may have to set `MIRIFLAGS=-Zmiri-env-forward=RUST_BACKTRACE` for the environment variable to have an effect
Expand Down
4 changes: 2 additions & 2 deletions src/tools/miri/tests/fail/panic/abort_unwind.stderr
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@

thread 'main' panicked at tests/fail/panic/abort_unwind.rs:LL:CC:
thread 'main' ($TID) panicked at tests/fail/panic/abort_unwind.rs:LL:CC:
PANIC!!!
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
note: in Miri, you may have to set `MIRIFLAGS=-Zmiri-env-forward=RUST_BACKTRACE` for the environment variable to have an effect

thread 'main' panicked at RUSTLIB/core/src/panicking.rs:LL:CC:
thread 'main' ($TID) panicked at RUSTLIB/core/src/panicking.rs:LL:CC:
panic in a function that cannot unwind
stack backtrace:
thread caused non-unwinding panic. aborting.
Expand Down
2 changes: 1 addition & 1 deletion src/tools/miri/tests/fail/panic/bad_unwind.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@

thread 'main' panicked at tests/fail/panic/bad_unwind.rs:LL:CC:
thread 'main' ($TID) panicked at tests/fail/panic/bad_unwind.rs:LL:CC:
explicit panic
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
note: in Miri, you may have to set `MIRIFLAGS=-Zmiri-env-forward=RUST_BACKTRACE` for the environment variable to have an effect
Expand Down
6 changes: 3 additions & 3 deletions src/tools/miri/tests/fail/panic/double_panic.stderr
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@

thread 'main' panicked at tests/fail/panic/double_panic.rs:LL:CC:
thread 'main' ($TID) panicked at tests/fail/panic/double_panic.rs:LL:CC:
first
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
note: in Miri, you may have to set `MIRIFLAGS=-Zmiri-env-forward=RUST_BACKTRACE` for the environment variable to have an effect

thread 'main' panicked at tests/fail/panic/double_panic.rs:LL:CC:
thread 'main' ($TID) panicked at tests/fail/panic/double_panic.rs:LL:CC:
second
stack backtrace:

thread 'main' panicked at RUSTLIB/core/src/panicking.rs:LL:CC:
thread 'main' ($TID) panicked at RUSTLIB/core/src/panicking.rs:LL:CC:
panic in a destructor during cleanup
thread caused non-unwinding panic. aborting.
error: abnormal termination: the program aborted execution
Expand Down
2 changes: 1 addition & 1 deletion src/tools/miri/tests/fail/panic/panic_abort1.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@

thread 'main' panicked at tests/fail/panic/panic_abort1.rs:LL:CC:
thread 'main' ($TID) panicked at tests/fail/panic/panic_abort1.rs:LL:CC:
panicking from libstd
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
note: in Miri, you may have to set `MIRIFLAGS=-Zmiri-env-forward=RUST_BACKTRACE` for the environment variable to have an effect
Expand Down
2 changes: 1 addition & 1 deletion src/tools/miri/tests/fail/panic/panic_abort2.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@

thread 'main' panicked at tests/fail/panic/panic_abort2.rs:LL:CC:
thread 'main' ($TID) panicked at tests/fail/panic/panic_abort2.rs:LL:CC:
42-panicking from libstd
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
note: in Miri, you may have to set `MIRIFLAGS=-Zmiri-env-forward=RUST_BACKTRACE` for the environment variable to have an effect
Expand Down
Loading
Loading