From 90fce805a35abd58b779cf7cabef533d56f16788 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Thu, 5 May 2022 17:01:39 +0200 Subject: [PATCH 1/4] Add debug tracing to FilePathMapping::map_prefix --- compiler/rustc_span/src/source_map.rs | 53 +++++++++++++++++---------- 1 file changed, 34 insertions(+), 19 deletions(-) diff --git a/compiler/rustc_span/src/source_map.rs b/compiler/rustc_span/src/source_map.rs index 505c0af953af1..03e82dde967ad 100644 --- a/compiler/rustc_span/src/source_map.rs +++ b/compiler/rustc_span/src/source_map.rs @@ -1098,28 +1098,43 @@ impl FilePathMapping { /// The return value is the remapped path and a boolean indicating whether /// the path was affected by the mapping. pub fn map_prefix(&self, path: PathBuf) -> (PathBuf, bool) { - // NOTE: We are iterating over the mapping entries from last to first - // because entries specified later on the command line should - // take precedence. - for &(ref from, ref to) in self.mapping.iter().rev() { - if let Ok(rest) = path.strip_prefix(from) { - let remapped = if rest.as_os_str().is_empty() { - // This is subtle, joining an empty path onto e.g. `foo/bar` will - // result in `foo/bar/`, that is, there'll be an additional directory - // separator at the end. This can lead to duplicated directory separators - // in remapped paths down the line. - // So, if we have an exact match, we just return that without a call - // to `Path::join()`. - to.clone() - } else { - to.join(rest) - }; + if path.as_os_str().is_empty() { + return (path, false); + } - return (remapped, true); + return remap_path_prefix(&self.mapping, path); + + #[instrument(level = "debug", skip(mapping))] + fn remap_path_prefix(mapping: &[(PathBuf, PathBuf)], path: PathBuf) -> (PathBuf, bool) { + // NOTE: We are iterating over the mapping entries from last to first + // because entries specified later on the command line should + // take precedence. + for &(ref from, ref to) in mapping.iter().rev() { + debug!("Trying to apply {:?} => {:?}", from, to); + + if let Ok(rest) = path.strip_prefix(from) { + let remapped = if rest.as_os_str().is_empty() { + // This is subtle, joining an empty path onto e.g. `foo/bar` will + // result in `foo/bar/`, that is, there'll be an additional directory + // separator at the end. This can lead to duplicated directory separators + // in remapped paths down the line. + // So, if we have an exact match, we just return that without a call + // to `Path::join()`. + to.clone() + } else { + to.join(rest) + }; + debug!("Match - remapped {:?} => {:?}", path, remapped); + + return (remapped, true); + } else { + debug!("No match - prefix {:?} does not match {:?}", from, path); + } } - } - (path, false) + debug!("Path {:?} was not remapped", path); + (path, false) + } } fn map_filename_prefix(&self, file: &FileName) -> (FileName, bool) { From 583880b0ff3a04712e66426d63b7474dc74c2165 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Thu, 5 May 2022 17:03:51 +0200 Subject: [PATCH 2/4] Move logic for making potentially remapped paths absolute into helper method. --- compiler/rustc_metadata/src/rmeta/encoder.rs | 93 ++++++-------------- compiler/rustc_span/src/source_map.rs | 79 +++++++++++++++++ 2 files changed, 108 insertions(+), 64 deletions(-) diff --git a/compiler/rustc_metadata/src/rmeta/encoder.rs b/compiler/rustc_metadata/src/rmeta/encoder.rs index 39e6ddb316d71..a382209e20670 100644 --- a/compiler/rustc_metadata/src/rmeta/encoder.rs +++ b/compiler/rustc_metadata/src/rmeta/encoder.rs @@ -33,18 +33,14 @@ use rustc_middle::ty::{self, SymbolName, Ty, TyCtxt}; use rustc_serialize::{opaque, Encodable, Encoder}; use rustc_session::config::CrateType; use rustc_session::cstore::{ForeignModule, LinkagePreference, NativeLib}; +use rustc_span::hygiene::{ExpnIndex, HygieneEncodeContext, MacroKind}; use rustc_span::symbol::{sym, Ident, Symbol}; use rustc_span::{ self, DebuggerVisualizerFile, ExternalSource, FileName, SourceFile, Span, SyntaxContext, }; -use rustc_span::{ - hygiene::{ExpnIndex, HygieneEncodeContext, MacroKind}, - RealFileName, -}; use rustc_target::abi::VariantIdx; use std::hash::Hash; use std::num::NonZeroUsize; -use std::path::Path; use tracing::{debug, trace}; pub(super) struct EncodeContext<'a, 'tcx> { @@ -490,6 +486,8 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { // is done. let required_source_files = self.required_source_files.take().unwrap(); + let working_directory = &self.tcx.sess.opts.working_dir; + let adapted = all_source_files .iter() .enumerate() @@ -502,66 +500,33 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { (!source_file.is_imported() || self.is_proc_macro) }) .map(|(_, source_file)| { - let mut adapted = match source_file.name { - FileName::Real(ref realname) => { - let mut adapted = (**source_file).clone(); - adapted.name = FileName::Real(match realname { - RealFileName::LocalPath(path_to_file) => { - // Prepend path of working directory onto potentially - // relative paths, because they could become relative - // to a wrong directory. - // We include `working_dir` as part of the crate hash, - // so it's okay for us to use it as part of the encoded - // metadata. - let working_dir = &self.tcx.sess.opts.working_dir; - match working_dir { - RealFileName::LocalPath(absolute) => { - // Although neither working_dir or the file name were subject - // to path remapping, the concatenation between the two may - // be. Hence we need to do a remapping here. - let joined = Path::new(absolute).join(path_to_file); - let (joined, remapped) = - source_map.path_mapping().map_prefix(joined); - if remapped { - RealFileName::Remapped { - local_path: None, - virtual_name: joined, - } - } else { - RealFileName::LocalPath(joined) - } - } - RealFileName::Remapped { local_path: _, virtual_name } => { - // If working_dir has been remapped, then we emit - // Remapped variant as the expanded path won't be valid - RealFileName::Remapped { - local_path: None, - virtual_name: Path::new(virtual_name) - .join(path_to_file), - } - } - } - } - RealFileName::Remapped { local_path: _, virtual_name } => { - RealFileName::Remapped { - // We do not want any local path to be exported into metadata - local_path: None, - virtual_name: virtual_name.clone(), - } - } - }); - adapted.name_hash = { - let mut hasher: StableHasher = StableHasher::new(); - adapted.name.hash(&mut hasher); - hasher.finish::() - }; - Lrc::new(adapted) + match source_file.name { + FileName::Real(ref original_file_name) => { + let adapted_file_name = + source_map.path_mapping().to_embeddable_absolute_path( + original_file_name.clone(), + working_directory, + ); + + if adapted_file_name != *original_file_name { + let mut adapted: SourceFile = (**source_file).clone(); + adapted.name = FileName::Real(adapted_file_name); + adapted.name_hash = { + let mut hasher: StableHasher = StableHasher::new(); + adapted.name.hash(&mut hasher); + hasher.finish::() + }; + Lrc::new(adapted) + } else { + // Nothing to adapt + source_file.clone() + } } - // expanded code, not from a file _ => source_file.clone(), - }; - + } + }) + .map(|mut source_file| { // We're serializing this `SourceFile` into our crate metadata, // so mark it as coming from this crate. // This also ensures that we don't try to deserialize the @@ -569,9 +534,9 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { // dependencies aren't loaded when we deserialize a proc-macro, // trying to remap the `CrateNum` would fail. if self.is_proc_macro { - Lrc::make_mut(&mut adapted).cnum = LOCAL_CRATE; + Lrc::make_mut(&mut source_file).cnum = LOCAL_CRATE; } - adapted + source_file }) .collect::>(); diff --git a/compiler/rustc_span/src/source_map.rs b/compiler/rustc_span/src/source_map.rs index 03e82dde967ad..44aa9f72809f6 100644 --- a/compiler/rustc_span/src/source_map.rs +++ b/compiler/rustc_span/src/source_map.rs @@ -1155,4 +1155,83 @@ impl FilePathMapping { other => (other.clone(), false), } } + + /// Expand a relative path to an absolute path with remapping taken into account. + /// Use this when absolute paths are required (e.g. debuginfo or crate metadata). + /// + /// The resulting `RealFileName` will have its `local_path` portion erased if + /// possible (i.e. if there's also a remapped path). + pub fn to_embeddable_absolute_path( + &self, + file_path: RealFileName, + working_directory: &RealFileName, + ) -> RealFileName { + match file_path { + // Anything that's already remapped we don't modify, except for erasing + // the `local_path` portion. + RealFileName::Remapped { local_path: _, virtual_name } => { + RealFileName::Remapped { + // We do not want any local path to be exported into metadata + local_path: None, + // We use the remapped name verbatim, even if it looks like a relative + // path. The assumption is that the user doesn't want us to further + // process paths that have gone through remapping. + virtual_name, + } + } + + RealFileName::LocalPath(unmapped_file_path) => { + // If no remapping has been applied yet, try to do so + let (new_path, was_remapped) = self.map_prefix(unmapped_file_path); + if was_remapped { + // It was remapped, so don't modify further + return RealFileName::Remapped { local_path: None, virtual_name: new_path }; + } + + if new_path.is_absolute() { + // No remapping has applied to this path and it is absolute, + // so the working directory cannot influence it either, so + // we are done. + return RealFileName::LocalPath(new_path); + } + + debug_assert!(new_path.is_relative()); + let unmapped_file_path_rel = new_path; + + match working_directory { + RealFileName::LocalPath(unmapped_working_dir_abs) => { + let file_path_abs = unmapped_working_dir_abs.join(unmapped_file_path_rel); + + // Although neither `working_directory` nor the file name were subject + // to path remapping, the concatenation between the two may be. Hence + // we need to do a remapping here. + let (file_path_abs, was_remapped) = self.map_prefix(file_path_abs); + if was_remapped { + RealFileName::Remapped { + // Erase the actual path + local_path: None, + virtual_name: file_path_abs, + } + } else { + // No kind of remapping applied to this path, so + // we leave it as it is. + RealFileName::LocalPath(file_path_abs) + } + } + RealFileName::Remapped { + local_path: _, + virtual_name: remapped_working_dir_abs, + } => { + // If working_directory has been remapped, then we emit + // Remapped variant as the expanded path won't be valid + RealFileName::Remapped { + local_path: None, + virtual_name: Path::new(remapped_working_dir_abs) + .join(unmapped_file_path_rel), + } + } + } + } + } + } } From 9e7b0ff2e11fba83c5d87cf871e6531d94edb2e5 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Thu, 5 May 2022 17:05:08 +0200 Subject: [PATCH 3/4] Add tests for FilePathMapping::to_embeddable_absolute_path(). --- compiler/rustc_span/src/source_map/tests.rs | 169 +++++++++++++++----- 1 file changed, 128 insertions(+), 41 deletions(-) diff --git a/compiler/rustc_span/src/source_map/tests.rs b/compiler/rustc_span/src/source_map/tests.rs index 481e015c66c25..be827cea8744b 100644 --- a/compiler/rustc_span/src/source_map/tests.rs +++ b/compiler/rustc_span/src/source_map/tests.rs @@ -313,82 +313,169 @@ impl SourceMapExtension for SourceMap { } } -fn map_path_prefix(mapping: &FilePathMapping, path: &str) -> String { +// Takes a unix-style path and returns a platform specific path. +fn path(p: &str) -> PathBuf { + path_str(p).into() +} + +// Takes a unix-style path and returns a platform specific path. +fn path_str(p: &str) -> String { + #[cfg(not(windows))] + { + return p.into(); + } + + #[cfg(windows)] + { + let mut path = p.replace('/', "\\"); + if let Some(rest) = path.strip_prefix('\\') { + path = ["X:\\", rest].concat(); + } + + path + } +} + +fn map_path_prefix(mapping: &FilePathMapping, p: &str) -> String { // It's important that we convert to a string here because that's what // later stages do too (e.g. in the backend), and comparing `Path` values // won't catch some differences at the string level, e.g. "abc" and "abc/" // compare as equal. - mapping.map_prefix(path.into()).0.to_string_lossy().to_string() + mapping.map_prefix(path(p)).0.to_string_lossy().to_string() } -#[cfg(unix)] #[test] fn path_prefix_remapping() { // Relative to relative { - let mapping = &FilePathMapping::new(vec![("abc/def".into(), "foo".into())]); + let mapping = &FilePathMapping::new(vec![(path("abc/def"), path("foo"))]); - assert_eq!(map_path_prefix(mapping, "abc/def/src/main.rs"), "foo/src/main.rs"); - assert_eq!(map_path_prefix(mapping, "abc/def"), "foo"); + assert_eq!(map_path_prefix(mapping, "abc/def/src/main.rs"), path_str("foo/src/main.rs")); + assert_eq!(map_path_prefix(mapping, "abc/def"), path_str("foo")); } // Relative to absolute { - let mapping = &FilePathMapping::new(vec![("abc/def".into(), "/foo".into())]); + let mapping = &FilePathMapping::new(vec![(path("abc/def"), path("/foo"))]); - assert_eq!(map_path_prefix(mapping, "abc/def/src/main.rs"), "/foo/src/main.rs"); - assert_eq!(map_path_prefix(mapping, "abc/def"), "/foo"); + assert_eq!(map_path_prefix(mapping, "abc/def/src/main.rs"), path_str("/foo/src/main.rs")); + assert_eq!(map_path_prefix(mapping, "abc/def"), path_str("/foo")); } // Absolute to relative { - let mapping = &FilePathMapping::new(vec![("/abc/def".into(), "foo".into())]); + let mapping = &FilePathMapping::new(vec![(path("/abc/def"), path("foo"))]); - assert_eq!(map_path_prefix(mapping, "/abc/def/src/main.rs"), "foo/src/main.rs"); - assert_eq!(map_path_prefix(mapping, "/abc/def"), "foo"); + assert_eq!(map_path_prefix(mapping, "/abc/def/src/main.rs"), path_str("foo/src/main.rs")); + assert_eq!(map_path_prefix(mapping, "/abc/def"), path_str("foo")); } // Absolute to absolute { - let mapping = &FilePathMapping::new(vec![("/abc/def".into(), "/foo".into())]); + let mapping = &FilePathMapping::new(vec![(path("/abc/def"), path("/foo"))]); - assert_eq!(map_path_prefix(mapping, "/abc/def/src/main.rs"), "/foo/src/main.rs"); - assert_eq!(map_path_prefix(mapping, "/abc/def"), "/foo"); + assert_eq!(map_path_prefix(mapping, "/abc/def/src/main.rs"), path_str("/foo/src/main.rs")); + assert_eq!(map_path_prefix(mapping, "/abc/def"), path_str("/foo")); } } -#[cfg(windows)] #[test] -fn path_prefix_remapping_from_relative2() { - // Relative to relative - { - let mapping = &FilePathMapping::new(vec![("abc\\def".into(), "foo".into())]); +fn path_prefix_remapping_expand_to_absolute() { + // "virtual" working directory is relative path + let mapping = + &FilePathMapping::new(vec![(path("/foo"), path("FOO")), (path("/bar"), path("BAR"))]); + let working_directory = path("/foo"); + let working_directory = RealFileName::Remapped { + local_path: Some(working_directory.clone()), + virtual_name: mapping.map_prefix(working_directory).0, + }; + + assert_eq!(working_directory.remapped_path_if_available(), path("FOO")); + + // Unmapped absolute path + assert_eq!( + mapping.to_embeddable_absolute_path( + RealFileName::LocalPath(path("/foo/src/main.rs")), + &working_directory + ), + RealFileName::Remapped { local_path: None, virtual_name: path("FOO/src/main.rs") } + ); - assert_eq!(map_path_prefix(mapping, "abc\\def\\src\\main.rs"), "foo\\src\\main.rs"); - assert_eq!(map_path_prefix(mapping, "abc\\def"), "foo"); - } + // Unmapped absolute path with unrelated working directory + assert_eq!( + mapping.to_embeddable_absolute_path( + RealFileName::LocalPath(path("/bar/src/main.rs")), + &working_directory + ), + RealFileName::Remapped { local_path: None, virtual_name: path("BAR/src/main.rs") } + ); - // Relative to absolute - { - let mapping = &FilePathMapping::new(vec![("abc\\def".into(), "X:\\foo".into())]); + // Unmapped absolute path that does not match any prefix + assert_eq!( + mapping.to_embeddable_absolute_path( + RealFileName::LocalPath(path("/quux/src/main.rs")), + &working_directory + ), + RealFileName::LocalPath(path("/quux/src/main.rs")), + ); - assert_eq!(map_path_prefix(mapping, "abc\\def\\src\\main.rs"), "X:\\foo\\src\\main.rs"); - assert_eq!(map_path_prefix(mapping, "abc\\def"), "X:\\foo"); - } + // Unmapped relative path + assert_eq!( + mapping.to_embeddable_absolute_path( + RealFileName::LocalPath(path("src/main.rs")), + &working_directory + ), + RealFileName::Remapped { local_path: None, virtual_name: path("FOO/src/main.rs") } + ); - // Absolute to relative - { - let mapping = &FilePathMapping::new(vec![("X:\\abc\\def".into(), "foo".into())]); + // Unmapped relative path with `./` + assert_eq!( + mapping.to_embeddable_absolute_path( + RealFileName::LocalPath(path("./src/main.rs")), + &working_directory + ), + RealFileName::Remapped { local_path: None, virtual_name: path("FOO/src/main.rs") } + ); - assert_eq!(map_path_prefix(mapping, "X:\\abc\\def\\src\\main.rs"), "foo\\src\\main.rs"); - assert_eq!(map_path_prefix(mapping, "X:\\abc\\def"), "foo"); - } + // Unmapped relative path that does not match any prefix + assert_eq!( + mapping.to_embeddable_absolute_path( + RealFileName::LocalPath(path("quux/src/main.rs")), + &RealFileName::LocalPath(path("/abc")), + ), + RealFileName::LocalPath(path("/abc/quux/src/main.rs")), + ); - // Absolute to absolute - { - let mapping = &FilePathMapping::new(vec![("X:\\abc\\def".into(), "X:\\foo".into())]); + // Already remapped absolute path + assert_eq!( + mapping.to_embeddable_absolute_path( + RealFileName::Remapped { + local_path: Some(path("/foo/src/main.rs")), + virtual_name: path("FOO/src/main.rs"), + }, + &working_directory + ), + RealFileName::Remapped { local_path: None, virtual_name: path("FOO/src/main.rs") } + ); - assert_eq!(map_path_prefix(mapping, "X:\\abc\\def\\src\\main.rs"), "X:\\foo\\src\\main.rs"); - assert_eq!(map_path_prefix(mapping, "X:\\abc\\def"), "X:\\foo"); - } + // Already remapped absolute path, with unrelated working directory + assert_eq!( + mapping.to_embeddable_absolute_path( + RealFileName::Remapped { + local_path: Some(path("/bar/src/main.rs")), + virtual_name: path("BAR/src/main.rs"), + }, + &working_directory + ), + RealFileName::Remapped { local_path: None, virtual_name: path("BAR/src/main.rs") } + ); + + // Already remapped relative path + assert_eq!( + mapping.to_embeddable_absolute_path( + RealFileName::Remapped { local_path: None, virtual_name: path("XYZ/src/main.rs") }, + &working_directory + ), + RealFileName::Remapped { local_path: None, virtual_name: path("XYZ/src/main.rs") } + ); } From 6411fef3aba5ba54a02b54b171b4e9bc83687ce9 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Thu, 5 May 2022 17:26:22 +0200 Subject: [PATCH 4/4] Properly apply path prefix remapping paths emitted into debuginfo. --- .../src/debuginfo/metadata.rs | 162 ++++++++++-------- .../rustc_codegen_llvm/src/debuginfo/mod.rs | 4 +- compiler/rustc_metadata/src/rmeta/encoder.rs | 7 + compiler/rustc_span/src/lib.rs | 6 +- compiler/rustc_span/src/source_map.rs | 2 + src/bootstrap/dist.rs | 1 + src/test/codegen/remap_path_prefix/main.rs | 2 +- .../run-make/remap-path-prefix-dwarf/Makefile | 77 +++++++++ .../remap-path-prefix-dwarf/src/quux.rs | 5 + 9 files changed, 193 insertions(+), 73 deletions(-) create mode 100644 src/test/run-make/remap-path-prefix-dwarf/Makefile create mode 100644 src/test/run-make/remap-path-prefix-dwarf/src/quux.rs diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs b/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs index f2cf3b1ef5c1e..97d3acb34ce75 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs @@ -36,20 +36,21 @@ use rustc_middle::ty::subst::GenericArgKind; use rustc_middle::ty::{self, AdtKind, Instance, ParamEnv, Ty, TyCtxt, COMMON_VTABLE_ENTRIES}; use rustc_session::config::{self, DebugInfo}; use rustc_span::symbol::Symbol; +use rustc_span::FileName; use rustc_span::FileNameDisplayPreference; -use rustc_span::{self, SourceFile, SourceFileHash}; +use rustc_span::{self, SourceFile}; use rustc_target::abi::{Align, Size}; use smallvec::smallvec; use tracing::debug; use libc::{c_longlong, c_uint}; use std::borrow::Cow; -use std::collections::hash_map::Entry; use std::fmt::{self, Write}; use std::hash::{Hash, Hasher}; use std::iter; use std::path::{Path, PathBuf}; use std::ptr; +use tracing::instrument; impl PartialEq for llvm::Metadata { fn eq(&self, other: &Self) -> bool { @@ -527,78 +528,105 @@ fn hex_encode(data: &[u8]) -> String { } pub fn file_metadata<'ll>(cx: &CodegenCx<'ll, '_>, source_file: &SourceFile) -> &'ll DIFile { - debug!("file_metadata: file_name: {:?}", source_file.name); - - let hash = Some(&source_file.src_hash); - 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() - .opts - .working_dir - .to_string_lossy(FileNameDisplayPreference::Remapped) - .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. - None - }; - file_metadata_raw(cx, file_name, directory, hash) -} - -pub fn unknown_file_metadata<'ll>(cx: &CodegenCx<'ll, '_>) -> &'ll DIFile { - file_metadata_raw(cx, None, None, None) -} - -fn file_metadata_raw<'ll>( - cx: &CodegenCx<'ll, '_>, - file_name: Option, - directory: Option, - hash: Option<&SourceFileHash>, -) -> &'ll DIFile { - let key = (file_name, directory); - - match debug_context(cx).created_files.borrow_mut().entry(key) { - Entry::Occupied(o) => o.get(), - Entry::Vacant(v) => { - let (file_name, directory) = v.key(); - debug!("file_metadata: file_name: {:?}, directory: {:?}", file_name, directory); - - let file_name = file_name.as_deref().unwrap_or(""); - let directory = directory.as_deref().unwrap_or(""); - - let (hash_kind, hash_value) = match hash { - Some(hash) => { - let kind = match hash.kind { - rustc_span::SourceFileHashAlgorithm::Md5 => llvm::ChecksumKind::MD5, - rustc_span::SourceFileHashAlgorithm::Sha1 => llvm::ChecksumKind::SHA1, - rustc_span::SourceFileHashAlgorithm::Sha256 => llvm::ChecksumKind::SHA256, - }; - (kind, hex_encode(hash.hash_bytes())) + let cache_key = Some((source_file.name_hash, source_file.src_hash)); + return debug_context(cx) + .created_files + .borrow_mut() + .entry(cache_key) + .or_insert_with(|| alloc_new_file_metadata(cx, source_file)); + + #[instrument(skip(cx, source_file), level = "debug")] + fn alloc_new_file_metadata<'ll>( + cx: &CodegenCx<'ll, '_>, + source_file: &SourceFile, + ) -> &'ll DIFile { + debug!(?source_file.name); + + let (directory, file_name) = match &source_file.name { + FileName::Real(filename) => { + let working_directory = &cx.sess().opts.working_dir; + debug!(?working_directory); + + let filename = cx + .sess() + .source_map() + .path_mapping() + .to_embeddable_absolute_path(filename.clone(), working_directory); + + // Construct the absolute path of the file + let abs_path = filename.remapped_path_if_available(); + debug!(?abs_path); + + if let Ok(rel_path) = + abs_path.strip_prefix(working_directory.remapped_path_if_available()) + { + // If the compiler's working directory (which also is the DW_AT_comp_dir of + // the compilation unit) is a prefix of the path we are about to emit, then + // only emit the part relative to the working directory. + // Because of path remapping we sometimes see strange things here: `abs_path` + // might actually look like a relative path + // (e.g. `/src/lib.rs`), so if we emit it without + // taking the working directory into account, downstream tooling will + // interpret it as `//src/lib.rs`, + // which makes no sense. Usually in such cases the working directory will also + // be remapped to `` or some other prefix of the path + // we are remapping, so we end up with + // `//src/lib.rs`. + // By moving the working directory portion into the `directory` part of the + // DIFile, we allow LLVM to emit just the relative path for DWARF, while + // still emitting the correct absolute path for CodeView. + ( + working_directory.to_string_lossy(FileNameDisplayPreference::Remapped), + rel_path.to_string_lossy().into_owned(), + ) + } else { + ("".into(), abs_path.to_string_lossy().into_owned()) } - None => (llvm::ChecksumKind::None, String::new()), - }; + } + other => ("".into(), other.prefer_remapped().to_string_lossy().into_owned()), + }; - let file_metadata = unsafe { - llvm::LLVMRustDIBuilderCreateFile( - DIB(cx), - file_name.as_ptr().cast(), - file_name.len(), - directory.as_ptr().cast(), - directory.len(), - hash_kind, - hash_value.as_ptr().cast(), - hash_value.len(), - ) - }; + let hash_kind = match source_file.src_hash.kind { + rustc_span::SourceFileHashAlgorithm::Md5 => llvm::ChecksumKind::MD5, + rustc_span::SourceFileHashAlgorithm::Sha1 => llvm::ChecksumKind::SHA1, + rustc_span::SourceFileHashAlgorithm::Sha256 => llvm::ChecksumKind::SHA256, + }; + let hash_value = hex_encode(source_file.src_hash.hash_bytes()); - v.insert(file_metadata); - file_metadata + unsafe { + llvm::LLVMRustDIBuilderCreateFile( + DIB(cx), + file_name.as_ptr().cast(), + file_name.len(), + directory.as_ptr().cast(), + directory.len(), + hash_kind, + hash_value.as_ptr().cast(), + hash_value.len(), + ) } } } +pub fn unknown_file_metadata<'ll>(cx: &CodegenCx<'ll, '_>) -> &'ll DIFile { + debug_context(cx).created_files.borrow_mut().entry(None).or_insert_with(|| unsafe { + let file_name = ""; + let directory = ""; + let hash_value = ""; + + llvm::LLVMRustDIBuilderCreateFile( + DIB(cx), + file_name.as_ptr().cast(), + file_name.len(), + directory.as_ptr().cast(), + directory.len(), + llvm::ChecksumKind::None, + hash_value.as_ptr().cast(), + hash_value.len(), + ) + }) +} + trait MsvcBasicName { fn msvc_basic_name(self) -> &'static str; } diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs b/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs index 6a164557a4719..0910e7c94ea12 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs @@ -31,7 +31,7 @@ use rustc_middle::ty::{self, Instance, ParamEnv, Ty, TypeFoldable}; use rustc_session::config::{self, DebugInfo}; use rustc_session::Session; use rustc_span::symbol::Symbol; -use rustc_span::{self, BytePos, Pos, SourceFile, SourceFileAndLine, Span}; +use rustc_span::{self, BytePos, Pos, SourceFile, SourceFileAndLine, SourceFileHash, Span}; use rustc_target::abi::Size; use libc::c_uint; @@ -61,7 +61,7 @@ pub struct CodegenUnitDebugContext<'ll, 'tcx> { llcontext: &'ll llvm::Context, llmod: &'ll llvm::Module, builder: &'ll mut DIBuilder<'ll>, - created_files: RefCell, Option), &'ll DIFile>>, + created_files: RefCell, &'ll DIFile>>, type_map: metadata::TypeMap<'ll, 'tcx>, namespace_map: RefCell>, diff --git a/compiler/rustc_metadata/src/rmeta/encoder.rs b/compiler/rustc_metadata/src/rmeta/encoder.rs index a382209e20670..086f1bd94b6c0 100644 --- a/compiler/rustc_metadata/src/rmeta/encoder.rs +++ b/compiler/rustc_metadata/src/rmeta/encoder.rs @@ -500,6 +500,13 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { (!source_file.is_imported() || self.is_proc_macro) }) .map(|(_, source_file)| { + // At export time we expand all source file paths to absolute paths because + // downstream compilation sessions can have a different compiler working + // directory, so relative paths from this or any other upstream crate + // won't be valid anymore. + // + // At this point we also erase the actual on-disk path and only keep + // the remapped version -- as is necessary for reproducible builds. match source_file.name { FileName::Real(ref original_file_name) => { let adapted_file_name = diff --git a/compiler/rustc_span/src/lib.rs b/compiler/rustc_span/src/lib.rs index 7357cebf62eb9..1f4578c08a3f2 100644 --- a/compiler/rustc_span/src/lib.rs +++ b/compiler/rustc_span/src/lib.rs @@ -335,8 +335,8 @@ impl fmt::Display for FileNameDisplay<'_> { } } -impl FileNameDisplay<'_> { - pub fn to_string_lossy(&self) -> Cow<'_, str> { +impl<'a> FileNameDisplay<'a> { + pub fn to_string_lossy(&self) -> Cow<'a, str> { match self.inner { FileName::Real(ref inner) => inner.to_string_lossy(self.display_pref), _ => Cow::from(format!("{}", self)), @@ -1153,7 +1153,7 @@ impl FromStr for SourceFileHashAlgorithm { } /// The hash of the on-disk source file used for debug info. -#[derive(Copy, Clone, PartialEq, Eq, Debug)] +#[derive(Copy, Clone, PartialEq, Eq, Debug, Hash)] #[derive(HashStable_Generic, Encodable, Decodable)] pub struct SourceFileHash { pub kind: SourceFileHashAlgorithm, diff --git a/compiler/rustc_span/src/source_map.rs b/compiler/rustc_span/src/source_map.rs index 44aa9f72809f6..020ae3ad0c78c 100644 --- a/compiler/rustc_span/src/source_map.rs +++ b/compiler/rustc_span/src/source_map.rs @@ -1099,6 +1099,8 @@ impl FilePathMapping { /// the path was affected by the mapping. pub fn map_prefix(&self, path: PathBuf) -> (PathBuf, bool) { if path.as_os_str().is_empty() { + // Exit early if the path is empty and therefore there's nothing to remap. + // This is mostly to reduce spam for `RUSTC_LOG=[remap_path_prefix]`. return (path, false); } diff --git a/src/bootstrap/dist.rs b/src/bootstrap/dist.rs index 6181a611ec315..16727f4398dff 100644 --- a/src/bootstrap/dist.rs +++ b/src/bootstrap/dist.rs @@ -2047,6 +2047,7 @@ impl Step for RustDev { "llvm-cov", "llvm-dwp", "llvm-nm", + "llvm-dwarfdump", ] { tarball.add_file(src_bindir.join(exe(bin, target)), "bin", 0o755); } diff --git a/src/test/codegen/remap_path_prefix/main.rs b/src/test/codegen/remap_path_prefix/main.rs index b13d576295c60..381f11ff1efcc 100644 --- a/src/test/codegen/remap_path_prefix/main.rs +++ b/src/test/codegen/remap_path_prefix/main.rs @@ -22,7 +22,7 @@ fn main() { } // Here we check that local debuginfo is mapped correctly. -// CHECK: !DIFile(filename: "/the/src/remap_path_prefix/main.rs", directory: "/the/cwd" +// CHECK: !DIFile(filename: "/the/src/remap_path_prefix/main.rs", directory: "" // And here that debuginfo from other crates are expanded to absolute paths. // CHECK: !DIFile(filename: "/the/aux-src/remap_path_prefix_aux.rs", directory: "" diff --git a/src/test/run-make/remap-path-prefix-dwarf/Makefile b/src/test/run-make/remap-path-prefix-dwarf/Makefile new file mode 100644 index 0000000000000..561a343d60b94 --- /dev/null +++ b/src/test/run-make/remap-path-prefix-dwarf/Makefile @@ -0,0 +1,77 @@ +# This test makes sure that --remap-path-prefix has the expected effects on paths in debuginfo. +# It tests several cases, each of them has a detailed description attached to it. + +# ignore-windows + +SRC_DIR := $(abspath .) +SRC_DIR_PARENT := $(abspath ..) + +-include ../../run-make-fulldeps/tools.mk + +all: \ + abs_input_outside_working_dir \ + rel_input_remap_working_dir \ + rel_input_remap_working_dir_parent \ + rel_input_remap_working_dir_child \ + abs_input_inside_working_dir \ + abs_input_outside_working_dir + +# The compiler is called with an *ABSOLUTE PATH* as input, and that absolute path *is* within +# the working directory of the compiler. We are remapping the path that contains `src`. +abs_input_inside_working_dir: + # We explicitly switch to a directory that *is* a prefix of the directory our + # source code is contained in. + cd $(SRC_DIR) && $(RUSTC) $(SRC_DIR)/src/quux.rs -o "$(TMPDIR)/abs_input_inside_working_dir.rlib" -Cdebuginfo=2 --remap-path-prefix $(SRC_DIR)=REMAPPED + # We expect the path to the main source file to be remapped. + "$(LLVM_BIN_DIR)"/llvm-dwarfdump $(TMPDIR)/abs_input_inside_working_dir.rlib | $(CGREP) "REMAPPED/src/quux.rs" + # No weird duplication of remapped components (see #78479) + "$(LLVM_BIN_DIR)"/llvm-dwarfdump $(TMPDIR)/abs_input_inside_working_dir.rlib | $(CGREP) -v "REMAPPED/REMAPPED" + +# The compiler is called with an *ABSOLUTE PATH* as input, and that absolute path is *not* within +# the working directory of the compiler. We are remapping both the path that contains `src` and +# the working directory to the same thing. This setup corresponds to a workaround that is needed +# when trying to remap everything to something that looks like a local path. +# Relative paths are interpreted as relative to the compiler's working directory (e.g. in +# debuginfo). If we also remap the working directory, the compiler strip it from other paths so +# that the final outcome is the desired one again. +abs_input_outside_working_dir: + # We explicitly switch to a directory that is *not* a prefix of the directory our + # source code is contained in. + cd $(TMPDIR) && $(RUSTC) $(SRC_DIR)/src/quux.rs -o "$(TMPDIR)/abs_input_outside_working_dir.rlib" -Cdebuginfo=2 --remap-path-prefix $(SRC_DIR)=REMAPPED --remap-path-prefix $(TMPDIR)=REMAPPED + "$(LLVM_BIN_DIR)"/llvm-dwarfdump $(TMPDIR)/abs_input_outside_working_dir.rlib | $(CGREP) "REMAPPED/src/quux.rs" + # No weird duplication of remapped components (see #78479) + "$(LLVM_BIN_DIR)"/llvm-dwarfdump $(TMPDIR)/abs_input_outside_working_dir.rlib | $(CGREP) -v "REMAPPED/REMAPPED" + +# The compiler is called with a *RELATIVE PATH* as input. We are remapping the working directory of +# the compiler, which naturally is an implicit prefix of our relative input path. Debuginfo will +# expand the relative path to an absolute path and we expect the working directory to be remapped +# in that expansion. +rel_input_remap_working_dir: + cd $(SRC_DIR) && $(RUSTC) src/quux.rs -o "$(TMPDIR)/rel_input_remap_working_dir.rlib" -Cdebuginfo=2 --remap-path-prefix "$(SRC_DIR)=REMAPPED" + "$(LLVM_BIN_DIR)"/llvm-dwarfdump "$(TMPDIR)/rel_input_remap_working_dir.rlib" | $(CGREP) "REMAPPED/src/quux.rs" + # No weird duplication of remapped components (see #78479) + "$(LLVM_BIN_DIR)"/llvm-dwarfdump "$(TMPDIR)/rel_input_remap_working_dir.rlib" | $(CGREP) -v "REMAPPED/REMAPPED" + +# The compiler is called with a *RELATIVE PATH* as input. We are remapping a *SUB-DIRECTORY* of the +# compiler's working directory. This test makes sure that that directory is remapped even though it +# won't actually show up in this form in the compiler's SourceMap and instead is only constructed +# on demand during debuginfo generation. +rel_input_remap_working_dir_child: + cd $(SRC_DIR) && $(RUSTC) src/quux.rs -o "$(TMPDIR)/rel_input_remap_working_dir_child.rlib" -Cdebuginfo=2 --remap-path-prefix "$(SRC_DIR)/src=REMAPPED" + # We expect `src/quux.rs` to have been remapped to `REMAPPED/quux.rs`. + "$(LLVM_BIN_DIR)"/llvm-dwarfdump "$(TMPDIR)/rel_input_remap_working_dir_child.rlib" | $(CGREP) "REMAPPED/quux.rs" + # We don't want to find the path that we just remapped anywhere in the DWARF + "$(LLVM_BIN_DIR)"/llvm-dwarfdump "$(TMPDIR)/rel_input_remap_working_dir_child.rlib" | $(CGREP) -v "$(SRC_DIR)/src" + # No weird duplication of remapped components (see #78479) + "$(LLVM_BIN_DIR)"/llvm-dwarfdump "$(TMPDIR)/rel_input_remap_working_dir_child.rlib" | $(CGREP) -v "REMAPPED/REMAPPED" + +# The compiler is called with a *RELATIVE PATH* as input. We are remapping a *PARENT DIRECTORY* of +# the compiler's working directory. +rel_input_remap_working_dir_parent: + cd $(SRC_DIR) && $(RUSTC) src/quux.rs -o "$(TMPDIR)/rel_input_remap_working_dir_parent.rlib" -Cdebuginfo=2 --remap-path-prefix "$(SRC_DIR_PARENT)=REMAPPED" + # We expect `src/quux.rs` to have been remapped to `REMAPPED/remap-path-prefix-dwarf/src/quux.rs`. + "$(LLVM_BIN_DIR)"/llvm-dwarfdump "$(TMPDIR)/rel_input_remap_working_dir_parent.rlib" | $(CGREP) "REMAPPED/remap-path-prefix-dwarf/src/quux.rs" + # We don't want to find the path that we just remapped anywhere in the DWARF + "$(LLVM_BIN_DIR)"/llvm-dwarfdump "$(TMPDIR)/rel_input_remap_working_dir_parent.rlib" | $(CGREP) -v "$(SRC_DIR_PARENT)" + # No weird duplication of remapped components (see #78479) + "$(LLVM_BIN_DIR)"/llvm-dwarfdump "$(TMPDIR)/rel_input_remap_working_dir_parent.rlib" | $(CGREP) -v "REMAPPED/REMAPPED" diff --git a/src/test/run-make/remap-path-prefix-dwarf/src/quux.rs b/src/test/run-make/remap-path-prefix-dwarf/src/quux.rs new file mode 100644 index 0000000000000..38d5ef6194cd2 --- /dev/null +++ b/src/test/run-make/remap-path-prefix-dwarf/src/quux.rs @@ -0,0 +1,5 @@ +#![crate_type = "rlib"] + +pub fn foo() { + println!("foo"); +}