Stabilize -Zremap-path-scope

[Urgau]

Stabilization report of --remap-path-scope

Summary

RFC 3127 trim-paths aims to improve the current status of sanitizing paths emitted by the compiler via the --remap-path-prefix=FROM=TO command line flag, by offering a profile setting named trim-paths in Cargo to sanitize absolute paths introduced during compilation that may be embedded in the compiled binary executable or library.

As part of that RFC the compiler was asked to add the --remap-path-scope command-line flag to control the scoping of how paths get remapped in the resulting binary.

Tracking:

• Tracking Issue for trim-paths RFC 3127 #111540

What is stabilized

The rustc --remap-path-scope flag is being stabilized by this PR. It defines which scopes of paths should be remapped by --remap-path-prefix.

This flag accepts a comma-separated list of values and may be specified multiple times, in which case the scopes are aggregated together.

The valid scopes are:

• macro - apply remappings to the expansion of std::file!() macro. This is where paths in embedded panic messages come from
• diagnostics - apply remappings to printed compiler diagnostics
• debuginfo - apply remappings to debug informations
• coverage - apply remappings to coverage informations
• object - apply remappings to all paths in compiled executables or libraries, but not elsewhere. Currently an alias for macro,coverage,debuginfo.
• all (default) - an alias for all of the above, also equivalent to supplying only --remap-path-prefix without --remap-path-scope.

Example

# With `object` scope only the build outputs will be remapped, the diagnostics won't be remapped.
rustc --remap-path-prefix=$(PWD)=/remapped --remap-path-scope=object main.rs

What isn't stabilized

None of the Cargo facility is being stabilized in this stabilization PR, only the --remap-path-scope flag in rustc is being stabilized.

Design

RFC history

• RFC3127 - trim-paths

Answers to unresolved questions

What questions were left unresolved by the RFC? How have they been answered? Link to any relevant lang decisions.

There are no unresolved questions regarding --remap-path-scope.

(The tracking issue list a bunch of unresolved questions but they are for --remap-path-prefix or the bigger picture trim-paths in Cargo and are not related the functionality provided by --remap-path-scope.)

Post-RFC changes

The RFC described more scopes, in particularly regarding split debuginfo. Those scopes where removed after analysis by michaelwoerister of all the possible combinations in #111540 (comment).

Nightly extensions

There are no nightly extensions.

Doors closed

We are committing to having to having a flag that control which paths are being remapped based on a "scope".

Feedback

Call for testing

Has a "call for testing" been done? If so, what feedback was received?

No call for testing has been done per se but feedback has been received on both the rust-lang/rust and rust-lang/cargo tracking issue.

The feedback was mainly related to deficiencies in our best-effort --remap-path-prefix implementation, in particular regarding linkers added paths, which does not change anything for --remap-path-scope.

Nightly use

Do any known nightly users use this feature? Counting instances of #![feature(FEATURE_NAME)] on GitHub with grep might be informative.

Except for Cargo unstable trim-paths there doesn't appear any committed use on GitHub.

Implementation

Major parts

• rust/compiler/rustc_session/src/config.rs

  Lines 1373 to 1384 in b3f8586

  	pub struct RemapPathScopeComponents: u8 {
  	/// Apply remappings to the expansion of std::file!() macro
  	const MACRO = 1 << 0;
  	/// Apply remappings to printed compiler diagnostics
  	const DIAGNOSTICS = 1 << 1;
  	/// Apply remappings to debug information
  	const DEBUGINFO = 1 << 3;
  	/// An alias for `macro` and `debuginfo`. This ensures all paths in compiled
  	/// executables or libraries are remapped but not elsewhere.
  	const OBJECT = Self::MACRO.bits() | Self::DEBUGINFO.bits();
  	}

• rust/compiler/rustc_session/src/session.rs

  Line 1526 in b3f8586

  fn for_scope(&self, sess: &Session, scope: RemapPathScopeComponents) -> Self::Output<'_>;

• rust/compiler/rustc_span/src/lib.rs

  Lines 352 to 372 in b3f8586

  	pub enum FileNameEmbeddablePreference {
  	/// If a remapped path is available, only embed the `virtual_path` and omit the `local_path`.
  	///
  	/// Otherwise embed the local-path into the `virtual_path`.
  	RemappedOnly,
  	/// Embed the original path as well as its remapped `virtual_path` component if available.
  	LocalAndRemapped,
  	}
  	#[derive(Clone, Copy, Eq, PartialEq, Hash, Debug)]
  	pub enum FileNameDisplayPreference {
  	/// Display the path after the application of rewrite rules provided via `--remap-path-prefix`.
  	/// This is appropriate for paths that get embedded into files produced by the compiler.
  	Remapped,
  	/// Display the path before the application of rewrite rules provided via `--remap-path-prefix`.
  	/// This is appropriate for use in user-facing output (such as diagnostics).
  	Local,
  	/// Display only the filename, as a way to reduce the verbosity of the output.
  	/// This is appropriate for use in user-facing output (such as diagnostics).
  	Short,
  	}

Coverage

• tests/run-make/split-debuginfo/rmake.rs
• tests/ui/errors/remap-path-prefix.rs
• tests/ui/errors/remap-path-prefix-macro.rs
• tests/run-make/remap-path-prefix-dwarf/rmake.rs
• tests/run-make/remap-path-prefix/rmake.rs
• tests/ui/errors/remap-path-prefix-diagnostics.rs

Outstanding bugs

What outstanding bugs involve this feature? List them. Should any block the stabilization? Discuss why or why not.

There are no outstanding bugs regarding --remap-path-scope.

Outstanding FIXMEs

What FIXMEs are still in the code for that feature and why is it OK to leave them there?

There are no FIXME regarding --remap-path-scope in it-self.

Tool changes

What changes must be made to our other tools to support this feature. Has this work been done? Link to any relevant PRs and issues.

• rustdoc (both JSON, HTML and doctest)
  • rustdoc has support for --remap-path-prefix, it should probably also get support for --remap-path-scope, although rustdoc maybe want to adapt the scopes for it's use (replace debuginfo with documentation for example).

History

List issues and PRs that are important for understanding how we got here.

• Implement rustc part of RFC 3127 trim-paths #115214
• Simplify trim-paths feature by merging all debuginfo options together #122450
• Fix -Zremap-path-scope rmeta handling #139550
• Improve -Zremap-path-scope tests with dependency #140716

Acknowledgments

Summarize contributors to the feature by name for recognition and so that those people are notified about the stabilization. Does anyone who worked on this not think it should be stabilized right now? We'd like to hear about that if so.

• @cbeuw
• @michaelwoerister
• @weihanglo
• @Urgau

@rustbot labels +T-compiler +needs-fcp +F-trim-paths
r? @davidtwco

[weihanglo]

Thanks for driving the stabilization!

There are outstanding bugs regarding --remap-path-scope.

"There are no outstanding bugs" probably?

Nightly use

Do any known nightly users use this feature? Counting instances of #![feature(FEATURE_NAME)] on GitHub with grep might be informative.

Except for Cargo unstable trim-paths there doesn't appear any committed use on GitHub.

Don't want to block the stabilization. However, this sounds a bit odd that there is no major use cases beyond Cargo and rustc is heading toward stabilization alone. That may imply t-cargo should review again whether --remap-path-scope is useful and cover what they want at least (of course this is on me as I worked on Cargo side of this)

[Urgau]

"There are no outstanding bugs" probably?

As far as I know --remap-path-scope has no bugs. We have test for all the scopes, with/without dependency and with mixed scopes between crates.

However, this sounds a bit odd that there is no major use cases beyond Cargo and rustc is heading toward stabilization alone.

It doesn't really surprises me that there are no public use of it. It's quite a pain currently to setup --remap-path-prefix and -Zremap-path-scope in Cargo, as there is currently no support for it, which is what we are trying to solve. I suspect most users are using RUSTFLAGS which I don't think is typically committed.

That may imply t-cargo should review again whether --remap-path-scope is useful

Well, be able to see the not-remapped paths in diagnostics is quite nice thing.

The RFC specifically states that the default should be object for the release profile.

[weihanglo]

"There are no outstanding bugs" probably?

I meant the "no" is missing in the original PR description.

[weihanglo]

The other thing is that do we want to stabilize all options all at once. I remember that the RFC said we could optionally stabilize some of them. I am not sure whether macros and debuginfo standalone are useful, as well as diagnostics. The most useful thing as you mentioned is probably object and the original behavior all.

[davidtwco]

I don't know that macros or debuginfo would need to be useful on their own to be worth stabilising, we're able to support them relatively straightforwardly and they may be useful in combination with other options.

[Zalathar]

Currently the macro scope is also applied to the source file paths embedded in coverage mapping metadata, presumably due to the lack of any more suitable scope.

(Historically I believe they were under debuginfo, but that was eventually changed because coverage metadata is technically not debuginfo, and doesn't interact with split-debug settings.)

Tools like llvm-cov use those embedded paths not just for display to the user, but also for automatically locating and reading source files to prepare coverage reports. The llvm-cov tool does have support for remapping paths, though it would need to be configured by the user, and I'm not familiar with its fine details and limitations.

• Is this known and acknowledged as part of the stabilization?
• What are the stability constraints on changing this in the future, e.g. by adding a coverage scope or a separate catch-all scope under object?

[davidtwco]

What are the stability constraints on changing this in the future, e.g. by adding a coverage scope or a separate catch-all scope under object?

We should document that new coverage scopes may be added in the future. coverage seems like a reasonable example of such a scope.

[davidtwco]

Implementation LGTM

View changes since this review

[davidtwco]

@rfcbot fcp merge

[rust-rfcbot]

Team member @davidtwco has proposed to merge this. The next step is review by the rest of the tagged team members:

• @Mark-Simulacrum
• @Noratrieb
• @SparrowLii
• @cjgillot
• @davidtwco
• @estebank
• @jieyouxu
• @matthewjasper
• @nagisa
• @oli-obk
• @petrochenkov
• @spastorino
• @wesleywiser

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[nagisa]

I don't see this discussed here, but would specifying different remappings for each of the scope be something that somebody might want to do? If this came up, how would this design evolve to support such a use-case?

[Urgau]

Regarding #147611 (comment):

Tools like llvm-cov use those embedded paths not just for display to the user, but also for automatically locating and reading source files to prepare coverage reports. The llvm-cov tool does have support for remapping paths, though it would need to be configured by the user, and I'm not familiar with its fine details and limitations.

• Is this known and acknowledged as part of the stabilization?

This PR only stabilizes -Zremap-path-scope, which interacts with the already stable --remap-path-prefix, so this PR doesn't change anything regarding the current situation.

People using custom remap prefixes¹ already needs to manually any tell the tools they use how to un-remap the different prefixes they used.

• What are the stability constraints on changing this in the future, e.g. by adding a coverage scope or a separate catch-all scope under object?

As proposed by David I have adjusted the documentation to mention that the list of scopes is not exhaustive and may be increased in the future.

Footnotes

1. We provide support for the /rustc/ prefix in our rust-gdb script. ↩

[Urgau]

Currently the macro scope is also applied to the source file paths embedded in coverage mapping metadata, presumably due to the lack of any more suitable scope.

This is sub-optimal indeed :-/ We should either create a dedicated scope for it or put it under debuginfo.

EDIT: This has now been done in this PR. There is now a coverage scope.

(Historically I believe they were under debuginfo, but that was eventually changed because coverage metadata is technically not debuginfo, and doesn't interact with split-debug settings.)

Note that contrary to the RFC, the debuginfo scope doesn't have anymore any relationship with split debuginfo (see #111540 (comment) for more details).

[Urgau]

I don't see this discussed here, but would specifying different remappings for each of the scope be something that somebody might want to do? If this came up, how would this design evolve to support such a use-case?

It's been mentioned in #142917, but only as a workaround for some current limitations¹ and I prefer fixing the limitations rather than introducing workarounds.

Therefore, handling different remappings for each scope has not been very well thought out, but if we wanted to do that in the future I think we should do it on --remap-path-prefix directly (as --remap-path-scope is the global config) by introducing a parameter like so --remap-path-prefix /tmp=debuginfo:/virtual or --remap-path-prefix /tmp=/virtual:debuginfo.

Footnotes

1. In Support different paths remapping for different scopes #142917, Apple linker is undermining us by introducing absolute paths in the final binary for which we have little control over. (And to be clear, it's a issue for --remap-path-prefix not -Zremap-path-scope and it's explicitly allowed in our docs) ↩

[rustbot]

Some changes occurred in coverage instrumentation.

cc @Zalathar

Some changes occurred in coverage tests.

cc @Zalathar

[jieyouxu]

Nit: this is probably obvious, but it might be worth spelling out that, by implication, all thus necessarily can correspond to different scopes between releases.