[libc] [UnitTest] Create death tests

[abrachet]

Summary

This patch adds EXPECT_EXITS and EXPECT_DEATH macros for testing exit codes and deadly signals. They are less convoluted than their analogs in GTEST and don't have matchers but just take an int for either the exit code or the signal respectively. Nor do they have any regex match against the stdout/stderr of the child process.

[abrachet]

Stupidly I didn't branch against D74528, so I do not have an example test in this patch, but the death test for raise would look like EXPECT_DEATH([] { __llvm_libc::raise(SIGKILL); }, SIGKILL);

[sivachandra]

Overall looks good. But, one comment about eliminating the platform specificity from the change as proposed.

libc/utils/UnitTest/Test.cpp

15

Can we build a generic forking utility and put it in the testutils library I have proposed in my other patch? This will help us keep the unittest implementation from pulling in and using platform specific pieces.

libc/utils/UnitTest/Test.h

42

Instead of putting them in the internal namespace, can we nest them inside of of the Test class? It seems a bit odd for an internal class to be part of the public API of the Test class.

[abrachet]

• Create testutils directory modeled from D74652.
• Move platform specific process creation into Unix specific file.
• Add death test for raise

[abrachet]

Remove extraneous semicolon.

[gchatelet]

libc/utils/testutils/ExecuteFunction.h

35

[nit] Add new line

[sivachandra]

libc/utils/UnitTest/CMakeLists.txt

7

This shouldn't be required as we want to include like #include "utils/testutils/...".

libc/utils/UnitTest/Test.cpp

11

Use `utils/...' as suggested above.

254

Can we avoid calling strsignal in this file? May be add a signalAsString helper function in ExecuteFunction.h

libc/utils/UnitTest/Test.h

12

Use utils/... as suggested above.

libc/utils/testutils/ExecuteFunction.h

21

Call this struct ProcessStatus as it is actually that? Otherwise, it is easy to confuse with function return value.

29

Name the function invokeInSubprocess?

[abrachet]

• Include using "utils/" prefix,
• Create strsignal wrapper, signalAsString
• Rename InvokeResult to ProcessStatus
• Rename invokeFunction to invokeInSubprocess
• Added missing newline

[sivachandra]

In D74665#1890131, @abrachet wrote:

• Include using "utils/" prefix,
• Create strsignal wrapper, signalAsString
• Rename InvokeResult to ProcessStatus
• Rename invokeFunction to invokeInSubprocess
• Added missing newline

Thanks for listing the changes.

libc/utils/UnitTest/Test.h

9–10

Sigh! Thanks!

[abrachet]

libc/utils/UnitTest/Test.h

9–10

I forget it all the time too :)

[gchatelet]

Can you try building this with shared library ?

FAILED: lib/liblibc_test_utils.so 
: && /redacted/build-llvm/clang/bin/clang++ -fPIC -fPIC -fvisibility-inlines-hidden -Werror -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wstring-conversion -fdiagnostics-color -g  -Wl,-z,defs -Wl,-z,nodelete -fuse-ld=lld -Wl,--color-diagnostics -shared -Wl,-soname,liblibc_test_utils.so -o lib/liblibc_test_utils.so projects/libc/utils/testutils/CMakeFiles/libc_test_utils.dir/ExecuteFunctionUnix.cpp.o   && :
ld.lld: error: undefined symbol: llvm::outs()
>>> referenced by ExecuteFunctionUnix.cpp:35 (/redacted/git/llvm-project/libc/utils/testutils/ExecuteFunctionUnix.cpp:35)
>>>               projects/libc/utils/testutils/CMakeFiles/libc_test_utils.dir/ExecuteFunctionUnix.cpp.o:(__llvm_libc::testutils::invokeInSubprocess(__llvm_libc::testutils::FunctionCaller*))

ld.lld: error: undefined symbol: llvm::errs()
>>> referenced by ExecuteFunctionUnix.cpp:36 (/redatced/git/llvm-project/libc/utils/testutils/ExecuteFunctionUnix.cpp:36)
>>>               projects/libc/utils/testutils/CMakeFiles/libc_test_utils.dir/ExecuteFunctionUnix.cpp.o:(__llvm_libc::testutils::invokeInSubprocess(__llvm_libc::testutils::FunctionCaller*))

ld.lld: error: undefined symbol: llvm::raw_ostream::flush_nonempty()
>>> referenced by raw_ostream.h:157 (/redacted/git/llvm-project/llvm/include/llvm/Support/raw_ostream.h:157)
>>>               projects/libc/utils/testutils/CMakeFiles/libc_test_utils.dir/ExecuteFunctionUnix.cpp.o:(llvm::raw_ostream::flush())

ld.lld: error: undefined symbol: llvm::EnableABIBreakingChecks
>>> referenced by ExecuteFunctionUnix.cpp
>>>               projects/libc/utils/testutils/CMakeFiles/libc_test_utils.dir/ExecuteFunctionUnix.cpp.o:(llvm::VerifyEnableABIBreakingChecks)
clang: error: linker command failed with exit code 1 (use -v to see invocation)

libc/cmake/modules/LLVMLibCRules.cmake

function(add_libc_unittest target_name)
 
   set_target_properties(${target_name} PROPERTIES RUNTIME_OUTPUT_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR})
 
   add_dependencies(
     ${target_name}
     ${LIBC_UNITTEST_DEPENDS}
   )
 
-  target_link_libraries(${target_name} PRIVATE LibcUnitTest)
+  target_link_libraries(${target_name} PRIVATE LibcUnitTest libc_test_utils)
 
   add_custom_command(
     TARGET ${target_name}
     POST_BUILD
     COMMAND $<TARGET_FILE:${target_name}>
   )
   if(LIBC_UNITTEST_SUITE)
     add_dependencies(

libc/test/src/signal/raise_test.cpp

 //===----------------------- Unittests for raise --------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "include/signal.h"
 #include "src/signal/raise.h"
 #include "utils/UnitTest/Test.h"
 
 TEST(SignalTest, Raise) {
   // SIGCONT is ingored unless stopped, so we can use it to check the return
   // value of raise without needing to block.
   EXPECT_EQ(__llvm_libc::raise(SIGCONT), 0);
+
+  // SIGKILL is chosen because other fatal signals could be caught by sanitizers
+  // for example and incorrectly report test failure.
+  EXPECT_DEATH([] { __llvm_libc::raise(SIGKILL); }, SIGKILL);
 }

libc/utils/CMakeLists.txt

 add_subdirectory(CPP)
 add_subdirectory(HdrGen)
+add_subdirectory(testutils)
 add_subdirectory(UnitTest)
 add_subdirectory(benchmarks)

libc/utils/UnitTest/CMakeLists.txt

 add_llvm_library(
   LibcUnitTest
   Test.cpp
   Test.h
   LINK_COMPONENTS Support
 )
-target_include_directories(LibcUnitTest PUBLIC ${LIBC_SOURCE_DIR})
+target_include_directories(LibcUnitTest PUBLIC ${LIBC_SOURCE_DIR} ${LIBC_SOURCE_DIR}/utils/)
 add_dependencies(LibcUnitTest standalone_cpp)

libc/utils/UnitTest/Test.h

 //===------------------ Base class for libc unittests -----------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
-// This file can only include headers from utils/CPP/. No other header should be
-// included.
+// This file can only include headers from utils/CPP/ or utils/testutils. No
+// other headers should be included.
 
+#include "testutils/ExecuteFunction.h"
 #include "utils/CPP/TypeTraits.h"
 
 namespace __llvm_libc {
 namespace testing {
 
 class RunContext;
 
 // Only the following conditions are supported. Notice that we do not have

 
 namespace internal {
 
 template <typename ValType>
 bool test(RunContext &Ctx, TestCondition Cond, ValType LHS, ValType RHS,
           const char *LHSStr, const char *RHSStr, const char *File,
           unsigned long Line);
 
 } // namespace internal
 
 // NOTE: One should not create instances and call methods on them directly. One
 // should use the macros TEST or TEST_F to write test cases.
 class Test {
 private:
   Test *Next = nullptr;
 
 public:
protected:
   static bool testStrEq(RunContext &Ctx, const char *LHS, const char *RHS,
                         const char *LHSStr, const char *RHSStr,
                         const char *File, unsigned long Line);
 
   static bool testStrNe(RunContext &Ctx, const char *LHS, const char *RHS,
                         const char *LHSStr, const char *RHSStr,
                         const char *File, unsigned long Line);
 
+  static bool testProcessExits(RunContext &Ctx, testutils::FunctionCaller *Func,
+                               int ExitCode, const char *LHSStr,
+                               const char *RHSStr, const char *File,
+                               unsigned long Line);
+
+  static bool testProcessKilled(RunContext &Ctx,
+                                testutils::FunctionCaller *Func, int Signal,
+                                const char *LHSStr, const char *RHSStr,
+                                const char *File, unsigned long Line);
+
+  template <typename Func> testutils::FunctionCaller *createCallable(Func f) {
+    struct Callable : public testutils::FunctionCaller {
+      Func f;
+      Callable(Func f) : f(f) {}
+      void operator()() override { f(); }
+    };
+
+    return new Callable(f);
+  }
+
 private:
   virtual void Run(RunContext &Ctx) = 0;
   virtual const char *getName() const = 0;
 
   static Test *Start;
   static Test *End;
 };
 
#define ASSERT_TRUE(VAL)                                                       \
   if (!EXPECT_TRUE(VAL))                                                       \
   return
 
 #define EXPECT_FALSE(VAL) EXPECT_EQ((VAL), false)
 
 #define ASSERT_FALSE(VAL)                                                      \
   if (!EXPECT_FALSE(VAL))                                                      \
   return
+
+#define EXPECT_EXITS(FUNC, EXIT)                                               \
+  __llvm_libc::testing::Test::testProcessExits(                                \
+      Ctx, __llvm_libc::testing::Test::createCallable(FUNC), EXIT, #FUNC,      \
+      #EXIT, __FILE__, __LINE__)
+
+#define ASSERT_EXITS(FUNC, EXIT)                                               \
+  if (!EXPECT_EXITS(FUNC, EXIT))                                               \
+  return
+
+#define EXPECT_DEATH(FUNC, SIG)                                                \
+  __llvm_libc::testing::Test::testProcessKilled(                               \
+      Ctx, __llvm_libc::testing::Test::createCallable(FUNC), SIG, #FUNC, #SIG, \
+      __FILE__, __LINE__)
+
+#define ASSERT_DEATH(FUNC, EXIT)                                               \
+  if (!EXPECT_DEATH(FUNC, EXIT))                                               \
+  return

libc/utils/UnitTest/Test.cpp

 //===--------- Implementation of the base class for libc unittests --------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "Test.h"
 
+#include "testutils/ExecuteFunction.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/raw_ostream.h"
 
 namespace __llvm_libc {
 namespace testing {
 
 // This need not be a class as all it has is a single read-write state variable.
 // But, we make it class as then its implementation can be hidden from the
 // header file.
 class RunContext {
 public:
   enum RunResult { Result_Pass = 1, Result_Fail = 2 };

 
 bool Test::testStrNe(RunContext &Ctx, const char *LHS, const char *RHS,
                      const char *LHSStr, const char *RHSStr, const char *File,
                      unsigned long Line) {
   return internal::test(Ctx, Cond_NE, llvm::StringRef(LHS),
                         llvm::StringRef(RHS), LHSStr, RHSStr, File, Line);
 }
 
+bool Test::testProcessKilled(RunContext &Ctx, testutils::FunctionCaller *Func,
+                             int Signal, const char *LHSStr, const char *RHSStr,
+                             const char *File, unsigned long Line) {
+  testutils::InvokeResult Result = testutils::invokeFunction(Func);
+
+  if (Result.exitedNormally()) {
+    Ctx.markFail();
+    llvm::outs() << File << ":" << Line << ": FAILURE\n"
+                 << "Expected " << LHSStr
+                 << " to be killed by a signal\nBut it exited normally!\n";
+    return false;
+  }
+
+  int KilledBy = Result.getFatalSignal();
+  assert(KilledBy != 0 && "Not killed by any signal");
+  if (Signal == -1 || KilledBy == Signal)
+    return true;
+
+  Ctx.markFail();
+  llvm::outs() << File << ":" << Line << ": FAILURE\n"
+               << "              Expected: " << LHSStr << '\n'
+               << "To be killed by signal: " << Signal << '\n'
+               << "              Which is: " << ::strsignal(Signal) << '\n'
+               << "  But it was killed by: " << KilledBy << '\n'
+               << "              Which is: " << ::strsignal(KilledBy) << '\n';
+  return false;
+}
+
+bool Test::testProcessExits(RunContext &Ctx, testutils::FunctionCaller *Func,
+                            int ExitCode, const char *LHSStr,
+                            const char *RHSStr, const char *File,
+                            unsigned long Line) {
+  testutils::InvokeResult Result = testutils::invokeFunction(Func);
+
+  if (!Result.exitedNormally()) {
+    Ctx.markFail();
+    llvm::outs() << File << ":" << Line << ": FAILURE\n"
+                 << "Expected " << RHSStr << '\n'
+                 << "to exit with exit code " << ExitCode << '\n'
+                 << "But it exited abnormally!\n";
+    return false;
+  }
+
+  int ActualExit = Result.getExitCode();
+  if (ActualExit == ExitCode)
+    return true;
+
+  Ctx.markFail();
+  llvm::outs() << File << ":" << Line << ": FAILURE\n"
+               << "Expected exit code of: " << LHSStr << '\n'
+               << "             Which is: " << ActualExit << '\n'
+               << "       To be equal to: " << RHSStr << '\n'
+               << "             Which is: " << ExitCode << '\n';
+  return false;
+}
+
 } // namespace testing
 } // namespace __llvm_libc
 
 int main() { return __llvm_libc::testing::Test::runTests(); }

libc/utils/testutils/CMakeLists.txt

+add_library(
+  libc_test_utils
+  ExecuteFunction.h
+)
+
+if(CMAKE_HOST_UNIX)
+  target_sources(libc_test_utils PRIVATE ExecuteFunctionUnix.cpp)
+endif()

libc/utils/testutils/ExecuteFunction.h

+//===---------------------- ExecuteFunction.h -------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_UTILS_TESTUTILS_EXECUTEFUNCTION_H
+#define LLVM_LIBC_UTILS_TESTUTILS_EXECUTEFUNCTION_H
+
+namespace __llvm_libc {
+namespace testutils {
+
+class FunctionCaller {
+public:
+  virtual ~FunctionCaller() {}
+  virtual void operator()() = 0;
+};
+
+struct InvokeResult {
+  int PlatformDefined;
+
+  bool exitedNormally();
+  int getExitCode();
+  int getFatalSignal();
+};
+
+InvokeResult invokeFunction(FunctionCaller *Func);
+
+} // namespace testutils
+} // namespace __llvm_libc
+
+#endif // LLVM_LIBC_UTILS_TESTUTILS_EXECUTEFUNCTION_H
 No newline at end of file

libc/utils/testutils/ExecuteFunctionUnix.cpp

+//===------- ExecuteFunction implementation for Unix-like Systems ---------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "ExecuteFunction.h"
+#include "llvm/Support/raw_ostream.h"
+#include <cassert>
+#include <cstdlib>
+#include <signal.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
+namespace __llvm_libc {
+namespace testutils {
+
+bool InvokeResult::exitedNormally() { return WIFEXITED(PlatformDefined); }
+
+int InvokeResult::getExitCode() {
+  assert(exitedNormally() && "Abnormal termination, no exit code");
+  return WEXITSTATUS(PlatformDefined);
+}
+
+int InvokeResult::getFatalSignal() {
+  if (exitedNormally())
+    return 0;
+  return WTERMSIG(PlatformDefined);
+}
+
+InvokeResult invokeFunction(FunctionCaller *Func) {
+  // Don't copy the buffers into the child process and print twice.
+  llvm::outs().flush();
+  llvm::errs().flush();
+  pid_t Pid = ::fork();
+  if (!Pid) {
+    (*Func)();
+    std::exit(0);
+  }
+
+  int WStatus;
+  ::waitpid(Pid, &WStatus, 0);
+  delete Func;
+  return {WStatus};
+}
+
+} // namespace testutils
+} // namespace __llvm_libc