[cfe-commits] [Review] First step towards PR13231 - vftable generation with -cxx-abi microsoft

Timur Iskhodzhanov timurrrr at google.com
Sat Dec 29 06:48:00 PST 2012


Please review the new patch attached.

I've put an extra FIXME for the RTTI handling
and added a new check to the constructor CodeGen test.

Timur Iskhodzhanov,
Google Russia



2012/12/29 Timur Iskhodzhanov <timurrrr at google.com>:
> 2012/12/4 Timur Iskhodzhanov <timurrrr at google.com>
>> > Okay, but I'm curious about something here.  It looks like you're not even
>> > allocating space in the v-table for the RTTI pointer if RTTI is disabled?
>> Yes - but it turned out this is not necessary.
>> When I was working on the patch initially the COFF writer formatted
>> vftables wrong, so I thought this was a problem.
>> Now that COFF is fixed, turns out I can put RTTI back - thanks for noticing!
> I take my words back (again).
> The vftable must have AddressPoint=0 in the single-inheritance case;
> I've found this while working on the virtual dtor patch.
>
> If the AddressPoint=4 and the class looks like this:
>   struct C {
>     virtual void foo();
>     virtual ~C();
>   }
> the call to foo() jumps into ~C() instead if the constructor is
> compiled by Clang (works OK if compiled by VC).
>
> I'll prepare a newer vtable layout patch to fix that.
>
>> > That means that every translation unit using the class needs to agree
>> > about whether RTTI is enabled, or else they might miscompile.  If that's
>> > what MSVC does, fine, but if not and what you're really trying to do is
>> > just punt on RTTI for now, why not just always emit RTTI pointers as null?
>> >
>> >> b) I had to remove "AddressPoint != 0" assertion since AddressPoint
>> >> should be 0 for some classes in -cxx-abi microsoft,
>> >
>> > You could weaken the assertion instead of removing it.
>> it's uint64_t, so weakening the "x != 0" assertion is just assert('x
>> is any'), right? :)
>> However, now that I've put RTTI pointer back, I can put the assertion
>> back as well.
> I'll have to remove the assertion again :(
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bug_13231_4.patch
Type: application/octet-stream
Size: 12280 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121229/be2f97a3/attachment.obj>


More information about the cfe-commits mailing list