[patch] Don't let virtual calls and dynamic casts call MarkVTableUsed()

Nico Weber thakis at chromium.org
Sun Jan 25 22:26:12 PST 2015


Finally landed in r227073 (took a bit since I wanted to write some tests
for apple-kext mode first, which I did in r227068 after making the existing
apple-kext tests actually run in r227029). Thanks!

On Wed, Jan 14, 2015 at 2:55 PM, Reid Kleckner <rnk at google.com> wrote:

> lgtm
>
> On Tue, Jan 13, 2015 at 1:42 PM, Nico Weber <thakis at chromium.org> wrote:
>
>> Tiny update: I missed a MarkVTableUsed() call in SemaInit.cpp. After
>> removing that, Sema::BasePathInvolvesVirtualBase() is dead, so remove that
>> too.
>>
>> On Tue, Jan 13, 2015 at 11:44 AM, Reid Kleckner <rnk at google.com> wrote:
>>
>>> On Tue, Jan 13, 2015 at 11:24 AM, Reid Kleckner <rnk at google.com> wrote:
>>>
>>>> On Tue, Jan 13, 2015 at 11:22 AM, Nico Weber <thakis at chromium.org>
>>>> wrote:
>>>>
>>>>> On Tue, Jan 13, 2015 at 11:11 AM, Reid Kleckner <rnk at google.com>
>>>>> wrote:
>>>>>
>>>>>> I'm concerned that if you don't mark the vtable used enough, then
>>>>>> codegen will crash trying to emit a vtable for a class that it assumed
>>>>>> would be marked used because it's compiling a virtual call to a method of
>>>>>> such a class. However, it looks like Rafael completely nuked the
>>>>>> available_externally vtable emission optimization in r189852, which I
>>>>>> forgot about. The vtable code still has lots of rigging to allow
>>>>>> available_externally vtable emission, though.
>>>>>>
>>>>>
>>>>> At the moment, the only thing in coding adding to the DeferredVTables
>>>>> vector in codegen is getAddrOfVTable(), and that's only called for structor
>>>>> body emission (and apple kext vcalls). So I think this should be fine.
>>>>>
>>>>
>>>> Right, but we used to call it more, prior to PR13124 and r189852. I'm
>>>> reading that bug to see if it's something we want to add back.
>>>>
>>>
>>> I'm starting to think that even if we want to bring back the
>>> available_externally vtable optimization, it should only happen
>>> optimistically and should not rely on Sema to mark the vtable used. Looking
>>> at PR13124, though, that seems difficult...
>>>
>>> Anyway, I drop my objections. The available_externally vtable
>>> optimization doesn't exist and turns out to be very hard to implement in
>>> Clang today. =/
>>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150125/143b7323/attachment.html>


More information about the cfe-commits mailing list