about summary refs log tree commit diff
path: root/compiler/rustc_span/src
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2022-05-18 12:45:44 +0000
committerbors <bors@rust-lang.org>2022-05-18 12:45:44 +0000
commit936eba3b348e65b658b60218cc9237f02abdbeb4 (patch)
tree170ce02aaa1fca72948775fc8adaccf2952f82e7 /compiler/rustc_span/src
parente5732a21711e7cefa6eb22e1790406b269d6197a (diff)
parent6411fef3aba5ba54a02b54b171b4e9bc83687ce9 (diff)
downloadrust-936eba3b348e65b658b60218cc9237f02abdbeb4.tar.gz
rust-936eba3b348e65b658b60218cc9237f02abdbeb4.zip
Auto merge of #96867 - michaelwoerister:path-prefix-fixes-2, r=davidtwco
--remap-path-prefix: Fix duplicated path components in debuginfo

This PR fixes an issue with `--remap-path-prefix` where path components could appear twice in the remapped version of the path (e.g. https://github.com/rust-lang/rust/issues/78479). The underlying problem was that `--remap-path-prefix` is often used to map an absolute path to something that looks like a relative path, e.g.:

```
--remap-path-prefix=/home/calvin/.cargo/registry/src/github.com-1ecc6299db9ec823=crates.io",
```

and relative paths in debuginfo are interpreted as being relative to the compilation directory. So if Cargo invokes the compiler with `/home/calvin/.cargo/registry/src/github.com-1ecc6299db9ec823/some_crate-0.1.0/src/lib.rs` as input and `/home/calvin/.cargo/registry/src/github.com-1ecc6299db9ec823/some_crate-0.1.0` as the compiler's working directory, then debuginfo will state that the working directory was `crates.io/some_crate-0.1.0` and the file is question was `crates.io/some_crate-0.1.0/src/lib.rs`, which combined gives the path:

```
crates.io/some_crate-0.1.0/crates.io/some_crate-0.1.0/src/lib.rs
```

With this PR the compiler will detect this situation and set up debuginfo in LLVM in a way that makes it strip the duplicated path components when emitting DWARF.

The PR also extracts the logic for making remapped paths absolute into a common helper function that is now used by debuginfo too (instead of just during crate metadata generation).
Diffstat (limited to 'compiler/rustc_span/src')
-rw-r--r--compiler/rustc_span/src/lib.rs6
-rw-r--r--compiler/rustc_span/src/source_map.rs134
-rw-r--r--compiler/rustc_span/src/source_map/tests.rs169
3 files changed, 246 insertions, 63 deletions
diff --git a/compiler/rustc_span/src/lib.rs b/compiler/rustc_span/src/lib.rs
index 7357cebf62e..1f4578c08a3 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 505c0af953a..020ae3ad0c7 100644
--- a/compiler/rustc_span/src/source_map.rs
+++ b/compiler/rustc_span/src/source_map.rs
@@ -1098,28 +1098,45 @@ 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() {
+            // 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);
+        }
 
-                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) {
@@ -1140,4 +1157,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),
+                        }
+                    }
+                }
+            }
+        }
+    }
 }
diff --git a/compiler/rustc_span/src/source_map/tests.rs b/compiler/rustc_span/src/source_map/tests.rs
index 481e015c66c..be827cea874 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") }
+    );
 }