Add unstable option to nul-terminate location strings

Author: Darksonn

compiler/rustc_const_eval/src/util/caller_location.rs

@@ -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) };
 
@@ -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");

compiler/rustc_interface/src/tests.rs

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

compiler/rustc_session/src/config.rs

@@ -398,11 +398,12 @@ 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 }
+        Self { file: true, line: true, column: true, cstr: false }
     }
 }
 

compiler/rustc_session/src/options.rs

@@ -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";
@@ -647,6 +647,7 @@ pub mod parse {
             ld.line = false;
             ld.file = false;
             ld.column = false;
+            ld.cstr = false;
             if v == "none" {
                 return true;
             }
@@ -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,
                 }
             }
@@ -1904,8 +1906,8 @@ options! {
         "generate JSON tracing data file from LLVM data (default: no)"),
     location_detail: LocationDetail = (LocationDetail::all(), 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)"),

src/doc/unstable-book/src/compiler-flags/location-detail.md

@@ -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.
@@ -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.

tests/ui/panics/location-cstr.rs

@@ -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);
+}

tests/ui/panics/location-detail-panic-cstr.rs

@@ -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");
+}

tests/ui/panics/location-detail-panic-cstr.run.stderr

@@ -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