On 18 August 2015 at 14:46, Richard Smith <rich...@metafoo.co.uk> wrote:
> On Tue, Aug 18, 2015 at 1:52 PM, Gábor Horváth <xazax....@gmail.com> > wrote: > >> On 18 August 2015 at 13:41, Richard Smith <rich...@metafoo.co.uk> wrote: >> >>> On Tue, Aug 18, 2015 at 12:59 PM, Gábor Horváth <xazax....@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...) > Hope to CC the right list this time :) Well, it would end up on the bottom of my TODO list as well, so maybe we should both add it, and the one who gets there earlier should do this.
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
