diff mbox

[BZ,20628] make mallinfo saturating

Message ID xn1szvrice.fsf@greed.delorie.com
State New, archived
Headers

Commit Message

DJ Delorie Oct. 4, 2016, 8:04 p.m. UTC 
  "Carlos O'Donell" <carlos@redhat.com> writes:
> DJ, Care to make a version 2 of the patch?

Version 2.  Changed some of the counters to larger types and tested those
too, which took about 130Gb of virtual memory and lots of waiting...

	[BZ #20628]
	* malloc/malloc.c (int_mallinfo): Use saturating add instead of
	overflow to collect statistics into a fixed "int" container.

Comments

Paul Eggert Oct. 6, 2016, 12:45 a.m. UTC | #1 
On 10/04/2016 01:04 PM, DJ Delorie wrote:
> +/* Saturated add - add ADD to SUM.  If the result exceeds the range of
> +   "int", set SUM to UINT_MAX instead ((int)-1).  Assumes ADD and SUM
> +   are positive.  The published ABI prevents us from bumping "int" to
> +   a larger type.  */
> +#define SAT_ADD(SUM, ADD) \
> +  ({ INTERNAL_SIZE_T tmp = (INTERNAL_SIZE_T)(SUM) + (INTERNAL_SIZE_T)(ADD); SUM = (tmp > UINT_MAX) ? -1 : tmp; })
> +
> +/* Likewise, but assign ADD to SUM.  */
> +#define SAT_SET(SUM, ADD) \
> +  ({ SUM = ((INTERNAL_SIZE_T)(ADD) > UINT_MAX) ? -1 : (ADD); })
These don't look right, as INTERNAL_SIZE_T might not be wider than int, 
which means the + in SAT_ADD will wrap around before SAT_ADD gets a 
chance to check for overflow. Also, if SUM is INT_MIN then SAT_ADD(SUM, 
1) should set it to INT_MIN + 1, but that doesn't happen when 
INTERNAL_SIZE_T is wider than int.

How about something like the following instead?

   #define MIN(a, b) ((a)<(b)?(a):(b))

   /* Return the sum of the unsigned int A and the nonnegative integer B.
      If the result is not representable as an unsigned int, return 
UINT_MAX.
      B's type may be any integer type.  */
   #define UINT_SATURATED_ADD(a, b) \
      ({ unsigned a1 = a, b1 = MIN (b, UINT_MAX), sum1 = a1 + b1; \
         sum1 < a1 ? UINT_MAX : sum1; })

and then set 'm->smblks = UINT_SATURATED_ADD (m->sm_blks, nfastblocks);' 
and 'm->hblks = UINT_SATURATED_ADD (0, mp_.n_mmaps);', etc.
diff mbox

Patch

diff --git a/malloc/malloc.c b/malloc/malloc.c
index ef04360..e3f4693 100644
--- a/malloc/malloc.c
+++ b/malloc/malloc.c
@@ -224,6 +224,7 @@ 
 #include <unistd.h>
 #include <stdio.h>    /* needed for malloc_stats */
 #include <errno.h>
+#include <limits.h>
 
 #include <shlib-compat.h>
 
@@ -4594,8 +4595,8 @@  int_mallinfo (mstate av, struct mallinfo *m)
   mchunkptr p;
   INTERNAL_SIZE_T avail;
   INTERNAL_SIZE_T fastavail;
-  int nblocks;
-  int nfastblocks;
+  INTERNAL_SIZE_T nblocks;
+  INTERNAL_SIZE_T nfastblocks;
 
   /* Ensure initialization */
   if (av->top == 0)
@@ -4633,18 +4634,29 @@  int_mallinfo (mstate av, struct mallinfo *m)
         }
     }
 
-  m->smblks += nfastblocks;
-  m->ordblks += nblocks;
-  m->fordblks += avail;
-  m->uordblks += av->system_mem - avail;
-  m->arena += av->system_mem;
-  m->fsmblks += fastavail;
+/* Saturated add - add ADD to SUM.  If the result exceeds the range of
+   "int", set SUM to UINT_MAX instead ((int)-1).  Assumes ADD and SUM
+   are positive.  The published ABI prevents us from bumping "int" to
+   a larger type.  */
+#define SAT_ADD(SUM, ADD) \
+  ({ INTERNAL_SIZE_T tmp = (INTERNAL_SIZE_T)(SUM) + (INTERNAL_SIZE_T)(ADD); SUM = (tmp > UINT_MAX) ? -1 : tmp; })
+
+/* Likewise, but assign ADD to SUM.  */
+#define SAT_SET(SUM, ADD) \
+  ({ SUM = ((INTERNAL_SIZE_T)(ADD) > UINT_MAX) ? -1 : (ADD); })
+
+  SAT_ADD (m->smblks, nfastblocks);
+  SAT_ADD (m->ordblks, nblocks);
+  SAT_ADD (m->fordblks, avail);
+  SAT_ADD (m->uordblks, av->system_mem - avail);
+  SAT_ADD (m->arena, av->system_mem);
+  SAT_ADD (m->fsmblks, fastavail);
   if (av == &main_arena)
     {
-      m->hblks = mp_.n_mmaps;
-      m->hblkhd = mp_.mmapped_mem;
-      m->usmblks = 0;
-      m->keepcost = chunksize (av->top);
+      SAT_SET (m->hblks, mp_.n_mmaps);
+      SAT_SET (m->hblkhd, mp_.mmapped_mem);
+      SAT_SET (m->usmblks, 0);
+      SAT_SET (m->keepcost, chunksize (av->top));
     }
 }