Use union of AO_t and word for mse_descr to avoid type casting

(which breaks 'strict-aliasing' compiler optimization)

* include/private/gc_pmark.h (GC_ms_entry): Change type of mse_descr
from word to word_ptr_ao_u.
* include/private/gc_pmark.h (GC_mark_stack_top): Add FIXME.
* include/private/gc_pmark.h (OR_WORD_EXIT_IF_SET): Add 'volatile'
keyword to type cast for AO_or argument.
* include/private/gc_priv.h (counter_t): Add 'volatile' in case of
defining as AO_t (replace typedef with a macro).
* include/private/gc_priv.h (word_ptr_ao_u): New union type.
* include/private/gc_pmark.h (PUSH_OBJ): Add ".w" suffix to mse_descr.
* mark.c (GC_mark_from, GC_steal_mark_stack, GC_push_all): Likewise.
* mark.c (GC_steal_mark_stack): Use "ao" element of union to load and
clear mse_descr atomically; remove pointer type casts.

diff --git a/include/private/gc_pmark.h b/include/private/gc_pmark.h
index 04aa8c3cd7181bf21264fa09c1699975cb789a73..99937d35753da241e6ccae27538eb1925821a238 100644 (file)
--- a/include/private/gc_pmark.h
+++ b/include/private/gc_pmark.h
@@ -71,7 +71,8 @@ GC_EXTERN unsigned GC_n_mark_procs;
 
 typedef struct GC_ms_entry {
     ptr_t mse_start;    /* First word of object, word aligned.  */
-    GC_word mse_descr;  /* Descriptor; low order two bits are tags,     */
+    union word_ptr_ao_u mse_descr;
+                        /* Descriptor; low order two bits are tags,     */
                         /* as described in gc_mark.h.                   */
 } mse;
 
@@ -81,6 +82,7 @@ GC_EXTERN mse * GC_mark_stack_limit;
 
 #ifdef PARALLEL_MARK
   GC_EXTERN mse * volatile GC_mark_stack_top;
+                                /* FIXME: Use union to avoid casts to AO_t */
 #else
   GC_EXTERN mse * GC_mark_stack_top;
 #endif
@@ -140,7 +142,7 @@ GC_INNER mse * GC_signal_mark_stack_overflow(mse *msp);
           mark_stack_top = GC_signal_mark_stack_overflow(mark_stack_top); \
         } \
         mark_stack_top -> mse_start = (obj); \
-        mark_stack_top -> mse_descr = _descr; \
+        mark_stack_top -> mse_descr.w = _descr; \
     } \
 }
 
@@ -177,7 +179,7 @@ exit_label: ; \
 #   define OR_WORD_EXIT_IF_SET(addr, bits, exit_label) \
         { \
           if (!(*(addr) & (bits))) { \
-            AO_or((AO_t *)(addr), (bits)); \
+            AO_or((volatile AO_t *)(addr), (AO_t)(bits)); \
           } else { \
             goto exit_label; \
           } \

diff --git a/include/private/gc_priv.h b/include/private/gc_priv.h
index 3584e40fa426b59650ce394d67dcddaf27594430..e98e9d2d322ff2b4ea5b51240a2cda220e3a6402 100644 (file)
--- a/include/private/gc_priv.h
+++ b/include/private/gc_priv.h
@@ -806,7 +806,7 @@ typedef word page_hash_table[PHT_SIZE];
 
 #ifdef PARALLEL_MARK
 # include "atomic_ops.h"
-  typedef AO_t counter_t;
+# define counter_t volatile AO_t
 #else
   typedef size_t counter_t;
 # if defined(THREADS) && defined(MPROTECT_VDB)
@@ -814,6 +814,15 @@ typedef word page_hash_table[PHT_SIZE];
 # endif
 #endif /* !PARALLEL_MARK */
 
+union word_ptr_ao_u {
+  word w;
+  signed_word sw;
+  void *vp;
+# ifdef AO_HAVE_load
+    volatile AO_t ao;
+# endif
+};
+
 /* We maintain layout maps for heap blocks containing objects of a given */
 /* size.  Each entry in this map describes a byte offset and has the     */
 /* following type.                                                       */

diff --git a/mark.c b/mark.c
index 5bef7f9e70036904685232eaa7e26a49c7cfcc5d..5dfd0049c4393ab9454cbe7d2bd03f4130f82091 100644 (file)
--- a/mark.c
+++ b/mark.c
@@ -645,7 +645,7 @@ GC_INNER mse * GC_mark_from(mse *mark_stack_top, mse *mark_stack,
 # endif
   {
     current_p = mark_stack_top -> mse_start;
-    descr = mark_stack_top -> mse_descr;
+    descr = mark_stack_top -> mse_descr.w;
   retry:
     /* current_p and descr describe the current object.         */
     /* *mark_stack_top is vacant.                               */
@@ -675,7 +675,7 @@ GC_INNER mse * GC_mark_from(mse *mark_stack_top, mse *mark_stack,
                 && mark_stack_top < mark_stack_limit - 1) {
               int new_size = (descr/2) & ~(sizeof(word)-1);
               mark_stack_top -> mse_start = current_p;
-              mark_stack_top -> mse_descr = new_size + sizeof(word);
+              mark_stack_top -> mse_descr.w = new_size + sizeof(word);
                                         /* makes sure we handle         */
                                         /* misaligned pointers.         */
               mark_stack_top++;
@@ -693,8 +693,8 @@ GC_INNER mse * GC_mark_from(mse *mark_stack_top, mse *mark_stack,
 #         endif /* PARALLEL_MARK */
           mark_stack_top -> mse_start =
                 limit = current_p + WORDS_TO_BYTES(SPLIT_RANGE_WORDS-1);
-          mark_stack_top -> mse_descr =
-                        descr - WORDS_TO_BYTES(SPLIT_RANGE_WORDS-1);
+          mark_stack_top -> mse_descr.w =
+                                descr - WORDS_TO_BYTES(SPLIT_RANGE_WORDS-1);
 #         ifdef ENABLE_TRACE
             if (GC_trace_addr >= current_p
                 && GC_trace_addr < current_p + descr) {
@@ -915,17 +915,17 @@ STATIC mse * GC_steal_mark_stack(mse * low, mse * high, mse * local,
 
     GC_ASSERT(high >= low-1 && (word)(high - low + 1) <= GC_mark_stack_size);
     for (p = low; p <= high && i <= max; ++p) {
-        word descr = AO_load((volatile AO_t *) &(p -> mse_descr));
+        word descr = (word)AO_load(&p->mse_descr.ao);
         if (descr != 0) {
             /* Must be ordered after read of descr: */
-            AO_store_release_write((volatile AO_t *) &(p -> mse_descr), 0);
+            AO_store_release_write(&p->mse_descr.ao, 0);
             /* More than one thread may get this entry, but that's only */
             /* a minor performance problem.                             */
             ++top;
-            top -> mse_descr = descr;
+            top -> mse_descr.w = descr;
             top -> mse_start = p -> mse_start;
-            GC_ASSERT((top -> mse_descr & GC_DS_TAGS) != GC_DS_LENGTH ||
-                      top -> mse_descr < (word)GC_greatest_plausible_heap_addr
+            GC_ASSERT((top->mse_descr.w & GC_DS_TAGS) != GC_DS_LENGTH ||
+                      top->mse_descr.w < (word)GC_greatest_plausible_heap_addr
                                          - (word)GC_least_plausible_heap_addr);
             /* If this is a big object, count it as                     */
             /* size/256 + 1 objects.                                    */
@@ -1276,7 +1276,7 @@ GC_INNER void GC_push_all(ptr_t bottom, ptr_t top)
         length &= ~GC_DS_TAGS;
 #   endif
     GC_mark_stack_top -> mse_start = bottom;
-    GC_mark_stack_top -> mse_descr = length;
+    GC_mark_stack_top -> mse_descr.w = length;
 }
 
 #ifndef GC_DISABLE_INCREMENTAL

diff --git a/typd_mlc.c b/typd_mlc.c
index caba301e7d3e914d4778c1cdf347cc09d7f63165..ce5c4c82a576eda33a1d012861a2e2b85af3d3a9 100644 (file)
--- a/typd_mlc.c
+++ b/typd_mlc.c
@@ -410,8 +410,8 @@ STATIC mse * GC_typed_mark_proc(word * addr, mse * mark_stack_ptr,
             mark_stack_ptr = GC_signal_mark_stack_overflow(mark_stack_ptr);
         }
         mark_stack_ptr -> mse_start = (ptr_t)(addr + WORDSZ);
-        mark_stack_ptr -> mse_descr =
-                GC_MAKE_PROC(GC_typed_mark_proc_index, env+1);
+        mark_stack_ptr -> mse_descr.w =
+                        GC_MAKE_PROC(GC_typed_mark_proc_index, env + 1);
     }
     return(mark_stack_ptr);
 }
@@ -457,7 +457,7 @@ STATIC mse * GC_push_complex_descriptor(word *addr, complex_descriptor *d,
           for (i = 0; i < nelements; i++) {
               msp++;
               msp -> mse_start = current;
-              msp -> mse_descr = descr;
+              msp -> mse_descr.w = descr;
               current += sz;
           }
           return(msp);
@@ -522,12 +522,12 @@ STATIC mse * GC_array_mark_proc(word * addr, mse * mark_stack_ptr,
         GC_mark_stack_too_small = TRUE;
         new_mark_stack_ptr = orig_mark_stack_ptr + 1;
         new_mark_stack_ptr -> mse_start = (ptr_t)addr;
-        new_mark_stack_ptr -> mse_descr = sz | GC_DS_LENGTH;
+        new_mark_stack_ptr -> mse_descr.w = sz | GC_DS_LENGTH;
     } else {
         /* Push descriptor itself */
         new_mark_stack_ptr++;
         new_mark_stack_ptr -> mse_start = (ptr_t)(addr + nwords - 1);
-        new_mark_stack_ptr -> mse_descr = sizeof(word) | GC_DS_LENGTH;
+        new_mark_stack_ptr -> mse_descr.w = sizeof(word) | GC_DS_LENGTH;
     }
     return new_mark_stack_ptr;
 }