GitLab

Marge Bot pushed to branch master at Glasgow Haskell Compiler / GHC

Commits:

4bd193fa

by Sylvain Henry at 2026-08-13T10:17:00-04:00

RTS: fix LDV profiler's slop skipping (#27585)

processHeapForDead was the one heap scanner not updated for the slop
marker encoding introduced in #19048. It still assumed slop is zeroed:

    while (p < bd->free && !*p) p++;   // skip slop

so it stopped at the (StgWord)(-1) sentinel and passed it to
processHeapClosureForDead. IS_FORWARDING_PTR(-1) holds, hence a garbage
size was read out of LDVW and the scan ran off the block, tripping
ASSERT(p == bd->free) on a debug RTS and silently corrupting the census
otherwise.

The loop was hand-copied in four places, so factor it out into skipSlop
in ClosureMacros.h and use it in ProfHeap.c, Sanity.c, Printer.c and
LdvProfile.c.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

11 changed files:

changelog.d/fix-heap-census-large-arrays-19048
rts/LdvProfile.c
rts/PrimOps.cmm
rts/Printer.c
rts/ProfHeap.c
rts/include/rts/storage/ClosureMacros.h
rts/sm/Sanity.c
rts/sm/Storage.c
+ testsuite/tests/rts/T27585.hs
+ testsuite/tests/rts/T27585.stdout
testsuite/tests/rts/all.T

Changes:

changelog.d/fix-heap-census-large-arrays-19048

@@ -2,5 +2,5 @@ section: rts
  synopsis: Correctly mark slop bytes when shrinking large arrays.
    Heap census no longer traverses garbage-collected closures when profiling
    is off.
 -issues: #19048
 -mrs: !15685
 +issues: #19048 #27585
 +mrs: !15685 !16452

rts/LdvProfile.c

@@ -177,8 +177,8 @@ processHeapForDead( bdescr *bd )
          p = bd->start;
          while (p < bd->free) {
              p += processHeapClosureForDead((StgClosure *)p);
 -            while (p < bd->free && !*p)   // skip slop
 -                p++;
 +            // See Note [Skipping slop when scanning the heap] in ClosureMacros.h
 +            p = skipSlop(p, bd->free);
+         }
          ASSERT(p == bd->free);
          bd = bd->link;

rts/PrimOps.cmm

@@ -223,9 +223,8 @@ stg_isMutableByteArrayWeaklyPinnedzh ( gcptr mba )
  /* Note [shrink-array slop marker]
   * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   * When shrinkSmallMutableArray# or shrinkMutableByteArray# creates n words of
 - * slop at address `slop_start`, we write an O(1) marker so that the heap
 - * census (heapCensus in ProfHeap.c) and the sanity checker (checkHeapChain in
 - * Sanity.c) can skip over the slop without reading stale heap pointers.
 + * slop at address `slop_start`, we write an O(1) marker so that linear heap
 + * scans can skip over the slop without reading stale heap pointers.
+  *
   * The marker scheme (let n = number of slop words):
+  *
@@ -239,8 +238,8 @@ stg_isMutableByteArrayWeaklyPinnedzh ( gcptr mba )
+  *
   * An array may be shrunk multiple times, leaving consecutive slop regions.
   * Traversal code must therefore loop over all slop regions before advancing
 - * to the next live closure.  See the while-loops in heapCensusBlock (ProfHeap.c)
 - * and checkHeapChain (Sanity.c).
 + * to the next live closure.  See Note [Skipping slop when scanning the heap]
 + * in ClosureMacros.h.
   */
  // shrink size of MutableByteArray in-place

rts/Printer.c

@@ -1001,19 +1001,9 @@ findPtrBlocks (StgPtr p, bdescr *bd, StgPtr arr[], int arr_size, int i)
              if (UNTAG_CONST_CLOSURE((StgClosure*)*q) == (const StgClosure *)p) {
                  if (i < arr_size) {
                      for (r = bd->start; r < bd->free; r = end) {
 -                        // skip over marked slop; loop because an array
 -                        // may have been shrunk multiple times.
 -                        // See Note [shrink-array slop marker] in PrimOps.cmm.
 -                        while (r < bd->free) {
 -                            if (!*r) {
 -                                r++;
 -                            } else if (*r == (StgWord)(-1)) {
 -                                StgWord skip = *(r + 1);
 -                                r += 2 + skip;
 -                            } else {
 -                                break;
 -                            }
 -                        }
 +                        // See Note [Skipping slop when scanning the heap]
 +                        // in ClosureMacros.h
 +                        r = skipSlop(r, bd->free);
                          if (!LOOKS_LIKE_CLOSURE_PTR(r)) {
                              debugBelch("%p found at %p, no closure at %p\n",
                                         p, q, r);

rts/ProfHeap.c

@@ -1317,37 +1317,8 @@ heapCensusBlock(Census *census, bdescr *bd)
          p += size;
 -        /* skip over slop (zero words from large/pinned objects, or
 -           shrink-array slop markers); loop because an array may have been
 -           shrunk multiple times, leaving consecutive slop regions.
 -           See Note [slop on the heap] and Note [shrink-array slop marker]
 -           in PrimOps.cmm.
+-
 -           Note [skipping slop in the heap profiler]
 -           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 -           Slop left behind after major GC comes in two forms:
+-
 -            1. Zero words: alignment padding for large/pinned objects.
 -               We zero these explicitly (see MEMSET_SLOP_W in allocatePinned).
+-
 -            2. Shrink-array slop markers: written by stg_shrinkMutableByteArrayzh
 -               and stg_shrinkSmallMutableArrayzh in all build modes.  A single-word
 -               slop region is represented as a zero word; a multi-word region begins
 -               with the sentinel (StgWord)(-1) followed by a count of additional
 -               words.  See Note [shrink-array slop marker] in PrimOps.cmm.
+-
 -           Because an array can be shrunk multiple times, we loop until we
 -           see a word that looks like a valid info pointer. */
 -        while (p < bd->free) {
 -            if (!*p) {
 -                p++;
 -            } else if (*p == (StgWord)(-1)) {
 -                StgWord skip = *(p + 1);
 -                p += 2 + skip;
 -            } else {
 -                break;
 -            }
 -        }
 +        /* See Note [Skipping slop when scanning the heap] in ClosureMacros.h */
 +        p = skipSlop(p, bd->free);
+     }
+ }
@@ -1472,8 +1443,6 @@ heapCensusChain( Census *census, bdescr *bd )
          // of the associated block descriptor, thus introducing slop at the end
          // of the object.  This slop remains after GC, violating the assumption
          // of the loop below that all slop has been eliminated (#11627).
 -        // The slop isn't always zeroed (e.g. in non-profiling mode, cf
 -        // OVERWRITING_CLOSURE_OFS).
          // Consequently, we handle large ARR_WORDS objects as a special case.
          if (bd->flags & BF_LARGE) {
              StgPtr p = bd->start;

rts/include/rts/storage/ClosureMacros.h

@@ -634,6 +634,44 @@ INLINE_HEADER void writeSlopMarker(StgWord *slop, StgWord n)
+     }
+ }
 +// Note [Skipping slop when scanning the heap]
 +// ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 +// Every linear scan of the heap (heap census, LDV census, sanity checker,
 +// findPtr) has to step over slop between closures.  Slop comes in two forms:
 +//
 +//  1. Zero words: alignment padding for large/pinned objects, zeroed
 +//     explicitly (see MEMSET_SLOP_W in allocatePinned).
 +//
 +//  2. Slop markers: written by writeSlopMarker whenever a closure is
 +//     overwritten by a smaller one.  See Note [shrink-array slop marker]
 +//     in PrimOps.cmm for the encoding.
 +//
 +// A closure can be shrunk repeatedly, leaving consecutive slop regions, so we
 +// must loop until reaching a word that can't be slop.  Info pointers are never
 +// 0 or (StgWord)(-1), so such a word starts the next closure.
 +//
 +// All scanners must use skipSlop: if any one of them keeps its own copy of
 +// this loop it will go out of sync with the encoding (#27585).
 +INLINE_HEADER StgPtr skipSlop(StgPtr p, StgPtr end)
 +{
 +    while (p < end) {
 +        if (!*p) {
 +            // single-word slop region, or alignment padding
 +            p++;
 +        } else if (*p == (StgWord)(-1)) {
 +            // Multi-word slop region: sentinel, count, then that many words.
 +            // writeSlopMarker only writes the sentinel for n >= 2, so the count
 +            // word is always within the region.  We assert rather than bail out
 +            // so that a corrupt heap is reported instead of silently skipped.
 +            ASSERT(p + 1 < end);
 +            p += 2 + *(p + 1);
 +        } else {
 +            break;
 +        }
 +    }
 +    return p;
 +}
++
  INLINE_HEADER void
  markImmutableSlop (StgClosure *p,
            uint32_t offset,    /*< offset to start marking at, in words */

rts/sm/Sanity.c

@@ -605,19 +605,9 @@ void checkHeapChain (bdescr *bd)
                  ASSERT( size >= MIN_PAYLOAD_SIZE + sizeofW(StgHeader) );
                  p += size;
 -                /* skip slop; loop because an array may have been shrunk
 -                   multiple times.  See Note [slop on the heap] in Storage.c
 -                   and Note [shrink-array slop marker] in PrimOps.cmm. */
 -                while (p < bd->free) {
 -                    if (!*p) {
 -                        p++;
 -                    } else if (*p == (StgWord)(-1)) {
 -                        StgWord skip = *(p + 1);
 -                        p += 2 + skip;
 -                    } else {
 -                        break;
 -                    }
 -                }
 +                /* See Note [Skipping slop when scanning the heap]
 +                   in ClosureMacros.h */
 +                p = skipSlop(p, bd->free);
+             }
+         }
+     }

rts/sm/Storage.c

@@ -1035,9 +1035,10 @@ accountAllocation(Capability *cap, W_ n)
   *   leave slop behind depending on the size of the closure being
   *   overwritten. See Note [marking slop when overwriting immutable closures].
+  *
 - * To allow the heap profiler and sanity checker to linearly scan over heap
 - * blocks, slop must be identifiable without reading stale heap pointers.
 - * See Note [skipping slop in the heap profiler]
 + * To allow the heap profiler, the LDV profiler and the sanity checker to
 + * linearly scan over heap blocks, slop must be identifiable without reading
 + * stale heap pointers.
 + * See Note [Skipping slop when scanning the heap] in ClosureMacros.h
+  *
   * Shrunk-array slop has a further, concurrent reader: the non-moving GC mark
   * thread scans SmallMutArrPtrs payloads while the mutator may be shrinking
@@ -1207,7 +1208,7 @@ allocateMightFail (Capability *cap, W_ n)
   * When profiling we zero the space used for alignment. This allows us to
   * traverse pinned blocks in the heap profiler.
+  *
 - * See Note [skipping slop in the heap profiler]
 + * See Note [Skipping slop when scanning the heap] in ClosureMacros.h
   */
  #define MEMSET_SLOP_W(p, val, len_w) memset(p, val, (len_w) * sizeof(W_))

testsuite/tests/rts/T27585.hs

 +{-# LANGUAGE MagicHash, UnboxedTuples, BlockArguments #-}
 +module Main where
++
 +import GHC.Exts
 +import GHC.IO (IO(..))
 +import System.Mem (performMajorGC)
++
 +-- Lifted wrapper so SmallMutableArray# can be passed around.
 +data MArr = MArr (SmallMutableArray# RealWorld Integer)
++
 +-- Variant of T19048 for the LDV (biographical) profiler, +RTS -hb (#27585).
 +--
 +-- The array is promoted to the oldest generation *before* shrinking, so the
 +-- shrink-array slop marker is written into an old-generation block.  The next
 +-- major GC then runs LdvCensusForDead, whose linear heap scan
 +-- (processHeapForDead in rts/LdvProfile.c) must skip the slop correctly.
 +--
 +-- The array must stay below LARGE_OBJECT_THRESHOLD (409 words): large objects
 +-- live on the large_objects chain, which the census does not scan linearly.
 +main :: IO ()
 +main = do
 +  ma <- newArr
 +  fillArr ma 299
 +  -- Two major GCs promote the array to the oldest generation.
 +  performMajorGC
 +  performMajorGC
 +  -- Shrink: writes the slop marker over slots [10..299], in place, in an
 +  -- old-generation block.
 +  shrinkArr ma
 +  n <- getSize ma
 +  putStrLn $ "size after shrink = " ++ show n
 +  -- With -hb active, LdvCensusForDead scans the old blocks containing the
 +  -- slop marker.
 +  performMajorGC
 +  x <- readElem ma 0
 +  putStrLn $ "arr[0] = " ++ show x
 +  putStrLn "survived"
++
 +newArr :: IO MArr
 +newArr = IO \s -> case newSmallArray# 300# (0 :: Integer) s of
 +  (# s', ma #) -> (# s', MArr ma #)
++
 +-- Overwrite every slot with a distinct Integer so each holds a unique,
 +-- definitely non-zero heap pointer.
 +fillArr :: MArr -> Int -> IO ()
 +fillArr _ (-1) = pure ()
 +fillArr arr@(MArr ma) n@(I# n#) = do
 +  IO \s -> case writeSmallArray# ma n# (fromIntegral n :: Integer) s of
 +    s' -> (# s', () #)
 +  fillArr arr (n - 1)
++
 +shrinkArr :: MArr -> IO ()
 +shrinkArr (MArr ma) = IO \s ->
 +  case shrinkSmallMutableArray# ma 10# s of s' -> (# s', () #)
++
 +getSize :: MArr -> IO Int
 +getSize (MArr ma) = IO \s ->
 +  case getSizeofSmallMutableArray# ma s of (# s', n# #) -> (# s', I# n# #)
++
 +readElem :: MArr -> Int -> IO Integer
 +readElem (MArr ma) (I# i#) = IO \s -> readSmallArray# ma i# s

testsuite/tests/rts/T27585.stdout

 +size after shrink = 10
 +arr[0] = 0
 +survived

testsuite/tests/rts/all.T

@@ -710,3 +710,13 @@ test('T19048',
       , extra_run_opts('+RTS -hT -i0 -RTS')
       ],
       compile_and_run, ['-O -rtsopts'])
++
 +test('T27585',
 +     [ omit_ghci
 +     , no_check_hp
 +     , js_skip
 +     , when(have_profiling(), extra_ways(['prof_hb']))
 +     , only_ways(['prof_hb'])
 +     , extra_run_opts('+RTS -i0 -RTS')
 +     ],
 +     compile_and_run, ['-O -rtsopts'])