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 unstable option to nul-terminate location strings #135240

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
24 changes: 17 additions & 7 deletions compiler/rustc_const_eval/src/util/caller_location.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,25 @@ fn alloc_caller_location<'tcx>(
line: u32,
col: u32,
) -> MPlaceTy<'tcx> {
// Ensure that the filename itself does not contain nul bytes.
// This isn't possible via POSIX or Windows, but we should ensure no one
// ever does such a thing.
assert!(!filename.as_str().as_bytes().contains(&0));

let loc_details = ecx.tcx.sess.opts.unstable_opts.location_detail;
// 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_dedup(filename.as_str()).unwrap()

let file = if loc_details.file { filename.as_str() } else { "<redacted>" };

// These allocations 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_ptr = if loc_details.cstr {
ecx.allocate_bytes_dedup(format!("{file}\0").as_bytes()).unwrap()
} else {
ecx.allocate_str_dedup("<redacted>").unwrap()
ecx.allocate_bytes_dedup(file.as_bytes()).unwrap()
};
let file = file.map_provenance(CtfeProvenance::as_immutable);
// We don't include the zero-terminator in the length here.
let file_wide_ptr = Immediate::new_slice(file_ptr.into(), file.len().try_into().unwrap(), ecx);

let line = if loc_details.line { Scalar::from_u32(line) } else { Scalar::from_u32(0) };
let col = if loc_details.column { Scalar::from_u32(col) } else { Scalar::from_u32(0) };

Expand All @@ -37,7 +47,7 @@ fn alloc_caller_location<'tcx>(
let location = ecx.allocate(loc_layout, MemoryKind::CallerLocation).unwrap();

// Initialize fields.
ecx.write_immediate(file.to_ref(ecx), &ecx.project_field(&location, 0).unwrap())
ecx.write_immediate(file_wide_ptr, &ecx.project_field(&location, 0).unwrap())
.expect("writing to memory we just allocated cannot fail");
ecx.write_scalar(line, &ecx.project_field(&location, 1).unwrap())
.expect("writing to memory we just allocated cannot fail");
Expand Down
7 changes: 6 additions & 1 deletion compiler/rustc_interface/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -804,7 +804,12 @@ fn test_unstable_options_tracking_hash() {
tracked!(lint_llvm_ir, true);
tracked!(llvm_module_flag, vec![("bar".to_string(), 123, "max".to_string())]);
tracked!(llvm_plugins, vec![String::from("plugin_name")]);
tracked!(location_detail, LocationDetail { file: true, line: false, column: false });
tracked!(location_detail, LocationDetail {
file: true,
line: false,
column: false,
cstr: false
});
tracked!(maximal_hir_to_mir_coverage, true);
tracked!(merge_functions, Some(MergeFunctions::Disabled));
tracked!(mir_emit_retag, true);
Expand Down
6 changes: 4 additions & 2 deletions compiler/rustc_session/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -398,11 +398,13 @@ pub struct LocationDetail {
pub file: bool,
pub line: bool,
pub column: bool,
pub cstr: bool,
}

impl LocationDetail {
pub(crate) fn all() -> Self {
Self { file: true, line: true, column: true }
/// The default value for this option.
pub(crate) fn default_value() -> Self {
Self { file: true, line: true, column: true, cstr: false }
}
}

Expand Down
10 changes: 6 additions & 4 deletions compiler/rustc_session/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,7 @@ mod desc {
"either a boolean (`yes`, `no`, `on`, `off`, etc), `thin`, `fat`, or omitted";
pub(crate) const parse_linker_plugin_lto: &str =
"either a boolean (`yes`, `no`, `on`, `off`, etc), or the path to the linker plugin";
pub(crate) const parse_location_detail: &str = "either `none`, or a comma separated list of location details to track: `file`, `line`, or `column`";
pub(crate) const parse_location_detail: &str = "either `none`, or a comma separated list of location details to track: `file`, `line`, `column` or `cstr`";
pub(crate) const parse_fmt_debug: &str = "either `full`, `shallow`, or `none`";
pub(crate) const parse_switch_with_opt_path: &str =
"an optional path to the profiling data output directory";
Expand Down Expand Up @@ -647,6 +647,7 @@ pub mod parse {
ld.line = false;
ld.file = false;
ld.column = false;
ld.cstr = false;
if v == "none" {
return true;
}
Expand All @@ -655,6 +656,7 @@ pub mod parse {
"file" => ld.file = true,
"line" => ld.line = true,
"column" => ld.column = true,
"cstr" => ld.cstr = true,
_ => return false,
}
}
Expand Down Expand Up @@ -1902,10 +1904,10 @@ options! {
"a list LLVM plugins to enable (space separated)"),
llvm_time_trace: bool = (false, parse_bool, [UNTRACKED],
"generate JSON tracing data file from LLVM data (default: no)"),
location_detail: LocationDetail = (LocationDetail::all(), parse_location_detail, [TRACKED],
location_detail: LocationDetail = (LocationDetail::default_value(), parse_location_detail, [TRACKED],
"what location details should be tracked when using caller_location, either \
`none`, or a comma separated list of location details, for which \
valid options are `file`, `line`, and `column` (default: `file,line,column`)"),
`none`, or a comma separated list of location details, for which valid \
options are `file`, `line`, `column`, and `cstr` (default: `file,line,column`)"),
ls: Vec<String> = (Vec::new(), parse_list, [UNTRACKED],
"decode and print various parts of the crate metadata for a library crate \
(space separated)"),
Expand Down
6 changes: 6 additions & 0 deletions src/doc/unstable-book/src/compiler-flags/location-detail.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ within this list are:
- `file` - the filename of the panic will be included in the panic output
- `line` - the source line of the panic will be included in the panic output
- `column` - the source column of the panic will be included in the panic output
- `cstr` - whether the location strings should be nul-terminated

Any combination of these three options are supported. Alternatively, you can pass
`none` to this option, which results in no location details being tracked.
Expand All @@ -40,5 +41,10 @@ and column information is identical for all panics, these branches can be fused,
can lead to substantial code size savings, especially for small embedded binaries with
many panics.

Although the `cstr` option makes location strings one byte longer, some linkers have
functionality for deduplicating nul-terminated strings, so using this option may decrease
binary size under some circumstances. Some applications may also use this option to pass
location strings into C or C++ code that relies on nul-terminated strings.

The savings from this option are amplified when combined with the use of `-Zbuild-std`, as
otherwise paths for panics within the standard library are still included in your binary.
13 changes: 13 additions & 0 deletions tests/ui/panics/location-cstr.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
//@ run-pass
//@ compile-flags: -Zlocation-detail=line,file,cstr

use std::panic::Location;

fn main() {
let location = Location::caller();
let len = location.file().len();
let ptr = location.file().as_ptr();

let zero_terminator = unsafe { core::ptr::read(ptr.add(len)) };
assert_eq!(zero_terminator, 0u8);
}
8 changes: 8 additions & 0 deletions tests/ui/panics/location-detail-panic-cstr.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
//@ run-fail
//@ check-run-results
//@ compile-flags: -Zlocation-detail=line,column,file,cstr
//@ exec-env:RUST_BACKTRACE=0

fn main() {
panic!("with-nul-terminator");
}
4 changes: 4 additions & 0 deletions tests/ui/panics/location-detail-panic-cstr.run.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@

thread 'main' panicked at $DIR/location-detail-panic-cstr.rs:7:5:
with-nul-terminator
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Loading