[PATCH][RFC] Poison bitmap_head->obstack

Richard Biener rguenther@suse.de
Tue Dec 4 13:16:00 GMT 2018


This tries to make bugs like that in PR88317 harder to create by
introducing a bitmap_release function that can be used as
pendant to bitmap_initialize for non-allocated bitmap heads.
The function makes sure to poison the bitmaps obstack member
so the obstack the bitmap was initialized with can be safely
released.

The patch also adds a default constructor to bitmap_head
doing the same, but for C++ reason initializes to a
all-zero bitmap_obstack rather than 0xdeadbeef because
the latter isn't possible in constexpr context (it is
by using unions but then things start to look even more ugly).

The stage1 compiler might end up with a few extra runtime
initializers but constexpr makes sure they'll vanish for
later stages.

I had to paper over that you-may-not-use-memset-to-zero classes
with non-trivial constructors warning in two places and I
had to teach gengtype about CONSTEXPR (probably did so in
an awkward way - suggestions and pointers into gengtype
appreciated).

Bootstrapped (with host GCC 4.8) on x86_64-unknown-linux-gnu,
testing in progress.

The LRA issue seems to be rare enough (on x86_64...) that
I didn't trip over it sofar.

Comments?  Do we want this?  Not sure how we can easily
discover all bitmap_clear () users that should really
use bitmap_release (suggestion for a better name appreciated
as well - I thought about bitmap_uninitialize)

Richard.

2018-12-04  Richard Biener  <rguenther@suse.de>

	* bitmap.c (bitmap_head::crashme): Define.
	* bitmap.h (bitmap_head): Add constexpr default constructor
	poisoning the obstack member.
	(bitmap_head::crashme): Declare.
	(bitmap_release): New function clearing a bitmap and poisoning
	the obstack member.
	* gengtype.c (main): Make it recognize CONSTEXPR.

	* lra-constraints.c (lra_inheritance): Use bitmap_release
	instead of bitmap_clear.

	* ira.c (ira): Work around warning.
	* regrename.c (create_new_chain): Likewise.

Index: gcc/bitmap.c
===================================================================
--- gcc/bitmap.c	(revision 266772)
+++ gcc/bitmap.c	(working copy)
@@ -26,6 +26,10 @@ along with GCC; see the file COPYING3.
 /* Memory allocation statistics purpose instance.  */
 mem_alloc_description<bitmap_usage> bitmap_mem_desc;
 
+/* Static zero-initialized bitmap obstack used for default initialization
+   of bitmap_head.  */
+bitmap_obstack bitmap_head::crashme;
+
 static bitmap_element *bitmap_tree_listify_from (bitmap, bitmap_element *);
 
 /* Register new bitmap.  */
Index: gcc/bitmap.h
===================================================================
--- gcc/bitmap.h	(revision 266772)
+++ gcc/bitmap.h	(working copy)
@@ -323,6 +323,12 @@ struct GTY((chain_next ("%h.next"), chai
    already pointed to by the chain started by first, so GTY((skip)) it.  */
 
 struct GTY(()) bitmap_head {
+  static bitmap_obstack crashme;
+  /* Poison obstack to not make it not a valid initialized GC bitmap.  */
+  CONSTEXPR bitmap_head()
+    : indx(0), tree_form(false), first(NULL), current(NULL),
+      obstack (&crashme)
+  {}
   /* Index of last element looked at.  */
   unsigned int indx;
   /* False if the bitmap is in list form; true if the bitmap is in tree form.
@@ -441,6 +446,18 @@ bitmap_initialize (bitmap head, bitmap_o
     bitmap_register (head PASS_MEM_STAT);
 }
 
+/* Release a bitmap (but not its head).  This is suitable for pairing with
+   bitmap_initialize.  */
+
+static inline void
+bitmap_release (bitmap head)
+{
+  bitmap_clear (head);
+  /* Poison the obstack pointer so the obstack can be safely released.
+     Do not zero it as the bitmap then becomes initialized GC.  */
+  head->obstack = (bitmap_obstack *)0xdeadbeef;
+}
+
 /* Allocate and free bitmaps from obstack, malloc and gc'd memory.  */
 extern bitmap bitmap_alloc (bitmap_obstack *obstack CXX_MEM_STAT_INFO);
 #define BITMAP_ALLOC bitmap_alloc
Index: gcc/gengtype.c
===================================================================
--- gcc/gengtype.c	(revision 266772)
+++ gcc/gengtype.c	(working copy)
@@ -5205,6 +5205,7 @@ main (int argc, char **argv)
       POS_HERE (do_scalar_typedef ("void", &pos));
       POS_HERE (do_scalar_typedef ("machine_mode", &pos));
       POS_HERE (do_scalar_typedef ("fixed_size_mode", &pos));
+      POS_HERE (do_scalar_typedef ("CONSTEXPR", &pos));
       POS_HERE (do_typedef ("PTR", 
 			    create_pointer (resolve_typedef ("void", &pos)),
 			    &pos));
Index: gcc/ira.c
===================================================================
--- gcc/ira.c	(revision 266772)
+++ gcc/ira.c	(working copy)
@@ -5417,7 +5417,7 @@ ira (FILE *f)
 	    = ((struct ira_spilled_reg_stack_slot *)
 	       ira_allocate (max_regno
 			     * sizeof (struct ira_spilled_reg_stack_slot)));
-	  memset (ira_spilled_reg_stack_slots, 0,
+	  memset ((void *)ira_spilled_reg_stack_slots, 0,
 		  max_regno * sizeof (struct ira_spilled_reg_stack_slot));
 	}
     }
Index: gcc/lra-constraints.c
===================================================================
--- gcc/lra-constraints.c	(revision 266772)
+++ gcc/lra-constraints.c	(working copy)
@@ -6647,11 +6647,11 @@ lra_inheritance (void)
 	   inherit_in_ebb.  */
 	update_ebb_live_info (BB_HEAD (start_bb), BB_END (bb));
     }
-  bitmap_clear (&ebb_global_regs);
-  bitmap_clear (&temp_bitmap);
-  bitmap_clear (&live_regs);
-  bitmap_clear (&invalid_invariant_regs);
-  bitmap_clear (&check_only_regs);
+  bitmap_release (&ebb_global_regs);
+  bitmap_release (&temp_bitmap);
+  bitmap_release (&live_regs);
+  bitmap_release (&invalid_invariant_regs);
+  bitmap_release (&check_only_regs);
   free (usage_insns);
 
   timevar_pop (TV_LRA_INHERITANCE);
Index: gcc/regrename.c
===================================================================
--- gcc/regrename.c	(revision 266772)
+++ gcc/regrename.c	(working copy)
@@ -231,7 +231,7 @@ create_new_chain (unsigned this_regno, u
   struct du_chain *this_du;
   int nregs;
 
-  memset (head, 0, sizeof *head);
+  memset ((void *)head, 0, sizeof *head);
   head->next_chain = open_chains;
   head->regno = this_regno;
   head->nregs = this_nregs;



More information about the Gcc-patches mailing list