[MLton-commit] r4988

Matthew Fluet fluet at mlton.org
Tue Dec 19 10:08:42 PST 2006


Fixed an assertion failure in GC_arrayAllocate with -align 8:

fenrir:~/devel/mlton/mlton.svn.trunk/regression fluet$ ../build/bin/mlton -debug true -align 8 flat-array.2.sml 
fenrir:~/devel/mlton/mlton.svn.trunk/regression fluet$ ./flat-array.2
gc/array-allocate.c:91: assert(next <= last) failed.
Abort trap

With -align 8, we may need to leave an extra 32-bit word at the end of
the array in order to properly align the next object.  Hence, last
(where last = frontier + arraySize and where arraySize =
align(bytesPerElement * numElements + GC_ARRAY_HEADER_SIZE)) may
correspond to an address beyond the end of the array proper.  The
loops to initialize all pointers in an array with BOGUS_OBJPTR use p <
last as a termination condition.  Hence, the loops could access
addresses beyond the end of the array proper.  

When the array has only object pointers, at worst, we could access one
extra 32-bit word beyond the end of the array proper, but that extra
32-bit word had already been accounted for in the bytesRequested and
the heap check; so, while we may access an address beyond the end of
the array proper, that address "belongs" to the array and is certainly
within the heap.

On the other hand, when the array has both non object pointers and
object pointers and last correspondst to an address beyond the end of
the array proper, then we attempt to initialize an extra array
element.  Since there will be at least one word or non object pointer
data, we will access at least one object pointer at an address that is
strictly beyond the space allocated for the array and may even be
outside the heap.

The solution is simple: compute last according to the size of the
array proper, but bump the frontier (or oldGenSize and
cumulativeStatistics.bytesAllocated) by the aligned size.


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

U   mlton/trunk/runtime/gc/array-allocate.c

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

Modified: mlton/trunk/runtime/gc/array-allocate.c
===================================================================
--- mlton/trunk/runtime/gc/array-allocate.c	2006-12-19 16:14:45 UTC (rev 4987)
+++ mlton/trunk/runtime/gc/array-allocate.c	2006-12-19 18:08:40 UTC (rev 4988)
@@ -10,66 +10,74 @@
                           size_t ensureBytesFree, 
                           GC_arrayLength numElements, 
                           GC_header header) {
-  uintmax_t arraySizeMax;
-  size_t arraySize;
+  uintmax_t arraySizeMax, arraySizeAlignedMax;
+  size_t arraySize, arraySizeAligned;
   size_t bytesPerElement;
   uint16_t bytesNonObjptrs;
   uint16_t numObjptrs;
   pointer frontier;
   pointer last;
-  pointer res;
+  pointer result;
 
   splitHeader(s, header, NULL, NULL, &bytesNonObjptrs, &numObjptrs);
   if (DEBUG)
     fprintf (stderr, "GC_arrayAllocate (%zu, "FMTARRLEN", "FMTHDR")\n",
              ensureBytesFree, numElements, header);
   bytesPerElement = bytesNonObjptrs + (numObjptrs * OBJPTR_SIZE);
-  arraySizeMax = 
-    alignMax ((uintmax_t)bytesPerElement * (uintmax_t)numElements + GC_ARRAY_HEADER_SIZE,
-              s->alignment);
-  if (arraySizeMax >= (uintmax_t)SIZE_MAX)
+  arraySizeMax =
+    (uintmax_t)bytesPerElement * (uintmax_t)numElements + GC_ARRAY_HEADER_SIZE;
+  arraySizeAlignedMax = alignMax (arraySizeMax, s->alignment);
+  if (arraySizeAlignedMax >= (uintmax_t)SIZE_MAX)
     die ("Out of memory: cannot allocate array with %s bytes.",
-         uintmaxToCommaString(arraySizeMax));
+         uintmaxToCommaString(arraySizeAlignedMax));
   arraySize = (size_t)arraySizeMax;
-  if (arraySize < GC_ARRAY_HEADER_SIZE + OBJPTR_SIZE)
+  arraySizeAligned = (size_t)arraySizeAlignedMax;
+  if (arraySizeAligned < GC_ARRAY_HEADER_SIZE + OBJPTR_SIZE) {
     /* Create space for forwarding pointer. */
-    arraySize = GC_ARRAY_HEADER_SIZE + OBJPTR_SIZE;
+    arraySize = GC_ARRAY_HEADER_SIZE;
+    arraySizeAligned = align(GC_ARRAY_HEADER_SIZE + OBJPTR_SIZE, s->alignment);
+  }
   if (DEBUG_ARRAY)
-    fprintf (stderr, "array with "FMTARRLEN" elts of size %zu and total size %s.  Ensure %s bytes free.\n",
+    fprintf (stderr, 
+             "Array with "FMTARRLEN" elts of size %zu and size %s and aligned size %s.  "
+             "Ensure %s bytes free.\n",
              numElements, bytesPerElement, 
              uintmaxToCommaString(arraySize),
+             uintmaxToCommaString(arraySizeAligned),
              uintmaxToCommaString(ensureBytesFree));
-  if (arraySize >= s->controls.oldGenArraySize) {
-    if (not hasHeapBytesFree (s, arraySize, ensureBytesFree)) {
+  if (arraySizeAligned >= s->controls.oldGenArraySize) {
+    if (not hasHeapBytesFree (s, arraySizeAligned, ensureBytesFree)) {
       enter (s);
-      performGC (s, arraySize, ensureBytesFree, FALSE, TRUE);
+      performGC (s, arraySizeAligned, ensureBytesFree, FALSE, TRUE);
       leave (s);
     }
     frontier = s->heap.start + s->heap.oldGenSize;
-    last = frontier + arraySize;
-    s->heap.oldGenSize += arraySize;
-    s->cumulativeStatistics.bytesAllocated += arraySize;
+    s->heap.oldGenSize += arraySizeAligned;
+    s->cumulativeStatistics.bytesAllocated += arraySizeAligned;
   } else {
     size_t bytesRequested;
+    pointer newFrontier;
 
-    bytesRequested = arraySize + ensureBytesFree;
+    bytesRequested = arraySizeAligned + ensureBytesFree;
     if (not hasHeapBytesFree (s, 0, bytesRequested)) {
       enter (s);
       performGC (s, 0, bytesRequested, FALSE, TRUE);
       leave (s);
     }
     frontier = s->frontier;
-    last = frontier + arraySize;
-    assert (isFrontierAligned (s, last));
-    s->frontier = last;
+    newFrontier = frontier + arraySizeAligned;
+    assert (isFrontierAligned (s, newFrontier));
+    s->frontier = newFrontier;
   }
+  last = frontier + arraySize;
   *((GC_arrayCounter*)(frontier)) = 0;
   frontier = frontier + GC_ARRAY_COUNTER_SIZE;
   *((GC_arrayLength*)(frontier)) = numElements;
   frontier = frontier + GC_ARRAY_LENGTH_SIZE;
   *((GC_header*)(frontier)) = header;
   frontier = frontier + GC_HEADER_SIZE;
-  res = frontier;
+  result = frontier;
+  assert (isAligned ((size_t)result, s->alignment));
   /* Initialize all pointers with BOGUS_OBJPTR. */
   if (1 <= numObjptrs and 0 < numElements) {
     pointer p;
@@ -94,10 +102,10 @@
       }
     }
   }
-  GC_profileAllocInc (s, arraySize);
+  GC_profileAllocInc (s, arraySizeAligned);
   if (DEBUG_ARRAY) {
-    fprintf (stderr, "GC_arrayAllocate done.  res = "FMTPTR"  frontier = "FMTPTR"\n",
-             (uintptr_t)res, (uintptr_t)s->frontier);
+    fprintf (stderr, "GC_arrayAllocate done.  result = "FMTPTR"  frontier = "FMTPTR"\n",
+             (uintptr_t)result, (uintptr_t)s->frontier);
     displayGCState (s, stderr);
   }
   assert (ensureBytesFree <= (size_t)(s->limitPlusSlop - s->frontier));
@@ -105,5 +113,5 @@
    * unless we did the GC, we never set s->currentThread->stack->used
    * to reflect what the mutator did with stackTop.
    */
-  return res;
+  return result;
 }       




More information about the MLton-commit mailing list