r244416 - [modules] PR22534: Load files specified by -fmodule-file= eagerly. In particular, this avoids the need to re-parse module map files when using such a module.

[Richard Smith via cfe-commits]

On Tue, Aug 18, 2015 at 1:52 PM, Gábor Horváth <xazax.hun at gmail.com> wrote:

> On 18 August 2015 at 13:41, Richard Smith <richard at metafoo.co.uk> wrote:
>
>> On Tue, Aug 18, 2015 at 12:59 PM, Gábor Horváth <xazax.hun at gmail.com>
>> wrote:
>>
>>> Hi!
>>>
>>> In r244416 you made createModuleManager to call the Initialize method
>>> of the ASTConsumer.
>>> Because of this change the ASTConsumer::Initialize might be called twice
>>> in some scenarios.
>>>
>>> This change makes the Static Analyzer crash (use after free) in those
>>> scenarios. I think most of the ASTConsumers was not written with that in
>>> mind Initialize might be called twice for the object. This fact is also not
>>> covered by the documentation.
>>>
>>> In my opinion we should either guarantee that the Initialize method will
>>> be called only once during the lifetime of an ASTConsumer, or document the
>>> fact that, it might be called multiple times and notify the authors of the
>>> different consumers to update their classes accordingly (announcement on
>>> the mailing list?).
>>>
>>> What do you think?
>>>
>>
>> Fixed in r245346.
>>
>> It seems like an indicator of poor design that we have the initialize
>> function at all. I don't think there is any situation where we need or want
>> to create an ASTConsumer before we have enough information to create the
>> ASTContext, so perhaps we can just get rid of this function.
>>
>
> Thank you for fixing this!
>
> I agree that it would be awesome to get rid of that function.
>

CCing the right list again :)

Are you interested in looking into this? (If not, it's going to near the
bottom of a long list of TODO items for me...)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150818/c2d46c0d/attachment.html>