[Git][ghc/ghc][wip/T19146] 3 commits: rts: Assert that pointers aren't cleared

Ben Gamari (@bgamari) gitlab at gitlab.haskell.org
Tue May 2 19:49:52 UTC 2023



Ben Gamari pushed to branch wip/T19146 at Glasgow Haskell Compiler / GHC


Commits:
83b58e1a by Ben Gamari at 2023-05-02T15:49:39-04:00
rts: Assert that pointers aren't cleared

This turns many segmentation faults into much easier-to-debug assertion
failures by ensuring that LOOKS_LIKE_*_PTR checks recognize bit-patterns
produced by `+RTS -DZ` clearing as invalid pointers.

This is a bit ad-hoc but this is the debug runtime.

- - - - -
a41e2a77 by Ben Gamari at 2023-05-02T15:49:43-04:00
rts: Don't sanity-check StgTSO.global_link

See Note [Avoid dangling global_link pointers].

Fixes #19146.

- - - - -
593058d2 by Ben Gamari at 2023-05-02T15:49:44-04:00
rts: Introduce printGlobalThreads

- - - - -


7 changed files:

- rts/Threads.c
- rts/Threads.h
- rts/include/Cmm.h
- rts/include/MachDeps.h
- rts/include/rts/storage/ClosureMacros.h
- rts/sm/Evac.c
- rts/sm/Sanity.c


Changes:

=====================================
rts/Threads.c
=====================================
@@ -1008,6 +1008,20 @@ printAllThreads(void)
   }
 }
 
+void
+printGlobalThreads(void)
+{
+  for (uint32_t g = 0; g < RtsFlags.GcFlags.generations; g++) {
+    debugBelch("\ngen %d\n", g);
+    for (StgTSO *t = generations[g].threads; t != END_TSO_QUEUE; t = t->global_link) {
+      debugBelch("thread %p (id=%lu)\n", t, t->id);
+    }
+    for (StgTSO *t = generations[g].old_threads; t != END_TSO_QUEUE; t = t->global_link) {
+      debugBelch("thread %p (id=%lu) (old)\n", t, t->id);
+    }
+  }
+}
+
 // useful from gdb
 void
 printThreadQueue(StgTSO *t)


=====================================
rts/Threads.h
=====================================
@@ -43,6 +43,7 @@ W_   threadStackUnderflow (Capability *cap, StgTSO *tso);
 bool performTryPutMVar(Capability *cap, StgMVar *mvar, StgClosure *value);
 
 #if defined(DEBUG)
+void printGlobalThreads(void);
 void printThreadBlockage (StgTSO *tso);
 void printThreadStatus (StgTSO *t);
 void printAllThreads (void);


=====================================
rts/include/Cmm.h
=====================================
@@ -605,16 +605,20 @@
 #define BITMAP_SIZE(bitmap) ((bitmap) & BITMAP_SIZE_MASK)
 #define BITMAP_BITS(bitmap) ((bitmap) >> BITMAP_BITS_SHIFT)
 
+#define LOOKS_LIKE_PTR(p) ((p) != NULL && (p) != INVALID_GHC_POINTER)
+
 /* Debugging macros */
 #define LOOKS_LIKE_INFO_PTR(p)                                  \
-   ((p) != NULL &&                                              \
+   (LOOKS_LIKE_PTR(p) &&                                        \
     LOOKS_LIKE_INFO_PTR_NOT_NULL(p))
 
 #define LOOKS_LIKE_INFO_PTR_NOT_NULL(p)                         \
    ( (TO_W_(%INFO_TYPE(%STD_INFO(p))) != INVALID_OBJECT) &&     \
      (TO_W_(%INFO_TYPE(%STD_INFO(p))) <  N_CLOSURE_TYPES))
 
-#define LOOKS_LIKE_CLOSURE_PTR(p) (LOOKS_LIKE_INFO_PTR(GET_INFO(UNTAG(p))))
+#define LOOKS_LIKE_CLOSURE_PTR(p)                               \
+   ( LOOKS_LIKE_PTR(p) &&                                       \
+     LOOKS_LIKE_INFO_PTR(GET_INFO(UNTAG(p))))
 
 /*
  * The layout of the StgFunInfoExtra part of an info table changes


=====================================
rts/include/MachDeps.h
=====================================
@@ -107,6 +107,17 @@
 #endif
 #endif
 
+// This is the bit-pattern used by the RTS to clear freed memory
+// when +RTS -DZ is in use. The LOOKS_LIKE_*_PTR utilities use this
+// macro to catch clearly invalid pointers
+#if !defined(INVALID_GHC_POINTER)
+#if SIZEOF_HSWORD == 4
+#define INVALID_GHC_POINTER 0xaaaaaaaa
+#else
+#define INVALID_GHC_POINTER 0xaaaaaaaaaaaaaaaa
+#endif
+#endif
+
 #if !defined(TAG_BITS)
 #if SIZEOF_HSWORD == 4
 #define TAG_BITS                2


=====================================
rts/include/rts/storage/ClosureMacros.h
=====================================
@@ -270,6 +270,12 @@ EXTERN_INLINE StgClosure *TAG_CLOSURE(StgWord tag,StgClosure * p)
    make sense...
    -------------------------------------------------------------------------- */
 
+EXTERN_INLINE bool LOOKS_LIKE_PTR (const void* p);
+EXTERN_INLINE bool LOOKS_LIKE_PTR (const void* p)
+{
+    return p && (p != (const void*) INVALID_GHC_POINTER);
+}
+
 EXTERN_INLINE bool LOOKS_LIKE_INFO_PTR_NOT_NULL (StgWord p);
 EXTERN_INLINE bool LOOKS_LIKE_INFO_PTR_NOT_NULL (StgWord p)
 {
@@ -280,12 +286,13 @@ EXTERN_INLINE bool LOOKS_LIKE_INFO_PTR_NOT_NULL (StgWord p)
 EXTERN_INLINE bool LOOKS_LIKE_INFO_PTR (StgWord p);
 EXTERN_INLINE bool LOOKS_LIKE_INFO_PTR (StgWord p)
 {
-    return p && (IS_FORWARDING_PTR(p) || LOOKS_LIKE_INFO_PTR_NOT_NULL(p));
+    return LOOKS_LIKE_PTR((const void*) p) && (IS_FORWARDING_PTR(p) || LOOKS_LIKE_INFO_PTR_NOT_NULL(p));
 }
 
 EXTERN_INLINE bool LOOKS_LIKE_CLOSURE_PTR (const void *p);
 EXTERN_INLINE bool LOOKS_LIKE_CLOSURE_PTR (const void *p)
 {
+    if (!LOOKS_LIKE_PTR(p)) return false;
     const StgInfoTable *info = RELAXED_LOAD(&UNTAG_CONST_CLOSURE((const StgClosure *) (p))->header.info);
     return LOOKS_LIKE_INFO_PTR((StgWord) info);
 }


=====================================
rts/sm/Evac.c
=====================================
@@ -1030,8 +1030,10 @@ loop:
       return;
 
   case TSO:
+    {
       copy(p,info,q,sizeofW(StgTSO),gen_no);
       return;
+    }
 
   case STACK:
     {


=====================================
rts/sm/Sanity.c
=====================================
@@ -737,6 +737,27 @@ checkSTACK (StgStack *stack)
     checkStackChunk(sp, stack_end);
 }
 
+/*
+ * Note [Avoid dangling global_link pointers]
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ * TSOs are a bit odd in that they have a global_link pointer field
+ * which is not scavenged by the GC. This field is used to track the
+ * generations[_].[old_]threads lists and is ultimately updated by
+ * MarkWeak.c:tidyThreadList.
+
+ * Typically the fact that this field is not scavenged is fine as all reachable
+ * TSOs on the heap are guaranteed to be on some generation's thread list and
+ * therefore will be scavenged by tidyThreadList. However, the sanity checker
+ * poses a bit of a challenge here as it walks heap blocks directly and
+ * therefore may encounter TSOs which aren't reachable via the heap.
+ * For this reason, checkTSO does not check global_link. Instead, we only do
+ * so in checkGlobalTSOList, which by definition will only look at
+ * threads which are reachable via a thread list (and therefore must have won
+ * the forwarding-pointer race).
+ *
+ * See #19146.
+ */
+
 void
 checkTSO(StgTSO *tso)
 {
@@ -761,9 +782,11 @@ checkTSO(StgTSO *tso)
     ASSERT(LOOKS_LIKE_CLOSURE_PTR(tso->bq));
     ASSERT(LOOKS_LIKE_CLOSURE_PTR(tso->blocked_exceptions));
     ASSERT(LOOKS_LIKE_CLOSURE_PTR(tso->stackobj));
-    ASSERT(LOOKS_LIKE_CLOSURE_PTR(tso->global_link) &&
-            (tso->global_link == END_TSO_QUEUE ||
-             get_itbl((StgClosure*)tso->global_link)->type == TSO));
+
+    // This assertion is sadly not viable. See Note [Sanity-checking global_link].
+    //ASSERT(LOOKS_LIKE_CLOSURE_PTR(tso->global_link) &&
+    //        (tso->global_link == END_TSO_QUEUE ||
+    //         get_itbl((StgClosure*)tso->global_link)->type == TSO));
 
     if (tso->label) {
         ASSERT(LOOKS_LIKE_CLOSURE_PTR(tso->label));



View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/d2bcf3472a825bafc11e90b5e5f8b8d40cd6bc67...593058d280912576c595a0d6e3619c7f90cbaf0a

-- 
View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/d2bcf3472a825bafc11e90b5e5f8b8d40cd6bc67...593058d280912576c595a0d6e3619c7f90cbaf0a
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/20230502/7f5dd309/attachment-0001.html>


More information about the ghc-commits mailing list