From af0b0e00fba9b13c23dabe19fcabde15672f9dc5 Mon Sep 17 00:00:00 2001 From: Alex Brachet Date: Wed, 11 Mar 2020 23:57:20 -0400 Subject: [PATCH] [libc] [UnitTest] Add timeout to death tests Summary: This patch adds a timeout of 500ms to death tests. As we add multithreaded code and locks, deadlocks become more likely so timeout will be useful. Additionally: - Better error handling in `invokeSubprocess` - Makes `ProcessStatus`'s methods const Reviewers: sivachandra, MaskRay, gchatelet, PaulkaToast Reviewed By: sivachandra, PaulkaToast Subscribers: tschuett, libc-commits Differential Revision: https://reviews.llvm.org/D75651 --- libc/utils/UnitTest/Test.cpp | 30 ++++++++++++++++++-- libc/utils/testutils/ExecuteFunction.h | 24 ++++++++++++---- libc/utils/testutils/ExecuteFunctionUnix.cpp | 41 +++++++++++++++++++++++----- 3 files changed, 81 insertions(+), 14 deletions(-) diff --git a/libc/utils/UnitTest/Test.cpp b/libc/utils/UnitTest/Test.cpp index 608a4df..8add925 100644 --- a/libc/utils/UnitTest/Test.cpp +++ b/libc/utils/UnitTest/Test.cpp @@ -250,7 +250,20 @@ bool Test::testMatch(RunContext &Ctx, bool MatchResult, MatcherBase &Matcher, bool Test::testProcessKilled(RunContext &Ctx, testutils::FunctionCaller *Func, int Signal, const char *LHSStr, const char *RHSStr, const char *File, unsigned long Line) { - testutils::ProcessStatus Result = testutils::invokeInSubprocess(Func); + testutils::ProcessStatus Result = testutils::invokeInSubprocess(Func, 500); + + if (const char *error = Result.getError()) { + Ctx.markFail(); + llvm::outs() << File << ":" << Line << ": FAILURE\n" << error << '\n'; + return false; + } + + if (Result.timedOut()) { + Ctx.markFail(); + llvm::outs() << File << ":" << Line << ": FAILURE\n" + << "Process timed out after " << 500 << " miliseconds.\n"; + return false; + } if (Result.exitedNormally()) { Ctx.markFail(); @@ -281,7 +294,20 @@ bool Test::testProcessExits(RunContext &Ctx, testutils::FunctionCaller *Func, int ExitCode, const char *LHSStr, const char *RHSStr, const char *File, unsigned long Line) { - testutils::ProcessStatus Result = testutils::invokeInSubprocess(Func); + testutils::ProcessStatus Result = testutils::invokeInSubprocess(Func, 500); + + if (const char *error = Result.getError()) { + Ctx.markFail(); + llvm::outs() << File << ":" << Line << ": FAILURE\n" << error << '\n'; + return false; + } + + if (Result.timedOut()) { + Ctx.markFail(); + llvm::outs() << File << ":" << Line << ": FAILURE\n" + << "Process timed out after " << 500 << " miliseconds.\n"; + return false; + } if (!Result.exitedNormally()) { Ctx.markFail(); diff --git a/libc/utils/testutils/ExecuteFunction.h b/libc/utils/testutils/ExecuteFunction.h index f3ba4ae..80da4ca 100644 --- a/libc/utils/testutils/ExecuteFunction.h +++ b/libc/utils/testutils/ExecuteFunction.h @@ -9,6 +9,8 @@ #ifndef LLVM_LIBC_UTILS_TESTUTILS_EXECUTEFUNCTION_H #define LLVM_LIBC_UTILS_TESTUTILS_EXECUTEFUNCTION_H +#include + namespace __llvm_libc { namespace testutils { @@ -20,13 +22,25 @@ public: struct ProcessStatus { int PlatformDefined; - - bool exitedNormally(); - int getExitCode(); - int getFatalSignal(); + const char *failure = nullptr; + + static constexpr uintptr_t timeout = -1L; + + static ProcessStatus Error(const char *error) { return {0, error}; } + static ProcessStatus TimedOut() { + return {0, reinterpret_cast(timeout)}; + } + + bool timedOut() const { + return failure == reinterpret_cast(timeout); + } + const char *getError() const { return timedOut() ? nullptr : failure; } + bool exitedNormally() const; + int getExitCode() const; + int getFatalSignal() const; }; -ProcessStatus invokeInSubprocess(FunctionCaller *Func); +ProcessStatus invokeInSubprocess(FunctionCaller *Func, unsigned TimeoutMS = -1); const char *signalAsString(int Signum); diff --git a/libc/utils/testutils/ExecuteFunctionUnix.cpp b/libc/utils/testutils/ExecuteFunctionUnix.cpp index afc4ca1..e920e50 100644 --- a/libc/utils/testutils/ExecuteFunctionUnix.cpp +++ b/libc/utils/testutils/ExecuteFunctionUnix.cpp @@ -10,6 +10,8 @@ #include "llvm/Support/raw_ostream.h" #include #include +#include +#include #include #include #include @@ -17,32 +19,57 @@ namespace __llvm_libc { namespace testutils { -bool ProcessStatus::exitedNormally() { return WIFEXITED(PlatformDefined); } +bool ProcessStatus::exitedNormally() const { + return WIFEXITED(PlatformDefined); +} -int ProcessStatus::getExitCode() { +int ProcessStatus::getExitCode() const { assert(exitedNormally() && "Abnormal termination, no exit code"); return WEXITSTATUS(PlatformDefined); } -int ProcessStatus::getFatalSignal() { +int ProcessStatus::getFatalSignal() const { if (exitedNormally()) return 0; return WTERMSIG(PlatformDefined); } -ProcessStatus invokeInSubprocess(FunctionCaller *Func) { +ProcessStatus invokeInSubprocess(FunctionCaller *Func, unsigned timeoutMS) { + std::unique_ptr X(Func); + int pipeFDs[2]; + if (::pipe(pipeFDs) == -1) + return ProcessStatus::Error("pipe(2) failed"); + // Don't copy the buffers into the child process and print twice. llvm::outs().flush(); llvm::errs().flush(); pid_t Pid = ::fork(); + if (Pid == -1) + return ProcessStatus::Error("fork(2) failed"); + if (!Pid) { (*Func)(); std::exit(0); } + ::close(pipeFDs[1]); + + struct pollfd pollFD { + pipeFDs[0], 0, 0 + }; + // No events requested so this call will only return after the timeout or if + // the pipes peer was closed, signaling the process exited. + if (::poll(&pollFD, 1, timeoutMS) == -1) + return ProcessStatus::Error("poll(2) failed"); + // If the pipe wasn't closed by the child yet then timeout has expired. + if (!(pollFD.revents & POLLHUP)) { + ::kill(Pid, SIGKILL); + return ProcessStatus::TimedOut(); + } - int WStatus; - ::waitpid(Pid, &WStatus, 0); - delete Func; + int WStatus = 0; + int status = ::waitpid(Pid, &WStatus, WNOHANG); + assert(status == Pid && "wait call should not block here"); + (void)status; return {WStatus}; } -- 2.7.4