[libc-commits] [libc] af0b0e0 - [libc] [UnitTest] Add timeout to death tests

[Alex Brachet via libc-commits]

Author: Alex Brachet
Date: 2020-03-11T23:57:20-04:00
New Revision: af0b0e00fba9b13c23dabe19fcabde15672f9dc5

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

LOG: [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

Added: 
    

Modified: 
    libc/utils/UnitTest/Test.cpp
    libc/utils/testutils/ExecuteFunction.h
    libc/utils/testutils/ExecuteFunctionUnix.cpp

Removed: 
    


################################################################################
diff  --git a/libc/utils/UnitTest/Test.cpp b/libc/utils/UnitTest/Test.cpp
index 608a4df89979..8add925b760b 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 f3ba4aeda59b..80da4ca9457f 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 <stdint.h>
+
 namespace __llvm_libc {
 namespace testutils {
 
@@ -20,13 +22,25 @@ class FunctionCaller {
 
 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<const char *>(timeout)};
+  }
+
+  bool timedOut() const {
+    return failure == reinterpret_cast<const char *>(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 afc4ca1f1473..e920e50f7147 100644
--- a/libc/utils/testutils/ExecuteFunctionUnix.cpp
+++ b/libc/utils/testutils/ExecuteFunctionUnix.cpp
@@ -10,6 +10,8 @@
 #include "llvm/Support/raw_ostream.h"
 #include <cassert>
 #include <cstdlib>
+#include <memory>
+#include <poll.h>
 #include <signal.h>
 #include <sys/wait.h>
 #include <unistd.h>
@@ -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<FunctionCaller> 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};
 }