Skip to content

Commit

Permalink
Auto merge of #83813 - cbeuw:remap-std, r=michaelwoerister
Browse files Browse the repository at this point in the history
Fix `--remap-path-prefix` not correctly remapping `rust-src` component paths and unify handling of path mapping with virtualized paths

This PR fixes #73167 ("Binaries end up containing path to the rust-src component despite `--remap-path-prefix`") by preventing real local filesystem paths from reaching compilation output if the path is supposed to be remapped.

`RealFileName::Named` introduced in #72767 is now renamed as `LocalPath`, because this variant wraps a (most likely) valid local filesystem path.

`RealFileName::Devirtualized` is renamed as `Remapped` to be used for remapped path from a real path via `--remap-path-prefix` argument, as well as real path inferred from a virtualized (during compiler bootstrapping) `/rustc/...` path. The `local_path` field is now an `Option<PathBuf>`, as it will be set to `None` before serialisation, so it never reaches any build output. Attempting to serialise a non-`None` `local_path` will cause an assertion faliure.

When a path is remapped, a `RealFileName::Remapped` variant is created. The original path is preserved in `local_path` field and the remapped path is saved in `virtual_name` field. Previously, the `local_path` is directly modified which goes against its purpose of "suitable for reading from the file system on the local host".

`rustc_span::SourceFile`'s fields `unmapped_path` (introduced by #44940) and `name_was_remapped` (introduced by #41508 when `--remap-path-prefix` feature originally added) are removed, as these two pieces of information can be inferred from the `name` field: if it's anything other than a `FileName::Real(_)`, or if it is a `FileName::Real(RealFileName::LocalPath(_))`, then clearly `name_was_remapped` would've been false and `unmapped_path` would've been `None`. If it is a `FileName::Real(RealFileName::Remapped{local_path, virtual_name})`, then `name_was_remapped` would've been true and `unmapped_path` would've been `Some(local_path)`.

cc `@eddyb` who implemented `/rustc/...` path devirtualisation
  • Loading branch information
bors committed May 12, 2021
2 parents ac923d9 + 37dbe86 commit e1ff91f
Show file tree
Hide file tree
Showing 48 changed files with 442 additions and 265 deletions.
4 changes: 3 additions & 1 deletion compiler/rustc_builtin_macros/src/source_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,9 @@ pub fn expand_file(

let topmost = cx.expansion_cause().unwrap_or(sp);
let loc = cx.source_map().lookup_char_pos(topmost.lo());
base::MacEager::expr(cx.expr_str(topmost, Symbol::intern(&loc.file.name.to_string())))
base::MacEager::expr(
cx.expr_str(topmost, Symbol::intern(&loc.file.name.prefer_remapped().to_string_lossy())),
)
}

pub fn expand_stringify(
Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_codegen_cranelift/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,9 @@ impl<'tcx> FunctionCx<'_, '_, 'tcx> {
let topmost = span.ctxt().outer_expn().expansion_cause().unwrap_or(span);
let caller = self.tcx.sess.source_map().lookup_char_pos(topmost.lo());
let const_loc = self.tcx.const_caller_location((
rustc_span::symbol::Symbol::intern(&caller.file.name.to_string()),
rustc_span::symbol::Symbol::intern(
&caller.file.name.prefer_remapped().to_string_lossy(),
),
caller.line as u32,
caller.col_display as u32 + 1,
));
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_codegen_cranelift/src/debuginfo/line_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ fn line_program_add_file(
) -> FileId {
match &file.name {
FileName::Real(path) => {
let (dir_path, file_name) = split_path_dir_and_file(path.stable_name());
let (dir_path, file_name) = split_path_dir_and_file(path.remapped_path_if_available());
let dir_name = osstr_as_utf8_bytes(dir_path.as_os_str());
let file_name = osstr_as_utf8_bytes(file_name);

Expand All @@ -87,7 +87,7 @@ fn line_program_add_file(
filename => {
let dir_id = line_program.default_directory();
let dummy_file_name = LineString::new(
filename.to_string().into_bytes(),
filename.prefer_remapped().to_string().into_bytes(),
line_program.encoding(),
line_strings,
);
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_cranelift/src/debuginfo/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ impl<'tcx> DebugContext<'tcx> {
// FIXME: how to get version when building out of tree?
// Normally this would use option_env!("CFG_VERSION").
let producer = format!("cg_clif (rustc {})", "unknown version");
let comp_dir = tcx.sess.working_dir.0.to_string_lossy().into_owned();
let comp_dir = tcx.sess.working_dir.to_string_lossy(false).into_owned();
let (name, file_info) = match tcx.sess.local_crate_source_file.clone() {
Some(path) => {
let name = path.to_string_lossy().into_owned();
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -760,12 +760,12 @@ fn hex_encode(data: &[u8]) -> String {
}

pub fn file_metadata(cx: &CodegenCx<'ll, '_>, source_file: &SourceFile) -> &'ll DIFile {
debug!("file_metadata: file_name: {}", source_file.name);
debug!("file_metadata: file_name: {:?}", source_file.name);

let hash = Some(&source_file.src_hash);
let file_name = Some(source_file.name.to_string());
let file_name = Some(source_file.name.prefer_remapped().to_string());
let directory = if source_file.is_real_file() && !source_file.is_imported() {
Some(cx.sess().working_dir.0.to_string_lossy().to_string())
Some(cx.sess().working_dir.to_string_lossy(false).to_string())
} else {
// If the path comes from an upstream crate we assume it has been made
// independent of the compiler's working directory one way or another.
Expand Down Expand Up @@ -993,7 +993,7 @@ pub fn compile_unit_metadata(
let producer = format!("clang LLVM ({})", rustc_producer);

let name_in_debuginfo = name_in_debuginfo.to_string_lossy();
let work_dir = tcx.sess.working_dir.0.to_string_lossy();
let work_dir = tcx.sess.working_dir.to_string_lossy(false);
let flags = "\0";
let out_dir = &tcx.output_filenames(LOCAL_CRATE).out_directory;
let split_name = if tcx.sess.target_can_use_split_dwarf() {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_ssa/src/mir/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1144,7 +1144,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
let topmost = span.ctxt().outer_expn().expansion_cause().unwrap_or(span);
let caller = tcx.sess.source_map().lookup_char_pos(topmost.lo());
let const_loc = tcx.const_caller_location((
Symbol::intern(&caller.file.name.to_string()),
Symbol::intern(&caller.file.name.prefer_remapped().to_string_lossy()),
caller.line as u32,
caller.col_display as u32 + 1,
));
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,8 @@ impl AnnotateSnippetEmitterWriter {
}
// owned: line source, line index, annotations
type Owned = (String, usize, Vec<crate::snippet::Annotation>);
let origin = primary_lo.file.name.to_string();
let filename = primary_lo.file.name.prefer_local();
let origin = filename.to_string_lossy();
let annotated_files: Vec<Owned> = annotated_files
.into_iter()
.flat_map(|annotated_file| {
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_errors/src/emitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1324,7 +1324,7 @@ impl EmitterWriter {
buffer_msg_line_offset,
&format!(
"{}:{}:{}",
loc.file.name,
loc.file.name.prefer_local(),
sm.doctest_offset_line(&loc.file.name, loc.line),
loc.col.0 + 1,
),
Expand All @@ -1338,7 +1338,7 @@ impl EmitterWriter {
0,
&format!(
"{}:{}:{}: ",
loc.file.name,
loc.file.name.prefer_local(),
sm.doctest_offset_line(&loc.file.name, loc.line),
loc.col.0 + 1,
),
Expand All @@ -1362,12 +1362,12 @@ impl EmitterWriter {
};
format!(
"{}:{}{}",
annotated_file.file.name,
annotated_file.file.name.prefer_local(),
sm.doctest_offset_line(&annotated_file.file.name, first_line.line_index),
col
)
} else {
annotated_file.file.name.to_string()
format!("{}", annotated_file.file.name.prefer_local())
};
buffer.append(buffer_msg_line_offset + 1, &loc, Style::LineAndColumn);
for _ in 0..max_line_num_len {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_errors/src/json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,7 @@ impl DiagnosticSpan {
});

DiagnosticSpan {
file_name: start.file.name.to_string(),
file_name: start.file.name.prefer_local().to_string(),
byte_start: start.file.original_relative_byte_pos(span.lo()).0,
byte_end: start.file.original_relative_byte_pos(span.hi()).0,
line_start: start.line,
Expand Down
11 changes: 8 additions & 3 deletions compiler/rustc_expand/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1084,13 +1084,18 @@ impl<'a> ExtCtxt<'a> {
// after macro expansion (that is, they are unhygienic).
if !path.is_absolute() {
let callsite = span.source_callsite();
let mut result = match self.source_map().span_to_unmapped_path(callsite) {
FileName::Real(name) => name.into_local_path(),
let mut result = match self.source_map().span_to_filename(callsite) {
FileName::Real(name) => name
.into_local_path()
.expect("attempting to resolve a file path in an external file"),
FileName::DocTest(path, _) => path,
other => {
return Err(self.struct_span_err(
span,
&format!("cannot resolve relative path in non-file source `{}`", other),
&format!(
"cannot resolve relative path in non-file source `{}`",
other.prefer_local()
),
));
}
};
Expand Down
8 changes: 5 additions & 3 deletions compiler/rustc_expand/src/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -361,9 +361,11 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
// FIXME: Avoid visiting the crate as a `Mod` item,
// make crate a first class expansion target instead.
pub fn expand_crate(&mut self, mut krate: ast::Crate) -> ast::Crate {
let file_path = match self.cx.source_map().span_to_unmapped_path(krate.span) {
FileName::Real(name) => name.into_local_path(),
other => PathBuf::from(other.to_string()),
let file_path = match self.cx.source_map().span_to_filename(krate.span) {
FileName::Real(name) => name
.into_local_path()
.expect("attempting to resolve a file path in an external file"),
other => PathBuf::from(other.prefer_local().to_string()),
};
let dir_path = file_path.parent().unwrap_or(&file_path).to_owned();
self.cx.root_path = dir_path.clone();
Expand Down
7 changes: 4 additions & 3 deletions compiler/rustc_expand/src/proc_macro_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -638,10 +638,11 @@ impl server::SourceFile for Rustc<'_> {
match file.name {
FileName::Real(ref name) => name
.local_path()
.expect("attempting to get a file path in an imported file in `proc_macro::SourceFile::path`")
.to_str()
.expect("non-UTF8 file path in `proc_macro::SourceFile::path`")
.to_string(),
_ => file.name.to_string(),
_ => file.name.prefer_local().to_string(),
}
}
fn is_real(&mut self, file: &Self::SourceFile) -> bool {
Expand Down Expand Up @@ -785,7 +786,7 @@ fn ident_name_compatibility_hack(
if let ExpnKind::Macro { name: macro_name, .. } = orig_span.ctxt().outer_expn_data().kind {
let source_map = rustc.sess.source_map();
let filename = source_map.span_to_filename(orig_span);
if let FileName::Real(RealFileName::Named(path)) = filename {
if let FileName::Real(RealFileName::LocalPath(path)) = filename {
let matches_prefix = |prefix, filename| {
// Check for a path that ends with 'prefix*/src/<filename>'
let mut iter = path.components().rev();
Expand Down Expand Up @@ -848,7 +849,7 @@ fn ident_name_compatibility_hack(
if macro_name == sym::tuple_from_req && matches_prefix("actix-web", "extract.rs") {
let snippet = source_map.span_to_snippet(orig_span);
if snippet.as_deref() == Ok("$T") {
if let FileName::Real(RealFileName::Named(macro_path)) =
if let FileName::Real(RealFileName::LocalPath(macro_path)) =
source_map.span_to_filename(rustc.def_site)
{
if macro_path.to_string_lossy().contains("pin-project-internal-0.") {
Expand Down
16 changes: 11 additions & 5 deletions compiler/rustc_infer/src/infer/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1604,13 +1604,19 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
match (&terr, expected == found) {
(TypeError::Sorts(values), extra) => {
let sort_string = |ty: Ty<'tcx>| match (extra, ty.kind()) {
(true, ty::Opaque(def_id, _)) => format!(
" (opaque type at {})",
self.tcx
(true, ty::Opaque(def_id, _)) => {
let pos = self
.tcx
.sess
.source_map()
.mk_substr_filename(self.tcx.def_span(*def_id)),
),
.lookup_char_pos(self.tcx.def_span(*def_id).lo());
format!(
" (opaque type at <{}:{}:{}>)",
pos.file.name.prefer_local(),
pos.line,
pos.col.to_usize() + 1,
)
}
(true, _) => format!(" ({})", ty.sort_string(self.tcx)),
(false, _) => "".to_string(),
};
Expand Down
16 changes: 6 additions & 10 deletions compiler/rustc_interface/src/passes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ use rustc_session::output::{filename_for_input, filename_for_metadata};
use rustc_session::search_paths::PathKind;
use rustc_session::Session;
use rustc_span::symbol::{Ident, Symbol};
use rustc_span::{FileName, RealFileName};
use rustc_trait_selection::traits;
use rustc_typeck as typeck;
use tracing::{info, warn};
Expand Down Expand Up @@ -531,10 +530,10 @@ fn output_conflicts_with_dir(output_paths: &[PathBuf]) -> Option<PathBuf> {
check_output(output_paths, check)
}

fn escape_dep_filename(filename: &FileName) -> String {
fn escape_dep_filename(filename: &String) -> String {
// Apparently clang and gcc *only* escape spaces:
// http://llvm.org/klaus/clang/commit/9d50634cfc268ecc9a7250226dd5ca0e945240d4
filename.to_string().replace(" ", "\\ ")
filename.replace(" ", "\\ ")
}

// Makefile comments only need escaping newlines and `\`.
Expand Down Expand Up @@ -574,7 +573,7 @@ fn write_out_deps(
.iter()
.filter(|fmap| fmap.is_real_file())
.filter(|fmap| !fmap.is_imported())
.map(|fmap| escape_dep_filename(&fmap.unmapped_path.as_ref().unwrap_or(&fmap.name)))
.map(|fmap| escape_dep_filename(&fmap.name.prefer_local().to_string()))
.collect();

if let Some(ref backend) = sess.opts.debugging_opts.codegen_backend {
Expand All @@ -586,16 +585,13 @@ fn write_out_deps(
for cnum in resolver.cstore().crates_untracked() {
let source = resolver.cstore().crate_source_untracked(cnum);
if let Some((path, _)) = source.dylib {
let file_name = FileName::Real(RealFileName::Named(path));
files.push(escape_dep_filename(&file_name));
files.push(escape_dep_filename(&path.display().to_string()));
}
if let Some((path, _)) = source.rlib {
let file_name = FileName::Real(RealFileName::Named(path));
files.push(escape_dep_filename(&file_name));
files.push(escape_dep_filename(&path.display().to_string()));
}
if let Some((path, _)) = source.rmeta {
let file_name = FileName::Real(RealFileName::Named(path));
files.push(escape_dep_filename(&file_name));
files.push(escape_dep_filename(&path.display().to_string()));
}
}
});
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_interface/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -724,6 +724,7 @@ fn test_debugging_options_tracking_hash() {
tracked!(profile_emit, Some(PathBuf::from("abc")));
tracked!(relax_elf_relocations, Some(true));
tracked!(relro_level, Some(RelroLevel::Full));
tracked!(simulate_remapped_rust_src_base, Some(PathBuf::from("/rustc/abc")));
tracked!(report_delayed_bugs, true);
tracked!(sanitizer, SanitizerSet::ADDRESS);
tracked!(sanitizer_memory_track_origins, 2);
Expand Down
41 changes: 32 additions & 9 deletions compiler/rustc_metadata/src/rmeta/decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1651,9 +1651,11 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
if let Some(virtual_dir) = virtual_rust_source_base_dir {
if let Some(real_dir) = &sess.opts.real_rust_source_base_dir {
if let rustc_span::FileName::Real(old_name) = name {
if let rustc_span::RealFileName::Named(one_path) = old_name {
if let Ok(rest) = one_path.strip_prefix(virtual_dir) {
let virtual_name = one_path.clone();
if let rustc_span::RealFileName::Remapped { local_path: _, virtual_name } =
old_name
{
if let Ok(rest) = virtual_name.strip_prefix(virtual_dir) {
let virtual_name = virtual_name.clone();

// The std library crates are in
// `$sysroot/lib/rustlib/src/rust/library`, whereas other crates
Expand Down Expand Up @@ -1689,8 +1691,8 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
virtual_name.display(),
new_path.display(),
);
let new_name = rustc_span::RealFileName::Devirtualized {
local_path: new_path,
let new_name = rustc_span::RealFileName::Remapped {
local_path: Some(new_path),
virtual_name,
};
*old_name = new_name;
Expand All @@ -1710,7 +1712,6 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
// containing the information we need.
let rustc_span::SourceFile {
mut name,
name_was_remapped,
src_hash,
start_pos,
end_pos,
Expand All @@ -1722,11 +1723,34 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
..
} = source_file_to_import;

// If this file is under $sysroot/lib/rustlib/src/ but has not been remapped
// during rust bootstrapping by `remap-debuginfo = true`, and the user
// wish to simulate that behaviour by -Z simulate-remapped-rust-src-base,
// then we change `name` to a similar state as if the rust was bootstrapped
// with `remap-debuginfo = true`.
// This is useful for testing so that tests about the effects of
// `try_to_translate_virtual_to_real` don't have to worry about how the
// compiler is bootstrapped.
if let Some(virtual_dir) =
&sess.opts.debugging_opts.simulate_remapped_rust_src_base
{
if let Some(real_dir) = &sess.opts.real_rust_source_base_dir {
if let rustc_span::FileName::Real(ref mut old_name) = name {
if let rustc_span::RealFileName::LocalPath(local) = old_name {
if let Ok(rest) = local.strip_prefix(real_dir) {
*old_name = rustc_span::RealFileName::Remapped {
local_path: None,
virtual_name: virtual_dir.join(rest),
};
}
}
}
}
}

// If this file's path has been remapped to `/rustc/$hash`,
// we might be able to reverse that (also see comments above,
// on `try_to_translate_virtual_to_real`).
// FIXME(eddyb) we could check `name_was_remapped` here,
// but in practice it seems to be always `false`.
try_to_translate_virtual_to_real(&mut name);

let source_length = (end_pos - start_pos).to_usize();
Expand All @@ -1751,7 +1775,6 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {

let local_version = sess.source_map().new_imported_source_file(
name,
name_was_remapped,
src_hash,
name_hash,
source_length,
Expand Down
Loading

0 comments on commit e1ff91f

Please sign in to comment.