Partially mitigate quadratic-time malloc tests in unit tests

Malloc failure testing is quadratic in the number of allocations. To
test a failure at allocation N, we must first run the previous N-1
allocations. Now that we have combined GTest binaries, this does not
work very well.

Use the test listener to reset the counter across independent tests. We
assume failures in a previous test won't interfere in the next one and
run each test's counter in parallel.

The assumption isn't *quite* true because we have a lot of internal
init-once machinery that is reused across otherwise "independent" tests,
but it's close enough that I was able to find some bugs, fixed in the
next commit. That said, the tests still take too long to run to
completion.

Bug: 127
Change-Id: I6836793448fbdc740a8cc424361e6b3dd66fb8a6
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56926
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>

crypto/internal.h

diff --git a/crypto/internal.h b/crypto/internal.h
index 46a4e70..576ad85 100644
--- a/crypto/internal.h
+++ b/crypto/internal.h
@@ -225,6 +225,16 @@
 #define OPENSSL_SSE2
 #endif
 
+#if defined(BORINGSSL_MALLOC_FAILURE_TESTING)
+// OPENSSL_reset_malloc_counter_for_testing, when malloc testing is enabled,
+// resets the internal malloc counter, to simulate further malloc failures. This
+// should be called in between independent tests, at a point where failure from
+// a previous test will not impact subsequent ones.
+OPENSSL_EXPORT void OPENSSL_reset_malloc_counter_for_testing(void);
+#else
+OPENSSL_INLINE void OPENSSL_reset_malloc_counter_for_testing(void) {}
+#endif
+
 
 // Pointer utility functions.
 

crypto/mem.c

diff --git a/crypto/mem.c b/crypto/mem.c
index abba4a4..97a85e9 100644
--- a/crypto/mem.c
+++ b/crypto/mem.c
@@ -146,11 +146,14 @@
     CRYPTO_STATIC_MUTEX_INIT;
 static uint64_t current_malloc_count = 0;
 static uint64_t malloc_number_to_fail = 0;
-static int malloc_failure_enabled = 0, break_on_malloc_fail = 0;
+static int malloc_failure_enabled = 0, break_on_malloc_fail = 0,
+           any_malloc_failed = 0;
 
 static void malloc_exit_handler(void) {
   CRYPTO_STATIC_MUTEX_lock_read(&malloc_failure_lock);
-  if (malloc_failure_enabled && current_malloc_count > malloc_number_to_fail) {
+  if (any_malloc_failed) {
+    // Signal to the test driver that some allocation failed, so it knows to
+    // increment the counter and continue.
     _exit(88);
   }
   CRYPTO_STATIC_MUTEX_unlock_read(&malloc_failure_lock);
@@ -183,6 +186,7 @@
   CRYPTO_STATIC_MUTEX_lock_write(&malloc_failure_lock);
   int should_fail = current_malloc_count == malloc_number_to_fail;
   current_malloc_count++;
+  any_malloc_failed = any_malloc_failed || should_fail;
   CRYPTO_STATIC_MUTEX_unlock_write(&malloc_failure_lock);
 
   if (should_fail && break_on_malloc_fail) {
@@ -194,6 +198,12 @@
   return should_fail;
 }
 
+void OPENSSL_reset_malloc_counter_for_testing(void) {
+  CRYPTO_STATIC_MUTEX_lock_write(&malloc_failure_lock);
+  current_malloc_count = 0;
+  CRYPTO_STATIC_MUTEX_unlock_write(&malloc_failure_lock);
+}
+
 #else
 static int should_fail_allocation(void) { return 0; }
 #endif

crypto/test/gtest_main.h

diff --git a/crypto/test/gtest_main.h b/crypto/test/gtest_main.h
index 20ccf21..05d468e 100644
--- a/crypto/test/gtest_main.h
+++ b/crypto/test/gtest_main.h
@@ -31,13 +31,15 @@
 #include <signal.h>
 #endif
 
+#include "../internal.h"
+
 
 BSSL_NAMESPACE_BEGIN
 
-class ErrorTestEventListener : public testing::EmptyTestEventListener {
+class TestEventListener : public testing::EmptyTestEventListener {
  public:
-  ErrorTestEventListener() {}
-  ~ErrorTestEventListener() override {}
+  TestEventListener() {}
+  ~TestEventListener() override {}
 
   void OnTestEnd(const testing::TestInfo &test_info) override {
     if (test_info.result()->Failed()) {
@@ -48,6 +50,13 @@
       // error queue without printing.
       ERR_clear_error();
     }
+
+    // Malloc failure testing is quadratic in the number of mallocs. Running
+    // multiple tests sequentially thus scales badly. Reset the malloc counter
+    // between tests. This way we will test, each test with the first allocation
+    // failing, then the second, and so on, until the test with the most
+    // allocations runs out.
+    OPENSSL_reset_malloc_counter_for_testing();
   }
 };
 
@@ -75,8 +84,7 @@
   signal(SIGPIPE, SIG_IGN);
 #endif
 
-  testing::UnitTest::GetInstance()->listeners().Append(
-      new ErrorTestEventListener);
+  testing::UnitTest::GetInstance()->listeners().Append(new TestEventListener);
 }
 
 BSSL_NAMESPACE_END