Re: [Gc] [PATCH] FIXUP_POINTER compilation

Ivan Maidanski ivmai at mail.ru
Mon Jan 19 04:43:31 PST 2009


Hi!

Adam Warner <lists at consulting.net.nz> wrote:
> 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

I've changed this patch slightly so that it doesn't require typeof() nor "twin versions" of any macro that contain FIXUP_POINTER() anymore. The arg for FIXUP_POINTER() is always of "word" L-value type now.

Bye.

-------------- next part --------------
diff -ru bdwgc/finalize.c updated/bdwgc/finalize.c
--- bdwgc/finalize.c	2008-12-24 19:38:14.000000000 +0300
+++ updated/bdwgc/finalize.c	2009-01-19 12:37:48.000000000 +0300
@@ -259,7 +259,8 @@
 {
     hdr * hhdr = HDR(p);
     word descr = hhdr -> hb_descr;
-    ptr_t q, r;
+    ptr_t q;
+    word r;
     ptr_t scan_limit;
     ptr_t target_limit = p + hhdr -> hb_sz - 1;
     
@@ -269,8 +270,8 @@
        scan_limit = target_limit + 1 - sizeof(word);
     }
     for (q = p; q <= scan_limit; q += ALIGNMENT) {
-    	r = *(ptr_t *)q;
-    	if (r < p || r > target_limit) {
+    	r = *(word *)q;
+    	if ((ptr_t)r < p || (ptr_t)r > target_limit) {
     	    GC_PUSH_ONE_HEAP(r, q);
     	}
     }
diff -ru bdwgc/include/private/gc_pmark.h updated/bdwgc/include/private/gc_pmark.h
--- bdwgc/include/private/gc_pmark.h	2008-10-31 22:25:52.000000000 +0300
+++ updated/bdwgc/include/private/gc_pmark.h	2009-01-19 12:52:10.000000000 +0300
@@ -370,10 +370,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
 
 /*
@@ -387,13 +387,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 */
@@ -411,8 +411,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)); \
diff -ru bdwgc/include/private/gcconfig.h updated/bdwgc/include/private/gcconfig.h
--- bdwgc/include/private/gcconfig.h	2008-11-13 11:23:54.000000000 +0300
+++ updated/bdwgc/include/private/gcconfig.h	2009-01-19 12:21:56.000000000 +0300
@@ -2275,7 +2275,7 @@
 # endif
 
 # if !defined(FIXUP_POINTER) && defined(POINTER_MASK)
-#   define FIXUP_POINTER(p) (p) = ((p) & (POINTER_MASK) << POINTER_SHIFT)
+#   define FIXUP_POINTER(p) (p = ((p) & POINTER_MASK) << POINTER_SHIFT)
 # endif
 
 # if defined(FIXUP_POINTER)
diff -ru bdwgc/mark.c updated/bdwgc/mark.c
--- bdwgc/mark.c	2008-11-20 18:30:08.000000000 +0300
+++ updated/bdwgc/mark.c	2009-01-19 12:41:20.000000000 +0300
@@ -1335,7 +1335,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);
 }
 
 GC_API struct GC_ms_entry * GC_CALL GC_mark_and_push(void *obj,
@@ -1472,7 +1472,7 @@
     word * b = (word *)(((word) bottom + ALIGNMENT-1) & ~(ALIGNMENT-1));
     word * t = (word *)(((word) top) & ~(ALIGNMENT-1));
     register word *p;
-    register ptr_t q;
+    register word q;
     register word *lim;
     register ptr_t greatest_ha = GC_greatest_plausible_heap_addr;
     register ptr_t least_ha = GC_least_plausible_heap_addr;
@@ -1484,8 +1484,8 @@
     /* to be valid.						*/
       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);
+	q = *p;
+	GC_PUSH_ONE_STACK(q, p);
       }
 #   undef GC_greatest_plausible_heap_addr
 #   undef GC_least_plausible_heap_addr
@@ -1549,25 +1549,25 @@
 # if GC_GRANULE_WORDS == 1
 #   define USE_PUSH_MARKED_ACCELERATORS
 #   define PUSH_GRANULE(q) \
-		{ ptr_t qcontents = (ptr_t)((q)[0]); \
+		{ word qcontents = (q)[0]; \
 	          GC_PUSH_ONE_HEAP(qcontents, (q)); }
 # elif GC_GRANULE_WORDS == 2
 #   define USE_PUSH_MARKED_ACCELERATORS
 #   define PUSH_GRANULE(q) \
-		{ ptr_t qcontents = (ptr_t)((q)[0]); \
+		{ word qcontents = (q)[0]; \
 	          GC_PUSH_ONE_HEAP(qcontents, (q)); \
-		  qcontents = (ptr_t)((q)[1]); \
+		  qcontents = (q)[1]; \
 	          GC_PUSH_ONE_HEAP(qcontents, (q)+1); }
 # elif GC_GRANULE_WORDS == 4
 #   define USE_PUSH_MARKED_ACCELERATORS
 #   define PUSH_GRANULE(q) \
-		{ ptr_t qcontents = (ptr_t)((q)[0]); \
+		{ word qcontents = (q)[0]; \
 	          GC_PUSH_ONE_HEAP(qcontents, (q)); \
-		  qcontents = (ptr_t)((q)[1]); \
+		  qcontents = (q)[1]; \
 	          GC_PUSH_ONE_HEAP(qcontents, (q)+1); \
-		  qcontents = (ptr_t)((q)[2]); \
+		  qcontents = (q)[2]; \
 	          GC_PUSH_ONE_HEAP(qcontents, (q)+2); \
-		  qcontents = (ptr_t)((q)[3]); \
+		  qcontents = (q)[3]; \
 	          GC_PUSH_ONE_HEAP(qcontents, (q)+3); }
 # endif
 #endif


More information about the Gc mailing list