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

Measure binary size impact of implicit caller location #70579

Open
anp opened this issue Mar 30, 2020 · 9 comments
Open

Measure binary size impact of implicit caller location #70579

anp opened this issue Mar 30, 2020 · 9 comments
Labels
F-track_caller `#![feature(track_caller)]` I-heavy Issue: Problems and improvements with respect to binary size of generated code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@anp
Copy link
Member

anp commented Mar 30, 2020

The largest unanswered questions about implicit caller location are about the binary size impact of the feature and how to offer users control over it.

There are three ways I see that #[track_caller] inflates binaries:

  1. code bloat from panicking branches that were previously fused
  2. encoding the Location structs themselves
  3. encoding the &'static str for every filename referenced by a Location

We need to determine the impact to the ecosystem for each of these before deciding on mitigation strategy.

(1) was the main concern in the RFC but it was predicated on the assumption that #[track_caller] would affect inlining (an artifact of the implementation proposed). Since it doesn't affect inlining now, the only lost optimization in panicking branches would be for cases where LLVM previously used knowledge of a single constant panic location to make the failure path of a function smaller.

We've not yet had any reports of this and some ad-hoc measurements don't show any difference. This matches my mental model of LLVM's ability to optimize calling conventions which is "very expensive magic". I'm open to ideas for how to better assess this, including "wait and see".

TLDR Re (2) and (3),

Locations are 24 bytes when size_of::<usize>() == size_of::<u64>() and we don't currently measure how many we encode. We should put these in their own section to make it easier to measure.

Which filenames do we encode? In practice: probably all source files. We should put these in their own section too.

@anp
Copy link
Member Author

anp commented Mar 30, 2020

@eddyb could you provide some pointers on how to put static allocations in their own sections? I assume it's a change to librustc_codegen_ssa?

@jonas-schievink jonas-schievink added F-track_caller `#![feature(track_caller)]` I-heavy Issue: Problems and improvements with respect to binary size of generated code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 30, 2020
@eddyb
Copy link
Member

eddyb commented Mar 30, 2020

@anp Yeah, look at how #[link_section] is handled and do the same.

@Mark-Simulacrum
Copy link
Member

Have we looked at a diff of a stage1 (or stage2) rustc build? rustc has a lot of expect/unwrap etc code so I suspect if there's essentially no delta then I'm not too worried. I haven't noticed nightlies getting much bigger so I suspect we in practice aren't growing by too much.

@eddyb
Copy link
Member

eddyb commented Mar 30, 2020

@Mark-Simulacrum this reminds me that we're not measuring size changes on perf.rust-lang.org.

@anp
Copy link
Member Author

anp commented Apr 5, 2020

Filed: rust-lang/rustc-perf#643

@anp
Copy link
Member Author

anp commented Apr 5, 2020

OK, here's a gist documenting an instrumentation-based approach. TLDR: rustc with #[track_caller] in the standard library uses ~0.68% of its binary size on the static Location allocations and their filenames but the binary is still quite a bit smaller than a matching stage1 build. I haven't yet been able to observe a conclusive net increase in binary size due to the feature.

The fact that intuitively we know this could cause a net binary bloat makes me think that we want to polish up the custom sections patch in my gist. It's useful instrumentation for the future.

I'm not sure whether this is sufficient motivation to block stabilization on #70580. Thoughts?

@tmandry
Copy link
Member

tmandry commented Apr 7, 2020

The best way of calculating the impact of the feature itself would be doing two stage2 builds, one with track_caller turned on and one with it off. From your analysis now we know it's no more than 810 kB for rustc.

Percentage of the overall binary size is probably useful, but it's hard to say without seeing more binaries. For a large enough system I'd guess it'd be approximately the same overall, but for smaller binaries it could take up a higher percentage.

Manishearth added a commit to Manishearth/rust that referenced this issue Jun 30, 2020
Stabilize `#[track_caller]`.

# Stabilization Report

RFC: [2091]
Tracking issue: rust-lang#47809

## Summary

From the [rustc-dev-guide chapter][dev-guide]:

> Take this example program:

```rust
fn main() {
    let foo: Option<()> = None;
    foo.unwrap(); // this should produce a useful panic message!
}
```

> Prior to Rust 1.42, panics like this `unwrap()` printed a location in libcore:

```
$ rustc +1.41.0 example.rs; example.exe
thread 'main' panicked at 'called `Option::unwrap()` on a `None` value',...core\macros\mod.rs:15:40
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace.
```

> As of 1.42, we get a much more helpful message:

```
$ rustc +1.42.0 example.rs; example.exe
thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', example.rs:3:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
```

> These error messages are achieved through a combination of changes to `panic!` internals to make use of `core::panic::Location::caller` and a number of `#[track_caller]` annotations in the standard library which propagate caller information.

The attribute adds an implicit caller location argument to the ABI of annotated functions, but does not affect the type or MIR of the function. We implement the feature entirely in codegen and in the const evaluator.

## Bottom Line

This PR stabilizes the use of `#[track_caller]` everywhere, including traits and extern blocks. It also stabilizes `core::panic::Location::caller`, although the use of that function in a const context remains gated by `#![feature(const_caller_location)]`.

The implementation for the feature already changed the output of panic messages for a number of std functions, as described in the [1.42 release announcement]. The attribute's use in `Index` and `IndexMut` traits is visible to users since 1.44.

## Tests

All of the tests for this feature live under [src/test/ui/rfc-2091-track-caller][tests] in the repo.

Noteworthy cases:

* [use of attr in std]
  * validates user-facing benefit of the feature
* [trait attribute inheritance]
  * covers subtle behavior designed during implementation and not RFC'd
* [const/codegen equivalence]
  * this was the result of a suspected edge case and investigation
* [diverging function support]
  * covers an unresolved question from the RFC
* [fn pointers and shims]
  * covers important potential sources of unsoundness

## Documentation

The rustc-dev-guide now has a chapter on [Implicit Caller Location][dev-guide].

I have an [open PR to the reference][attr-reference-pr] documenting the attribute.

The intrinsic's [wrapper] includes some examples as well.

## Implementation History

* 2019-10-02: [`#[track_caller]` feature gate (RFC 2091 1/N) rust-lang#65037](rust-lang#65037)
  * Picked up the patch that @ayosec had started on the feature gate.
* 2019-10-13: [Add `Instance::resolve_for_fn_ptr` (RFC 2091 rust-lang#2/N) rust-lang#65182](rust-lang#65182)
* 2019-10-20: ~~[WIP Add MIR argument for #[track_caller] (RFC 2091 3/N) rust-lang#65258](rust-lang#65258
  * Abandoned approach to send location as a MIR argument.
* 2019-10-28: [`std::panic::Location` is a lang_item, add `core::intrinsics::caller_location` (RFC 2091 3/N) rust-lang#65664](rust-lang#65664)
* 2019-12-07: [Implement #[track_caller] attribute. (RFC 2091 4/N) rust-lang#65881](rust-lang#65881)
* 2020-01-04: [libstd uses `core::panic::Location` where possible. rust-lang#67137](rust-lang#67137)
* 2020-01-08: [`Option::{expect,unwrap}` and `Result::{expect, expect_err, unwrap, unwrap_err}` have `#[track_caller]` rust-lang#67887](rust-lang#67887)
* 2020-01-20: [Fix #[track_caller] and function pointers rust-lang#68302](rust-lang#68302) (fixed rust-lang#68178)
* 2020-03-23: [#[track_caller] in traits rust-lang#69251](rust-lang#69251)
* 2020-03-24: [#[track_caller] on core::ops::{Index, IndexMut}. rust-lang#70234](rust-lang#70234)
* 2020-04-08 [Support `#[track_caller]` on functions in `extern "Rust" { ... }` rust-lang#70916](rust-lang#70916)

## Unresolveds

### From the RFC

> Currently the RFC simply prohibit applying #[track_caller] to trait methods as a future-proofing
> measure.

**Resolved.** See the dev-guide documentation and the tests section above.

> Diverging functions should be supported.

**Resolved.** See the tests section above.

> The closure foo::{{closure}} should inherit most attributes applied to the function foo, ...

**Resolved.** This unknown was related to specifics of the implementation which were made irrelevant by the final implementation.

### Binary Size

I [instrumented track_caller to use custom sections][measure-size] in a local build and discovered relatively minor binary size usage for the feature overall. I'm leaving the issue open to discuss whether we want to upstream custom section support.

There's an [open issue to discuss mitigation strategies][mitigate-size]. Some decisions remain about the "right" strategies to reduce size without overly constraining the compiler implementation. I'd be excited to see someone carry that work forward but my opinion is that we shouldn't block stabilization on implementing compiler flags for redaction.

### Specialization

There's an [open issue][specialization] on the semantics of the attribute in specialization chains. I'm inclined to move forward with stabilization without an exact resolution here given that specialization is itself unstable, but I also think it should be an easy question to resolve.

### Location only points to the start of a call span

rust-lang#69977 was resolved by rust-lang#73182, and the next step should probably be to [extend `Location` with a notion of the end of a call](rust-lang#73554).

### Regression of std's panic messages

rust-lang#70963 should be resolved by serializing span hygeine to crate metadata: rust-lang#68686.

[2091]: https://github.com/rust-lang/rfcs/blob/master/text/2091-inline-semantic.md
[dev-guide]: https://rustc-dev-guide.rust-lang.org/codegen/implicit-caller-location.html
[specialization]: rust-lang#70293
[measure-size]: rust-lang#70579
[mitigate-size]: rust-lang#70580
[attr-reference-pr]: rust-lang/reference#742
[wrapper]: https://doc.rust-lang.org/nightly/core/panic/struct.Location.html#method.caller
[tests]: https://github.com/rust-lang/rust/tree/master/src/test/ui/rfc-2091-track-caller
[const/codegen equivalence]: https://github.com/rust-lang/rust/blob/master/src/test/ui/rfc-2091-track-caller/caller-location-fnptr-rt-ctfe-equiv.rs
[diverging function support]: https://github.com/rust-lang/rust/blob/master/src/test/ui/rfc-2091-track-caller/diverging-caller-location.rs
[use of attr in std]: https://github.com/rust-lang/rust/blob/master/src/test/ui/rfc-2091-track-caller/std-panic-locations.rs
[fn pointers and shims]: https://github.com/rust-lang/rust/blob/master/src/test/ui/rfc-2091-track-caller/tracked-fn-ptr-with-arg.rs
[trait attribute inheritance]: https://github.com/rust-lang/rust/blob/master/src/test/ui/rfc-2091-track-caller/tracked-trait-impls.rs
[1.42 release announcement]: https://blog.rust-lang.org/2020/03/12/Rust-1.42.html#useful-line-numbers-in-option-and-result-panic-messages
Manishearth added a commit to Manishearth/rust that referenced this issue Jun 30, 2020
Stabilize `#[track_caller]`.

# Stabilization Report

RFC: [2091]
Tracking issue: rust-lang#47809

## Summary

From the [rustc-dev-guide chapter][dev-guide]:

> Take this example program:

```rust
fn main() {
    let foo: Option<()> = None;
    foo.unwrap(); // this should produce a useful panic message!
}
```

> Prior to Rust 1.42, panics like this `unwrap()` printed a location in libcore:

```
$ rustc +1.41.0 example.rs; example.exe
thread 'main' panicked at 'called `Option::unwrap()` on a `None` value',...core\macros\mod.rs:15:40
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace.
```

> As of 1.42, we get a much more helpful message:

```
$ rustc +1.42.0 example.rs; example.exe
thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', example.rs:3:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
```

> These error messages are achieved through a combination of changes to `panic!` internals to make use of `core::panic::Location::caller` and a number of `#[track_caller]` annotations in the standard library which propagate caller information.

The attribute adds an implicit caller location argument to the ABI of annotated functions, but does not affect the type or MIR of the function. We implement the feature entirely in codegen and in the const evaluator.

## Bottom Line

This PR stabilizes the use of `#[track_caller]` everywhere, including traits and extern blocks. It also stabilizes `core::panic::Location::caller`, although the use of that function in a const context remains gated by `#![feature(const_caller_location)]`.

The implementation for the feature already changed the output of panic messages for a number of std functions, as described in the [1.42 release announcement]. The attribute's use in `Index` and `IndexMut` traits is visible to users since 1.44.

## Tests

All of the tests for this feature live under [src/test/ui/rfc-2091-track-caller][tests] in the repo.

Noteworthy cases:

* [use of attr in std]
  * validates user-facing benefit of the feature
* [trait attribute inheritance]
  * covers subtle behavior designed during implementation and not RFC'd
* [const/codegen equivalence]
  * this was the result of a suspected edge case and investigation
* [diverging function support]
  * covers an unresolved question from the RFC
* [fn pointers and shims]
  * covers important potential sources of unsoundness

## Documentation

The rustc-dev-guide now has a chapter on [Implicit Caller Location][dev-guide].

I have an [open PR to the reference][attr-reference-pr] documenting the attribute.

The intrinsic's [wrapper] includes some examples as well.

## Implementation History

* 2019-10-02: [`#[track_caller]` feature gate (RFC 2091 1/N) rust-lang#65037](rust-lang#65037)
  * Picked up the patch that @ayosec had started on the feature gate.
* 2019-10-13: [Add `Instance::resolve_for_fn_ptr` (RFC 2091 rust-lang#2/N) rust-lang#65182](rust-lang#65182)
* 2019-10-20: ~~[WIP Add MIR argument for #[track_caller] (RFC 2091 3/N) rust-lang#65258](rust-lang#65258
  * Abandoned approach to send location as a MIR argument.
* 2019-10-28: [`std::panic::Location` is a lang_item, add `core::intrinsics::caller_location` (RFC 2091 3/N) rust-lang#65664](rust-lang#65664)
* 2019-12-07: [Implement #[track_caller] attribute. (RFC 2091 4/N) rust-lang#65881](rust-lang#65881)
* 2020-01-04: [libstd uses `core::panic::Location` where possible. rust-lang#67137](rust-lang#67137)
* 2020-01-08: [`Option::{expect,unwrap}` and `Result::{expect, expect_err, unwrap, unwrap_err}` have `#[track_caller]` rust-lang#67887](rust-lang#67887)
* 2020-01-20: [Fix #[track_caller] and function pointers rust-lang#68302](rust-lang#68302) (fixed rust-lang#68178)
* 2020-03-23: [#[track_caller] in traits rust-lang#69251](rust-lang#69251)
* 2020-03-24: [#[track_caller] on core::ops::{Index, IndexMut}. rust-lang#70234](rust-lang#70234)
* 2020-04-08 [Support `#[track_caller]` on functions in `extern "Rust" { ... }` rust-lang#70916](rust-lang#70916)

## Unresolveds

### From the RFC

> Currently the RFC simply prohibit applying #[track_caller] to trait methods as a future-proofing
> measure.

**Resolved.** See the dev-guide documentation and the tests section above.

> Diverging functions should be supported.

**Resolved.** See the tests section above.

> The closure foo::{{closure}} should inherit most attributes applied to the function foo, ...

**Resolved.** This unknown was related to specifics of the implementation which were made irrelevant by the final implementation.

### Binary Size

I [instrumented track_caller to use custom sections][measure-size] in a local build and discovered relatively minor binary size usage for the feature overall. I'm leaving the issue open to discuss whether we want to upstream custom section support.

There's an [open issue to discuss mitigation strategies][mitigate-size]. Some decisions remain about the "right" strategies to reduce size without overly constraining the compiler implementation. I'd be excited to see someone carry that work forward but my opinion is that we shouldn't block stabilization on implementing compiler flags for redaction.

### Specialization

There's an [open issue][specialization] on the semantics of the attribute in specialization chains. I'm inclined to move forward with stabilization without an exact resolution here given that specialization is itself unstable, but I also think it should be an easy question to resolve.

### Location only points to the start of a call span

rust-lang#69977 was resolved by rust-lang#73182, and the next step should probably be to [extend `Location` with a notion of the end of a call](rust-lang#73554).

### Regression of std's panic messages

rust-lang#70963 should be resolved by serializing span hygeine to crate metadata: rust-lang#68686.

[2091]: https://github.com/rust-lang/rfcs/blob/master/text/2091-inline-semantic.md
[dev-guide]: https://rustc-dev-guide.rust-lang.org/codegen/implicit-caller-location.html
[specialization]: rust-lang#70293
[measure-size]: rust-lang#70579
[mitigate-size]: rust-lang#70580
[attr-reference-pr]: rust-lang/reference#742
[wrapper]: https://doc.rust-lang.org/nightly/core/panic/struct.Location.html#method.caller
[tests]: https://github.com/rust-lang/rust/tree/master/src/test/ui/rfc-2091-track-caller
[const/codegen equivalence]: https://github.com/rust-lang/rust/blob/master/src/test/ui/rfc-2091-track-caller/caller-location-fnptr-rt-ctfe-equiv.rs
[diverging function support]: https://github.com/rust-lang/rust/blob/master/src/test/ui/rfc-2091-track-caller/diverging-caller-location.rs
[use of attr in std]: https://github.com/rust-lang/rust/blob/master/src/test/ui/rfc-2091-track-caller/std-panic-locations.rs
[fn pointers and shims]: https://github.com/rust-lang/rust/blob/master/src/test/ui/rfc-2091-track-caller/tracked-fn-ptr-with-arg.rs
[trait attribute inheritance]: https://github.com/rust-lang/rust/blob/master/src/test/ui/rfc-2091-track-caller/tracked-trait-impls.rs
[1.42 release announcement]: https://blog.rust-lang.org/2020/03/12/Rust-1.42.html#useful-line-numbers-in-option-and-result-panic-messages
Manishearth added a commit to Manishearth/rust that referenced this issue Jul 1, 2020
Stabilize `#[track_caller]`.

# Stabilization Report

RFC: [2091]
Tracking issue: rust-lang#47809

## Summary

From the [rustc-dev-guide chapter][dev-guide]:

> Take this example program:

```rust
fn main() {
    let foo: Option<()> = None;
    foo.unwrap(); // this should produce a useful panic message!
}
```

> Prior to Rust 1.42, panics like this `unwrap()` printed a location in libcore:

```
$ rustc +1.41.0 example.rs; example.exe
thread 'main' panicked at 'called `Option::unwrap()` on a `None` value',...core\macros\mod.rs:15:40
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace.
```

> As of 1.42, we get a much more helpful message:

```
$ rustc +1.42.0 example.rs; example.exe
thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', example.rs:3:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
```

> These error messages are achieved through a combination of changes to `panic!` internals to make use of `core::panic::Location::caller` and a number of `#[track_caller]` annotations in the standard library which propagate caller information.

The attribute adds an implicit caller location argument to the ABI of annotated functions, but does not affect the type or MIR of the function. We implement the feature entirely in codegen and in the const evaluator.

## Bottom Line

This PR stabilizes the use of `#[track_caller]` everywhere, including traits and extern blocks. It also stabilizes `core::panic::Location::caller`, although the use of that function in a const context remains gated by `#![feature(const_caller_location)]`.

The implementation for the feature already changed the output of panic messages for a number of std functions, as described in the [1.42 release announcement]. The attribute's use in `Index` and `IndexMut` traits is visible to users since 1.44.

## Tests

All of the tests for this feature live under [src/test/ui/rfc-2091-track-caller][tests] in the repo.

Noteworthy cases:

* [use of attr in std]
  * validates user-facing benefit of the feature
* [trait attribute inheritance]
  * covers subtle behavior designed during implementation and not RFC'd
* [const/codegen equivalence]
  * this was the result of a suspected edge case and investigation
* [diverging function support]
  * covers an unresolved question from the RFC
* [fn pointers and shims]
  * covers important potential sources of unsoundness

## Documentation

The rustc-dev-guide now has a chapter on [Implicit Caller Location][dev-guide].

I have an [open PR to the reference][attr-reference-pr] documenting the attribute.

The intrinsic's [wrapper] includes some examples as well.

## Implementation History

* 2019-10-02: [`#[track_caller]` feature gate (RFC 2091 1/N) rust-lang#65037](rust-lang#65037)
  * Picked up the patch that @ayosec had started on the feature gate.
* 2019-10-13: [Add `Instance::resolve_for_fn_ptr` (RFC 2091 rust-lang#2/N) rust-lang#65182](rust-lang#65182)
* 2019-10-20: ~~[WIP Add MIR argument for #[track_caller] (RFC 2091 3/N) rust-lang#65258](rust-lang#65258
  * Abandoned approach to send location as a MIR argument.
* 2019-10-28: [`std::panic::Location` is a lang_item, add `core::intrinsics::caller_location` (RFC 2091 3/N) rust-lang#65664](rust-lang#65664)
* 2019-12-07: [Implement #[track_caller] attribute. (RFC 2091 4/N) rust-lang#65881](rust-lang#65881)
* 2020-01-04: [libstd uses `core::panic::Location` where possible. rust-lang#67137](rust-lang#67137)
* 2020-01-08: [`Option::{expect,unwrap}` and `Result::{expect, expect_err, unwrap, unwrap_err}` have `#[track_caller]` rust-lang#67887](rust-lang#67887)
* 2020-01-20: [Fix #[track_caller] and function pointers rust-lang#68302](rust-lang#68302) (fixed rust-lang#68178)
* 2020-03-23: [#[track_caller] in traits rust-lang#69251](rust-lang#69251)
* 2020-03-24: [#[track_caller] on core::ops::{Index, IndexMut}. rust-lang#70234](rust-lang#70234)
* 2020-04-08 [Support `#[track_caller]` on functions in `extern "Rust" { ... }` rust-lang#70916](rust-lang#70916)

## Unresolveds

### From the RFC

> Currently the RFC simply prohibit applying #[track_caller] to trait methods as a future-proofing
> measure.

**Resolved.** See the dev-guide documentation and the tests section above.

> Diverging functions should be supported.

**Resolved.** See the tests section above.

> The closure foo::{{closure}} should inherit most attributes applied to the function foo, ...

**Resolved.** This unknown was related to specifics of the implementation which were made irrelevant by the final implementation.

### Binary Size

I [instrumented track_caller to use custom sections][measure-size] in a local build and discovered relatively minor binary size usage for the feature overall. I'm leaving the issue open to discuss whether we want to upstream custom section support.

There's an [open issue to discuss mitigation strategies][mitigate-size]. Some decisions remain about the "right" strategies to reduce size without overly constraining the compiler implementation. I'd be excited to see someone carry that work forward but my opinion is that we shouldn't block stabilization on implementing compiler flags for redaction.

### Specialization

There's an [open issue][specialization] on the semantics of the attribute in specialization chains. I'm inclined to move forward with stabilization without an exact resolution here given that specialization is itself unstable, but I also think it should be an easy question to resolve.

### Location only points to the start of a call span

rust-lang#69977 was resolved by rust-lang#73182, and the next step should probably be to [extend `Location` with a notion of the end of a call](rust-lang#73554).

### Regression of std's panic messages

rust-lang#70963 should be resolved by serializing span hygeine to crate metadata: rust-lang#68686.

[2091]: https://github.com/rust-lang/rfcs/blob/master/text/2091-inline-semantic.md
[dev-guide]: https://rustc-dev-guide.rust-lang.org/codegen/implicit-caller-location.html
[specialization]: rust-lang#70293
[measure-size]: rust-lang#70579
[mitigate-size]: rust-lang#70580
[attr-reference-pr]: rust-lang/reference#742
[wrapper]: https://doc.rust-lang.org/nightly/core/panic/struct.Location.html#method.caller
[tests]: https://github.com/rust-lang/rust/tree/master/src/test/ui/rfc-2091-track-caller
[const/codegen equivalence]: https://github.com/rust-lang/rust/blob/master/src/test/ui/rfc-2091-track-caller/caller-location-fnptr-rt-ctfe-equiv.rs
[diverging function support]: https://github.com/rust-lang/rust/blob/master/src/test/ui/rfc-2091-track-caller/diverging-caller-location.rs
[use of attr in std]: https://github.com/rust-lang/rust/blob/master/src/test/ui/rfc-2091-track-caller/std-panic-locations.rs
[fn pointers and shims]: https://github.com/rust-lang/rust/blob/master/src/test/ui/rfc-2091-track-caller/tracked-fn-ptr-with-arg.rs
[trait attribute inheritance]: https://github.com/rust-lang/rust/blob/master/src/test/ui/rfc-2091-track-caller/tracked-trait-impls.rs
[1.42 release announcement]: https://blog.rust-lang.org/2020/03/12/Rust-1.42.html#useful-line-numbers-in-option-and-result-panic-messages
Manishearth added a commit to Manishearth/rust that referenced this issue Jul 1, 2020
Stabilize `#[track_caller]`.

# Stabilization Report

RFC: [2091]
Tracking issue: rust-lang#47809

## Summary

From the [rustc-dev-guide chapter][dev-guide]:

> Take this example program:

```rust
fn main() {
    let foo: Option<()> = None;
    foo.unwrap(); // this should produce a useful panic message!
}
```

> Prior to Rust 1.42, panics like this `unwrap()` printed a location in libcore:

```
$ rustc +1.41.0 example.rs; example.exe
thread 'main' panicked at 'called `Option::unwrap()` on a `None` value',...core\macros\mod.rs:15:40
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace.
```

> As of 1.42, we get a much more helpful message:

```
$ rustc +1.42.0 example.rs; example.exe
thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', example.rs:3:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
```

> These error messages are achieved through a combination of changes to `panic!` internals to make use of `core::panic::Location::caller` and a number of `#[track_caller]` annotations in the standard library which propagate caller information.

The attribute adds an implicit caller location argument to the ABI of annotated functions, but does not affect the type or MIR of the function. We implement the feature entirely in codegen and in the const evaluator.

## Bottom Line

This PR stabilizes the use of `#[track_caller]` everywhere, including traits and extern blocks. It also stabilizes `core::panic::Location::caller`, although the use of that function in a const context remains gated by `#![feature(const_caller_location)]`.

The implementation for the feature already changed the output of panic messages for a number of std functions, as described in the [1.42 release announcement]. The attribute's use in `Index` and `IndexMut` traits is visible to users since 1.44.

## Tests

All of the tests for this feature live under [src/test/ui/rfc-2091-track-caller][tests] in the repo.

Noteworthy cases:

* [use of attr in std]
  * validates user-facing benefit of the feature
* [trait attribute inheritance]
  * covers subtle behavior designed during implementation and not RFC'd
* [const/codegen equivalence]
  * this was the result of a suspected edge case and investigation
* [diverging function support]
  * covers an unresolved question from the RFC
* [fn pointers and shims]
  * covers important potential sources of unsoundness

## Documentation

The rustc-dev-guide now has a chapter on [Implicit Caller Location][dev-guide].

I have an [open PR to the reference][attr-reference-pr] documenting the attribute.

The intrinsic's [wrapper] includes some examples as well.

## Implementation History

* 2019-10-02: [`#[track_caller]` feature gate (RFC 2091 1/N) rust-lang#65037](rust-lang#65037)
  * Picked up the patch that @ayosec had started on the feature gate.
* 2019-10-13: [Add `Instance::resolve_for_fn_ptr` (RFC 2091 rust-lang#2/N) rust-lang#65182](rust-lang#65182)
* 2019-10-20: ~~[WIP Add MIR argument for #[track_caller] (RFC 2091 3/N) rust-lang#65258](rust-lang#65258
  * Abandoned approach to send location as a MIR argument.
* 2019-10-28: [`std::panic::Location` is a lang_item, add `core::intrinsics::caller_location` (RFC 2091 3/N) rust-lang#65664](rust-lang#65664)
* 2019-12-07: [Implement #[track_caller] attribute. (RFC 2091 4/N) rust-lang#65881](rust-lang#65881)
* 2020-01-04: [libstd uses `core::panic::Location` where possible. rust-lang#67137](rust-lang#67137)
* 2020-01-08: [`Option::{expect,unwrap}` and `Result::{expect, expect_err, unwrap, unwrap_err}` have `#[track_caller]` rust-lang#67887](rust-lang#67887)
* 2020-01-20: [Fix #[track_caller] and function pointers rust-lang#68302](rust-lang#68302) (fixed rust-lang#68178)
* 2020-03-23: [#[track_caller] in traits rust-lang#69251](rust-lang#69251)
* 2020-03-24: [#[track_caller] on core::ops::{Index, IndexMut}. rust-lang#70234](rust-lang#70234)
* 2020-04-08 [Support `#[track_caller]` on functions in `extern "Rust" { ... }` rust-lang#70916](rust-lang#70916)

## Unresolveds

### From the RFC

> Currently the RFC simply prohibit applying #[track_caller] to trait methods as a future-proofing
> measure.

**Resolved.** See the dev-guide documentation and the tests section above.

> Diverging functions should be supported.

**Resolved.** See the tests section above.

> The closure foo::{{closure}} should inherit most attributes applied to the function foo, ...

**Resolved.** This unknown was related to specifics of the implementation which were made irrelevant by the final implementation.

### Binary Size

I [instrumented track_caller to use custom sections][measure-size] in a local build and discovered relatively minor binary size usage for the feature overall. I'm leaving the issue open to discuss whether we want to upstream custom section support.

There's an [open issue to discuss mitigation strategies][mitigate-size]. Some decisions remain about the "right" strategies to reduce size without overly constraining the compiler implementation. I'd be excited to see someone carry that work forward but my opinion is that we shouldn't block stabilization on implementing compiler flags for redaction.

### Specialization

There's an [open issue][specialization] on the semantics of the attribute in specialization chains. I'm inclined to move forward with stabilization without an exact resolution here given that specialization is itself unstable, but I also think it should be an easy question to resolve.

### Location only points to the start of a call span

rust-lang#69977 was resolved by rust-lang#73182, and the next step should probably be to [extend `Location` with a notion of the end of a call](rust-lang#73554).

### Regression of std's panic messages

rust-lang#70963 should be resolved by serializing span hygeine to crate metadata: rust-lang#68686.

[2091]: https://github.com/rust-lang/rfcs/blob/master/text/2091-inline-semantic.md
[dev-guide]: https://rustc-dev-guide.rust-lang.org/codegen/implicit-caller-location.html
[specialization]: rust-lang#70293
[measure-size]: rust-lang#70579
[mitigate-size]: rust-lang#70580
[attr-reference-pr]: rust-lang/reference#742
[wrapper]: https://doc.rust-lang.org/nightly/core/panic/struct.Location.html#method.caller
[tests]: https://github.com/rust-lang/rust/tree/master/src/test/ui/rfc-2091-track-caller
[const/codegen equivalence]: https://github.com/rust-lang/rust/blob/master/src/test/ui/rfc-2091-track-caller/caller-location-fnptr-rt-ctfe-equiv.rs
[diverging function support]: https://github.com/rust-lang/rust/blob/master/src/test/ui/rfc-2091-track-caller/diverging-caller-location.rs
[use of attr in std]: https://github.com/rust-lang/rust/blob/master/src/test/ui/rfc-2091-track-caller/std-panic-locations.rs
[fn pointers and shims]: https://github.com/rust-lang/rust/blob/master/src/test/ui/rfc-2091-track-caller/tracked-fn-ptr-with-arg.rs
[trait attribute inheritance]: https://github.com/rust-lang/rust/blob/master/src/test/ui/rfc-2091-track-caller/tracked-trait-impls.rs
[1.42 release announcement]: https://blog.rust-lang.org/2020/03/12/Rust-1.42.html#useful-line-numbers-in-option-and-result-panic-messages
Manishearth added a commit to Manishearth/rust that referenced this issue Jul 1, 2020
Stabilize `#[track_caller]`.

# Stabilization Report

RFC: [2091]
Tracking issue: rust-lang#47809

## Summary

From the [rustc-dev-guide chapter][dev-guide]:

> Take this example program:

```rust
fn main() {
    let foo: Option<()> = None;
    foo.unwrap(); // this should produce a useful panic message!
}
```

> Prior to Rust 1.42, panics like this `unwrap()` printed a location in libcore:

```
$ rustc +1.41.0 example.rs; example.exe
thread 'main' panicked at 'called `Option::unwrap()` on a `None` value',...core\macros\mod.rs:15:40
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace.
```

> As of 1.42, we get a much more helpful message:

```
$ rustc +1.42.0 example.rs; example.exe
thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', example.rs:3:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
```

> These error messages are achieved through a combination of changes to `panic!` internals to make use of `core::panic::Location::caller` and a number of `#[track_caller]` annotations in the standard library which propagate caller information.

The attribute adds an implicit caller location argument to the ABI of annotated functions, but does not affect the type or MIR of the function. We implement the feature entirely in codegen and in the const evaluator.

## Bottom Line

This PR stabilizes the use of `#[track_caller]` everywhere, including traits and extern blocks. It also stabilizes `core::panic::Location::caller`, although the use of that function in a const context remains gated by `#![feature(const_caller_location)]`.

The implementation for the feature already changed the output of panic messages for a number of std functions, as described in the [1.42 release announcement]. The attribute's use in `Index` and `IndexMut` traits is visible to users since 1.44.

## Tests

All of the tests for this feature live under [src/test/ui/rfc-2091-track-caller][tests] in the repo.

Noteworthy cases:

* [use of attr in std]
  * validates user-facing benefit of the feature
* [trait attribute inheritance]
  * covers subtle behavior designed during implementation and not RFC'd
* [const/codegen equivalence]
  * this was the result of a suspected edge case and investigation
* [diverging function support]
  * covers an unresolved question from the RFC
* [fn pointers and shims]
  * covers important potential sources of unsoundness

## Documentation

The rustc-dev-guide now has a chapter on [Implicit Caller Location][dev-guide].

I have an [open PR to the reference][attr-reference-pr] documenting the attribute.

The intrinsic's [wrapper] includes some examples as well.

## Implementation History

* 2019-10-02: [`#[track_caller]` feature gate (RFC 2091 1/N) rust-lang#65037](rust-lang#65037)
  * Picked up the patch that @ayosec had started on the feature gate.
* 2019-10-13: [Add `Instance::resolve_for_fn_ptr` (RFC 2091 rust-lang#2/N) rust-lang#65182](rust-lang#65182)
* 2019-10-20: ~~[WIP Add MIR argument for #[track_caller] (RFC 2091 3/N) rust-lang#65258](rust-lang#65258
  * Abandoned approach to send location as a MIR argument.
* 2019-10-28: [`std::panic::Location` is a lang_item, add `core::intrinsics::caller_location` (RFC 2091 3/N) rust-lang#65664](rust-lang#65664)
* 2019-12-07: [Implement #[track_caller] attribute. (RFC 2091 4/N) rust-lang#65881](rust-lang#65881)
* 2020-01-04: [libstd uses `core::panic::Location` where possible. rust-lang#67137](rust-lang#67137)
* 2020-01-08: [`Option::{expect,unwrap}` and `Result::{expect, expect_err, unwrap, unwrap_err}` have `#[track_caller]` rust-lang#67887](rust-lang#67887)
* 2020-01-20: [Fix #[track_caller] and function pointers rust-lang#68302](rust-lang#68302) (fixed rust-lang#68178)
* 2020-03-23: [#[track_caller] in traits rust-lang#69251](rust-lang#69251)
* 2020-03-24: [#[track_caller] on core::ops::{Index, IndexMut}. rust-lang#70234](rust-lang#70234)
* 2020-04-08 [Support `#[track_caller]` on functions in `extern "Rust" { ... }` rust-lang#70916](rust-lang#70916)

## Unresolveds

### From the RFC

> Currently the RFC simply prohibit applying #[track_caller] to trait methods as a future-proofing
> measure.

**Resolved.** See the dev-guide documentation and the tests section above.

> Diverging functions should be supported.

**Resolved.** See the tests section above.

> The closure foo::{{closure}} should inherit most attributes applied to the function foo, ...

**Resolved.** This unknown was related to specifics of the implementation which were made irrelevant by the final implementation.

### Binary Size

I [instrumented track_caller to use custom sections][measure-size] in a local build and discovered relatively minor binary size usage for the feature overall. I'm leaving the issue open to discuss whether we want to upstream custom section support.

There's an [open issue to discuss mitigation strategies][mitigate-size]. Some decisions remain about the "right" strategies to reduce size without overly constraining the compiler implementation. I'd be excited to see someone carry that work forward but my opinion is that we shouldn't block stabilization on implementing compiler flags for redaction.

### Specialization

There's an [open issue][specialization] on the semantics of the attribute in specialization chains. I'm inclined to move forward with stabilization without an exact resolution here given that specialization is itself unstable, but I also think it should be an easy question to resolve.

### Location only points to the start of a call span

rust-lang#69977 was resolved by rust-lang#73182, and the next step should probably be to [extend `Location` with a notion of the end of a call](rust-lang#73554).

### Regression of std's panic messages

rust-lang#70963 should be resolved by serializing span hygeine to crate metadata: rust-lang#68686.

[2091]: https://github.com/rust-lang/rfcs/blob/master/text/2091-inline-semantic.md
[dev-guide]: https://rustc-dev-guide.rust-lang.org/codegen/implicit-caller-location.html
[specialization]: rust-lang#70293
[measure-size]: rust-lang#70579
[mitigate-size]: rust-lang#70580
[attr-reference-pr]: rust-lang/reference#742
[wrapper]: https://doc.rust-lang.org/nightly/core/panic/struct.Location.html#method.caller
[tests]: https://github.com/rust-lang/rust/tree/master/src/test/ui/rfc-2091-track-caller
[const/codegen equivalence]: https://github.com/rust-lang/rust/blob/master/src/test/ui/rfc-2091-track-caller/caller-location-fnptr-rt-ctfe-equiv.rs
[diverging function support]: https://github.com/rust-lang/rust/blob/master/src/test/ui/rfc-2091-track-caller/diverging-caller-location.rs
[use of attr in std]: https://github.com/rust-lang/rust/blob/master/src/test/ui/rfc-2091-track-caller/std-panic-locations.rs
[fn pointers and shims]: https://github.com/rust-lang/rust/blob/master/src/test/ui/rfc-2091-track-caller/tracked-fn-ptr-with-arg.rs
[trait attribute inheritance]: https://github.com/rust-lang/rust/blob/master/src/test/ui/rfc-2091-track-caller/tracked-trait-impls.rs
[1.42 release announcement]: https://blog.rust-lang.org/2020/03/12/Rust-1.42.html#useful-line-numbers-in-option-and-result-panic-messages
@hudson-ayers
Copy link
Contributor

I took a stab at measuring this for a TockOS kernel binary. I did so by following the recommendation at #70580 (comment) , modifying those lines to replace all filenames with "", all line numbers with 0, and all column numbers with 0. I then built a new stage 1 compiler, and used that compiler to re-compile the tock "Imix" kernel binary (https://github.com/tock/tock/tree/master/boards/imix).

I chose to use a Tock kernel binary as the subject of this test because many uses of Tock are severely size-constrained. The Tock Imix kernel is required to remain smaller than 180 kB to fit in the space available on that platform. Other Tock deployments require even smaller sizes. I consider binary size impact on these classes of embedded devices more important than the impact on traditional binaries, where size is less likely to be a significant concern anyway.

  • Tock Imix kernel size, rust nightly-2021-10-11: 175,304 bytes
  • Tock Imix kernel size, all Panic locations fused: 158,356 bytes (-16,948 bytes / -9.6%)
  • Tock Imix kernel size, keeping filenames but setting all line/column numbers to 0: 163,752 bytes (-11,552 bytes / -6.6%)
  • Tock Imix kernel size, keeping only line numbers but setting filenames to "" and column to 0: 170,040 bytes (-5,264 bytes / -3%)
  • Tock Imix kernel size, keeping line numbers + column numbers, setting filenames to "": 172,856 bytes (-2,448 bytes / -1.4%)
  • Tock Imix kernel size, keeping filename only but trimming paths (e.g. kernel/src/sched.rs --> sched.rs): 162,196 bytes (-13,108 bytes / -7.5%)

Key things to note:

  • Tock uses -Z build-std=core,compiler-builtins by default to rebuild the core library with size optimizations. This means that not only does this approach remove all panic strings from the Tock binary, but also from the core library. Severely size constrained applications are likely to do this as well.
  • Tock uses remap-path-prefix for both the Tock root directory and the core library (necessary bc of the use of build-std), so paths are already shorter than they might be in more typical projects
  • When this issue was first created, I suspect the goal was to measure the overhead of #[track_caller] compared to the status quo before #[track_caller]. However, since https://github.com/libstd uses core::panic::Location where possible. #67137 , all panic locations are derived via #[track_caller], so any estimate of code size without #[track_caller] is really an estimate of code size with no panic locations in the binary at all (as opposed to an estimate where all unwrap/expect calls just use the same location in the core library).

Finally, I also took a measurement of code size with the compiler configured such that requires_caller_location() always returns false. This means that functions annotated with #[track_caller] do not get an extra argument added to pass a location. This is nice, because if we aren't using the location anyway the extra argument is just wasted size. The downside is that this change would break code not using build-std, and would break dylibs (I think).

  • Tock imix kernel size, track_caller completely disabled: 157,204 bytes (-18,100 bytes / -10.3%)

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Oct 22, 2021
…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?
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Oct 22, 2021
…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?
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Oct 23, 2021
…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?
@benpye
Copy link

benpye commented Aug 2, 2023

I think it would be interesting to allow putting the CallerLocation allocations in a different section - similar to the instrumentation change in #70579 (comment) . My thinking is that I would log the pointer to the core::panic::Location structure which would be in a no-load section of the binary - and so would be stripped from the flash image. A post processing tool could them take the log messages + the unstripped ELF and give you the original filename:line:column.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F-track_caller `#![feature(track_caller)]` I-heavy Issue: Problems and improvements with respect to binary size of generated code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

7 participants