[llvm] r273195 - Fix a relatively nasty bug with fs::getPathFromOpenFD() on Windows. The GetFinalPathNameByHandle API does not behave as documented; if given a buffer that has enough space for the path but not the null terminator, the call will return the number of characters required *without* the null terminator (despite being documented otherwise) and it will not set GetLastError(). The result was that this function would return a bogus path and no error. Instead, ensure there is sufficient space for ...
Aaron Ballman via llvm-commits
llvm-commits at lists.llvm.org
Mon Jun 20 13:28:49 PDT 2016
Author: aaronballman
Date: Mon Jun 20 15:28:49 2016
New Revision: 273195
URL: http://llvm.org/viewvc/llvm-project?rev=273195&view=rev
Log:
Fix a relatively nasty bug with fs::getPathFromOpenFD() on Windows. The GetFinalPathNameByHandle API does not behave as documented; if given a buffer that has enough space for the path but not the null terminator, the call will return the number of characters required *without* the null terminator (despite being documented otherwise) and it will not set GetLastError(). The result was that this function would return a bogus path and no error. Instead, ensure there is sufficient space for a null terminator (we already strip it off manually for compatibility with older versions of Windows).
Modified:
llvm/trunk/lib/Support/Windows/Path.inc
llvm/trunk/unittests/Support/Path.cpp
Modified: llvm/trunk/lib/Support/Windows/Path.inc
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Windows/Path.inc?rev=273195&r1=273194&r2=273195&view=diff
==============================================================================
--- llvm/trunk/lib/Support/Windows/Path.inc (original)
+++ llvm/trunk/lib/Support/Windows/Path.inc Mon Jun 20 15:28:49 2016
@@ -821,11 +821,19 @@ std::error_code getPathFromOpenFD(int FD
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
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();
Modified: llvm/trunk/unittests/Support/Path.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/Path.cpp?rev=273195&r1=273194&r2=273195&view=diff
==============================================================================
--- llvm/trunk/unittests/Support/Path.cpp (original)
+++ llvm/trunk/unittests/Support/Path.cpp Mon Jun 20 15:28:49 2016
@@ -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;
More information about the llvm-commits
mailing list