Skip to content
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

Add nul-terminated filename for #[track_caller] #131828

Closed
wants to merge 4 commits into from
Closed
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
7 changes: 5 additions & 2 deletions compiler/rustc_const_eval/src/util/caller_location.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,15 @@ 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()
oli-obk marked this conversation as resolved.
Show resolved Hide resolved
} else {
// 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("<redacted>", MemoryKind::CallerLocation, Mutability::Not).unwrap()
ecx.allocate_str("<redacted>\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) };
Expand Down
60 changes: 47 additions & 13 deletions library/core/src/panic/location.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#[cfg(not(bootstrap))]
use crate::ffi::CStr;
use crate::fmt;

/// A struct containing information about the location of a panic.
Expand Down Expand Up @@ -32,7 +34,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,
}
Expand Down Expand Up @@ -127,7 +133,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:
// <https://users.rust-lang.org/t/slicing-strings-in-const/119836>

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.
Expand Down Expand Up @@ -179,24 +208,29 @@ 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 {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note that this internal constructor does not seem to be used anymore.

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();

// 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) }
}
}

#[stable(feature = "panic_hook_display", since = "1.26.0")]
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)
}
}
Loading