[PATCH] D46942: Make sure getCanonicalName returns absolute paths even on VFS.

[Eric Liu via Phabricator via cfe-commits]

ioeric created this revision.
ioeric added a reviewer: bkramer.
Herald added a subscriber: cfe-commits.

Generally, canonical paths are expected to be absolute. This is the
behavior for Windows build. But for unix, it might return relative paths when
failing to resolve real path (e.g. unsupported symlink on VFS).


Repository:
  rC Clang

https://reviews.llvm.org/D46942

Files:
  lib/Basic/FileManager.cpp
  unittests/Basic/FileManagerTest.cpp


Index: unittests/Basic/FileManagerTest.cpp
===================================================================
--- unittests/Basic/FileManagerTest.cpp
+++ unittests/Basic/FileManagerTest.cpp
@@ -322,4 +322,35 @@
   EXPECT_EQ(Path, ExpectedResult);
 }
 
+TEST_F(FileManagerTest, GetCanonicalNameReturnsAbsolutePathOnVFS) {
+  SmallString<64> CustomWorkingDir;
+#ifdef _WIN32
+  CustomWorkingDir = "C:\\test";
+#else
+  CustomWorkingDir = "/test";
+#endif
+  llvm::sys::path::append(CustomWorkingDir, "some", "weird", "path");
+
+  auto FS =
+      IntrusiveRefCntPtr<vfs::InMemoryFileSystem>(new vfs::InMemoryFileSystem);
+  // setCurrentworkingdirectory must finish without error.
+  ASSERT_TRUE(!FS->setCurrentWorkingDirectory(CustomWorkingDir));
+  SmallString<64> FooPath("weird/foo/foo.cpp");
+  FS->addFile(FooPath.str(), 0, llvm::MemoryBuffer::getMemBuffer(""));
+
+  FileSystemOptions Opts;
+  FileManager Manager(Opts, FS);
+
+  // Use relative path to open the directory.
+  const DirectoryEntry *Dir =
+      Manager.getDirectory(llvm::sys::path::parent_path(FooPath.str()));
+  EXPECT_TRUE(Dir != nullptr);
+  EXPECT_FALSE(llvm::sys::path::is_absolute(Dir->getName()));
+
+  SmallString<64> AbsFooPath(CustomWorkingDir);
+  llvm::sys::path::append(AbsFooPath, FooPath);
+  EXPECT_EQ(Manager.getCanonicalName(Dir),
+            llvm::sys::path::parent_path(AbsFooPath.str()));
+}
+
 } // anonymous namespace
Index: lib/Basic/FileManager.cpp
===================================================================
--- lib/Basic/FileManager.cpp
+++ lib/Basic/FileManager.cpp
@@ -532,15 +532,17 @@
   if (Known != CanonicalDirNames.end())
     return Known->second;
 
-  StringRef CanonicalName(Dir->getName());
+  // Canonical name should still be an absolute path even if realpath fails.
+  llvm::SmallString<128> AbsDirPath(Dir->getName());
+  makeAbsolutePath(AbsDirPath);
+  StringRef CanonicalName = AbsDirPath.str().copy(CanonicalNameStorage);
 
 #ifdef LLVM_ON_UNIX
   char CanonicalNameBuf[PATH_MAX];
-  if (realpath(Dir->getName().str().c_str(), CanonicalNameBuf))
+  if (realpath(AbsDirPath.c_str(), CanonicalNameBuf))
     CanonicalName = StringRef(CanonicalNameBuf).copy(CanonicalNameStorage);
 #else
   SmallString<256> CanonicalNameBuf(CanonicalName);
-  llvm::sys::fs::make_absolute(CanonicalNameBuf);
   llvm::sys::path::native(CanonicalNameBuf);
   // We've run into needing to remove '..' here in the wild though, so
   // remove it.



Index: unittests/Basic/FileManagerTest.cpp
===================================================================
--- unittests/Basic/FileManagerTest.cpp
+++ unittests/Basic/FileManagerTest.cpp
@@ -322,4 +322,35 @@
   EXPECT_EQ(Path, ExpectedResult);
 }
 
+TEST_F(FileManagerTest, GetCanonicalNameReturnsAbsolutePathOnVFS) {
+  SmallString<64> CustomWorkingDir;
+#ifdef _WIN32
+  CustomWorkingDir = "C:\\test";
+#else
+  CustomWorkingDir = "/test";
+#endif
+  llvm::sys::path::append(CustomWorkingDir, "some", "weird", "path");
+
+  auto FS =
+      IntrusiveRefCntPtr<vfs::InMemoryFileSystem>(new vfs::InMemoryFileSystem);
+  // setCurrentworkingdirectory must finish without error.
+  ASSERT_TRUE(!FS->setCurrentWorkingDirectory(CustomWorkingDir));
+  SmallString<64> FooPath("weird/foo/foo.cpp");
+  FS->addFile(FooPath.str(), 0, llvm::MemoryBuffer::getMemBuffer(""));
+
+  FileSystemOptions Opts;
+  FileManager Manager(Opts, FS);
+
+  // Use relative path to open the directory.
+  const DirectoryEntry *Dir =
+      Manager.getDirectory(llvm::sys::path::parent_path(FooPath.str()));
+  EXPECT_TRUE(Dir != nullptr);
+  EXPECT_FALSE(llvm::sys::path::is_absolute(Dir->getName()));
+
+  SmallString<64> AbsFooPath(CustomWorkingDir);
+  llvm::sys::path::append(AbsFooPath, FooPath);
+  EXPECT_EQ(Manager.getCanonicalName(Dir),
+            llvm::sys::path::parent_path(AbsFooPath.str()));
+}
+
 } // anonymous namespace
Index: lib/Basic/FileManager.cpp
===================================================================
--- lib/Basic/FileManager.cpp
+++ lib/Basic/FileManager.cpp
@@ -532,15 +532,17 @@
   if (Known != CanonicalDirNames.end())
     return Known->second;
 
-  StringRef CanonicalName(Dir->getName());
+  // Canonical name should still be an absolute path even if realpath fails.
+  llvm::SmallString<128> AbsDirPath(Dir->getName());
+  makeAbsolutePath(AbsDirPath);
+  StringRef CanonicalName = AbsDirPath.str().copy(CanonicalNameStorage);
 
 #ifdef LLVM_ON_UNIX
   char CanonicalNameBuf[PATH_MAX];
-  if (realpath(Dir->getName().str().c_str(), CanonicalNameBuf))
+  if (realpath(AbsDirPath.c_str(), CanonicalNameBuf))
     CanonicalName = StringRef(CanonicalNameBuf).copy(CanonicalNameStorage);
 #else
   SmallString<256> CanonicalNameBuf(CanonicalName);
-  llvm::sys::fs::make_absolute(CanonicalNameBuf);
   llvm::sys::path::native(CanonicalNameBuf);
   // We've run into needing to remove '..' here in the wild though, so
   // remove it.

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits