[MLton-commit] r7461

[Matthew Fluet]

Replaced use of memcpy by memmove in mark-compact garbage collector.

Fixed bug in the mark-compact garbage collector where the C library's
memcpy was used to move objects during the compaction phase.  Objects
could be moved by an arbitrarily small amount, leading to cases where
the object's destination overlapped with the object's source.  This
could lead to heap corruption and segmentation faults with newer
versions of gcc and/or glibc, which assume that src and dst in a
memcpy do not overlap.
----------------------------------------------------------------------

U   mlton/trunk/doc/changelog
U   mlton/trunk/runtime/gc/mark-compact.c
U   mlton/trunk/runtime/gc/virtual-memory.c

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

Modified: mlton/trunk/doc/changelog
===================================================================
--- mlton/trunk/doc/changelog	2010-05-09 21:50:42 UTC (rev 7460)
+++ mlton/trunk/doc/changelog	2010-05-13 12:48:35 UTC (rev 7461)
@@ -62,6 +62,13 @@
     * Eliminated top-level 'type int = Int.int' in output.
     * Include (*#line line:col "file.grm" *) directives in output.
 
+* 2010-05-12
+  - Fixed bug in the mark-compact garbage collector where the C
+    library's memcpy was used to move objects during the compaction
+    phase; this could lead to heap corruption and segmentation faults
+    with newer versions of gcc and/or glibc, which assume that src and
+    dst in a memcpy do not overlap.
+
 * 2010-03-12
   - Fixed bug in elaboration of datatype declarations with withtype
     bindings.

Modified: mlton/trunk/runtime/gc/mark-compact.c
===================================================================
--- mlton/trunk/runtime/gc/mark-compact.c	2010-05-09 21:50:42 UTC (rev 7460)
+++ mlton/trunk/runtime/gc/mark-compact.c	2010-05-13 12:48:35 UTC (rev 7461)
@@ -1,4 +1,5 @@
-/* Copyright (C) 1999-2008 Henry Cejtin, Matthew Fluet, Suresh
+/* Copyright (C) 2010 Matthew Fluet.
+ * Copyright (C) 1999-2008 Henry Cejtin, Matthew Fluet, Suresh
  *    Jagannathan, and Stephen Weeks.
  * Copyright (C) 1997-2000 NEC Research Institute.
  *
@@ -310,9 +311,9 @@
       *headerp = header & ~MARK_MASK;
       /* slide */
       if (DEBUG_MARK_COMPACT)
-        fprintf (stderr, "sliding "FMTPTR" down %"PRIuMAX"\n",
-                 (uintptr_t)front, (uintmax_t)gap);
-      GC_memcpy (front, front - gap, size);
+        fprintf (stderr, "sliding "FMTPTR" down %"PRIuMAX" to "FMTPTR"\n",
+                 (uintptr_t)front, (uintmax_t)gap, (uintptr_t)(front - gap));
+      GC_memmove (front, front - gap, size);
       gap += skipGap;
       front += size + skipFront;
       goto updateObject;

Modified: mlton/trunk/runtime/gc/virtual-memory.c
===================================================================
--- mlton/trunk/runtime/gc/virtual-memory.c	2010-05-09 21:50:42 UTC (rev 7460)
+++ mlton/trunk/runtime/gc/virtual-memory.c	2010-05-13 12:48:35 UTC (rev 7461)
@@ -1,4 +1,5 @@
-/* Copyright (C) 1999-2008 Henry Cejtin, Matthew Fluet, Suresh
+/* Copyright (C) 2010 Matthew Fluet.
+ * Copyright (C) 1999-2008 Henry Cejtin, Matthew Fluet, Suresh
  *    Jagannathan, and Stephen Weeks.
  * Copyright (C) 1997-2000 NEC Research Institute.
  *
@@ -25,8 +26,21 @@
   assert (isAligned ((size_t)src, sizeof(unsigned int)));
   assert (isAligned ((size_t)dst, sizeof(unsigned int)));
   assert (isAligned (size, sizeof(unsigned int)));
-  assert (dst <= src or src + size <= dst);
   if (src == dst)
     return;
+  assert (! (src <= dst and dst < src + size));
+  assert (! (dst <= src and src < dst + size));
   memcpy (dst, src, size);
 }
+
+static inline void GC_memmove (pointer src, pointer dst, size_t size) {
+  if (DEBUG_DETAILED)
+    fprintf (stderr, "GC_memmove ("FMTPTR", "FMTPTR", %"PRIuMAX")\n",
+             (uintptr_t)src, (uintptr_t)dst, (uintmax_t)size);
+  assert (isAligned ((size_t)src, sizeof(unsigned int)));
+  assert (isAligned ((size_t)dst, sizeof(unsigned int)));
+  assert (isAligned (size, sizeof(unsigned int)));
+  if (src == dst)
+    return;
+  memmove (dst, src, size);
+}