[Gc] Fix for ensuring atomicity of in-line printing

Ivan Maidanski ivmai at mail.ru
Tue Oct 28 14:46:18 PST 2008


Hi!

This patch replaces (where easy/possible) groups of several adjacent printf() calls doing printing on the same line into the equivalent single printf() calls. Useful on Win32+threads. GC_err_puts(path/__FILE__) calls are left untouched because of possible buffer overflow in GC_err_printf().

PS. The patch also fixes a typo in the comment for GC_printf().

Bye.

-------------- next part --------------
diff -ru bdwgc/allchblk.c updated/bdwgc/allchblk.c
--- bdwgc/allchblk.c	2008-10-27 12:59:42.000000000 +0300
+++ updated/bdwgc/allchblk.c	2008-10-28 22:11:28.000000000 +0300
@@ -121,15 +121,11 @@
       while (h != 0) {
         hhdr = HDR(h);
         sz = hhdr -> hb_sz;
-    	GC_printf("\t%p size %lu ", h, (unsigned long)sz);
     	total_free += sz;
-        if (GC_is_black_listed(h, HBLKSIZE) != 0) {
-             GC_printf("start black listed\n");
-        } else if (GC_is_black_listed(h, hhdr -> hb_sz) != 0) {
-             GC_printf("partially black listed\n");
-        } else {
-             GC_printf("not black listed\n");
-        }
+    	GC_printf("\t%p size %lu %s black listed\n", h, (unsigned long)sz, 
+		GC_is_black_listed(h, HBLKSIZE) != 0 ? "start" :
+		GC_is_black_listed(h, hhdr -> hb_sz) != 0 ? "partially" :
+							"not");
         h = hhdr -> hb_next;
       }
     }
@@ -180,9 +176,8 @@
 	GC_printf("***Section from %p to %p\n", start, end);
 	for (p = start; p < end;) {
 	    hhdr = HDR(p);
-	    GC_printf("\t%p ", p);
 	    if (IS_FORWARDING_ADDR_OR_NIL(hhdr)) {
-		GC_printf("Missing header!!(%p)\n", hhdr);
+		GC_printf("\t%p Missing header!!(%p)\n", p, hhdr);
 		p += HBLKSIZE;
 		continue;
 	    }
@@ -191,13 +186,9 @@
 					divHBLKSZ(hhdr -> hb_sz));
 	        int actual_index;
 		
-		GC_printf("\tfree block of size 0x%lx bytes",
-			  (unsigned long)(hhdr -> hb_sz));
-	 	if (IS_MAPPED(hhdr)) {
-		    GC_printf("\n");
-		} else {
-		    GC_printf("(unmapped)\n");
-		}
+		GC_printf("\t%p\tfree block of size 0x%lx bytes%s\n", p,
+			  (unsigned long)(hhdr -> hb_sz),
+			  IS_MAPPED(hhdr) ? "" : " (unmapped)");
 		actual_index = free_list_index_of(hhdr);
 		if (-1 == actual_index) {
 		    GC_printf("\t\tBlock not on free list %d!!\n",
@@ -208,7 +199,7 @@
 		}
 		p += hhdr -> hb_sz;
 	    } else {
-		GC_printf("\tused for blocks of size 0x%lx bytes\n",
+		GC_printf("\t%p\tused for blocks of size 0x%lx bytes\n", p,
 			  (unsigned long)(hhdr -> hb_sz));
 		p += HBLKSIZE * OBJ_SZ_TO_BLOCKS(hhdr -> hb_sz);
 	    }
diff -ru bdwgc/alloc.c updated/bdwgc/alloc.c
--- bdwgc/alloc.c	2008-10-28 17:26:16.000000000 +0300
+++ updated/bdwgc/alloc.c	2008-10-28 22:34:16.000000000 +0300
@@ -137,9 +137,9 @@
     time_diff = MS_TIME_DIFF(current_time,GC_start_time);
     if (time_diff >= GC_time_limit) {
 	if (GC_print_stats) {
-	    GC_log_printf("Abandoning stopped marking after ");
-	    GC_log_printf("%lu msecs", time_diff);
-	    GC_log_printf("(attempt %d)\n", GC_n_attempts);
+	    GC_log_printf(
+		"Abandoning stopped marking after %lu msecs (attempt %d)\n",
+		time_diff, GC_n_attempts);
 	}
     	return(1);
     }
@@ -540,11 +540,10 @@
 	
     GC_gc_no++;
     if (GC_print_stats) {
-      GC_log_printf("Collection %lu reclaimed %ld bytes",
-		    (unsigned long)GC_gc_no - 1,
-	   	    (long)GC_bytes_found);
-      GC_log_printf(" ---> heapsize = %lu bytes\n",
-      	        (unsigned long) GC_heapsize);
+      GC_log_printf(
+	     "Collection %lu reclaimed %ld bytes ---> heapsize = %lu bytes\n",
+	     (unsigned long)(GC_gc_no - 1), (long)GC_bytes_found,
+	     (unsigned long)GC_heapsize);
         /* Printf arguments may be pushed in funny places.  Clear the	*/
         /* space.							*/
       GC_log_printf("");
@@ -780,14 +779,16 @@
 	}
 
     if (GC_print_stats == VERBOSE) {
-	GC_log_printf(
-		  "Immediately reclaimed %ld bytes in heap of size %lu bytes",
-	          (long)GC_bytes_found,
-	          (unsigned long)GC_heapsize);
 #	ifdef USE_MUNMAP
-	  GC_log_printf("(%lu unmapped)", (unsigned long)GC_unmapped_bytes);
+	  GC_log_printf("Immediately reclaimed %ld bytes in heap"
+			" of size %lu bytes (%lu unmapped)\n",
+			(long)GC_bytes_found, (unsigned long)GC_heapsize,
+			(unsigned long)GC_unmapped_bytes);
+#	else
+	  GC_log_printf("Immediately reclaimed %ld bytes in heap"
+			" of size %lu bytes\n",
+			(long)GC_bytes_found, (unsigned long)GC_heapsize);
 #	endif
-	GC_log_printf("\n");
     }
 
     /* Reset or increment counters for next cycle */
@@ -927,13 +928,12 @@
         struct hblk *h;
         unsigned nbl = 0;
         
-    	GC_printf("Section %d from %p to %p ", i,
-    		   start, start + len);
     	for (h = (struct hblk *)start; h < (struct hblk *)(start + len); h++) {
     	    if (GC_is_black_listed(h, HBLKSIZE)) nbl++;
     	}
-    	GC_printf("%lu/%lu blacklisted\n", (unsigned long)nbl,
-    		   (unsigned long)(len/HBLKSIZE));
+    	GC_printf("Section %d from %p to %p %lu/%lu blacklisted\n",
+		  i, start, start + len,
+    		  (unsigned long)nbl, (unsigned long)(len/HBLKSIZE));
     }
 }
 # endif
diff -ru bdwgc/dbg_mlc.c updated/bdwgc/dbg_mlc.c
--- bdwgc/dbg_mlc.c	2008-10-27 18:18:38.000000000 +0300
+++ updated/bdwgc/dbg_mlc.c	2008-10-29 01:08:22.000000000 +0300
@@ -411,21 +411,20 @@
 {
     register oh * ohdr = (oh *)GC_base(p);
     
-    GC_err_printf("%p in or near object at %p(", clobbered_addr, p);
     if (clobbered_addr <= (ptr_t)(&(ohdr -> oh_sz))
         || ohdr -> oh_string == 0) {
-        GC_err_printf("<smashed>, appr. sz = %ld)\n",
-		      (unsigned long)(GC_size((ptr_t)ohdr) - DEBUG_BYTES));
+	GC_err_printf(
+		"%p in or near object at %p(<smashed>, appr. sz = %lu)\n",
+		clobbered_addr, p,
+		(unsigned long)(GC_size((ptr_t)ohdr) - DEBUG_BYTES));
     } else {
-        if ((word)(ohdr -> oh_string) < HBLKSIZE) {
-            GC_err_puts("(smashed string)");
-        } else if (ohdr -> oh_string[0] == '\0') {
-            GC_err_puts("EMPTY(smashed?)");
-        } else {
-            GC_err_puts(ohdr -> oh_string);
-        }
-        GC_err_printf(":%ld, sz=%ld)\n", (unsigned long)(ohdr -> oh_int),
-        			          (unsigned long)(ohdr -> oh_sz));
+	GC_err_printf("%p in or near object at %p(%s:%lu, sz=%lu)\n",
+		clobbered_addr, p,
+		(word)(ohdr -> oh_string) < HBLKSIZE ? "(smashed string)" :
+		ohdr -> oh_string[0] == '\0' ? "EMPTY(smashed?)" :
+						ohdr -> oh_string,
+		(unsigned long)(ohdr -> oh_int),
+		(unsigned long)(ohdr -> oh_sz));
         PRINT_CALL_CHAIN(ohdr);
     }
 }
@@ -709,7 +708,7 @@
     }
     if ((ptr_t)p - (ptr_t)base != sizeof(oh)) {
         GC_err_printf(
-        	  "GC_debug_free called on pointer %p wo debugging info\n", p);
+        	 "GC_debug_free called on pointer %p w/o debugging info\n", p);
     } else {
 #     ifndef SHORT_DBG_HDRS
         clobbered = GC_check_annotated_obj((oh *)base);
@@ -788,7 +787,7 @@
     }
     if ((ptr_t)p - (ptr_t)base != sizeof(oh)) {
         GC_err_printf(
-        	"GC_debug_realloc called on pointer %p wo debugging info\n", p);
+              "GC_debug_realloc called on pointer %p w/o debugging info\n", p);
         return(GC_realloc(p, lb));
     }
     hhdr = HDR(base);
diff -ru bdwgc/mark_rts.c updated/bdwgc/mark_rts.c
--- bdwgc/mark_rts.c	2008-10-25 19:15:18.000000000 +0400
+++ updated/bdwgc/mark_rts.c	2008-10-28 22:13:34.000000000 +0300
@@ -46,14 +46,10 @@
     size_t total = 0;
     
     for (i = 0; i < n_root_sets; i++) {
-        GC_printf("From %p to %p ",
+        GC_printf("From %p to %p%s\n",
         	  GC_static_roots[i].r_start,
-        	  GC_static_roots[i].r_end);
-        if (GC_static_roots[i].r_tmp) {
-            GC_printf(" (temporary)\n");
-        } else {
-            GC_printf("\n");
-        }
+        	  GC_static_roots[i].r_end,
+		  GC_static_roots[i].r_tmp ? " (temporary)" : "");
         total += GC_static_roots[i].r_end - GC_static_roots[i].r_start;
     }
     GC_printf("Total size: %ld\n", (unsigned long) total);
diff -ru bdwgc/misc.c updated/bdwgc/misc.c
--- bdwgc/misc.c	2008-10-27 15:49:38.000000000 +0300
+++ updated/bdwgc/misc.c	2008-10-29 00:53:08.000000000 +0300
@@ -970,7 +970,7 @@
 #endif
 /* A version of printf that is unlikely to call malloc, and is thus safer */
 /* to call from the collector in case malloc has been bound to GC_malloc. */
-/* Floating point arguments ans formats should be avoided, since fp	  */
+/* Floating point arguments and formats should be avoided, since fp	  */
 /* conversion is more likely to allocate.				  */
 /* Assumes that no more than BUFSZ-1 characters are written at once.	  */
 void GC_printf(const char *format, ...)
diff -ru bdwgc/win32_threads.c updated/bdwgc/win32_threads.c
--- bdwgc/win32_threads.c	2008-10-28 18:10:51.000000000 +0300
+++ updated/bdwgc/win32_threads.c	2008-10-28 21:59:00.000000000 +0300
@@ -1003,12 +1003,8 @@
   }
 # ifndef SMALL_CONFIG
     if (GC_print_stats == VERBOSE) {
-      GC_log_printf("Pushed %d thread stacks ", nthreads);
-      if (GC_win32_dll_threads) {
-    	GC_log_printf("based on DllMain thread tracking\n");
-      } else {
-    	GC_log_printf("\n");
-      }
+      GC_log_printf("Pushed %d thread stacks%s\n", nthreads,
+	     GC_win32_dll_threads ? " based on DllMain thread tracking" : "");
     }
 # endif
   if (!found_me && !GC_in_thread_creation)


More information about the Gc mailing list