<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On 18 August 2015 at 14:46, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div class="h5">On Tue, Aug 18, 2015 at 1:52 PM, Gábor Horváth <span dir="ltr"><<a href="mailto:xazax.hun@gmail.com" target="_blank">xazax.hun@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><div><div class="gmail_extra"><div class="gmail_quote">On 18 August 2015 at 13:41, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span>On Tue, Aug 18, 2015 at 12:59 PM, Gábor Horváth <span dir="ltr"><<a href="mailto:xazax.hun@gmail.com" target="_blank">xazax.hun@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Hi!<div><br></div><div>In <span style="font-family:'Helvetica Neue';font-size:12px">r244416</span><span style="font-family:'Helvetica Neue';font-size:12px"> you made </span>createModuleManager to call the Initialize method of the ASTConsumer.</div><div>Because of this change the ASTConsumer::Initialize might be called twice in some scenarios.</div><div><br></div><div>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.</div><div><br></div><div>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?).</div><div><br></div><div>What do you think?</div></div></blockquote><div><br></div></span><div>Fixed in r245346.</div><div><br></div><div>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.</div></div></div></div>
</blockquote></div><br></div></div></div><div class="gmail_extra">Thank you for fixing this!</div><div class="gmail_extra"><br></div><div class="gmail_extra">I agree that it would be awesome to get rid of that function.</div></div></blockquote><div><br></div></div></div><div>CCing the right list again :)</div><div><br></div><div>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...)</div></div></div></div></blockquote><div><br></div><div>Hope to CC the right list this time :)</div><div><br></div><div>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. </div></div><br></div></div>