[PATCH] D65675: [Path] Fix bug in make_absolute logic

[Jonas Devlieghere via Phabricator via llvm-commits]

JDevlieghere created this revision.
JDevlieghere added a reviewer: Bigcheese.
Herald added subscribers: kristina, dexonsmith, hiraditya.
Herald added a project: LLVM.

While writing a VFS test case I discovered an issue in the `make_absolute` implementation for path with a `//net` style root. These are used in the VFS test cases because they are legal absolute paths on both Windows and unix.

> // NOTE: in the tests below, we use '//root/' as our root directory, since it is
>  // a legal *absolute* path on Windows as well as *nix.

The test worked fine for a regular Unix path, which led me to look at the implementation. The problem appears to be that we're trying to use the root name from the relative path, which I believe is incorrect.


Repository:
  rL LLVM

https://reviews.llvm.org/D65675

Files:
  llvm/lib/Support/Path.cpp
  llvm/unittests/Support/Path.cpp


Index: llvm/unittests/Support/Path.cpp
===================================================================
--- llvm/unittests/Support/Path.cpp
+++ llvm/unittests/Support/Path.cpp
@@ -185,10 +185,19 @@
     path::native(*i, temp_store);
   }
 
-  SmallString<32> Relative("foo.cpp");
-  sys::fs::make_absolute("/root", Relative);
-  Relative[5] = '/'; // Fix up windows paths.
-  ASSERT_EQ("/root/foo.cpp", Relative);
+  {
+    SmallString<32> Relative("foo.cpp");
+    sys::fs::make_absolute("/root", Relative);
+    Relative[5] = '/'; // Fix up windows paths.
+    ASSERT_EQ("/root/foo.cpp", Relative);
+  }
+
+  {
+    SmallString<32> Relative("foo.cpp");
+    sys::fs::make_absolute("//root", Relative);
+    Relative[6] = '/'; // Fix up windows paths.
+    ASSERT_EQ("//root/foo.cpp", Relative);
+  }
 }
 
 TEST(Support, FilenameParent) {
Index: llvm/lib/Support/Path.cpp
===================================================================
--- llvm/lib/Support/Path.cpp
+++ llvm/lib/Support/Path.cpp
@@ -885,13 +885,13 @@
   }
 
   if (rootName && !rootDirectory) {
-    StringRef pRootName      = path::root_name(p);
+    StringRef bRootName      = path::root_name(current_dir);
     StringRef bRootDirectory = path::root_directory(current_dir);
     StringRef bRelativePath  = path::relative_path(current_dir);
     StringRef pRelativePath  = path::relative_path(p);
 
     SmallString<128> res;
-    path::append(res, pRootName, bRootDirectory, bRelativePath, pRelativePath);
+    path::append(res, bRootName, bRootDirectory, bRelativePath, pRelativePath);
     path.swap(res);
     return;
   }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D65675.213120.patch
Type: text/x-patch
Size: 1603 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190802/a0172858/attachment.bin>