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...)
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
