Skip to content

Commit

Permalink
Rollup merge of rust-lang#89920 - hudson-ayers:location-detail-contro…
Browse files Browse the repository at this point in the history
…l, r=davidtwco

Implement -Z location-detail flag

This PR implements the `-Z location-detail` flag as described in rust-lang/rfcs#2091 .

`-Z location-detail=val` controls what location details are tracked when using `caller_location`. This allows users to control what location details are printed as part of panic messages, by allowing them to exclude any combination of filenames, line numbers, and column numbers. This option is intended to provide users with a way to mitigate the size impact of `#[track_caller]`.

Some measurements of the savings of this approach on an embedded binary can be found here: rust-lang#70579 (comment) .

Closes rust-lang#70580 (unless people want to leave that open as a place for discussion of further improvements).

This is my first real PR to rust, so any help correcting mistakes / understanding side effects / improving my tests is appreciated :)

I have one question: RFC 2091 specified this as a debugging option (I think that is what -Z implies?). Does that mean this can never be stabilized without a separate MCP? If so, do I need to submit an MCP now, or is the initial RFC specifying this option sufficient for this to be merged as is, and then an MCP would be needed for eventual stabilization?
  • Loading branch information
matthiaskrgr authored Oct 22, 2021
2 parents 38d274f + b802629 commit f95ad81
Show file tree
Hide file tree
Showing 13 changed files with 135 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,17 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
line: u32,
col: u32,
) -> MPlaceTy<'tcx, M::PointerTag> {
let file =
self.allocate_str(&filename.as_str(), MemoryKind::CallerLocation, Mutability::Not);
let line = Scalar::from_u32(line);
let col = Scalar::from_u32(col);
let loc_details = &self.tcx.sess.opts.debugging_opts.location_detail;
let file = if loc_details.file {
self.allocate_str(&filename.as_str(), MemoryKind::CallerLocation, Mutability::Not)
} 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
self.allocate_str("<redacted>", MemoryKind::CallerLocation, Mutability::Not)
};
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) };

// Allocate memory for `CallerLocation` struct.
let loc_ty = self
Expand Down
5 changes: 4 additions & 1 deletion compiler/rustc_interface/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ use rustc_errors::{emitter::HumanReadableErrorType, registry, ColorConfig};
use rustc_session::config::InstrumentCoverage;
use rustc_session::config::Strip;
use rustc_session::config::{build_configuration, build_session_options, to_crate_config};
use rustc_session::config::{rustc_optgroups, ErrorOutputType, ExternLocation, Options, Passes};
use rustc_session::config::{
rustc_optgroups, ErrorOutputType, ExternLocation, LocationDetail, Options, Passes,
};
use rustc_session::config::{CFGuard, ExternEntry, LinkerPluginLto, LtoCli, SwitchWithOptPath};
use rustc_session::config::{
Externs, OutputType, OutputTypes, SymbolManglingVersion, WasiExecModel,
Expand Down Expand Up @@ -733,6 +735,7 @@ fn test_debugging_options_tracking_hash() {
tracked!(instrument_mcount, true);
tracked!(link_only, true);
tracked!(llvm_plugins, vec![String::from("plugin_name")]);
tracked!(location_detail, LocationDetail { file: true, line: false, column: false });
tracked!(merge_functions, Some(MergeFunctions::Disabled));
tracked!(mir_emit_retag, true);
tracked!(mir_opt_level, Some(4));
Expand Down
17 changes: 16 additions & 1 deletion compiler/rustc_session/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,20 @@ impl LinkerPluginLto {
}
}

/// The different settings that can be enabled via the `-Z location-detail` flag.
#[derive(Clone, PartialEq, Hash, Debug)]
pub struct LocationDetail {
pub file: bool,
pub line: bool,
pub column: bool,
}

impl LocationDetail {
pub fn all() -> Self {
Self { file: true, line: true, column: true }
}
}

#[derive(Clone, PartialEq, Hash, Debug)]
pub enum SwitchWithOptPath {
Enabled(Option<PathBuf>),
Expand Down Expand Up @@ -2422,7 +2436,7 @@ crate mod dep_tracking {
use super::LdImpl;
use super::{
CFGuard, CrateType, DebugInfo, ErrorOutputType, InstrumentCoverage, LinkerPluginLto,
LtoCli, OptLevel, OutputType, OutputTypes, Passes, SourceFileHashAlgorithm,
LocationDetail, LtoCli, OptLevel, OutputType, OutputTypes, Passes, SourceFileHashAlgorithm,
SwitchWithOptPath, SymbolManglingVersion, TrimmedDefPaths,
};
use crate::lint;
Expand Down Expand Up @@ -2513,6 +2527,7 @@ crate mod dep_tracking {
Option<LdImpl>,
OutputType,
RealFileName,
LocationDetail,
);

impl<T1, T2> DepTrackingHash for (T1, T2)
Expand Down
24 changes: 24 additions & 0 deletions compiler/rustc_session/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,8 @@ mod desc {
"either a boolean (`yes`, `no`, `on`, `off`, etc), `thin`, `fat`, or omitted";
pub const parse_linker_plugin_lto: &str =
"either a boolean (`yes`, `no`, `on`, `off`, etc), or the path to the linker plugin";
pub const parse_location_detail: &str =
"comma seperated list of location details to track: `file`, `line`, or `column`";
pub const parse_switch_with_opt_path: &str =
"an optional path to the profiling data output directory";
pub const parse_merge_functions: &str = "one of: `disabled`, `trampolines`, or `aliases`";
Expand Down Expand Up @@ -484,6 +486,25 @@ mod parse {
}
}

crate fn parse_location_detail(ld: &mut LocationDetail, v: Option<&str>) -> bool {
if let Some(v) = v {
ld.line = false;
ld.file = false;
ld.column = false;
for s in v.split(',') {
match s {
"file" => ld.file = true,
"line" => ld.line = true,
"column" => ld.column = true,
_ => return false,
}
}
true
} else {
false
}
}

crate fn parse_opt_comma_list(slot: &mut Option<Vec<String>>, v: Option<&str>) -> bool {
match v {
Some(s) => {
Expand Down Expand Up @@ -1152,6 +1173,9 @@ 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],
"comma seperated list of location details to be tracked when using caller_location \
valid options are `file`, `line`, and `column` (default: all)"),
ls: bool = (false, parse_bool, [UNTRACKED],
"list the symbols defined by a library crate (default: no)"),
macro_backtrace: bool = (false, parse_bool, [UNTRACKED],
Expand Down
43 changes: 43 additions & 0 deletions src/doc/unstable-book/src/compiler-flags/location-detail.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
# `location-detail`

The tracking issue for this feature is: [#70580](https://github.com/rust-lang/rust/issues/70580).

------------------------

Option `-Z location-detail=val` controls what location details are tracked when
using `caller_location`. This allows users to control what location details
are printed as part of panic messages, by allowing them to exclude any combination
of filenames, line numbers, and column numbers. This option is intended to provide
users with a way to mitigate the size impact of `#[track_caller]`.

This option supports a comma separated list of location details to be included. Valid options
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

Any combination of these three options are supported. If this option is not specified,
all three are included by default.

An example of a panic output when using `-Z location-detail=line`:
```text
panicked at 'Process blink had a fault', <redacted>:323:0
```

The code size savings from this option are two-fold. First, the `&'static str` values
for each path to a file containing a panic are removed from the binary. For projects
with deep directory structures and many files with panics, this can add up. This category
of savings can only be realized by excluding filenames from the panic output. Second,
savings can be realized by allowing multiple panics to be fused into a single panicking
branch. It is often the case that within a single file, multiple panics with the same
panic message exist -- e.g. two calls to `Option::unwrap()` in a single line, or
two calls to `Result::expect()` on adjacent lines. If column and line information
are included in the `Location` struct passed to the panic handler, these branches cannot
be fused, as the output is different depending on which panic occurs. However if line
and column information is identical for all panics, these branches can be fused, which
can lead to substantial code size savings, especially for small embedded binaries with
many panics.

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.
7 changes: 7 additions & 0 deletions src/test/ui/panics/location-detail-panic-no-column.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// run-fail
// check-run-results
// compile-flags: -Zlocation-detail=line,file

fn main() {
panic!("column-redacted");
}
2 changes: 2 additions & 0 deletions src/test/ui/panics/location-detail-panic-no-column.run.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
thread 'main' panicked at 'column-redacted', $DIR/location-detail-panic-no-column.rs:6:0
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
7 changes: 7 additions & 0 deletions src/test/ui/panics/location-detail-panic-no-file.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// run-fail
// check-run-results
// compile-flags: -Zlocation-detail=line,column

fn main() {
panic!("file-redacted");
}
2 changes: 2 additions & 0 deletions src/test/ui/panics/location-detail-panic-no-file.run.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
thread 'main' panicked at 'file-redacted', <redacted>:6:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
7 changes: 7 additions & 0 deletions src/test/ui/panics/location-detail-panic-no-line.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// run-fail
// check-run-results
// compile-flags: -Zlocation-detail=file,column

fn main() {
panic!("line-redacted");
}
2 changes: 2 additions & 0 deletions src/test/ui/panics/location-detail-panic-no-line.run.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
thread 'main' panicked at 'line-redacted', $DIR/location-detail-panic-no-line.rs:0:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
8 changes: 8 additions & 0 deletions src/test/ui/panics/location-detail-unwrap-no-file.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

fn main() {
let opt: Option<u32> = None;
opt.unwrap();
}
2 changes: 2 additions & 0 deletions src/test/ui/panics/location-detail-unwrap-no-file.run.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', <redacted>:7:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

0 comments on commit f95ad81

Please sign in to comment.