[llvm] [Support] Report EISDIR when opening a directory (PR #79880)
via llvm-commits
llvm-commits at lists.llvm.org
Wed Apr 10 06:47:41 PDT 2024
https://github.com/azhan92 updated https://github.com/llvm/llvm-project/pull/79880
>From 066a8ca18b48780ce5291963312564d639ab1661 Mon Sep 17 00:00:00 2001
From: Alison Zhang <alisonzhang at ibm.com>
Date: Mon, 29 Jan 2024 13:27:08 -0500
Subject: [PATCH 1/6] Report eisdir on aix
---
llvm/lib/Support/Unix/Path.inc | 7 +++++++
.../tools/llvm-symbolizer/input-file-err.test | 2 --
llvm/unittests/Support/CommandLineTest.cpp | 2 --
llvm/unittests/Support/Path.cpp | 16 ++++++++++++++++
4 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/llvm/lib/Support/Unix/Path.inc b/llvm/lib/Support/Unix/Path.inc
index 968e2c459f3fb1..ea38cd82640620 100644
--- a/llvm/lib/Support/Unix/Path.inc
+++ b/llvm/lib/Support/Unix/Path.inc
@@ -1024,6 +1024,13 @@ std::error_code openFile(const Twine &Name, int &ResultFD,
auto Open = [&]() { return ::open(P.begin(), OpenFlags, Mode); };
if ((ResultFD = sys::RetryAfterSignal(-1, Open)) < 0)
return errnoAsErrorCode();
+ if (Access == FA_Read) {
+ struct stat Status;
+ if (fstat(ResultFD, &Status) == -1)
+ return errnoAsErrorCode();
+ if (S_ISDIR(Status.st_mode))
+ return make_error_code(errc::is_a_directory);
+ }
#ifndef O_CLOEXEC
if (!(Flags & OF_ChildInherit)) {
int r = fcntl(ResultFD, F_SETFD, FD_CLOEXEC);
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 different 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..604a8803f60e69 100644
--- a/llvm/unittests/Support/Path.cpp
+++ b/llvm/unittests/Support/Path.cpp
@@ -1296,6 +1296,22 @@ TEST_F(FileSystemTest, UTF8ToUTF16DirectoryIteration) {
}
#endif
+TEST_F(FileSystemTest, OpenDirectoryAsFile) {
+ ASSERT_NO_ERROR(fs::create_directory(Twine(TestDirectory)));
+ ASSERT_EQ(fs::create_directory(Twine(TestDirectory), false),
+ errc::file_exists);
+
+ int FD;
+ std::error_code EC;
+ EC = fs::openFileForRead(Twine(TestDirectory), FD);
+ ASSERT_EQ(EC, errc::is_a_directory);
+ EC = fs::openFileForWrite(Twine(TestDirectory), FD);
+ ASSERT_EQ(EC, errc::is_a_directory);
+
+ ASSERT_NO_ERROR(fs::remove_directories(Twine(TestDirectory)));
+ ::close(FD);
+}
+
TEST_F(FileSystemTest, Remove) {
SmallString<64> BaseDir;
SmallString<64> Paths[4];
>From 267039f1ae1c00b44a385fa8bf620c83b85bf039 Mon Sep 17 00:00:00 2001
From: Alison Zhang <alisonzhang at ibm.com>
Date: Mon, 29 Jan 2024 13:33:27 -0500
Subject: [PATCH 2/6] Fix formatting error
---
llvm/lib/Support/Unix/Path.inc | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/llvm/lib/Support/Unix/Path.inc b/llvm/lib/Support/Unix/Path.inc
index ea38cd82640620..aeb8d25faf3f63 100644
--- a/llvm/lib/Support/Unix/Path.inc
+++ b/llvm/lib/Support/Unix/Path.inc
@@ -1025,12 +1025,12 @@ std::error_code openFile(const Twine &Name, int &ResultFD,
if ((ResultFD = sys::RetryAfterSignal(-1, Open)) < 0)
return errnoAsErrorCode();
if (Access == FA_Read) {
- struct stat Status;
+ struct stat Status;
if (fstat(ResultFD, &Status) == -1)
return errnoAsErrorCode();
if (S_ISDIR(Status.st_mode))
return make_error_code(errc::is_a_directory);
- }
+ }
#ifndef O_CLOEXEC
if (!(Flags & OF_ChildInherit)) {
int r = fcntl(ResultFD, F_SETFD, FD_CLOEXEC);
>From 76670d22858958fcf3f569459b34c9040b583d45 Mon Sep 17 00:00:00 2001
From: Alison Zhang <alisonzhang at ibm.com>
Date: Wed, 14 Feb 2024 10:40:57 -0500
Subject: [PATCH 3/6] Add ifdef for AIX/MVS
---
llvm/lib/Support/Unix/Path.inc | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/llvm/lib/Support/Unix/Path.inc b/llvm/lib/Support/Unix/Path.inc
index aeb8d25faf3f63..372e163919254e 100644
--- a/llvm/lib/Support/Unix/Path.inc
+++ b/llvm/lib/Support/Unix/Path.inc
@@ -1024,13 +1024,17 @@ std::error_code openFile(const Twine &Name, int &ResultFD,
auto Open = [&]() { return ::open(P.begin(), OpenFlags, Mode); };
if ((ResultFD = sys::RetryAfterSignal(-1, Open)) < 0)
return errnoAsErrorCode();
- if (Access == FA_Read) {
- struct stat Status;
- if (fstat(ResultFD, &Status) == -1)
- return errnoAsErrorCode();
- if (S_ISDIR(Status.st_mode))
- return make_error_code(errc::is_a_directory);
- }
+ // The underlying operation on these platforms allow opening directories
+ // for reading in more cases than other platforms.
+ #if defined(__MVS__) || defined(_AIX)
+ if (Access == FA_Read) {
+ struct stat Status;
+ if (fstat(ResultFD, &Status) == -1)
+ return errnoAsErrorCode();
+ if (S_ISDIR(Status.st_mode))
+ return make_error_code(errc::is_a_directory);
+ }
+ #endif
#ifndef O_CLOEXEC
if (!(Flags & OF_ChildInherit)) {
int r = fcntl(ResultFD, F_SETFD, FD_CLOEXEC);
>From fe7209d730cb05cdccdd337617b6f320a32ded6c Mon Sep 17 00:00:00 2001
From: Alison Zhang <alisonzhang at ibm.com>
Date: Wed, 14 Feb 2024 11:26:38 -0500
Subject: [PATCH 4/6] Fix formatting
---
llvm/lib/Support/Unix/Path.inc | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/llvm/lib/Support/Unix/Path.inc b/llvm/lib/Support/Unix/Path.inc
index 372e163919254e..277ac67a236972 100644
--- a/llvm/lib/Support/Unix/Path.inc
+++ b/llvm/lib/Support/Unix/Path.inc
@@ -1024,17 +1024,17 @@ std::error_code openFile(const Twine &Name, int &ResultFD,
auto Open = [&]() { return ::open(P.begin(), OpenFlags, Mode); };
if ((ResultFD = sys::RetryAfterSignal(-1, Open)) < 0)
return errnoAsErrorCode();
- // The underlying operation on these platforms allow opening directories
- // for reading in more cases than other platforms.
- #if defined(__MVS__) || defined(_AIX)
- if (Access == FA_Read) {
- struct stat Status;
- if (fstat(ResultFD, &Status) == -1)
- return errnoAsErrorCode();
- if (S_ISDIR(Status.st_mode))
- return make_error_code(errc::is_a_directory);
- }
- #endif
+// The underlying operation on these platforms allow opening directories
+// for reading in more cases than other platforms.
+#if defined(__MVS__) || defined(_AIX)
+ if (Access == FA_Read) {
+ struct stat Status;
+ if (fstat(ResultFD, &Status) == -1)
+ return errnoAsErrorCode();
+ if (S_ISDIR(Status.st_mode))
+ return make_error_code(errc::is_a_directory);
+ }
+#endif
#ifndef O_CLOEXEC
if (!(Flags & OF_ChildInherit)) {
int r = fcntl(ResultFD, F_SETFD, FD_CLOEXEC);
>From cdb862387ad6ea2043ad86406b20796590821d79 Mon Sep 17 00:00:00 2001
From: Alison Zhang <alisonzhang at ibm.com>
Date: Tue, 9 Apr 2024 16:30:01 -0400
Subject: [PATCH 5/6] Fix failing lit test on linux
---
llvm/lib/Support/Unix/Path.inc | 20 +++++++++-----------
llvm/unittests/Support/Path.cpp | 23 ++++++++++++++++++-----
2 files changed, 27 insertions(+), 16 deletions(-)
diff --git a/llvm/lib/Support/Unix/Path.inc b/llvm/lib/Support/Unix/Path.inc
index 277ac67a236972..30f0481b3d6ed6 100644
--- a/llvm/lib/Support/Unix/Path.inc
+++ b/llvm/lib/Support/Unix/Path.inc
@@ -1024,17 +1024,6 @@ std::error_code openFile(const Twine &Name, int &ResultFD,
auto Open = [&]() { return ::open(P.begin(), OpenFlags, Mode); };
if ((ResultFD = sys::RetryAfterSignal(-1, Open)) < 0)
return errnoAsErrorCode();
-// The underlying operation on these platforms allow opening directories
-// for reading in more cases than other platforms.
-#if defined(__MVS__) || defined(_AIX)
- if (Access == FA_Read) {
- struct stat Status;
- if (fstat(ResultFD, &Status) == -1)
- return errnoAsErrorCode();
- if (S_ISDIR(Status.st_mode))
- return make_error_code(errc::is_a_directory);
- }
-#endif
#ifndef O_CLOEXEC
if (!(Flags & OF_ChildInherit)) {
int r = fcntl(ResultFD, F_SETFD, FD_CLOEXEC);
@@ -1202,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/unittests/Support/Path.cpp b/llvm/unittests/Support/Path.cpp
index 604a8803f60e69..4a0cf711dacb8f 100644
--- a/llvm/unittests/Support/Path.cpp
+++ b/llvm/unittests/Support/Path.cpp
@@ -1296,21 +1296,34 @@ TEST_F(FileSystemTest, UTF8ToUTF16DirectoryIteration) {
}
#endif
-TEST_F(FileSystemTest, OpenDirectoryAsFile) {
+TEST_F(FileSystemTest, OpenDirectoryAsFileForRead) {
ASSERT_NO_ERROR(fs::create_directory(Twine(TestDirectory)));
ASSERT_EQ(fs::create_directory(Twine(TestDirectory), false),
errc::file_exists);
+ std::string Buf(5, '?');
+ Expected<fs::file_t> FD = fs::openNativeFileForRead(TestDirectory);
+ ASSERT_NO_ERROR(errorToErrorCode(FD.takeError()));
+ auto Close = make_scope_exit([&] { fs::closeFile(*FD); });
+ Expected<size_t> BytesRead = fs::readNativeFile(
+ *FD, MutableArrayRef(&*Buf.begin(), Buf.size()));
+ ASSERT_EQ(errorToErrorCode(BytesRead.takeError()),
+ errc::is_a_directory);
+}
+
+TEST_F(FileSystemTest, OpenDirectoryAsFileForWrite) {
+ ASSERT_NO_ERROR(fs::create_directory(Twine(TestDirectory)));
+ ASSERT_EQ(fs::create_directory(Twine(TestDirectory), false),
+ errc::file_exists);
+
int FD;
- std::error_code EC;
- EC = fs::openFileForRead(Twine(TestDirectory), FD);
- ASSERT_EQ(EC, errc::is_a_directory);
+ std::error_code EC;
EC = fs::openFileForWrite(Twine(TestDirectory), FD);
ASSERT_EQ(EC, errc::is_a_directory);
ASSERT_NO_ERROR(fs::remove_directories(Twine(TestDirectory)));
::close(FD);
-}
+}
TEST_F(FileSystemTest, Remove) {
SmallString<64> BaseDir;
>From 37a2b469d5c3ec50a4829f598bfa9c0d16b19202 Mon Sep 17 00:00:00 2001
From: Alison Zhang <alisonzhang at ibm.com>
Date: Wed, 10 Apr 2024 09:47:23 -0400
Subject: [PATCH 6/6] fix formatting
---
llvm/lib/Support/Unix/Path.inc | 2 +-
llvm/unittests/Support/Path.cpp | 17 ++++++++---------
2 files changed, 9 insertions(+), 10 deletions(-)
diff --git a/llvm/lib/Support/Unix/Path.inc b/llvm/lib/Support/Unix/Path.inc
index 30f0481b3d6ed6..6e679f74869f0f 100644
--- a/llvm/lib/Support/Unix/Path.inc
+++ b/llvm/lib/Support/Unix/Path.inc
@@ -1199,7 +1199,7 @@ Expected<size_t> readNativeFile(file_t FD, MutableArrayRef<char> Buf) {
return errorCodeToError(errnoAsErrorCode());
if (S_ISDIR(Status.st_mode))
return errorCodeToError(make_error_code(errc::is_a_directory));
-#endif
+#endif
return NumRead;
}
diff --git a/llvm/unittests/Support/Path.cpp b/llvm/unittests/Support/Path.cpp
index 4a0cf711dacb8f..4659dab59874cd 100644
--- a/llvm/unittests/Support/Path.cpp
+++ b/llvm/unittests/Support/Path.cpp
@@ -1305,25 +1305,24 @@ TEST_F(FileSystemTest, OpenDirectoryAsFileForRead) {
Expected<fs::file_t> FD = fs::openNativeFileForRead(TestDirectory);
ASSERT_NO_ERROR(errorToErrorCode(FD.takeError()));
auto Close = make_scope_exit([&] { fs::closeFile(*FD); });
- Expected<size_t> BytesRead = fs::readNativeFile(
- *FD, MutableArrayRef(&*Buf.begin(), Buf.size()));
- ASSERT_EQ(errorToErrorCode(BytesRead.takeError()),
- errc::is_a_directory);
+ Expected<size_t> BytesRead =
+ fs::readNativeFile(*FD, MutableArrayRef(&*Buf.begin(), Buf.size()));
+ ASSERT_EQ(errorToErrorCode(BytesRead.takeError()), errc::is_a_directory);
}
TEST_F(FileSystemTest, OpenDirectoryAsFileForWrite) {
- ASSERT_NO_ERROR(fs::create_directory(Twine(TestDirectory)));
+ ASSERT_NO_ERROR(fs::create_directory(Twine(TestDirectory)));
ASSERT_EQ(fs::create_directory(Twine(TestDirectory), false),
- errc::file_exists);
-
+ errc::file_exists);
+
int FD;
- std::error_code EC;
+ std::error_code EC;
EC = fs::openFileForWrite(Twine(TestDirectory), FD);
ASSERT_EQ(EC, errc::is_a_directory);
ASSERT_NO_ERROR(fs::remove_directories(Twine(TestDirectory)));
::close(FD);
-}
+}
TEST_F(FileSystemTest, Remove) {
SmallString<64> BaseDir;
More information about the llvm-commits
mailing list