Fix a relatively nasty bug with fs::getPathFromOpenFD() on Windows. The GetFinalPathN...

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@273195 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Support/Windows/Path.inc b/lib/Support/Windows/Path.inc
index 5f588d7884e0256c2594d2e3f9961b2e8e688ca4..8e5931ce6d802f7da65a7cbee9d5ded6c3bf0af9 100644 (file)
--- a/lib/Support/Windows/Path.inc
+++ b/lib/Support/Windows/Path.inc
@@ -821,11 +821,19 @@ std::error_code getPathFromOpenFD(int FD, SmallVectorImpl<char> &ResultPath) {
 
   DWORD CharCount;
   do {
+    // FIXME: We should be using the W version of this API and converting the
+    // resulting path to UTF-8 to handle non-ASCII file paths.
     CharCount = ::GetFinalPathNameByHandleA(FileHandle, ResultPath.begin(),
-                                            ResultPath.capacity(), FILE_NAME_NORMALIZED);
-    if (CharCount <= ResultPath.capacity())
+                                            ResultPath.capacity(),
+                                            FILE_NAME_NORMALIZED);
+    if (CharCount < ResultPath.capacity())
       break;
-    ResultPath.reserve(CharCount);
+
+    // Reserve sufficient space for the path as well as the null character. Even
+    // though the API does not document that it is required, if we reserve just
+    // CharCount space, the function call will not store the resulting path and
+    // still report success.
+    ResultPath.reserve(CharCount + 1);
   } while (true);
 
   if (CharCount == 0)
@@ -833,7 +841,8 @@ std::error_code getPathFromOpenFD(int FD, SmallVectorImpl<char> &ResultPath) {
 
   ResultPath.set_size(CharCount);
 
-  // On earlier Windows releases, the character count includes the terminating null.
+  // On earlier Windows releases, the character count includes the terminating
+  // null.
   if (ResultPath.back() == '\0')
     ResultPath.pop_back();
 

diff --git a/unittests/Support/Path.cpp b/unittests/Support/Path.cpp
index 705a90bd0a320aba620adcdfed2d693f9b8dae08..74b74ac4195cdb15e3202553c17010a1b78bcdf3 100644 (file)
--- a/unittests/Support/Path.cpp
+++ b/unittests/Support/Path.cpp
@@ -1024,6 +1024,39 @@ TEST_F(FileSystemTest, PathFromFD) {
   ::close(FileDescriptor);
 }
 
+TEST_F(FileSystemTest, PathFromFDWin32) {
+  // Create a temp file.
+  int FileDescriptor;
+  SmallString<64> TempPath;
+  ASSERT_NO_ERROR(
+    fs::createTemporaryFile("prefix", "temp", FileDescriptor, TempPath));
+
+  // Make sure it exists.
+  ASSERT_TRUE(sys::fs::exists(Twine(TempPath)));
+  
+  SmallVector<char, 8> ResultPath;
+  std::error_code ErrorCode =
+    fs::getPathFromOpenFD(FileDescriptor, ResultPath);
+
+  if (!ErrorCode) {
+    // Now that we know how much space is required for the path, create a path
+    // buffer with exactly enough space (sans null terminator, which should not
+    // be present), and call getPathFromOpenFD again to ensure that the API
+    // properly handles exactly-sized buffers.
+    SmallVector<char, 8> ExactSizedPath(ResultPath.size());
+    ErrorCode = fs::getPathFromOpenFD(FileDescriptor, ExactSizedPath);
+    ResultPath = ExactSizedPath;
+  }
+
+  if (!ErrorCode) {
+    fs::UniqueID D1, D2;
+    ASSERT_NO_ERROR(fs::getUniqueID(Twine(TempPath), D1));
+    ASSERT_NO_ERROR(fs::getUniqueID(Twine(ResultPath), D2));
+    ASSERT_EQ(D1, D2);
+  }
+  ::close(FileDescriptor);
+}
+
 TEST_F(FileSystemTest, OpenFileForRead) {
   // Create a temp file.
   int FileDescriptor;