[Git][ghc/ghc][wip/25232] rts: Fix segfault when using non-moving GC with profiling

[Zubin (@wz1000)]

Zubin pushed to branch wip/25232 at Glasgow Haskell Compiler / GHC


Commits:
bfe8ae39 by Zubin Duggal at 2024-09-22T01:09:41+05:30
rts: Fix segfault when using non-moving GC with profiling

`nonMovingCollect()` swaps out the `static_flag` value used as a
sentinel for `gct->scavenged_static_objects`, but the subsequent call
`resetStaticObjectForProfiling()` sees the old value of `static_flag` used as
the sentinel and segfaults. So we must call `resetStaticObjectForProfiling()`
before calling `nonMovingCollect()` as otherwise it looks for the incorrect
sentinel value

Fixes #25232 and #23958

Also teach the testsuite driver about nonmoving profiling ways
and stop disabling metric collection when nonmoving GC is enabled.

- - - - -


5 changed files:

- rts/sm/GC.c
- testsuite/config/ghc
- testsuite/driver/testlib.py
- + testsuite/tests/rts/T25232.hs
- testsuite/tests/rts/all.T


Changes:

=====================================
rts/sm/GC.c
=====================================
@@ -854,6 +854,18 @@ GarbageCollect (struct GcConfig config,
       }
   }
 
+#if defined(PROFILING)
+  // resetStaticObjectForProfiling() must be called before
+  // zeroing below.
+  // It must also be called before nonMovingCollect() as that
+  // swaps out the value of static_flag used as a sentinel
+  // in gct->scavenged_static_objects
+  // Not doing this lead to #25232 and #23958
+
+  // ToDo: fix the gct->scavenged_static_objects below
+  resetStaticObjectForProfiling(&g_retainerTraverseState, gct->scavenged_static_objects);
+#endif
+
   // Mark and sweep the oldest generation.
   // N.B. This can only happen after we've moved
   // oldest_gen->scavenged_large_objects back to oldest_gen->large_objects.
@@ -946,14 +958,6 @@ GarbageCollect (struct GcConfig config,
       checkUnload();
   }
 
-#if defined(PROFILING)
-  // resetStaticObjectForProfiling() must be called before
-  // zeroing below.
-
-  // ToDo: fix the gct->scavenged_static_objects below
-  resetStaticObjectForProfiling(&g_retainerTraverseState, gct->scavenged_static_objects);
-#endif
-
   // Start any pending finalizers.  Must be after
   // updateStableTables() and stableUnlock() (see #4221).
   RELEASE_SM_LOCK;


=====================================
testsuite/config/ghc
=====================================
@@ -125,6 +125,8 @@ config.way_flags = {
     'ext-interp'   : ['-fexternal-interpreter'],
     'nonmoving'    : [],
     'nonmoving_thr': ['-threaded'],
+    'nonmoving_prof': ['-prof'],
+    'nonmoving_thr_prof': ['-prof', '-threaded'],
     'nonmoving_thr_sanity': ['-threaded', '-debug'],
     'nonmoving_thr_ghc': ['+RTS', '-xn', '-N2', '-RTS', '-threaded'],
     'compacting_gc': [],


=====================================
testsuite/driver/testlib.py
=====================================
@@ -848,15 +848,12 @@ def _collect_stats(name: TestName, opts, metrics, deviation: Optional[int],
     if config.compiler_debugged and is_compiler_stats_test:
         opts.skip = True
 
-    # If there are any residency testing metrics then turn on RESIDENCY_OPTS and
-    # omit nonmoving GC ways, which don't support profiling.
+    # If there are any residency testing metrics then turn on RESIDENCY_OPTS
     if residency_testing_metrics() & metrics:
         if is_compiler_stats_test:
             _extra_hc_opts(name, opts, RESIDENCY_OPTS)
         else:
             _extra_run_opts(name, opts, RESIDENCY_OPTS)
-        # The nonmoving collector does not support -G1
-        _omit_ways(name, opts, [WayName(name) for name in ['nonmoving', 'nonmoving_thr', 'nonmoving_thr_ghc']])
 
     # How to read the result of the performance test
     def read_stats_file(way, metric_name):


=====================================
testsuite/tests/rts/T25232.hs
=====================================
@@ -0,0 +1,14 @@
+module Main where
+
+import Control.Monad
+import Control.Monad.Trans.State.Strict
+import Data.IORef
+
+main :: IO ()
+main = do
+  ref <- newIORef ()
+  replicateM_ 1000 $ withIORef ref $ runStateT (pure ())
+
+withIORef :: IORef a -> (a -> IO (b, a)) -> IO b
+withIORef ref f =
+  readIORef ref >>= f >>= \(b, a) -> writeIORef ref a >> pure b


=====================================
testsuite/tests/rts/all.T
=====================================
@@ -616,3 +616,5 @@ test('IOManager', [js_skip, when(arch('wasm32'), skip), when(opsys('mingw32'), s
                   compile_and_run, [''])
 
 test('T24142', [req_target_smp], compile_and_run, ['-threaded -with-rtsopts "-N2"'])
+
+test('T25232', [req_profiling, extra_ways(['nonmoving', 'nonmoving_prof', 'nonmoving_thr_prof'])], compile_and_run, [''])



View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/commit/bfe8ae393d0d1bc056bdc9f5dfb31d79ec805562

-- 
View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/commit/bfe8ae393d0d1bc056bdc9f5dfb31d79ec805562
You're receiving this email because of your account on gitlab.haskell.org.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.haskell.org/pipermail/ghc-commits/attachments/20240921/487da0a6/attachment-0001.html>