[MLton-commit] r6527

[Matthew Fluet]

Avoid an overflow when initializing an __mpz_struct for an IntInf result.

The _mp_alloc field of an __mpz_struct is declared as an int.  On a
64-bit architecture, the computation of the number of available limbs
from the space available in the heap occurs at 64-bits.  With
>8Gb heap and sizeof(mp_limb_t) == 4 or >16Gb heap and
sizeof(mp_limb_t) == 8, the result will overflow when being assigned
to an int.

----------------------------------------------------------------------

U   mlton/trunk/runtime/gc/int-inf.c
U   mlton/trunk/runtime/gc/int-inf.h

----------------------------------------------------------------------

Modified: mlton/trunk/runtime/gc/int-inf.c
===================================================================
--- mlton/trunk/runtime/gc/int-inf.c	2008-04-03 14:28:06 UTC (rev 6526)
+++ mlton/trunk/runtime/gc/int-inf.c	2008-04-04 21:43:57 UTC (rev 6527)
@@ -1,5 +1,5 @@
-/* Copyright (C) 1999-2005, 2007 Henry Cejtin, Matthew Fluet, Suresh
- *    Jagannathan, and Stephen Weeks.
+/* Copyright (C) 1999-2005, 2007-2008 Henry Cejtin, Matthew Fluet,
+ *    Suresh Jagannathan, and Stephen Weeks.
  * Copyright (C) 1997-2000 NEC Research Institute.
  *
  * MLton is released under a BSD-style license.
@@ -57,26 +57,29 @@
       const objptr highBitMask = (objptr)1 << (CHAR_BIT * OBJPTR_SIZE - 1);
       bool neg = (arg & highBitMask) != (objptr)0;
       if (neg) {
-        res->_mp_size = - (mp_size_t)LIMBS_PER_OBJPTR;
+        res->_mp_size = - LIMBS_PER_OBJPTR;
         arg = -((arg >> 1) | highBitMask);
       } else {
-        res->_mp_size = (mp_size_t)LIMBS_PER_OBJPTR;
+        res->_mp_size = LIMBS_PER_OBJPTR;
         arg = (arg >> 1);
       }
-      for (unsigned int i = 0; i < LIMBS_PER_OBJPTR; i++) {
+      for (int i = 0; i < LIMBS_PER_OBJPTR; i++) {
         space[i] = (mp_limb_t)arg;
         // The conditional below is to quell a gcc warning:
         //   right shift count >= width of type
         // When 1 == LIMBS_PER_OBJPTR, the for loop will not continue,
         // so the shift doesn't matter.
         arg = arg >> (1 == LIMBS_PER_OBJPTR ?
-                        0 :
-                        CHAR_BIT * sizeof(mp_limb_t));
+                      0 : CHAR_BIT * sizeof(mp_limb_t));
       }
     }
   } else {
     bp = toBignum (s, arg);
-    res->_mp_alloc = bp->length - 1;
+    /* The _mp_alloc field is declared as int.  
+     * No possibility of an overflowing assignment, as all *huge*
+     * intInfs must have come from some previous GnuMP evaluation.
+     */
+    res->_mp_alloc = (int)(bp->length - 1);
     res->_mp_d = (mp_limb_t*)(bp->obj.body.limbs);
     res->_mp_size = bp->obj.body.isneg ? - res->_mp_alloc : res->_mp_alloc;
   }
@@ -95,13 +98,19 @@
 void initIntInfRes (GC_state s, __mpz_struct *res,
                     __attribute__ ((unused)) size_t bytes) {
   GC_intInf bp;
+  size_t nlimbs;
 
   assert (bytes <= (size_t)(s->limitPlusSlop - s->frontier));
   bp = (GC_intInf)s->frontier;
   /* We have as much space for the limbs as there is to the end of the
    * heap.  Divide by (sizeof(mp_limb_t)) to get number of limbs.
    */
-  res->_mp_alloc = (s->limitPlusSlop - (pointer)bp->obj.body.limbs) / (sizeof(mp_limb_t));
+  nlimbs = ((size_t)(s->limitPlusSlop - (pointer)bp->obj.body.limbs)) / (sizeof(mp_limb_t));
+  /* The _mp_alloc field is declared as int. 
+   * Avoid an overflowing assignment, which could happen with huge
+   * heaps.
+   */
+  res->_mp_alloc = (int)(min(nlimbs,(size_t)INT_MAX));
   res->_mp_d = (mp_limb_t*)(bp->obj.body.limbs);
   res->_mp_size = 0; /* is this necessary? */
 }
@@ -116,7 +125,7 @@
  */
 objptr finiIntInfRes (GC_state s, __mpz_struct *res, size_t bytes) {
   GC_intInf bp;
-  mp_size_t size;
+  int size;
 
   assert ((res->_mp_size == 0)
           or (res->_mp_d[(res->_mp_size < 0
@@ -136,6 +145,7 @@
     size = - size;
   } else
     bp->obj.body.isneg = FALSE;
+  assert (size >= 0);
   if (size <= 1) {
     uintmax_t val, ans;
 
@@ -159,8 +169,8 @@
     }
   }
   setFrontier (s, (pointer)(&bp->obj.body.limbs[size]), bytes);
-  bp->counter = 0;
-  bp->length = size + 1; /* +1 for isneg field */
+  bp->counter = (GC_arrayCounter)0;
+  bp->length = (GC_arrayLength)(size + 1); /* +1 for isneg field */
   bp->header = GC_INTINF_HEADER;
   return pointerToObjptr ((pointer)&bp->obj, s->heap.start);
 }

Modified: mlton/trunk/runtime/gc/int-inf.h
===================================================================
--- mlton/trunk/runtime/gc/int-inf.h	2008-04-03 14:28:06 UTC (rev 6526)
+++ mlton/trunk/runtime/gc/int-inf.h	2008-04-04 21:43:57 UTC (rev 6527)
@@ -1,4 +1,4 @@
-/* Copyright (C) 1999-2007 Henry Cejtin, Matthew Fluet, Suresh
+/* Copyright (C) 1999-2008 Henry Cejtin, Matthew Fluet, Suresh
  *    Jagannathan, and Stephen Weeks.
  * Copyright (C) 1997-2000 NEC Research Institute.
  *
@@ -54,7 +54,7 @@
                     (sizeof(objptr) % sizeof(mp_limb_t) == 0));
 #define LIMBS_PER_OBJPTR ( \
         sizeof(mp_limb_t) >= sizeof(objptr) ? \
-        1 : sizeof(objptr) / sizeof(mp_limb_t))
+        1 : (int)(sizeof(objptr) / sizeof(mp_limb_t)))
 
 void initIntInf (GC_state s);
 static inline void fillIntInfArg (GC_state s, objptr arg, __mpz_struct *res,