about summary refs log tree commit diff
diff options
context:
space:
mode:
authorDylan DPC <99973273+Dylan-DPC@users.noreply.github.com>2022-04-29 23:54:43 +0200
committerGitHub <noreply@github.com>2022-04-29 23:54:43 +0200
commit548fca6927a644b4cde66893f5db6f4aff3f8f33 (patch)
treee8ccf5035b4e28eb52e7f8c09bb31b958c0f9338
parent2986bef53473d9c0692d3130d8289e234db0b2ea (diff)
parent3614bd3c45a8d0e5f27c0736e8966f6456a5cd75 (diff)
downloadrust-548fca6927a644b4cde66893f5db6f4aff3f8f33.tar.gz
rust-548fca6927a644b4cde66893f5db6f4aff3f8f33.zip
Rollup merge of #96562 - michaelwoerister:path-remapping-fixes, r=oli-obk
Fix duplicate directory separator in --remap-path-prefix.

The compiler will currently emit duplicate directory separators when `--remap-path-prefix` has an exact match of the working directory and it is invoked with a relative path to the main source file. For example

```bash
rustc src/main.rs -Cdebuginfo=2 --remap-path-prefix="$(pwd)=abc"
```

will produce the path `abc//src/main.rs` in debuginfo. This is because `some_path.join("")` will append a directory separator to `some_path` and then LLVM does not check if the working directory already ends a directory separator before concatenating it with the relative path.
-rw-r--r--compiler/rustc_span/src/source_map.rs14
-rw-r--r--compiler/rustc_span/src/source_map/tests.rs80
-rw-r--r--src/test/codegen/remap_path_prefix/main.rs2
3 files changed, 94 insertions, 2 deletions
diff --git a/compiler/rustc_span/src/source_map.rs b/compiler/rustc_span/src/source_map.rs
index 95177102dcf..460b5c18fc1 100644
--- a/compiler/rustc_span/src/source_map.rs
+++ b/compiler/rustc_span/src/source_map.rs
@@ -1102,7 +1102,19 @@ impl FilePathMapping {
         //       take precedence.
         for &(ref from, ref to) in self.mapping.iter().rev() {
             if let Ok(rest) = path.strip_prefix(from) {
-                return (to.join(rest), true);
+                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)
+                };
+
+                return (remapped, true);
             }
         }
 
diff --git a/compiler/rustc_span/src/source_map/tests.rs b/compiler/rustc_span/src/source_map/tests.rs
index f13979941ab..481e015c66c 100644
--- a/compiler/rustc_span/src/source_map/tests.rs
+++ b/compiler/rustc_span/src/source_map/tests.rs
@@ -312,3 +312,83 @@ impl SourceMapExtension for SourceMap {
         }
     }
 }
+
+fn map_path_prefix(mapping: &FilePathMapping, path: &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()
+}
+
+#[cfg(unix)]
+#[test]
+fn path_prefix_remapping() {
+    // Relative to relative
+    {
+        let mapping = &FilePathMapping::new(vec![("abc/def".into(), "foo".into())]);
+
+        assert_eq!(map_path_prefix(mapping, "abc/def/src/main.rs"), "foo/src/main.rs");
+        assert_eq!(map_path_prefix(mapping, "abc/def"), "foo");
+    }
+
+    // Relative to absolute
+    {
+        let mapping = &FilePathMapping::new(vec![("abc/def".into(), "/foo".into())]);
+
+        assert_eq!(map_path_prefix(mapping, "abc/def/src/main.rs"), "/foo/src/main.rs");
+        assert_eq!(map_path_prefix(mapping, "abc/def"), "/foo");
+    }
+
+    // Absolute to relative
+    {
+        let mapping = &FilePathMapping::new(vec![("/abc/def".into(), "foo".into())]);
+
+        assert_eq!(map_path_prefix(mapping, "/abc/def/src/main.rs"), "foo/src/main.rs");
+        assert_eq!(map_path_prefix(mapping, "/abc/def"), "foo");
+    }
+
+    // Absolute to absolute
+    {
+        let mapping = &FilePathMapping::new(vec![("/abc/def".into(), "/foo".into())]);
+
+        assert_eq!(map_path_prefix(mapping, "/abc/def/src/main.rs"), "/foo/src/main.rs");
+        assert_eq!(map_path_prefix(mapping, "/abc/def"), "/foo");
+    }
+}
+
+#[cfg(windows)]
+#[test]
+fn path_prefix_remapping_from_relative2() {
+    // Relative to relative
+    {
+        let mapping = &FilePathMapping::new(vec![("abc\\def".into(), "foo".into())]);
+
+        assert_eq!(map_path_prefix(mapping, "abc\\def\\src\\main.rs"), "foo\\src\\main.rs");
+        assert_eq!(map_path_prefix(mapping, "abc\\def"), "foo");
+    }
+
+    // Relative to absolute
+    {
+        let mapping = &FilePathMapping::new(vec![("abc\\def".into(), "X:\\foo".into())]);
+
+        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");
+    }
+
+    // Absolute to relative
+    {
+        let mapping = &FilePathMapping::new(vec![("X:\\abc\\def".into(), "foo".into())]);
+
+        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");
+    }
+
+    // Absolute to absolute
+    {
+        let mapping = &FilePathMapping::new(vec![("X:\\abc\\def".into(), "X:\\foo".into())]);
+
+        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");
+    }
+}
diff --git a/src/test/codegen/remap_path_prefix/main.rs b/src/test/codegen/remap_path_prefix/main.rs
index 698dfe6b4f3..b13d576295c 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: "/the/cwd"
 
 // And here that debuginfo from other crates are expanded to absolute paths.
 // CHECK: !DIFile(filename: "/the/aux-src/remap_path_prefix_aux.rs", directory: ""