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.

diff --git a/tests/test.c b/tests/test.c
index a4b1bd12ef1d160c11f1de674eb3762285fc35f8..29805afa775d4c394d0f8bed9deee7359079f0e1 100644 (file)
--- 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;
   }