Filesystem/Windows: fix inconsistency in readNativeFileSlice API

Summary:
The windows version implementation of readNativeFileSlice, was trying to
match the POSIX behavior of not treating EOF as an error, but it was
only handling the case of reading from a pipe. Attempting to read past
the end of a regular file returns a slightly different error code, which
needs to be handled too. This patch adds ERROR_HANDLE_EOF to the list of
error codes to be treated as an end of file, and adds some unit tests
for the API.

This issue was found while attempting to land D66224, which caused a bunch of
lldb tests to start failing on windows.

Reviewers: rnk, aganea

Subscribers: kristina, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D66344

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

diff --git a/lib/Support/Windows/Path.inc b/lib/Support/Windows/Path.inc
index 5704930aeeccb3dc73a310b6bd0788cba7fb2715..00a4e669abb8b8c0bf807ea7e64bec7bea33367b 100644 (file)
--- a/lib/Support/Windows/Path.inc
+++ b/lib/Support/Windows/Path.inc
@@ -1229,8 +1229,8 @@ std::error_code readNativeFileImpl(file_t FileHandle, char *BufPtr, size_t Bytes
   *BytesRead = BytesRead32;
   if (!Success) {
     DWORD Err = ::GetLastError();
-    // Pipe EOF is not an error.
-    if (Err == ERROR_BROKEN_PIPE)
+    // EOF is not an error.
+    if (Err == ERROR_BROKEN_PIPE || Err == ERROR_HANDLE_EOF)
       return std::error_code();
     return mapWindowsError(Err);
   }

diff --git a/unittests/Support/Path.cpp b/unittests/Support/Path.cpp
index 1802b0031ad7bcaf00de4bb24f49f064b90a3a27..2aadf89161c1ece0a2d94b27d3d4742baa2cc7c4 100644 (file)
--- a/unittests/Support/Path.cpp
+++ b/unittests/Support/Path.cpp
@@ -20,8 +20,9 @@
 #include "llvm/Support/Host.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/raw_ostream.h"
-#include "gtest/gtest.h"
+#include "llvm/Testing/Support/Error.h"
 #include "gmock/gmock.h"
+#include "gtest/gtest.h"
 
 #ifdef _WIN32
 #include "llvm/ADT/ArrayRef.h"
@@ -1513,6 +1514,30 @@ TEST_F(FileSystemTest, ReadWriteFileCanReadOrWrite) {
   verifyWrite(FD, "Buzz", true);
 }
 
+TEST_F(FileSystemTest, readNativeFileSlice) {
+  char Data[10] = {'0', '1', '2', '3', '4', 0, 0, 0, 0, 0};
+  createFileWithData(NonExistantFile, false, fs::CD_CreateNew,
+                     StringRef(Data, 5));
+  FileRemover Cleanup(NonExistantFile);
+  Expected<fs::file_t> FD = fs::openNativeFileForRead(NonExistantFile);
+  ASSERT_THAT_EXPECTED(FD, Succeeded());
+  char Buf[10];
+  const auto &Read = [&](size_t Offset,
+                         size_t ToRead) -> Expected<ArrayRef<char>> {
+    std::memset(Buf, 0x47, sizeof(Buf));
+    if (std::error_code EC = fs::readNativeFileSlice(
+            *FD, makeMutableArrayRef(Buf, ToRead), Offset))
+      return errorCodeToError(EC);
+    return makeArrayRef(Buf, ToRead);
+  };
+  EXPECT_THAT_EXPECTED(Read(0, 5), HasValue(makeArrayRef(Data + 0, 5)));
+  EXPECT_THAT_EXPECTED(Read(0, 3), HasValue(makeArrayRef(Data + 0, 3)));
+  EXPECT_THAT_EXPECTED(Read(2, 3), HasValue(makeArrayRef(Data + 2, 3)));
+  EXPECT_THAT_EXPECTED(Read(0, 6), HasValue(makeArrayRef(Data + 0, 6)));
+  EXPECT_THAT_EXPECTED(Read(2, 6), HasValue(makeArrayRef(Data + 2, 6)));
+  EXPECT_THAT_EXPECTED(Read(5, 5), HasValue(makeArrayRef(Data + 5, 5)));
+}
+
 TEST_F(FileSystemTest, is_local) {
   bool TestDirectoryIsLocal;
   ASSERT_NO_ERROR(fs::is_local(TestDirectory, TestDirectoryIsLocal));