]> granicus.if.org Git - gc/commitdiff
Workaround TSan false positive in write fault handler
authorIvan Maidanski <ivmai@mail.ru>
Fri, 10 Nov 2017 15:14:55 +0000 (18:14 +0300)
committerIvan Maidanski <ivmai@mail.ru>
Fri, 10 Nov 2017 15:17:32 +0000 (18:17 +0300)
The data races reported between GC_write_fault_handler,
catch_exception_raise and GC_install_header, GC_remove_header are OK
as the added or removed header should be already unprotected.

* os_dep.c [MPROTECT_VDB && THREADS] (is_header_found_async): New
static function (with GC_ATTR_NO_SANITIZE_THREAD attribute).
* os_dep.c [MPROTECT_VDB && !THREADS] (is_header_found_async): New
macro.
* os_dep.c [MPROTECT_VDB && !DARWIN] (GC_write_fault_handler): Use
is_header_found_async() instead of HDR(addr)!=0.
* os_dep.c [MPROTECT_VDB && DARWIN] (catch_exception_raise): Likewise.

os_dep.c

index 690a4e5e8a42ca6b5b6e5d933c82364473d8529b..e790cacb31a52184dad752b2015ae1194d2e5747 100644 (file)
--- a/os_dep.c
+++ b/os_dep.c
@@ -3149,9 +3149,26 @@ GC_API GC_push_other_roots_proc GC_CALL GC_get_push_other_roots(void)
     }
   }
 #endif /* !AO_HAVE_test_and_set_acquire */
+
+  /* This function is used only by the fault handler.  Potential data   */
+  /* race between this function and GC_install_header, GC_remove_header */
+  /* should not be harmful because the added or removed header should   */
+  /* be already unprotected.                                            */
+  static GC_bool is_header_found_async(void *addr) GC_ATTR_NO_SANITIZE_THREAD
+  {
+#   ifdef HASH_TL
+      hdr *result;
+      GET_HDR((ptr_t)addr, result);
+      return result != NULL;
+#   else
+      return HDR_INNER(addr) != NULL;
+#   endif
+  }
+
 #else /* !THREADS */
 # define async_set_pht_entry_from_index(db, index) \
                         set_pht_entry_from_index(db, index)
+# define is_header_found_async(addr) (HDR(addr) != NULL)
 #endif /* !THREADS */
 
 #ifndef DARWIN
@@ -3236,13 +3253,13 @@ GC_API GC_push_other_roots_proc GC_CALL GC_get_push_other_roots(void)
             /* Address is only within the correct physical page.        */
             in_allocd_block = FALSE;
             for (i = 0; i < divHBLKSZ(GC_page_size); i++) {
-              if (HDR(h+i) != 0) {
+              if (is_header_found_async(&h[i])) {
                 in_allocd_block = TRUE;
                 break;
               }
             }
 #       else
-            in_allocd_block = (HDR(addr) != 0);
+            in_allocd_block = is_header_found_async(addr);
 #       endif
         if (!in_allocd_block) {
             /* FIXME - We should make sure that we invoke the   */
@@ -4366,7 +4383,7 @@ catch_exception_raise(mach_port_t exception_port GC_ATTR_UNUSED,
 
   /* This is the address that caused the fault */
   addr = (char*) exc_state.DARWIN_EXC_STATE_DAR;
-  if (HDR(addr) == 0) {
+  if (!is_header_found_async(addr)) {
     /* Ugh... just like the SIGBUS problem above, it seems we get       */
     /* a bogus KERN_PROTECTION_FAILURE every once and a while.  We wait */
     /* till we get a bunch in a row before doing anything about it.     */