This is the mail archive of the gcc@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: GCC 4.2.0 Status Report (2007-03-22)


Diego Novillo wrote:

> This one seems to be a bug in the C++ FE, compounded by alias analysis
> papering over the issue.

Doh!  Thank you for tracking this down.

> Mark, does this look OK?  (not tested yet)
> 
> Index: cp/class.c
> ===================================================================
> --- cp/class.c  (revision 123332)
> +++ cp/class.c  (working copy)
> @@ -7102,6 +7102,7 @@
>        /* Figure out the position to which the VPTR should point.  */
>        vtbl = TREE_PURPOSE (l);
>        vtbl = build1 (ADDR_EXPR, vtbl_ptr_type_node, vtbl);
> +      cxx_mark_addressable (vtbl);
>        index = size_binop (PLUS_EXPR,
>                           size_int (non_fn_entries),
>                           size_int (list_length (TREE_VALUE (l))));

Echoing Jason, I think it would be best to use:

  vtbl = build_nop (vtbl_ptr_type_node, build_address (vtbl));

That will also make the tree type-correct.  Right now, we're building an
ADDR_EXPR with type "void (*)(void)", even though the function may be
"int ()(S* this, double f)".  So, by using build_address and build_nop,
we'll get more correct code, plus get the addressability thing right.

My only concern is that we seem to have gone to some lengths to avoid
doing this.  I think that the reason is that we create vtable
initializers even for vtables we aren't going to emit in this
translation unit.  So, if you make the change above, you'll mark
functions addressable which are never going to be emitted.  I don't see
anything wrong with that; I just wonder why we didn't do it.  (You can
see that mark_vtable_entries, which is called for emitted vtables, sets
TREE_ADDRESSABLE by hand.)

So, I think the right fix is (a) the change above, (b) remove the
TREE_ADDRESSABLE setting from mark_vtable_entries (possibly replacing it
with an assert.)

If that works, it's OK.  If not, and you want to punt it back, go ahead.

Thanks again for working on this!

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]