From 908fe369fc39be0a81ec6dd029f388ff673e4c46 Mon Sep 17 00:00:00 2001 From: Ivan Maidanski Date: Fri, 10 Nov 2017 18:14:55 +0300 Subject: [PATCH] Workaround TSan false positive in write fault handler 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 | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/os_dep.c b/os_dep.c index 690a4e5e..e790cacb 100644 --- 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. */ -- 2.40.0