From 2dfb7e4fe8bd72c8b9801c6c485d80df92e8c20a Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Thu, 22 Jun 2017 14:18:55 +0000 Subject: [PATCH] Revert "[Support] Add RetryAfterSignal helper function" and subsequent fix The fix in r306003 uncovered a pretty fundamental problem that libc++ implementation of std::result_of does not handle the prototype of open(2) correctly (presumably because it contains ...). This makes the whole function unusable in its current form, so I am also reverting the original commit (r305892), which introduced the function, at least until I figure out a way to solve the libc++ issue. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@306005 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/Support/Errno.h | 13 ----------- lib/Support/MemoryBuffer.cpp | 13 ++++++----- lib/Support/Unix/Path.inc | 12 ++++++---- lib/Support/Unix/Process.inc | 12 +++++++--- unittests/Support/CMakeLists.txt | 1 - unittests/Support/ErrnoTest.cpp | 38 -------------------------------- 6 files changed, 25 insertions(+), 64 deletions(-) delete mode 100644 unittests/Support/ErrnoTest.cpp diff --git a/include/llvm/Support/Errno.h b/include/llvm/Support/Errno.h index 201b5598c02..4ce65e7dc83 100644 --- a/include/llvm/Support/Errno.h +++ b/include/llvm/Support/Errno.h @@ -16,7 +16,6 @@ #include #include -#include namespace llvm { namespace sys { @@ -30,18 +29,6 @@ std::string StrError(); /// Like the no-argument version above, but uses \p errnum instead of errno. std::string StrError(int errnum); -template > -inline typename ResultT::type RetryAfterSignal(typename ResultT::type Fail, - const Fun &F, - const Args &... As) { - typename ResultT::type Res; - do - Res = F(As...); - while (Res == Fail && errno == EINTR); - return Res; -} - } // namespace sys } // namespace llvm diff --git a/lib/Support/MemoryBuffer.cpp b/lib/Support/MemoryBuffer.cpp index 85e782b2c04..227e792d83d 100644 --- a/lib/Support/MemoryBuffer.cpp +++ b/lib/Support/MemoryBuffer.cpp @@ -240,9 +240,11 @@ getMemoryBufferForStream(int FD, const Twine &BufferName) { // Read into Buffer until we hit EOF. do { Buffer.reserve(Buffer.size() + ChunkSize); - ReadBytes = sys::RetryAfterSignal(-1, read, FD, Buffer.end(), ChunkSize); - if (ReadBytes == -1) + ReadBytes = read(FD, Buffer.end(), ChunkSize); + if (ReadBytes == -1) { + if (errno == EINTR) continue; return std::error_code(errno, std::generic_category()); + } Buffer.set_size(Buffer.size() + ReadBytes); } while (ReadBytes != 0); @@ -389,12 +391,13 @@ getOpenFileImpl(int FD, const Twine &Filename, uint64_t FileSize, while (BytesLeft) { #ifdef HAVE_PREAD - ssize_t NumRead = sys::RetryAfterSignal(-1, ::pread, FD, BufPtr, BytesLeft, - MapSize - BytesLeft + Offset); + ssize_t NumRead = ::pread(FD, BufPtr, BytesLeft, MapSize-BytesLeft+Offset); #else - ssize_t NumRead = sys::RetryAfterSignal(-1, ::read, FD, BufPtr, BytesLeft); + ssize_t NumRead = ::read(FD, BufPtr, BytesLeft); #endif if (NumRead == -1) { + if (errno == EINTR) + continue; // Error while reading. return std::error_code(errno, std::generic_category()); } diff --git a/lib/Support/Unix/Path.inc b/lib/Support/Unix/Path.inc index 45097eb918b..b6774692595 100644 --- a/lib/Support/Unix/Path.inc +++ b/lib/Support/Unix/Path.inc @@ -737,8 +737,10 @@ std::error_code openFileForRead(const Twine &Name, int &ResultFD, #ifdef O_CLOEXEC OpenFlags |= O_CLOEXEC; #endif - if ((ResultFD = sys::RetryAfterSignal(-1, open, P.begin(), OpenFlags)) < 0) - return std::error_code(errno, std::generic_category()); + while ((ResultFD = open(P.begin(), OpenFlags)) < 0) { + if (errno != EINTR) + return std::error_code(errno, std::generic_category()); + } #ifndef O_CLOEXEC int r = fcntl(ResultFD, F_SETFD, FD_CLOEXEC); (void)r; @@ -798,8 +800,10 @@ std::error_code openFileForWrite(const Twine &Name, int &ResultFD, SmallString<128> Storage; StringRef P = Name.toNullTerminatedStringRef(Storage); - if ((ResultFD = sys::RetryAfterSignal(-1, open, P.begin(), OpenFlags, Mode)) < 0) - return std::error_code(errno, std::generic_category()); + while ((ResultFD = open(P.begin(), OpenFlags, Mode)) < 0) { + if (errno != EINTR) + return std::error_code(errno, std::generic_category()); + } #ifndef O_CLOEXEC int r = fcntl(ResultFD, F_SETFD, FD_CLOEXEC); (void)r; diff --git a/lib/Support/Unix/Process.inc b/lib/Support/Unix/Process.inc index 2d466209468..1d0143c6716 100644 --- a/lib/Support/Unix/Process.inc +++ b/lib/Support/Unix/Process.inc @@ -207,10 +207,13 @@ std::error_code Process::FixupStandardFileDescriptors() { for (int StandardFD : StandardFDs) { struct stat st; errno = 0; - if (RetryAfterSignal(-1, fstat, StandardFD, &st) < 0) { + while (fstat(StandardFD, &st) < 0) { assert(errno && "expected errno to be set if fstat failed!"); // fstat should return EBADF if the file descriptor is closed. - if (errno != EBADF) + if (errno == EBADF) + break; + // retry fstat if we got EINTR, otherwise bubble up the failure. + if (errno != EINTR) return std::error_code(errno, std::generic_category()); } // if fstat succeeds, move on to the next FD. @@ -219,8 +222,11 @@ std::error_code Process::FixupStandardFileDescriptors() { assert(errno == EBADF && "expected errno to have EBADF at this point!"); if (NullFD < 0) { - if ((NullFD = RetryAfterSignal(-1, open, "/dev/null", O_RDWR)) < 0) + while ((NullFD = open("/dev/null", O_RDWR)) < 0) { + if (errno == EINTR) + continue; return std::error_code(errno, std::generic_category()); + } } if (NullFD == StandardFD) diff --git a/unittests/Support/CMakeLists.txt b/unittests/Support/CMakeLists.txt index 641163e39ed..e2a6561089b 100644 --- a/unittests/Support/CMakeLists.txt +++ b/unittests/Support/CMakeLists.txt @@ -21,7 +21,6 @@ add_llvm_unittest(SupportTests DebugTest.cpp EndianStreamTest.cpp EndianTest.cpp - ErrnoTest.cpp ErrorOrTest.cpp ErrorTest.cpp FileOutputBufferTest.cpp diff --git a/unittests/Support/ErrnoTest.cpp b/unittests/Support/ErrnoTest.cpp deleted file mode 100644 index 888c162822d..00000000000 --- a/unittests/Support/ErrnoTest.cpp +++ /dev/null @@ -1,38 +0,0 @@ -//===- ErrnoTest.cpp - Error handling unit tests --------------------------===// -// -// The LLVM Compiler Infrastructure -// -// This file is distributed under the University of Illinois Open Source -// License. See LICENSE.TXT for details. -// -//===----------------------------------------------------------------------===// - -#include "llvm/Support/Errno.h" -#include "gtest/gtest.h" - -using namespace llvm::sys; - -static int *ReturnPointer() { return new int(47); } - -TEST(ErrnoTest, RetryAfterSignal) { - EXPECT_EQ(1, RetryAfterSignal(-1, [] { return 1; })); - - EXPECT_EQ(-1, RetryAfterSignal(-1, [] { - errno = EAGAIN; - return -1; - })); - EXPECT_EQ(EAGAIN, errno); - - unsigned calls = 0; - EXPECT_EQ(1, RetryAfterSignal(-1, [&calls] { - errno = EINTR; - ++calls; - return calls == 1 ? -1 : 1; - })); - EXPECT_EQ(2u, calls); - - EXPECT_EQ(1, RetryAfterSignal(-1, [](int x) { return x; }, 1)); - - std::unique_ptr P{RetryAfterSignal(nullptr, ReturnPointer)}; - EXPECT_EQ(47, *P); -} -- 2.50.1