[llvm] [Support] Report EISDIR when opening a directory (PR #79880)
via llvm-commits
llvm-commits at lists.llvm.org
Mon Apr 15 09:11:17 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/9] 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/9] 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/9] 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/9] 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/9] 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/9] 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;
>From 6b3669366d26a6f7174d328095aeb44b3ee830ea Mon Sep 17 00:00:00 2001
From: Alison Zhang <alisonzhang at ibm.com>
Date: Thu, 11 Apr 2024 15:08:26 -0400
Subject: [PATCH 7/9] Disable test on Windows
---
llvm/unittests/Support/Path.cpp | 2 ++
1 file changed, 2 insertions(+)
diff --git a/llvm/unittests/Support/Path.cpp b/llvm/unittests/Support/Path.cpp
index 4659dab59874cd..c423a2b209184a 100644
--- a/llvm/unittests/Support/Path.cpp
+++ b/llvm/unittests/Support/Path.cpp
@@ -1296,6 +1296,7 @@ TEST_F(FileSystemTest, UTF8ToUTF16DirectoryIteration) {
}
#endif
+#ifndef _WIN32
TEST_F(FileSystemTest, OpenDirectoryAsFileForRead) {
ASSERT_NO_ERROR(fs::create_directory(Twine(TestDirectory)));
ASSERT_EQ(fs::create_directory(Twine(TestDirectory), false),
@@ -1309,6 +1310,7 @@ TEST_F(FileSystemTest, OpenDirectoryAsFileForRead) {
fs::readNativeFile(*FD, MutableArrayRef(&*Buf.begin(), Buf.size()));
ASSERT_EQ(errorToErrorCode(BytesRead.takeError()), errc::is_a_directory);
}
+#endif
TEST_F(FileSystemTest, OpenDirectoryAsFileForWrite) {
ASSERT_NO_ERROR(fs::create_directory(Twine(TestDirectory)));
>From b667596e78d16c613ef0b6d2d16d1ea1c3493f78 Mon Sep 17 00:00:00 2001
From: Alison Zhang <alisonzhang at ibm.com>
Date: Mon, 15 Apr 2024 11:08:44 -0400
Subject: [PATCH 8/9] Fix test on Windows
---
llvm/unittests/Support/Path.cpp | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/llvm/unittests/Support/Path.cpp b/llvm/unittests/Support/Path.cpp
index c423a2b209184a..c0bd1817647a37 100644
--- a/llvm/unittests/Support/Path.cpp
+++ b/llvm/unittests/Support/Path.cpp
@@ -1296,7 +1296,6 @@ TEST_F(FileSystemTest, UTF8ToUTF16DirectoryIteration) {
}
#endif
-#ifndef _WIN32
TEST_F(FileSystemTest, OpenDirectoryAsFileForRead) {
ASSERT_NO_ERROR(fs::create_directory(Twine(TestDirectory)));
ASSERT_EQ(fs::create_directory(Twine(TestDirectory), false),
@@ -1304,13 +1303,16 @@ TEST_F(FileSystemTest, OpenDirectoryAsFileForRead) {
std::string Buf(5, '?');
Expected<fs::file_t> FD = fs::openNativeFileForRead(TestDirectory);
- ASSERT_NO_ERROR(errorToErrorCode(FD.takeError()));
+#ifdef _WIN32
+ ASSERT_EQ(errorToErrorCode(BytesRead.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()));
ASSERT_EQ(errorToErrorCode(BytesRead.takeError()), errc::is_a_directory);
-}
#endif
+}
TEST_F(FileSystemTest, OpenDirectoryAsFileForWrite) {
ASSERT_NO_ERROR(fs::create_directory(Twine(TestDirectory)));
@@ -1320,10 +1322,11 @@ TEST_F(FileSystemTest, OpenDirectoryAsFileForWrite) {
int FD;
std::error_code EC;
EC = fs::openFileForWrite(Twine(TestDirectory), FD);
+ if (!EC)
+ ::close(FD);
ASSERT_EQ(EC, errc::is_a_directory);
ASSERT_NO_ERROR(fs::remove_directories(Twine(TestDirectory)));
- ::close(FD);
}
TEST_F(FileSystemTest, Remove) {
>From 7b3bc497b061fc4d7e2f60a0b31df3b279b8c241 Mon Sep 17 00:00:00 2001
From: Alison Zhang <alisonzhang at ibm.com>
Date: Mon, 15 Apr 2024 12:10:49 -0400
Subject: [PATCH 9/9] Do not create new directory for each test
---
llvm/unittests/Support/Path.cpp | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/llvm/unittests/Support/Path.cpp b/llvm/unittests/Support/Path.cpp
index c0bd1817647a37..74e63568d661d5 100644
--- a/llvm/unittests/Support/Path.cpp
+++ b/llvm/unittests/Support/Path.cpp
@@ -1297,10 +1297,6 @@ TEST_F(FileSystemTest, UTF8ToUTF16DirectoryIteration) {
#endif
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);
#ifdef _WIN32
@@ -1315,18 +1311,12 @@ TEST_F(FileSystemTest, OpenDirectoryAsFileForRead) {
}
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::openFileForWrite(Twine(TestDirectory), FD);
if (!EC)
::close(FD);
ASSERT_EQ(EC, errc::is_a_directory);
-
- ASSERT_NO_ERROR(fs::remove_directories(Twine(TestDirectory)));
}
TEST_F(FileSystemTest, Remove) {
More information about the llvm-commits
mailing list