internals: Use negative numbers to indicate no malloc failure simulation

If we set the countdown to simulating a failed allocation to
_DBUS_INT_MAX, then it will decrement every time we allocate memory,
eventually reaching 0 and triggering a simulated malloc failure.
In practice this does not happen during unit testing, because all of
our tests are (intentionally!) short enough that this can't happen,
but it can happen if a build of dbus with embedded tests enabled is
used for the "real" dbus-daemon or a "real" D-Bus service, either
during debugging or unintentionally, as noted on dbus/dbus!493.

We cannot simply special-case `_DBUS_INT_MAX` to never be decremented,
because _dbus_test_oom_handling() relies on the counter being
decremented even while we are not simulating malloc failure, as a way
to count the number of allocations as an upper bound for how long to
set the countdown during subsequent test runs.

Instead, reserve all negative numbers to represent the absence of
malloc failure simulation, while still being able to count allocations
by comparing two different negative numbers.

Resolves: https://gitlab.freedesktop.org/dbus/dbus/-/issues/535
Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit 0735c401a7b3c9e56096fcde8139e773067302bd)

diff --git a/dbus/dbus-internals.c b/dbus/dbus-internals.c
index 6a23ff5b5dc313a8b4db7b55512b34cdf0af0f0d..dd58137260a1198c5bdee2c0727df5f07620e206 100644 (file)
--- a/dbus/dbus-internals.c
+++ b/dbus/dbus-internals.c
@@ -1097,7 +1097,7 @@ run_failing_each_malloc (int                    n_mallocs,
       n_mallocs -= 1;
     }
 
-  _dbus_set_fail_alloc_counter (_DBUS_INT_MAX);
+  _dbus_set_fail_alloc_counter (-1);
 
   return TRUE;
 }                        
@@ -1127,14 +1127,18 @@ _dbus_test_oom_handling (const char             *description,
 
   /* Run once to see about how many mallocs are involved */
   
-  _dbus_set_fail_alloc_counter (_DBUS_INT_MAX);
+  _dbus_set_fail_alloc_counter (-1);
 
   _dbus_test_diag ("Running \"%s\" once to count mallocs", description);
 
   if (!(* func) (data, TRUE))
     return FALSE;
 
-  approx_mallocs = _DBUS_INT_MAX - _dbus_get_fail_alloc_counter ();
+  /* We have decremented the counter once per allocation, so for example
+   * if there were 10 allocations, it will have changed from -1 to -11.
+   * Subtract from -1 to get the positive number of allocations that took
+   * place. */
+  approx_mallocs = -1 - _dbus_get_fail_alloc_counter ();
 
   _dbus_test_diag ("\"%s\" has about %d mallocs in total",
                    description, approx_mallocs);

diff --git a/dbus/dbus-internals.h b/dbus/dbus-internals.h
index 4aef24ca72ee1b688bd5bbc7d0c36f74628a9d0d..0035a0c2de8064ee8b6fb8dbd0e05a09b5ed29a4 100644 (file)
--- a/dbus/dbus-internals.h
+++ b/dbus/dbus-internals.h
@@ -384,7 +384,7 @@ dbus_bool_t _dbus_test_oom_handling (const char             *description,
                                      void                   *data);
 #else
 #define _dbus_set_fail_alloc_counter(n)
-#define _dbus_get_fail_alloc_counter _DBUS_INT_MAX
+#define _dbus_get_fail_alloc_counter (-1)
 
 /* These are constant expressions so that blocks
  * they protect should be optimized away

diff --git a/dbus/dbus-memory.c b/dbus/dbus-memory.c
index cf3dd881a19fd6db9094c8d4a1547d93ded366f9..7a11b38551fd7a93b30e7b5132250906c329d3cd 100644 (file)
--- a/dbus/dbus-memory.c
+++ b/dbus/dbus-memory.c
@@ -105,7 +105,7 @@
 static dbus_bool_t debug_initialized = FALSE;
 static int fail_nth = -1;
 static size_t fail_size = 0;
-static int fail_alloc_counter = _DBUS_INT_MAX;
+static int fail_alloc_counter = -1;
 static int n_failures_per_failure = 1;
 static int n_failures_this_failure = 0;
 static dbus_bool_t guards = FALSE;
@@ -190,7 +190,11 @@ _dbus_disable_mem_pools (void)
  * Sets the number of allocations until we simulate a failed
  * allocation. If set to 0, the next allocation to run
  * fails; if set to 1, one succeeds then the next fails; etc.
- * Set to _DBUS_INT_MAX to not fail anything. 
+ *
+ * Set to a negative number to not fail anything.
+ * In this case, the counter is still decremented every time we allocate
+ * memory (until it reaches _DBUS_INT_MIN), which can be used to estimate
+ * the number of allocations in a particular unit test case.
  *
  * @param until_next_fail number of successful allocs before one fails
  */
@@ -208,7 +212,11 @@ _dbus_set_fail_alloc_counter (int until_next_fail)
 
 /**
  * Gets the number of successful allocs until we'll simulate
- * a failed alloc.
+ * a failed alloc, or negative if we will not simulate a failed alloc.
+ *
+ * If negative, the counter indicates the number of allocations since
+ * it was set to a known value, which can be used to count allocations:
+ * see _dbus_set_fail_alloc_counter() for details.
  *
  * @returns current counter value
  */
@@ -257,8 +265,20 @@ _dbus_decrement_fail_alloc_counter (void)
 {
   _dbus_initialize_malloc_debug ();
 
-  if (fail_alloc_counter <= 0)
+  if (fail_alloc_counter < 0)
+    {
+      /* We never fail in this case, but we still decrement the counter,
+       * so that _dbus_test_oom_handling() can use it to count allocations.
+       * Saturate at _DBUS_INT_MIN to avoid undefined integer overflow
+       * (or in this case underflow). */
+      if (fail_alloc_counter > _DBUS_INT_MIN)
+        fail_alloc_counter -= 1;
+
+      return FALSE;
+    }
+  else if (fail_alloc_counter == 0)
     {
+      /* It's time to pretend we ran out of memory. */
       if (backtrace_on_fail_alloc)
         _dbus_print_backtrace ();
 
@@ -267,11 +287,7 @@ _dbus_decrement_fail_alloc_counter (void)
       n_failures_this_failure += 1;
       if (n_failures_this_failure >= n_failures_per_failure)
         {
-          if (fail_nth >= 0)
-            fail_alloc_counter = fail_nth;
-          else
-            fail_alloc_counter = _DBUS_INT_MAX;
-
+          fail_alloc_counter = fail_nth;
           n_failures_this_failure = 0;
 
           _dbus_verbose ("reset fail alloc counter to %d\n", fail_alloc_counter);
@@ -281,6 +297,8 @@ _dbus_decrement_fail_alloc_counter (void)
     }
   else
     {
+      /* Don't fail this allocation, but count down to the next time we
+       * will fail an allocation. */
       fail_alloc_counter -= 1;
       return FALSE;
     }

diff --git a/dbus/dbus-mempool.c b/dbus/dbus-mempool.c
index 5bc4a97b640f6f6e6c3c993cfede1aa2972ae8c3..c817c049aea96b82d62a650db610d2fce5210410 100644 (file)
--- a/dbus/dbus-mempool.c
+++ b/dbus/dbus-mempool.c
@@ -317,7 +317,7 @@ _dbus_mem_pool_alloc (DBusMemPool *pool)
                * malloc here for purposes of failed alloc simulation.
                */
               saved_counter = _dbus_get_fail_alloc_counter ();
-              _dbus_set_fail_alloc_counter (_DBUS_INT_MAX);
+              _dbus_set_fail_alloc_counter (-1);
 #endif
           
               if (pool->zero_elements)