[Clang] Restore replace_path_prefix instead of startswith

In D49466, sys::path::replace_path_prefix was used instead startswith for -f[macro/debug/file]-prefix-map options. However those were reverted later (commit rG3bb24bf25767ef5bbcef958b484e7a06d8689204) due to broken Windows tests. This patch restores those replace_path_prefix calls. It also modifies the prefix matching to be case-insensitive under Windows. Differential Revision : https://reviews.llvm.org/D76869
llvm · May 13, 2020 · 7a8edcb · 7a8edcb
1 parent 33d96bf
commit 7a8edcb
Show file tree

Hide file tree

Showing 10 changed files with 125 additions and 39 deletions.
diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -470,10 +470,14 @@ CGDebugInfo::createFile(StringRef FileName,
 }
 
 std::string CGDebugInfo::remapDIPath(StringRef Path) const {
+  if (DebugPrefixMap.empty())
+    return Path.str();
+
+  SmallString<256> P = Path;
   for (const auto &Entry : DebugPrefixMap)
-    if (Path.startswith(Entry.first))
-      return (Twine(Entry.second) + Path.substr(Entry.first.size())).str();
-  return Path.str();
+    if (llvm::sys::path::replace_path_prefix(P, Entry.first, Entry.second))
+      break;
+  return P.str().str();
 }
 
 unsigned CGDebugInfo::getLineNumber(SourceLocation Loc) {

diff --git a/clang/lib/Lex/PPMacroExpansion.cpp b/clang/lib/Lex/PPMacroExpansion.cpp
@@ -1456,10 +1456,8 @@ static void remapMacroPath(
     const std::map<std::string, std::string, std::greater<std::string>>
         &MacroPrefixMap) {
   for (const auto &Entry : MacroPrefixMap)
-    if (Path.startswith(Entry.first)) {
-      Path = (Twine(Entry.second) + Path.substr(Entry.first.size())).str();
+    if (llvm::sys::path::replace_path_prefix(Path, Entry.first, Entry.second))
       break;
-    }
 }
 
 /// ExpandBuiltinMacro - If an identifier token is read that is to be expanded
@@ -1543,8 +1541,8 @@ void Preprocessor::ExpandBuiltinMacro(Token &Tok) {
       } else {
         FN += PLoc.getFilename();
       }
-      Lexer::Stringify(FN);
       remapMacroPath(FN, PPOpts->MacroPrefixMap);
+      Lexer::Stringify(FN);
       OS << '"' << FN << '"';
     }
     Tok.setKind(tok::string_literal);

diff --git a/clang/test/Preprocessor/file_test.h → ...ssor/Inputs/include-file-test/file_test.h b/clang/test/Preprocessor/file_test.h → ...ssor/Inputs/include-file-test/file_test.h
diff --git a/clang/test/Preprocessor/file_test.c b/clang/test/Preprocessor/file_test.c
@@ -1,23 +1,23 @@
-// XFAIL: system-windows
+// UNSUPPORTED: system-windows
 // RUN: %clang -E -ffile-prefix-map=%p=/UNLIKELY_PATH/empty -c -o - %s | FileCheck %s
 // RUN: %clang -E -fmacro-prefix-map=%p=/UNLIKELY_PATH/empty -c -o - %s | FileCheck %s
 // RUN: %clang -E -fmacro-prefix-map=%p=/UNLIKELY_PATH=empty -c -o - %s | FileCheck %s -check-prefix CHECK-EVIL
 // RUN: %clang -E -fmacro-prefix-map=%p/= -c -o - %s | FileCheck %s --check-prefix CHECK-REMOVE
 
 filename: __FILE__
-#include "file_test.h"
+#include "Inputs/include-file-test/file_test.h"
 
-// CHECK: filename: "/UNLIKELY_PATH/empty{{/|\\\\}}file_test.c"
-// CHECK: filename: "/UNLIKELY_PATH/empty{{/|\\\\}}file_test.h"
-// CHECK: basefile: "/UNLIKELY_PATH/empty{{/|\\\\}}file_test.c"
+// CHECK: filename: "/UNLIKELY_PATH/empty/file_test.c"
+// CHECK: filename: "/UNLIKELY_PATH/empty/Inputs/include-file-test/file_test.h"
+// CHECK: basefile: "/UNLIKELY_PATH/empty/file_test.c"
 // CHECK-NOT: filename:
 
-// CHECK-EVIL: filename: "/UNLIKELY_PATH=empty{{/|\\\\}}file_test.c"
-// CHECK-EVIL: filename: "/UNLIKELY_PATH=empty{{/|\\\\}}file_test.h"
-// CHECK-EVIL: basefile: "/UNLIKELY_PATH=empty{{/|\\\\}}file_test.c"
+// CHECK-EVIL: filename: "/UNLIKELY_PATH=empty/file_test.c"
+// CHECK-EVIL: filename: "/UNLIKELY_PATH=empty/Inputs/include-file-test/file_test.h"
+// CHECK-EVIL: basefile: "/UNLIKELY_PATH=empty/file_test.c"
 // CHECK-EVIL-NOT: filename:
 
 // CHECK-REMOVE: filename: "file_test.c"
-// CHECK-REMOVE: filename: "file_test.h"
+// CHECK-REMOVE: filename: "Inputs/include-file-test/file_test.h"
 // CHECK-REMOVE: basefile: "file_test.c"
 // CHECK-REMOVE-NOT: filename:
diff --git a/clang/test/Preprocessor/file_test_windows.c b/clang/test/Preprocessor/file_test_windows.c
@@ -0,0 +1,29 @@
+// REQUIRES: system-windows
+// RUN: %clang -E -ffile-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | FileCheck %s
+// RUN: %clang -E -fmacro-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | FileCheck %s
+// RUN: %clang -E -fmacro-prefix-map=%p=A:\UNLIKELY_PATH=empty -c -o - %s | FileCheck %s -check-prefix CHECK-EVIL
+// RUN: %clang -E -fmacro-prefix-map=%p/iNPUTS\=A:\UNLIKELY_PATH_INC\ -fmacro-prefix-map=%p/=A:\UNLIKELY_PATH_BASE\ -c -o - %s | FileCheck %s -check-prefix CHECK-CASE
+// RUN: %clang -E -fmacro-prefix-map=%p\= -c -o - %s | FileCheck %s --check-prefix CHECK-REMOVE
+
+filename: __FILE__
+#include "Inputs/include-file-test/file_test.h"
+
+// CHECK: filename: "A:\\UNLIKELY_PATH\\empty\\file_test_windows.c"
+// CHECK: filename: "A:\\UNLIKELY_PATH\\empty/Inputs/include-file-test/file_test.h"
+// CHECK: basefile: "A:\\UNLIKELY_PATH\\empty\\file_test_windows.c"
+// CHECK-NOT: filename:
+
+// CHECK-EVIL: filename: "A:\\UNLIKELY_PATH=empty\\file_test_windows.c"
+// CHECK-EVIL: filename: "A:\\UNLIKELY_PATH=empty/Inputs/include-file-test/file_test.h"
+// CHECK-EVIL: basefile: "A:\\UNLIKELY_PATH=empty\\file_test_windows.c"
+// CHECK-EVIL-NOT: filename:
+
+// CHECK-CASE: filename: "A:\\UNLIKELY_PATH_BASE\\file_test_windows.c"
+// CHECK-CASE: filename: "A:\\UNLIKELY_PATH_INC\\include-file-test/file_test.h"
+// CHECK-CASE: basefile: "A:\\UNLIKELY_PATH_BASE\\file_test_windows.c"
+// CHECK-CASE-NOT: filename:
+
+// CHECK-REMOVE: filename: "file_test_windows.c"
+// CHECK-REMOVE: filename: "Inputs/include-file-test/file_test.h"
+// CHECK-REMOVE: basefile: "file_test_windows.c"
+// CHECK-REMOVE-NOT: filename:
diff --git a/llvm/include/llvm/Support/Path.h b/llvm/include/llvm/Support/Path.h
@@ -167,9 +167,12 @@ void replace_extension(SmallVectorImpl<char> &path, const Twine &extension,
 ///        start with \a NewPrefix.
 /// @param OldPrefix The path prefix to strip from \a Path.
 /// @param NewPrefix The path prefix to replace \a NewPrefix with.
+/// @param style The style used to match the prefix. Exact match using
+/// Posix style, case/separator insensitive match for Windows style.
 /// @result true if \a Path begins with OldPrefix
 bool replace_path_prefix(SmallVectorImpl<char> &Path, StringRef OldPrefix,
-                         StringRef NewPrefix);
+                         StringRef NewPrefix,
+                         Style style = Style::native);
 
 /// Append to path.
 ///

diff --git a/llvm/lib/DWARFLinker/DWARFLinker.cpp b/llvm/lib/DWARFLinker/DWARFLinker.cpp
@@ -1941,10 +1941,14 @@ static uint64_t getDwoId(const DWARFDie &CUDie, const DWARFUnit &Unit) {
 
 static std::string remapPath(StringRef Path,
                              const objectPrefixMap &ObjectPrefixMap) {
+  if (ObjectPrefixMap.empty())
+    return Path.str();
+
+  SmallString<256> p = Path;
   for (const auto &Entry : ObjectPrefixMap)
-    if (Path.startswith(Entry.first))
-      return (Twine(Entry.second) + Path.substr(Entry.first.size())).str();
-  return Path.str();
+    if (llvm::sys::path::replace_path_prefix(p, Entry.first, Entry.second))
+      break;
+  return p.str().str();
 }
 
 bool DWARFLinker::registerModuleReference(

diff --git a/llvm/lib/MC/MCContext.cpp b/llvm/lib/MC/MCContext.cpp
@@ -642,13 +642,17 @@ void MCContext::addDebugPrefixMapEntry(const std::string &From,
 
 void MCContext::RemapDebugPaths() {
   const auto &DebugPrefixMap = this->DebugPrefixMap;
+  if (DebugPrefixMap.empty())
+    return;
+
   const auto RemapDebugPath = [&DebugPrefixMap](std::string &Path) {
-    for (const auto &Entry : DebugPrefixMap)
-      if (StringRef(Path).startswith(Entry.first)) {
-        std::string RemappedPath =
-            (Twine(Entry.second) + Path.substr(Entry.first.size())).str();
-        Path.swap(RemappedPath);
+    SmallString<256> P(Path);
+    for (const auto &Entry : DebugPrefixMap) {
+      if (llvm::sys::path::replace_path_prefix(P, Entry.first, Entry.second)) {
+        Path = P.str().str();
+        break;
       }
+    }
   };
 
   // Remap compilation directory.

diff --git a/llvm/lib/Support/Path.cpp b/llvm/lib/Support/Path.cpp
@@ -496,13 +496,32 @@ void replace_extension(SmallVectorImpl<char> &path, const Twine &extension,
   path.append(ext.begin(), ext.end());
 }
 
+static bool starts_with(StringRef Path, StringRef Prefix,
+                        Style style = Style::native) {
+  // Windows prefix matching : case and separator insensitive
+  if (real_style(style) == Style::windows) {
+    if (Path.size() < Prefix.size())
+      return false;
+    for (size_t I = 0, E = Prefix.size(); I != E; ++I) {
+      bool SepPath = is_separator(Path[I], style);
+      bool SepPrefix = is_separator(Prefix[I], style);
+      if (SepPath != SepPrefix)
+        return false;
+      if (!SepPath && toLower(Path[I]) != toLower(Prefix[I]))
+        return false;
+    }
+    return true;
+  }
+  return Path.startswith(Prefix);
+}
+
 bool replace_path_prefix(SmallVectorImpl<char> &Path, StringRef OldPrefix,
-                         StringRef NewPrefix) {
+                         StringRef NewPrefix, Style style) {
   if (OldPrefix.empty() && NewPrefix.empty())
     return false;
 
   StringRef OrigPath(Path.begin(), Path.size());
-  if (!OrigPath.startswith(OldPrefix))
+  if (!starts_with(OrigPath, OldPrefix, style))
     return false;
 
   // If prefixes have the same size we can simply copy the new one over.

diff --git a/llvm/unittests/Support/Path.cpp b/llvm/unittests/Support/Path.cpp
@@ -1311,48 +1311,73 @@ TEST(Support, ReplacePathPrefix) {
   SmallString<64> Path1("/foo");
   SmallString<64> Path2("/old/foo");
   SmallString<64> Path3("/oldnew/foo");
+  SmallString<64> Path4("C:\\old/foo\\bar");
   SmallString<64> OldPrefix("/old");
   SmallString<64> OldPrefixSep("/old/");
+  SmallString<64> OldPrefixWin("c:/oLD/F");
   SmallString<64> NewPrefix("/new");
   SmallString<64> NewPrefix2("/longernew");
   SmallString<64> EmptyPrefix("");
+  bool Found;
 
   SmallString<64> Path = Path1;
-  path::replace_path_prefix(Path, OldPrefix, NewPrefix);
+  Found = path::replace_path_prefix(Path, OldPrefix, NewPrefix);
+  EXPECT_FALSE(Found);
   EXPECT_EQ(Path, "/foo");
   Path = Path2;
-  path::replace_path_prefix(Path, OldPrefix, NewPrefix);
+  Found = path::replace_path_prefix(Path, OldPrefix, NewPrefix);
+  EXPECT_TRUE(Found);
   EXPECT_EQ(Path, "/new/foo");
   Path = Path2;
-  path::replace_path_prefix(Path, OldPrefix, NewPrefix2);
+  Found = path::replace_path_prefix(Path, OldPrefix, NewPrefix2);
+  EXPECT_TRUE(Found);
   EXPECT_EQ(Path, "/longernew/foo");
   Path = Path1;
-  path::replace_path_prefix(Path, EmptyPrefix, NewPrefix);
+  Found = path::replace_path_prefix(Path, EmptyPrefix, NewPrefix);
+  EXPECT_TRUE(Found);
   EXPECT_EQ(Path, "/new/foo");
   Path = Path2;
-  path::replace_path_prefix(Path, OldPrefix, EmptyPrefix);
+  Found = path::replace_path_prefix(Path, OldPrefix, EmptyPrefix);
+  EXPECT_TRUE(Found);
   EXPECT_EQ(Path, "/foo");
   Path = Path2;
-  path::replace_path_prefix(Path, OldPrefixSep, EmptyPrefix);
+  Found = path::replace_path_prefix(Path, OldPrefixSep, EmptyPrefix);
+  EXPECT_TRUE(Found);
   EXPECT_EQ(Path, "foo");
   Path = Path3;
-  path::replace_path_prefix(Path, OldPrefix, NewPrefix);
+  Found = path::replace_path_prefix(Path, OldPrefix, NewPrefix);
+  EXPECT_TRUE(Found);
   EXPECT_EQ(Path, "/newnew/foo");
   Path = Path3;
-  path::replace_path_prefix(Path, OldPrefix, NewPrefix2);
+  Found = path::replace_path_prefix(Path, OldPrefix, NewPrefix2);
+  EXPECT_TRUE(Found);
   EXPECT_EQ(Path, "/longernewnew/foo");
   Path = Path1;
-  path::replace_path_prefix(Path, EmptyPrefix, NewPrefix);
+  Found = path::replace_path_prefix(Path, EmptyPrefix, NewPrefix);
+  EXPECT_TRUE(Found);
   EXPECT_EQ(Path, "/new/foo");
   Path = OldPrefix;
-  path::replace_path_prefix(Path, OldPrefix, NewPrefix);
+  Found = path::replace_path_prefix(Path, OldPrefix, NewPrefix);
+  EXPECT_TRUE(Found);
   EXPECT_EQ(Path, "/new");
   Path = OldPrefixSep;
-  path::replace_path_prefix(Path, OldPrefix, NewPrefix);
+  Found = path::replace_path_prefix(Path, OldPrefix, NewPrefix);
+  EXPECT_TRUE(Found);
   EXPECT_EQ(Path, "/new/");
   Path = OldPrefix;
-  path::replace_path_prefix(Path, OldPrefixSep, NewPrefix);
+  Found = path::replace_path_prefix(Path, OldPrefixSep, NewPrefix);
+  EXPECT_FALSE(Found);
   EXPECT_EQ(Path, "/old");
+  Path = Path4;
+  Found = path::replace_path_prefix(Path, OldPrefixWin, NewPrefix,
+                                    path::Style::windows);
+  EXPECT_TRUE(Found);
+  EXPECT_EQ(Path, "/newoo\\bar");
+  Path = Path4;
+  Found = path::replace_path_prefix(Path, OldPrefixWin, NewPrefix,
+                                    path::Style::posix);
+  EXPECT_FALSE(Found);
+  EXPECT_EQ(Path, "C:\\old/foo\\bar");
 }
 
 TEST_F(FileSystemTest, OpenFileForRead) {