From c7837f864970e3859a3b35b9b8c0858a3548a457 Mon Sep 17 00:00:00 2001 From: Ivan Maidanski Date: Thu, 2 Nov 2017 00:31:11 +0300 Subject: [PATCH] Eliminate TSan warnings (false positives) in gctest * tests/test.c (struct SEXPR): Declare first. * tests/test.c (union sexpr_or_aot_u): New type. * tests/test.c (struct SEXPR): Replace struct SEXPR* to sexpr_or_aot_u. * tests/test.c (car, cdr, small_cons_uncollectable): Add .p suffix when accessing sexpr_car and sexpr_cdr fields. * tests/test.c [GC_GCJ_SUPPORT] (fake_gcj_mark_proc, gcj_cons): Likewise. * tests/test.c [!VERY_SMALL_CONFIG] (small_cons): Likewise. * tests/test.c (atomic_car, atomic_cdr): New macro; add comment. * tests/test.c [!VERY_SMALL_CONFIG] (cons): Store atomic_car and atomic_cdr atomically. * tests/test.c [VERY_SMALL_CONFIG] (small_cons): Likewise. * tests/test.c [!VERY_SMALL_CONFIG] (cons): Ignore store random value at *p if p points inside r->sexpr_car/cdr; add comment. * tests/test.c (reverse1, check_ints): Use atomic_car() and atomic_cdr() instead of car() and cdr(), respectively. * tests/test.c (reverse_test_inner): Move A.dummy=17 to set_print_procs(). * tests/test.c (set_print_procs): Add comment. --- tests/test.c | 80 ++++++++++++++++++++++++++++++++++++++++-------------------- 1 file changed, 53 insertions(+), 27 deletions(-) diff --git a/tests/test.c b/tests/test.c index a4b1bd1..29805af 100644 --- a/tests/test.c +++ b/tests/test.c @@ -234,23 +234,35 @@ volatile AO_t extra_count = 0; /* Amount of space wasted in cons node; */ /* configuration. In the FIND_LEAK configuration, it should */ /* find lots of leaks, since we free almost nothing. */ -struct SEXPR { - struct SEXPR * sexpr_car; - struct SEXPR * sexpr_cdr; -}; +struct SEXPR; +typedef struct SEXPR * sexpr; +union sexpr_or_aot_u { + sexpr p; + volatile AO_t a; +}; -typedef struct SEXPR * sexpr; +struct SEXPR { + union sexpr_or_aot_u sexpr_car; + union sexpr_or_aot_u sexpr_cdr; +}; # define INT_TO_SEXPR(x) ((sexpr)(GC_word)(x)) # define SEXPR_TO_INT(x) ((int)(GC_word)(x)) # undef nil # define nil (INT_TO_SEXPR(0)) -# define car(x) ((x) -> sexpr_car) -# define cdr(x) ((x) -> sexpr_cdr) +# define car(x) ((x)->sexpr_car.p) +# define cdr(x) ((x)->sexpr_cdr.p) # define is_nil(x) ((x) == nil) +/* As mentioned in reverse_test_inner, car/cdr should be accessed */ +/* atomically to be thread-safe during AA.a global list reversal. */ +/* Otherwise TSan reports data race issues between [small_]cons(), */ +/* reverse1() and check_ints(). */ +# define atomic_car(x) (sexpr)AO_load(&(x)->sexpr_car.a) +# define atomic_cdr(x) (sexpr)AO_load(&(x)->sexpr_cdr.a) + /* Silly implementation of Lisp cons. Intentionally wastes lots of space */ /* to test collector. */ # ifdef VERY_SMALL_CONFIG @@ -272,13 +284,23 @@ sexpr cons (sexpr x, sexpr y) (void *)p); FAIL; } - *p = (int)((13 << 12) + ((p - (int *)r) & 0xfff)); +# ifdef AT_END + if ((word)p + sizeof(struct SEXPR) <= (word)r + (my_extra & ~7) + || (word)p >= (word)r + (my_extra & ~7) + sizeof(struct SEXPR)) +# else + /* The first 2 elements are written atomically below. */ + if ((word)p >= (word)r + sizeof(struct SEXPR)) +# endif + { + *p = (int)((13 << 12) + ((p - (int *)r) & 0xfff)); + } } # ifdef AT_END r = (sexpr)((char *)r + (my_extra & ~7)); # endif - r -> sexpr_car = x; - r -> sexpr_cdr = y; + /* See the comment for atomic_car/cdr. */ + AO_store(&r->sexpr_car.a, (AO_t)x); + AO_store(&r->sexpr_cdr.a, (AO_t)y); GC_END_STUBBORN_CHANGE(r); return(r); } @@ -315,12 +337,10 @@ struct GC_ms_entry * fake_gcj_mark_proc(word * addr, addr = (word *)GC_USR_PTR_FROM_BASE(addr); } x = (sexpr)(addr + 1); /* Skip the vtable pointer. */ - mark_stack_ptr = GC_MARK_AND_PUSH( - (void *)(x -> sexpr_cdr), mark_stack_ptr, - mark_stack_limit, (void * *)&(x -> sexpr_cdr)); - mark_stack_ptr = GC_MARK_AND_PUSH( - (void *)(x -> sexpr_car), mark_stack_ptr, - mark_stack_limit, (void * *)&(x -> sexpr_car)); + mark_stack_ptr = GC_MARK_AND_PUSH((void *)x->sexpr_cdr.p, mark_stack_ptr, + mark_stack_limit, (void **)(&x->sexpr_cdr.p)); + mark_stack_ptr = GC_MARK_AND_PUSH((void *)x->sexpr_car.p, mark_stack_ptr, + mark_stack_limit, (void **)(&x->sexpr_car.p)); return(mark_stack_ptr); } @@ -333,8 +353,13 @@ sexpr small_cons (sexpr x, sexpr y) CHECK_OUT_OF_MEMORY(r); AO_fetch_and_add1(&collectable_count); - r -> sexpr_car = x; - r -> sexpr_cdr = y; +# ifdef VERY_SMALL_CONFIG + AO_store(&r->sexpr_car.a, (AO_t)x); + AO_store(&r->sexpr_cdr.a, (AO_t)y); +# else + r->sexpr_car.p = x; + r->sexpr_cdr.p = y; +# endif return(r); } @@ -344,8 +369,8 @@ sexpr small_cons_uncollectable (sexpr x, sexpr y) CHECK_OUT_OF_MEMORY(r); AO_fetch_and_add1(&uncollectable_count); - r -> sexpr_car = x; - r -> sexpr_cdr = (sexpr)(~(GC_word)y); + r->sexpr_car.p = x; + r->sexpr_cdr.p = (sexpr)(~(GC_word)y); return(r); } @@ -361,8 +386,8 @@ sexpr small_cons_uncollectable (sexpr x, sexpr y) CHECK_OUT_OF_MEMORY(r); result = (sexpr)(r + 1); - result -> sexpr_car = x; - result -> sexpr_cdr = y; + result->sexpr_car.p = x; + result->sexpr_cdr.p = y; return(result); } #endif /* GC_GCJ_SUPPORT */ @@ -373,7 +398,7 @@ sexpr reverse1(sexpr x, sexpr y) if (is_nil(x)) { return(y); } else { - return( reverse1(cdr(x), cons(car(x), y)) ); + return reverse1(atomic_cdr(x), cons(atomic_car(x), y)); } } @@ -438,18 +463,18 @@ void check_ints(sexpr list, int low, int up) GC_printf("list is nil\n"); FAIL; } - if (SEXPR_TO_INT(car(car(list))) != low) { + if (SEXPR_TO_INT(atomic_car(atomic_car(list))) != low) { GC_printf( "List reversal produced incorrect list - collector is broken\n"); FAIL; } if (low == up) { - if (cdr(list) != nil) { + if (atomic_cdr(list) != nil) { GC_printf("List too long - collector is broken\n"); FAIL; } } else { - check_ints(cdr(list), low+1, up); + check_ints(atomic_cdr(list), low + 1, up); } } @@ -669,7 +694,6 @@ void *GC_CALLBACK reverse_test_inner(void *data) # define BIG 4500 # endif - A.dummy = 17; a_set(ints(1, 49)); b = ints(1, 50); c = ints(1, BIG); @@ -1209,6 +1233,8 @@ void typed_test(void) void set_print_procs(void) { + /* Set these global variables just once to avoid TSan false positives. */ + A.dummy = 17; GC_is_valid_displacement_print_proc = fail_proc1; GC_is_visible_print_proc = fail_proc1; } -- 2.7.4