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>
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.
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
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
