From 571ccabed0dd62b57e24cd0ffed06d5b6ac89976 Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Thu, 17 Oct 2024 11:59:44 +0200 Subject: [PATCH 1/4] Add nul-terminated filename for #[track_caller] Signed-off-by: Alice Ryhl --- .../src/util/caller_location.rs | 5 +- library/core/src/panic/location.rs | 66 +++++++++++++++---- 2 files changed, 57 insertions(+), 14 deletions(-) diff --git a/compiler/rustc_const_eval/src/util/caller_location.rs b/compiler/rustc_const_eval/src/util/caller_location.rs index 7f4c36835e449..a170ba949d320 100644 --- a/compiler/rustc_const_eval/src/util/caller_location.rs +++ b/compiler/rustc_const_eval/src/util/caller_location.rs @@ -20,7 +20,10 @@ fn alloc_caller_location<'tcx>( // This can fail if rustc runs out of memory right here. Trying to emit an error would be // pointless, since that would require allocating more memory than these short strings. let file = if loc_details.file { - ecx.allocate_str(filename.as_str(), MemoryKind::CallerLocation, Mutability::Not).unwrap() + let mut name = filename.as_str().to_string(); + name.push('\0'); + + ecx.allocate_str(&name, MemoryKind::CallerLocation, Mutability::Not).unwrap() } else { // FIXME: This creates a new allocation each time. It might be preferable to // perform this allocation only once, and re-use the `MPlaceTy`. diff --git a/library/core/src/panic/location.rs b/library/core/src/panic/location.rs index 1ad5c07d15cd0..ef69aec955653 100644 --- a/library/core/src/panic/location.rs +++ b/library/core/src/panic/location.rs @@ -1,5 +1,8 @@ use crate::fmt; +#[cfg(not(bootstrap))] +use crate::ffi::CStr; + /// A struct containing information about the location of a panic. /// /// This structure is created by [`PanicHookInfo::location()`] and [`PanicInfo::location()`]. @@ -32,7 +35,11 @@ use crate::fmt; #[derive(Copy, Clone, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)] #[stable(feature = "panic_hooks", since = "1.10.0")] pub struct Location<'a> { - file: &'a str, + // When not bootstrapping the compiler, it is an invariant that the last byte of this string + // slice is a nul-byte. + // + // When bootstrapping the compiler, this string may be missing the nul-terminator. + file_with_nul: &'a str, line: u32, col: u32, } @@ -127,7 +134,30 @@ impl<'a> Location<'a> { #[rustc_const_stable(feature = "const_location_fields", since = "1.79.0")] #[inline] pub const fn file(&self) -> &str { - self.file + // String slicing in const is very hard, see: + // + + let s = self.file_with_nul; + + #[cfg(bootstrap)] + if !matches!(s.as_bytes().last(), Some(0)) { + return s; + } + + #[cfg(debug_assertions)] + if !matches!(s.as_bytes().last(), Some(0)) { + panic!("filename is not nul terminated"); + } + + // SAFETY: The string contains a nul-byte, so the length is at least one. + let len = unsafe { s.len().unchecked_sub(1) }; + + // SAFETY: `s.as_ptr()` is valid for `len+1` bytes, so it is valid for `len` bytes. + let file = unsafe { core::slice::from_raw_parts(s.as_ptr(), len) }; + + // SAFETY: This is valid utf-8 because the original string is valid utf-8 and the last + // character was a nul-byte, so removing it does not cut a codepoint in half. + unsafe { core::str::from_utf8_unchecked(file) } } /// Returns the line number from which the panic originated. @@ -179,17 +209,27 @@ impl<'a> Location<'a> { pub const fn column(&self) -> u32 { self.col } -} -#[unstable( - feature = "panic_internals", - reason = "internal details of the implementation of the `panic!` and related macros", - issue = "none" -)] -impl<'a> Location<'a> { - #[doc(hidden)] - pub const fn internal_constructor(file: &'a str, line: u32, col: u32) -> Self { - Location { file, line, col } + /// Returns the name of the source file from which the panic originated as a nul-terminated + /// string. + /// + /// This function is like [`Location::file`], except that it returns a nul-terminated string. It + /// is mainly useful for passing the filename into C or C++ code. + #[must_use] + #[inline] + #[unstable(feature = "panic_file_with_nul", issue = "none")] + #[cfg(not(bootstrap))] + pub fn file_with_nul(&self) -> &CStr { + let file_with_nul = self.file_with_nul.as_bytes(); + + #[cfg(debug_assertions)] + if !matches!(file_with_nul.last(), Some(0)) { + panic!("filename is not nul terminated"); + } + + // SAFETY: This struct is only ever constructed by the compiler, which always inserts a + // nul-terminator in this string. + unsafe { CStr::from_bytes_with_nul_unchecked(file_with_nul) } } } @@ -197,6 +237,6 @@ impl<'a> Location<'a> { impl fmt::Display for Location<'_> { #[inline] fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(formatter, "{}:{}:{}", self.file, self.line, self.col) + write!(formatter, "{}:{}:{}", self.file(), self.line, self.col) } } From 6875a9c48315722795400e7401019703b7d79b8e Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Thu, 17 Oct 2024 12:22:27 +0200 Subject: [PATCH 2/4] fmt --- library/core/src/panic/location.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/library/core/src/panic/location.rs b/library/core/src/panic/location.rs index ef69aec955653..7e31e98704673 100644 --- a/library/core/src/panic/location.rs +++ b/library/core/src/panic/location.rs @@ -1,7 +1,6 @@ -use crate::fmt; - #[cfg(not(bootstrap))] use crate::ffi::CStr; +use crate::fmt; /// A struct containing information about the location of a panic. /// From 0ab305130008749e8e3b0d086fced73e133e25a9 Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Thu, 17 Oct 2024 13:18:44 +0200 Subject: [PATCH 3/4] redacted nul --- compiler/rustc_const_eval/src/util/caller_location.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_const_eval/src/util/caller_location.rs b/compiler/rustc_const_eval/src/util/caller_location.rs index a170ba949d320..123df6e7fb6c1 100644 --- a/compiler/rustc_const_eval/src/util/caller_location.rs +++ b/compiler/rustc_const_eval/src/util/caller_location.rs @@ -28,7 +28,7 @@ fn alloc_caller_location<'tcx>( // FIXME: This creates a new allocation each time. It might be preferable to // perform this allocation only once, and re-use the `MPlaceTy`. // See https://github.com/rust-lang/rust/pull/89920#discussion_r730012398 - ecx.allocate_str("", MemoryKind::CallerLocation, Mutability::Not).unwrap() + ecx.allocate_str("\0", MemoryKind::CallerLocation, Mutability::Not).unwrap() }; let file = file.map_provenance(CtfeProvenance::as_immutable); let line = if loc_details.line { Scalar::from_u32(line) } else { Scalar::from_u32(0) }; From 1bdb6064ed0c4ded29a4d8a3c463bffa972b679d Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Thu, 17 Oct 2024 15:01:45 +0200 Subject: [PATCH 4/4] remove assertion from file_with_nul --- library/core/src/panic/location.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/library/core/src/panic/location.rs b/library/core/src/panic/location.rs index 7e31e98704673..2f0ac1811b6a1 100644 --- a/library/core/src/panic/location.rs +++ b/library/core/src/panic/location.rs @@ -145,7 +145,7 @@ impl<'a> Location<'a> { #[cfg(debug_assertions)] if !matches!(s.as_bytes().last(), Some(0)) { - panic!("filename is not nul terminated"); + panic!("filename is not nul-terminated"); } // SAFETY: The string contains a nul-byte, so the length is at least one. @@ -221,11 +221,6 @@ impl<'a> Location<'a> { pub fn file_with_nul(&self) -> &CStr { let file_with_nul = self.file_with_nul.as_bytes(); - #[cfg(debug_assertions)] - if !matches!(file_with_nul.last(), Some(0)) { - panic!("filename is not nul terminated"); - } - // SAFETY: This struct is only ever constructed by the compiler, which always inserts a // nul-terminator in this string. unsafe { CStr::from_bytes_with_nul_unchecked(file_with_nul) }