[Gc] [PATCH] FIXUP_POINTER compilation

Adam Warner lists at consulting.net.nz
Tue Dec 16 03:47:11 PST 2008


Hi Hans,

Line 1341 of mark.c contains this macro:
GC_PUSH_ONE_STACK((ptr_t)p, MARKED_FROM_REGISTER);

When NEED_FIXUP_POINTER is defined the GC_PUSH_ONE_STACK includes the 
macro expansion of FIXUP_POINTER(p)

Substituting (ptr_t)p into FIXUP_POINTER(p) results in:
((ptr_t)p) = ...
The cast should be in the right hand expression, not the lvalue.

Depending upon the context a right hand cast should either be to a word 
pointer or a char pointer (i.e. to match the type of the variable being 
reassigned). I have fixed this using typeof(). If you want to avoid the 
use of typeof() then my prior testing indicates that twin versions of any 
macro that contain FIXUP_POINTER() will be required.

I also added casts in gc_pmark.h that were missing from the 
NEED_FIXUP_POINTER definition when compared with the !NEED_FIXUP_POINTER 
definition.

Compilation primarily tested with:
make clean &&
CFLAGS="-Werror -DPOINTER_SHIFT=0" ./configure --enable-threads=posix --
enable-parallel-mark --enable-large-config &&
make &&
make check

Regards,
Adam

$ cvs diff -u 2> /dev/null 
Index: mark.c
===================================================================
RCS file: /cvsroot/bdwgc/bdwgc/mark.c,v
retrieving revision 1.10
diff -u -r1.10 mark.c
--- mark.c	8 Nov 2008 00:29:55 -0000	1.10
+++ mark.c	16 Dec 2008 11:43:04 -0000
@@ -1338,7 +1338,7 @@
   void GC_push_one(word p)
 # endif
 {
-    GC_PUSH_ONE_STACK((ptr_t)p, MARKED_FROM_REGISTER);
+    GC_PUSH_ONE_STACK(p, MARKED_FROM_REGISTER);
 }
 
 struct GC_ms_entry *GC_mark_and_push(void *obj,
@@ -1488,7 +1488,7 @@
       lim = t - 1 /* longword */;
       for (p = b; p <= lim; p = (word *)(((ptr_t)p) + ALIGNMENT)) {
 	q = (ptr_t)(*p);
-	GC_PUSH_ONE_STACK((ptr_t)q, p);
+	GC_PUSH_ONE_STACK(q, p);
       }
 #   undef GC_greatest_plausible_heap_addr
 #   undef GC_least_plausible_heap_addr
Index: include/private/gc_pmark.h
===================================================================
RCS file: /cvsroot/bdwgc/bdwgc/include/private/gc_pmark.h,v
retrieving revision 1.8
diff -u -r1.8 gc_pmark.h
--- include/private/gc_pmark.h	22 Oct 2008 01:06:28 -0000	1.8
+++ include/private/gc_pmark.h	16 Dec 2008 11:43:05 -0000
@@ -366,10 +366,10 @@
 
 #if defined(PRINT_BLACK_LIST) || defined(KEEP_BACK_PTRS)
 #   define PUSH_ONE_CHECKED_STACK(p, source) \
-	GC_mark_and_push_stack(p, (ptr_t)(source))
+	GC_mark_and_push_stack((ptr_t)p, (ptr_t)(source))
 #else
 #   define PUSH_ONE_CHECKED_STACK(p, source) \
-	GC_mark_and_push_stack(p)
+	GC_mark_and_push_stack((ptr_t)p)
 #endif
 
 /*
@@ -383,13 +383,13 @@
 # if NEED_FIXUP_POINTER
     /* Try both the raw version and the fixed up one.	*/
 #   define GC_PUSH_ONE_STACK(p, source) \
-      if ((p) >= (ptr_t)GC_least_plausible_heap_addr 	\
-	 && (p) < (ptr_t)GC_greatest_plausible_heap_addr) {	\
+      if ((ptr_t)(p) >= (ptr_t)GC_least_plausible_heap_addr 	\
+	 && (ptr_t)(p) < (ptr_t)GC_greatest_plausible_heap_addr) {	\
 	 PUSH_ONE_CHECKED_STACK(p, source);	\
       } \
       FIXUP_POINTER(p); \
-      if ((p) >= (ptr_t)GC_least_plausible_heap_addr 	\
-	 && (p) < (ptr_t)GC_greatest_plausible_heap_addr) {	\
+      if ((ptr_t)(p) >= (ptr_t)GC_least_plausible_heap_addr 	\
+	 && (ptr_t)(p) < (ptr_t)GC_greatest_plausible_heap_addr) {	\
 	 PUSH_ONE_CHECKED_STACK(p, source);	\
       }
 # else /* !NEED_FIXUP_POINTER */
@@ -407,8 +407,8 @@
  */
 # define GC_PUSH_ONE_HEAP(p,source) \
     FIXUP_POINTER(p); \
-    if ((p) >= (ptr_t)GC_least_plausible_heap_addr 	\
-	 && (p) < (ptr_t)GC_greatest_plausible_heap_addr) {	\
+    if ((ptr_t)(p) >= (ptr_t)GC_least_plausible_heap_addr 	\
+	 && (ptr_t)(p) < (ptr_t)GC_greatest_plausible_heap_addr) {	\
 	    GC_mark_stack_top = GC_mark_and_push( \
 			    (void *)(p), GC_mark_stack_top, \
 			    GC_mark_stack_limit, (void * *)(source)); \
Index: include/private/gcconfig.h
===================================================================
RCS file: /cvsroot/bdwgc/bdwgc/include/private/gcconfig.h,v
retrieving revision 1.39
diff -u -r1.39 gcconfig.h
--- include/private/gcconfig.h	12 Nov 2008 01:08:51 -0000	1.39
+++ include/private/gcconfig.h	16 Dec 2008 11:43:05 -0000
@@ -2275,7 +2275,8 @@
 # endif
 
 # if !defined(FIXUP_POINTER) && defined(POINTER_MASK)
-#   define FIXUP_POINTER(p) (p) = ((p) & (POINTER_MASK) << POINTER_SHIFT)
+#   define FIXUP_POINTER(p) \
+    p = (typeof(p)) ((((GC_word)p) & (POINTER_MASK)) << (POINTER_SHIFT))
 # endif
 
 # if defined(FIXUP_POINTER)



More information about the Gc mailing list