[llvm] 678f19f - [Support] Report EISDIR when opening a directory (#79880)

[via llvm-commits]

Author: azhan92
Date: 2024-04-17T17:56:42-04:00
New Revision: 678f19f08296fec299438130cf5943714c590b7e

URL: https://github.com/llvm/llvm-project/commit/678f19f08296fec299438130cf5943714c590b7e
DIFF: https://github.com/llvm/llvm-project/commit/678f19f08296fec299438130cf5943714c590b7e.diff

LOG: [Support] Report EISDIR when opening a directory (#79880)

The test `llvm/unittests/Support/CommandLineTest.cpp` that handles
errors in expansion of response files was previously disabled for AIX.
Originally the code was dependent on `read` returning `EISDIR` which
occurs on platforms such as Linux. However, other platforms such as AIX
allow use of `read` on file descriptors for directories. This change
updates `readNativeFile` to produce `EISDIR` on AIX and z/OS when used
on a directory (instead of relying on the call to `read` to do so).

---------

Co-authored-by: Alison Zhang <alisonzhang at ibm.com>
Co-authored-by: James Henderson <46713263+jh7370 at users.noreply.github.com>

Added: 
    

Modified: 
    llvm/lib/Support/Unix/Path.inc
    llvm/test/tools/llvm-symbolizer/input-file-err.test
    llvm/unittests/Support/CommandLineTest.cpp
    llvm/unittests/Support/Path.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Support/Unix/Path.inc b/llvm/lib/Support/Unix/Path.inc
index 968e2c459f3fb1..6e679f74869f0f 100644
--- a/llvm/lib/Support/Unix/Path.inc
+++ b/llvm/lib/Support/Unix/Path.inc
@@ -1191,6 +1191,15 @@ Expected<size_t> readNativeFile(file_t FD, MutableArrayRef<char> Buf) {
   ssize_t NumRead = sys::RetryAfterSignal(-1, ::read, FD, Buf.data(), Size);
   if (ssize_t(NumRead) == -1)
     return errorCodeToError(errnoAsErrorCode());
+// The underlying operation on these platforms allow opening directories
+// for reading in more cases than other platforms.
+#if defined(__MVS__) || defined(_AIX)
+  struct stat Status;
+  if (fstat(FD, &Status) == -1)
+    return errorCodeToError(errnoAsErrorCode());
+  if (S_ISDIR(Status.st_mode))
+    return errorCodeToError(make_error_code(errc::is_a_directory));
+#endif
   return NumRead;
 }
 

diff  --git a/llvm/test/tools/llvm-symbolizer/input-file-err.test b/llvm/test/tools/llvm-symbolizer/input-file-err.test
index df14da2f433c01..76115b513470b9 100644
--- a/llvm/test/tools/llvm-symbolizer/input-file-err.test
+++ b/llvm/test/tools/llvm-symbolizer/input-file-err.test
@@ -1,5 +1,3 @@
-Failing on AIX due to D153595. The test expects a 
diff erent error message from the one given on AIX.
-XFAIL: target={{.*}}-aix{{.*}}
 RUN: not llvm-addr2line -e %p/Inputs/nonexistent 0x12 2>&1 | FileCheck %s --check-prefix=CHECK-NONEXISTENT-A2L -DMSG=%errc_ENOENT
 RUN: not llvm-addr2line -e %p/Inputs/nonexistent 2>&1 | FileCheck %s --check-prefix=CHECK-NONEXISTENT-A2L -DMSG=%errc_ENOENT
 CHECK-NONEXISTENT-A2L: llvm-addr2line{{.*}}: error: '{{.*}}Inputs/nonexistent': [[MSG]]

diff  --git a/llvm/unittests/Support/CommandLineTest.cpp b/llvm/unittests/Support/CommandLineTest.cpp
index bbeb9d5dc19bda..23f6081cd32a45 100644
--- a/llvm/unittests/Support/CommandLineTest.cpp
+++ b/llvm/unittests/Support/CommandLineTest.cpp
@@ -1117,7 +1117,6 @@ TEST(CommandLineTest, BadResponseFile) {
   ASSERT_STREQ(Argv[0], "clang");
   ASSERT_STREQ(Argv[1], AFileExp.c_str());
 
-#if !defined(_AIX) && !defined(__MVS__)
   std::string ADirExp = std::string("@") + std::string(ADir.path());
   Argv = {"clang", ADirExp.c_str()};
   Res = cl::ExpandResponseFiles(Saver, cl::TokenizeGNUCommandLine, Argv);
@@ -1125,7 +1124,6 @@ TEST(CommandLineTest, BadResponseFile) {
   ASSERT_EQ(2U, Argv.size());
   ASSERT_STREQ(Argv[0], "clang");
   ASSERT_STREQ(Argv[1], ADirExp.c_str());
-#endif
 }
 
 TEST(CommandLineTest, SetDefaultValue) {

diff  --git a/llvm/unittests/Support/Path.cpp b/llvm/unittests/Support/Path.cpp
index 837ca03216f87a..bdae7a8ee4b55b 100644
--- a/llvm/unittests/Support/Path.cpp
+++ b/llvm/unittests/Support/Path.cpp
@@ -1757,6 +1757,28 @@ TEST_F(FileSystemTest, OpenFileForRead) {
 #endif
 }
 
+TEST_F(FileSystemTest, OpenDirectoryAsFileForRead) {
+  std::string Buf(5, '?');
+  Expected<fs::file_t> FD = fs::openNativeFileForRead(TestDirectory);
+#ifdef _WIN32
+  EXPECT_EQ(errorToErrorCode(FD.takeError()), errc::is_a_directory);
+#else
+  ASSERT_THAT_EXPECTED(FD, Succeeded());
+  auto Close = make_scope_exit([&] { fs::closeFile(*FD); });
+  Expected<size_t> BytesRead =
+      fs::readNativeFile(*FD, MutableArrayRef(&*Buf.begin(), Buf.size()));
+  EXPECT_EQ(errorToErrorCode(BytesRead.takeError()), errc::is_a_directory);
+#endif
+}
+
+TEST_F(FileSystemTest, OpenDirectoryAsFileForWrite) {
+  int FD;
+  std::error_code EC = fs::openFileForWrite(Twine(TestDirectory), FD);
+  if (!EC)
+    ::close(FD);
+  EXPECT_EQ(EC, errc::is_a_directory);
+}
+
 static void createFileWithData(const Twine &Path, bool ShouldExistBefore,
                                fs::CreationDisposition Disp, StringRef Data) {
   int FD;