Fix heap corruption during stable name allocation
authorÖmer Sinan Ağacan <omeragacan@gmail.com>
Wed, 21 Nov 2018 17:03:38 +0000 (20:03 +0300)
committerBen Gamari <ben@smart-cactus.org>
Thu, 22 Nov 2018 19:01:30 +0000 (14:01 -0500)
See #15906 for the problem. To fix we simply call `allocate()` instead of
`ALLOC_PRIM()`. `allocate()` does not trigger GC when the nursery is full,
instead it extends it.

Test Plan:
This validates. memo001 now passes with `-debug` compile parameter. I'll add
another test that runs memo001 with `-debug` once I figure out how to use
stdout files for multiple tests.

Reviewers: simonmar, bgamari, erikd

Reviewed By: simonmar

Subscribers: rwbarton, carter

GHC Trac Issues: #15906

Differential Revision: https://phabricator.haskell.org/D5342

(cherry picked from commit 691aa715cf43bf9d88ee32bca37e471bae35adfb)

rts/PrimOps.cmm

index 6081fab..cbb8e54 100644 (file)
@@ -1848,14 +1848,25 @@ stg_makeStableNamezh ( P_ obj )
 {
     W_ index, sn_obj;
 
+    MAYBE_GC_P(stg_makeStableNamezh, obj);
+
     (index) = ccall lookupStableName(obj "ptr");
 
     /* Is there already a StableName for this heap object?
      *  stable_name_table is a pointer to an array of snEntry structs.
      */
     if ( snEntry_sn_obj(W_[stable_name_table] + index*SIZEOF_snEntry) == NULL ) {
-        ALLOC_PRIM (SIZEOF_StgStableName);
-        sn_obj = Hp - SIZEOF_StgStableName + WDS(1);
+        // At this point we have a snEntry, but it doesn't look as used to the
+        // GC yet because we don't have a StableName object for the sn_obj field
+        // (remember that sn_obj == NULL means the entry is free). So if we call
+        // GC here we end up skipping the snEntry in gcStableNameTable(). This
+        // caused #15906. Solution: use allocate(), which does not call GC.
+        //
+        // (Alternatively we could use a special value for the sn_obj field to
+        // indicate that the entry is being allocated and not free, but that's
+        // too complicated and doesn't buy us much. See D5342?id=18700.)
+        ("ptr" sn_obj) = ccall allocate(MyCapability() "ptr",
+                                        BYTES_TO_WDS(SIZEOF_StgStableName));
         SET_HDR(sn_obj, stg_STABLE_NAME_info, CCCS);
         StgStableName_sn(sn_obj) = index;
         snEntry_sn_obj(W_[stable_name_table] + index*SIZEOF_snEntry) = sn_obj;