[Lldb-commits] [lldb] fe19ae3 - normalize file path when searching the source map

[Walter Erquinigo via lldb-commits]

Author: Xu Jun
Date: 2021-11-01T22:13:55-07:00
New Revision: fe19ae352ce201e7bab8baac4587cc4334e742e4

URL: https://github.com/llvm/llvm-project/commit/fe19ae352ce201e7bab8baac4587cc4334e742e4
DIFF: https://github.com/llvm/llvm-project/commit/fe19ae352ce201e7bab8baac4587cc4334e742e4.diff

LOG: normalize file path when searching the source map

The key stored in the source map is a normalized path string with host's
path style, so it is also necessary to normalize the file path during
searching the map

Reviewed By: wallace, aprantl

Differential Revision: https://reviews.llvm.org/D112439

Added: 
    lldb/unittests/Target/FindFileTest.cpp

Modified: 
    lldb/source/Target/PathMappingList.cpp
    lldb/unittests/Target/CMakeLists.txt

Removed: 
    


################################################################################
diff  --git a/lldb/source/Target/PathMappingList.cpp b/lldb/source/Target/PathMappingList.cpp
index b660c310ef31a..cd76421cec18a 100644
--- a/lldb/source/Target/PathMappingList.cpp
+++ b/lldb/source/Target/PathMappingList.cpp
@@ -218,7 +218,12 @@ bool PathMappingList::ReverseRemapPath(const FileSpec &file, FileSpec &fixed) co
 }
 
 llvm::Optional<FileSpec> PathMappingList::FindFile(const FileSpec &orig_spec) const {
-  if (auto remapped = RemapPath(orig_spec.GetPath(), /*only_if_exists=*/true))
+  // We must normalize the orig_spec again using the host's path style,
+  // otherwise there will be mismatch between the host and remote platform
+  // if they use 
diff erent path styles.
+  if (auto remapped = RemapPath(
+          NormalizePath(ConstString(orig_spec.GetCString())).GetStringRef(),
+          /*only_if_exists=*/true))
     return remapped;
 
   return {};

diff  --git a/lldb/unittests/Target/CMakeLists.txt b/lldb/unittests/Target/CMakeLists.txt
index c126597c79daf..3b23550feaf9c 100644
--- a/lldb/unittests/Target/CMakeLists.txt
+++ b/lldb/unittests/Target/CMakeLists.txt
@@ -7,6 +7,7 @@ add_lldb_unittest(TargetTests
   PathMappingListTest.cpp
   RemoteAwarePlatformTest.cpp
   StackFrameRecognizerTest.cpp
+  FindFileTest.cpp
 
   LINK_LIBS
       lldbCore

diff  --git a/lldb/unittests/Target/FindFileTest.cpp b/lldb/unittests/Target/FindFileTest.cpp
new file mode 100644
index 0000000000000..30ac4dfc9f5d1
--- /dev/null
+++ b/lldb/unittests/Target/FindFileTest.cpp
@@ -0,0 +1,97 @@
+//===-- FindFileTest.cpp -------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "TestingSupport/TestUtilities.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Target/PathMappingList.h"
+#include "lldb/Utility/FileSpec.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/FileUtilities.h"
+#include "gtest/gtest.h"
+#include <utility>
+
+using namespace llvm;
+using namespace llvm::sys::fs;
+using namespace lldb_private;
+
+namespace {
+struct Matches {
+  FileSpec original;
+  llvm::StringRef remapped;
+  Matches(const char *o, const char *r) : original(o), remapped(r) {}
+  Matches(const char *o, llvm::sys::path::Style style, const char *r)
+      : original(o, style), remapped(r) {}
+};
+
+class FindFileTest : public testing::Test {
+public:
+  void SetUp() override {
+    FileSystem::Initialize();
+    HostInfo::Initialize();
+  }
+  void TearDown() override {
+    HostInfo::Terminate();
+    FileSystem::Terminate();
+  }
+};
+} // namespace
+
+static void TestFileFindings(const PathMappingList &map,
+                             llvm::ArrayRef<Matches> matches,
+                             llvm::ArrayRef<FileSpec> fails) {
+  for (const auto &fail : fails) {
+    SCOPED_TRACE(fail.GetCString());
+    EXPECT_FALSE(map.FindFile(fail));
+  }
+
+  for (const auto &match : matches) {
+    SCOPED_TRACE(match.original.GetPath() + " -> " + match.remapped);
+    llvm::Optional<FileSpec> remapped;
+
+    EXPECT_TRUE(bool(remapped = map.FindFile(match.original)));
+    EXPECT_TRUE(FileSpec(remapped.getValue()).GetPath() ==
+                ConstString(match.remapped).GetStringRef());
+  }
+}
+
+TEST_F(FindFileTest, FindFileTests) {
+  const auto *Info = testing::UnitTest::GetInstance()->current_test_info();
+  llvm::SmallString<128> DirName, FileName;
+  int fd;
+
+  ASSERT_NO_ERROR(createUniqueDirectory(Info->name(), DirName));
+
+  sys::path::append(FileName, Twine(DirName), Twine("test"));
+  ASSERT_NO_ERROR(openFile(FileName, fd, CD_CreateAlways, FA_Read, OF_None));
+
+  llvm::FileRemover dir_remover(DirName);
+  llvm::FileRemover file_remover(FileName);
+  PathMappingList map;
+
+  map.Append(ConstString("/old"), ConstString(DirName.str()), false);
+  map.Append(ConstString(R"(C:\foo)"), ConstString(DirName.str()), false);
+
+  Matches matches[] = {
+      {"/old", llvm::sys::path::Style::posix, DirName.c_str()},
+      {"/old/test", llvm::sys::path::Style::posix, FileName.c_str()},
+      {R"(C:\foo)", llvm::sys::path::Style::windows, DirName.c_str()},
+      {R"(C:\foo\test)", llvm::sys::path::Style::windows, FileName.c_str()}};
+
+  ArrayRef<FileSpec> fails{
+      // path not mapped
+      FileSpec("/foo", llvm::sys::path::Style::posix),
+      FileSpec("/new", llvm::sys::path::Style::posix),
+      FileSpec(R"(C:\new)", llvm::sys::path::Style::windows),
+      // path mapped, but file not exist
+      FileSpec("/old/test1", llvm::sys::path::Style::posix),
+      FileSpec(R"(C:\foo\test2)", llvm::sys::path::Style::windows)};
+
+  TestFileFindings(map, matches, fails);
+}