[c++] rewrite vtable initialization construction to use VECs

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

On Tue, 2010-06-15 at 10:59 -0700, Nathan Froyd wrote:


> 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);
> 

Nope, this isn't working.

/proj/opensrc/nightly/src/trunk/libstdc++-v3/libsupc++/exception:61:9:
internal compiler error: vector VEC(constructor_elt,base) replace domain
error, in build_vtbl_initializer at cp/class.c:7624
Please submit a full bug report,
with preprocessed source if appropriate.
See <http://gcc.gnu.org/bugs.html> for instructions.
make[4]: *** [ia64-hp-hpux11.23/bits/stdc++.h.gch/O2ggnu++0x.gch] Error
1

Should
	VEC_replace (constructor_elt, vid.inits, new_position, e);
be
	VEC_replace (constructor_elt, *inits, new_position, e);

It doesn't look like this changed in your patch, but it differs from
other VEC_replace calls.

Steve Ellcey
sje@cup.hp.com