From patchwork Tue Jun 15 17:59:09 2010
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: [c++] rewrite vtable initialization construction to use VECs
Date: Tue, 15 Jun 2010 07:59:09 -0000
From: Nathan Froyd <froydnj@codesourcery.com>
X-Patchwork-Id: 55766
Message-Id: <20100615175908.GB27105@codesourcery.com>
To: Steve Ellcey <sje@cup.hp.com>
Cc: Mark Mitchell <mark@codesourcery.com>, gcc-patches@gcc.gnu.org

On Tue, Jun 15, 2010 at 10:30:54AM -0700, Steve Ellcey wrote:
> On Tue, 2010-06-15 at 05:22 -0700, Nathan Froyd wrote:
> > Index: class.c
> > ===================================================================
> > --- class.c	(revision 160473)
> > +++ class.c	(working copy)
> > @@ -7624,7 +7624,7 @@ build_vtbl_initializer (tree binfo,
> >  
> >  	  for (j = 1; j < TARGET_VTABLE_DATA_ENTRY_DISTANCE; ++j)
> >  	    {
> > -	      constructor_elt *f = VEC_index (constructor_elt, *inits,
> > +	      constructor_elt *f = VEC_index (constructor_elt, vid.inits,
> >  					      new_position + j);
> >  	      f->index = NULL_TREE;
> >  	      f->value = build1 (NOP_EXPR, vtable_entry_type,
> 
> Well, the bootstrap worked and I got a compiler built but I get a bunch
> of failures during C++ and libstdc++ testing.  Does the attached failure
> and backtrace give you any ideas on where the problem might be?

My C++ runtime-fu is very weak.  I guess one of the vtable entries must
be NULL when it's not supposed to be.

Ah, I think I see the bug.  Before the patch, we had:

      tree cur, *prev;

      for (prev = &vid.inits; (cur = *prev); prev = &TREE_CHAIN (cur))
	{
	  tree add = cur;
	  int i;

	  for (i = 1; i < TARGET_VTABLE_DATA_ENTRY_DISTANCE; ++i)
	    add = tree_cons (NULL_TREE,
			     build1 (NOP_EXPR, vtable_entry_type,
				     null_pointer_node),
			     add);
	  *prev = add;
	}

I think I misread how this loop works: it's adding the padding *before*
the entries.  But my rewrite added the padding *after* the entries.  In
pictures, before my patch:

  +---+---+---+---+
  | 0 | A | 0 | B |
  +---+---+---+---+

after my patch:

  +---+---+---+---+
  | A | 0 | B | 0 |
  +---+---+---+---+

which would explain why we're seeing null pointers.

Would you try the below patch (against mainline, not on top of my
previous patch), please?  I think that should resolve the problems
you're seeing, assuming I didn't botch my arithmetic.

-Nathan


Index: class.c
===================================================================
--- class.c	(revision 160473)
+++ class.c	(working copy)
@@ -7618,14 +7618,15 @@ build_vtbl_initializer (tree binfo,
 	   ix--)
 	{
 	  int j;
-	  int new_position = TARGET_VTABLE_DATA_ENTRY_DISTANCE * ix;
+	  int new_position = (TARGET_VTABLE_DATA_ENTRY_DISTANCE * ix
+			      - (TARGET_VTABLE_DATA_ENTRY_DISTANCE - 1));
 
 	  VEC_replace (constructor_elt, vid.inits, new_position, e);
 
 	  for (j = 1; j < TARGET_VTABLE_DATA_ENTRY_DISTANCE; ++j)
 	    {
-	      constructor_elt *f = VEC_index (constructor_elt, *inits,
-					      new_position + j);
+	      constructor_elt *f = VEC_index (constructor_elt, vid.inits,
+					      new_position - j);
 	      f->index = NULL_TREE;
 	      f->value = build1 (NOP_EXPR, vtable_entry_type,
 				 null_pointer_node);