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).

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@273195 91177308-0d34-0410-b5e6-96231b3b80d8
This commit is contained in:
Aaron Ballman 2016-06-20 20:28:49 +00:00
parent 50d58070c0
commit 0e65d2f433
2 changed files with 46 additions and 4 deletions

View File

@ -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();

View File

@ -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;