From 8f3f9e0849926051f9b6c7bf35ad433900d21d2f Mon Sep 17 00:00:00 2001 From: Sylvain Audi Date: Wed, 29 Apr 2020 12:50:37 -0400 Subject: [PATCH] [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 --- include/llvm/Support/Path.h | 5 +++- lib/DWARFLinker/DWARFLinker.cpp | 10 +++++-- lib/MC/MCContext.cpp | 14 ++++++---- lib/Support/Path.cpp | 23 ++++++++++++++-- unittests/Support/Path.cpp | 49 +++++++++++++++++++++++++-------- 5 files changed, 78 insertions(+), 23 deletions(-) diff --git a/include/llvm/Support/Path.h b/include/llvm/Support/Path.h index 3b712c00dc7..cdfff2aa7a5 100644 --- a/include/llvm/Support/Path.h +++ b/include/llvm/Support/Path.h @@ -167,9 +167,12 @@ void replace_extension(SmallVectorImpl &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 &Path, StringRef OldPrefix, - StringRef NewPrefix); + StringRef NewPrefix, + Style style = Style::native); /// Append to path. /// diff --git a/lib/DWARFLinker/DWARFLinker.cpp b/lib/DWARFLinker/DWARFLinker.cpp index fff3763aea4..12b19e77a42 100644 --- a/lib/DWARFLinker/DWARFLinker.cpp +++ b/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/lib/MC/MCContext.cpp b/lib/MC/MCContext.cpp index 1bc313553af..92d99ec577b 100644 --- a/lib/MC/MCContext.cpp +++ b/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/lib/Support/Path.cpp b/lib/Support/Path.cpp index 775629074f6..37b3086fddf 100644 --- a/lib/Support/Path.cpp +++ b/lib/Support/Path.cpp @@ -496,13 +496,32 @@ void replace_extension(SmallVectorImpl &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 &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/unittests/Support/Path.cpp b/unittests/Support/Path.cpp index a577f1b744b..8e842a95b2b 100644 --- a/unittests/Support/Path.cpp +++ b/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) {