<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Nov 14, 2014 at 10:32 AM, Alexander Kornienko <span dir="ltr"><<a href="mailto:alexfh@google.com" target="_blank">alexfh@google.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"><br><div class="gmail_extra"><br><div class="gmail_quote"><span class="">On Thu, Nov 13, 2014 at 6:39 PM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span>On Thu, Nov 13, 2014 at 1:20 AM, Manuel Klimek <span dir="ltr"><<a href="mailto:klimek@google.com" target="_blank">klimek@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">+dblakie<div><br><div class="gmail_quote"><span>On Thu Nov 13 2014 at 1:53:39 AM Alexander Kornienko <<a href="mailto:alexfh@google.com" target="_blank">alexfh@google.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr">Thank you for the analysis and the proposed solution!<div><br></div><div>I can reproduce the issue (with any q.cpp that is not clang-tidy clean):</div><blockquote style="margin:0px 0px 0px 40px;border:none;padding:0px"><div><div>$ clang-tidy q.cpp -- --serialize-diagnostics test.dia</div></div><div><div>*** Error in `clang_tidy': free(): invalid pointer: 0x00007fffa65bb4d8 ***</div></div><div><div>Aborted (core dumped)</div></div></blockquote><div><div class="gmail_extra"><br></div><div class="gmail_extra">The patch seems correct to me and the way to distinguish between owning and non-owning constructors seems also fine. I'll commit the patch tomorrow if nobody offers a better solution.</div></div></div></blockquote><div><br></div></span><div>I don't really see anything better under the current restrictions. Perhaps David has an idea, he has done a lot of the unique_ptr migrations in llvm.</div></div></div></blockquote><div><br></div></span><div>At a cursory glance, this is the "conditional ownership" issue that's come up in a few places (and currently we have solutions that both look like this one (T*+unique_ptr<T> where the latter may be null but otherwise they both point to the same object) or bool+T* where the bool indicates ownership)<br><br>There is a thread on llvm/cfe-dev about whether we should introduce a reusable "conditional ownership" pointer, but the response from several people (Manuel, Chandler, and, depending on the day of the week, myself, etc) is that this kind of ownership complication is a bug in the design which we should fix at the source.<br><br>I'm still not sure if that's the case (that all cases of conditional ownership like this are design bugs) - but I'm sort of curious to see how they would look if we really tried to apply that logic.<br><br>As a side note: this patch looks way too subtle/dangerous as-is, even given the necessary conditional ownership semantics. Two branches of the if, one calls func(takeX()) the other calls func(unique_ptr<T>(takeX()) - that's pretty subtle (even though the "ownsClient" condition demonstrates what's going on there).<br><br>I'm not sure how much it's worth making this a bit tidier/more reliable (maybe Diags::takeClient should return a unique_ptr and just return null whenever !Diags.ownsClient() - and have a separate "getClient" function that can be called to get a raw pointer, regardless of ownership (careful if we have an ordering issue there - if takeClient nulls out the Diags' client, then we'd need to call 'get' before 'take', if takeClient just sets "OwnsClient" to false, then we can call them in either order)) - or if it's just going to be a bit lame until we go the whole way and remove the conditional ownership of the DiagnosticConsumer all the way up (or add a first class maybe-owning pointer type).</div></div></div></div></blockquote><div><br></div></span><div>Yeah, there are too many possible options here and ideally, it would be nice to unify all cases of conditional ownership or eliminate them. I suppose, that the latter may be rather difficult to make as the scope of the change may be wide and affect many public interfaces. But in any case, we need a centralized decision on what we want to do with the conditional ownership.</div></div></div></div></blockquote><div><br></div><div>Yep :s</div><div> </div><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>As for this patch, it should use the already existing getClient() function in the non-owning branch. I can commit a fix.<br></div></div></div></div></blockquote><div><br></div><div>Yeah, I think that'd help a lot.<br><br>(if you're feeling inclined - could you also make "takeClient" return by unique_ptr if that seems to fit (which I really hope it does)?)</div><div> </div><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><div><div class="h5"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><span><font color="#888888"><br><br>- David</font></span></div><div><div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div><div class="gmail_quote"><div><div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div><div class="gmail_extra"><br></div><div class="gmail_extra">-- Alex</div></div></div><div dir="ltr"><div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Nov 12, 2014 at 9:32 PM, Aaron Wishnick <span dir="ltr"><<a href="mailto:aaron.s.wishnick@gmail.com" target="_blank">aaron.s.wishnick@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr">Understanding the bug better, I've attached a patch that more correctly fixes this bug, by teaching ChainedDiagnosticConsumer how to not take ownership of one of its arguments, and having <span style="font-family:arial,sans-serif;font-size:13px">SetupSerializedDiagnostics() use it. Is there a more idiomatic way, in the LLVM project, of a "maybe" owning pointer? I see that some related functions take a "ShouldOwnClient" argument, but this seems a little more kludgy for two arguments with separate ownership.</span><div><span style="font-family:arial,sans-serif;font-size:13px"><br></span></div><div><span style="font-family:arial,sans-serif;font-size:13px">Thanks,</span></div><div><span style="font-family:arial,sans-serif;font-size:13px">Aaron</span></div></div><div><div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Nov 12, 2014 at 3:14 PM, Aaron Wishnick <span dir="ltr"><<a href="mailto:aaron.s.wishnick@gmail.com" target="_blank">aaron.s.wishnick@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr">Hi Alexander, sorry to dig up an old issue, but I've just gotten some more time to look into it. This is still reproducing for me on trunk, and I can see where the ChainedDiagnosticConsumer is created, as well as why it ends up trying to free a stack object. In short, there's a function SetupSerializedDiagnostics() in CompilerInstance.cpp that doesn't know how to handle the case where its DiagnosticsEngine doesn't own its client. This bug can be reproduced by using clang-tidy with a compilation database that uses the "--serialize-diagnostics" flag.<div><br></div><div>When I run a debug build with the arguments "clang-tidy -p /path/to/compile_commands.json /path/to/source.cpp", I get a failed assert in tools/clang/lib/Frontend/CompilerInstance.cpp, line 173, in SetupSerializedDiagnostics():</div><div><br></div><div><div>static void SetupSerializedDiagnostics(DiagnosticOptions *DiagOpts,</div><div> DiagnosticsEngine &Diags,</div><div> StringRef OutputFile) {</div><div> auto SerializedConsumer =</div><div> clang::serialized_diags::create(OutputFile, DiagOpts);</div><div><br></div><div> assert(Diags.ownsClient());</div><div> Diags.setClient(new ChainedDiagnosticConsumer(</div><div> std::unique_ptr<DiagnosticConsumer>(Diags.takeClient()),</div><div> std::move(SerializedConsumer)));</div><div>}</div></div><div><br></div><div>Stepping one stack frame up into createDiagnostics(), it looks like this code path is hit because the "if (!Opts->DiagnosticSerializationFile.empty())" condition on line 209 of CompilerInstance.cpp is met.</div><div><br></div><div><div>If I skip that assert, and continue, I get that same "pointer being freed was not allocated" error, once the program finishes and the ChainedDiagnosticConsumer is deleted. The address is from the stack, rather than the heap, and it corresponds to the value of "Diags.Client" before that call to "Diags.takeClient()." In other words, I think the problem is that the DiagnosticsEngine passed into SetupSerializedDiagnostics doesn't own its client, and the client is stack allocated, and then the client is stored in a unique_ptr which is owned by the ChainedDiagnosticConsumer.</div><div><br></div><div>Ultimately, I can see this comes from ClangTidy.cpp, line 470. This ClangTidyDiagnosticConsumer is created on the stack, and is the one that eventually ends up being freed, causing the bug.<br><div><br></div><div>I am using this in conjunction with Xcode: I am using xcodebuild to build my project, and then oclint-xcodebuild to generate the compile_commands.json database. Sure enough, all of the commands in the compilation database include the argument "--serialize-diagnostics /path/to/source.dia". If I remove these arguments, this bug doesn't occur. So, I think the issue is that SetupSerializedDiagnostics doesn't know how to handle the case where the DiagnosticsEngine doesn't own its client.</div><div><br></div><div>Hope this helps!</div><div><br></div><div>Best,</div><div>Aaron<div><div><br><div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Jun 23, 2014 at 9:51 AM, Alexander Kornienko <span dir="ltr"><<a href="mailto:alexfh@google.com" target="_blank">alexfh@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote"><div><div>On Mon, Jun 23, 2014 at 2:51 PM, Alexander Kornienko <span dir="ltr"><<a href="mailto:alexfh@google.com" target="_blank">alexfh@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote">
<div><div>On Thu, Jun 19, 2014 at 5:59 PM, Aaron Wishnick <span dir="ltr"><<a href="mailto:aaron.s.wishnick@gmail.com" target="_blank">aaron.s.wishnick@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr">When I run clang-tidy on OS X 10.9.3, I immediately get this output:<div>
<br></div><div><div><font face="courier new, monospace">clang-tidy(97903,0x7fff782fb310) malloc: *** error for object 0x7fff5fbfecd0: pointer being freed was not allocated</font></div>
<div><font face="courier new, monospace">*** set a breakpoint in malloc_error_break to debug</font></div></div><div><br></div><div>This occurs inside the destructor of ClangTidyDiagnosticConsumer. Here's my callstack:</div>
<div><br></div><div><div><font face="courier new, monospace">#4<span style="white-space:pre-wrap"> </span>0x000000010058e3e2 in ~ClangTidyDiagnosticConsumer at /Users/awishnick/clang-tidy/llvm/tools/clang/tools/extra/clang-tidy/ClangTidyDiagnosticConsumer.h:190</font></div>
<div><font face="courier new, monospace">#5<span style="white-space:pre-wrap"> </span>0x0000000100656a73 in std::__1::default_delete<clang::DiagnosticConsumer>::operator()(clang::DiagnosticConsumer*) const [inlined] at /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../lib/c++/v1/memory:2426</font></div>
<div><font face="courier new, monospace">#6<span style="white-space:pre-wrap"> </span>0x0000000100656a4b in std::__1::unique_ptr<clang::DiagnosticConsumer, std::__1::default_delete<clang::DiagnosticConsumer> >::reset(clang::DiagnosticConsumer*) [inlined] at /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../lib/c++/v1/memory:2625</font></div>
<div><font face="courier new, monospace">#7<span style="white-space:pre-wrap"> </span>0x00000001006569f5 in ~unique_ptr [inlined] at /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../lib/c++/v1/memory:2593</font></div>
<div><font face="courier new, monospace">#8<span style="white-space:pre-wrap"> </span>0x00000001006569f5 in ~unique_ptr [inlined] at /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../lib/c++/v1/memory:2593</font></div>
<div><font face="courier new, monospace">#9<span style="white-space:pre-wrap"> </span>0x00000001006569f5 in ~ChainedDiagnosticConsumer at /Users/awishnick/clang-tidy/llvm/tools/clang/include/clang/Frontend/ChainedDiagnosticConsumer.h:23</font></div>
<div><font face="courier new, monospace">#10<span style="white-space:pre-wrap"> </span>0x0000000100656595 in ~ChainedDiagnosticConsumer at /Users/awishnick/clang-tidy/llvm/tools/clang/include/clang/Frontend/ChainedDiagnosticConsumer.h:23</font></div>
<div><font face="courier new, monospace">#11<span style="white-space:pre-wrap"> </span>0x00000001006565b9 in ~ChainedDiagnosticConsumer at /Users/awishnick/clang-tidy/llvm/tools/clang/include/clang/Frontend/ChainedDiagnosticConsumer.h:23</font></div>
<div><font face="courier new, monospace">#12<span style="white-space:pre-wrap"> </span>0x00000001015eec84 in ~DiagnosticsEngine at /Users/awishnick/clang-tidy/llvm/tools/clang/lib/Basic/Diagnostic.cpp:68</font></div><div>
<font face="courier new, monospace">#13<span style="white-space:pre-wrap"> </span>0x00000001015eec35 in ~DiagnosticsEngine at /Users/awishnick/clang-tidy/llvm/tools/clang/lib/Basic/Diagnostic.cpp:66</font></div><div><font face="courier new, monospace">#14<span style="white-space:pre-wrap"> </span>0x00000001006bd3d3 in llvm::RefCountedBase<clang::DiagnosticsEngine>::Release() const at /Users/awishnick/clang-tidy/llvm/include/llvm/ADT/IntrusiveRefCntPtr.h:55</font></div>
<div><font face="courier new, monospace">#15<span style="white-space:pre-wrap"> </span>0x00000001006bd325 in llvm::IntrusiveRefCntPtrInfo<clang::DiagnosticsEngine>::release(clang::DiagnosticsEngine*) at /Users/awishnick/clang-tidy/llvm/include/llvm/ADT/IntrusiveRefCntPtr.h:90</font></div>
<div><font face="courier new, monospace">#16<span style="white-space:pre-wrap"> </span>0x00000001006bd2fd in llvm::IntrusiveRefCntPtr<clang::DiagnosticsEngine>::release() at /Users/awishnick/clang-tidy/llvm/include/llvm/ADT/IntrusiveRefCntPtr.h:199</font></div>
<div><font face="courier new, monospace">#17<span style="white-space:pre-wrap"> </span>0x00000001006bd2c5 in ~IntrusiveRefCntPtr at /Users/awishnick/clang-tidy/llvm/include/llvm/ADT/IntrusiveRefCntPtr.h:172</font></div>
<div><font face="courier new, monospace">#18<span style="white-space:pre-wrap"> </span>0x00000001006bbe15 in ~IntrusiveRefCntPtr at /Users/awishnick/clang-tidy/llvm/include/llvm/ADT/IntrusiveRefCntPtr.h:172</font></div>
<div><font face="courier new, monospace">#19<span style="white-space:pre-wrap"> </span>0x000000010065cbc1 in ~CompilerInstance at /Users/awishnick/clang-tidy/llvm/tools/clang/lib/Frontend/CompilerInstance.cpp:63</font></div>
<div><font face="courier new, monospace">#20<span style="white-space:pre-wrap"> </span>0x000000010065c505 in ~CompilerInstance at /Users/awishnick/clang-tidy/llvm/tools/clang/lib/Frontend/CompilerInstance.cpp:61</font></div>
<div><font face="courier new, monospace">#21<span style="white-space:pre-wrap"> </span>0x00000001005d6474 in clang::tooling::FrontendActionFactory::runInvocation(clang::CompilerInvocation*, clang::FileManager*, clang::DiagnosticConsumer*) at /Users/awishnick/clang-tidy/llvm/tools/clang/lib/Tooling/Tooling.cpp:270</font></div>
<div><font face="courier new, monospace">#22<span style="white-space:pre-wrap"> </span>0x00000001005d614f in clang::tooling::ToolInvocation::runInvocation(char const*, clang::driver::Compilation*, clang::CompilerInvocation*) at /Users/awishnick/clang-tidy/llvm/tools/clang/lib/Tooling/Tooling.cpp:243</font></div>
<div><font face="courier new, monospace">#23<span style="white-space:pre-wrap"> </span>0x00000001005d5290 in clang::tooling::ToolInvocation::run() at /Users/awishnick/clang-tidy/llvm/tools/clang/lib/Tooling/Tooling.cpp:229</font></div>
<div><font face="courier new, monospace">#24<span style="white-space:pre-wrap"> </span>0x00000001005d7b29 in clang::tooling::ClangTool::run(clang::tooling::ToolAction*) at /Users/awishnick/clang-tidy/llvm/tools/clang/lib/Tooling/Tooling.cpp:360</font></div>
<div><font face="courier new, monospace">#25<span style="white-space:pre-wrap"> </span>0x0000000100566cd2 in clang::tidy::runClangTidy(clang::tidy::ClangTidyOptionsProvider*, clang::tooling::CompilationDatabase const&, llvm::ArrayRef<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, std::__1::vector<clang::tidy::ClangTidyError, std::__1::allocator<clang::tidy::ClangTidyError> >*) at /Users/awishnick/clang-tidy/llvm/tools/clang/tools/extra/clang-tidy/ClangTidy.cpp:345</font></div>
<div><font face="courier new, monospace">#26<span style="white-space:pre-wrap"> </span>0x0000000100002a96 in main at /Users/awishnick/clang-tidy/llvm/tools/clang/tools/extra/clang-tidy/tool/ClangTidyMain.cpp:145</font></div>
</div><div><br></div><div>In short, it appears that ClangTool takes ownership of the diagnostic consumer, but it's being allocated on the stack. My fix is to allocate it on the heap instead. I've attached my patch. Please let me know if this assessment is incorrect, or if you'd like me to go about this differently.</div>
</div></blockquote><div><br></div></div></div><div>Well, the ownership of the diagnostic consumer shouldn't be transferred, and I don't see any evidence ClangTool::setDiagnosticConsumer expects this to happen. This all looks strange, and I'm investigating this.</div>
</div></div></div></blockquote><div><br></div></div></div><div>I wasn't able to reproduce this crash. Your stack trace has ChainedDiagnosticConsumer in it, which afaiu, it is only used twice in Clang, and both places don't seem to be unrelated to clang-tidy. Could you set a breakpoint in ChainedDiagnosticConsumer constructor and send me the stack trace where it gets called in clang-tidy? (or add an "assert(false);" there to get the stack trace on the console in the assertions-enabled build)</div><span>
<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">
<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr">
<div><br></div><div>Thanks!<span><font color="#888888"><br>Aaron</font></span></div><div><br></div></div>
<br>_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a></blockquote></div></div></div></blockquote></span></div>
</div></div>
</blockquote></div><br></div></div></div></div></div></div></div></div><div><div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Jun 23, 2014 at 9:51 AM, Alexander Kornienko <span dir="ltr"><<a href="mailto:alexfh@google.com" target="_blank">alexfh@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote"><div><div>On Mon, Jun 23, 2014 at 2:51 PM, Alexander Kornienko <span dir="ltr"><<a href="mailto:alexfh@google.com" target="_blank">alexfh@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote">
<div><div>On Thu, Jun 19, 2014 at 5:59 PM, Aaron Wishnick <span dir="ltr"><<a href="mailto:aaron.s.wishnick@gmail.com" target="_blank">aaron.s.wishnick@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr">When I run clang-tidy on OS X 10.9.3, I immediately get this output:<div>
<br></div><div><div><font face="courier new, monospace">clang-tidy(97903,0x7fff782fb310) malloc: *** error for object 0x7fff5fbfecd0: pointer being freed was not allocated</font></div>
<div><font face="courier new, monospace">*** set a breakpoint in malloc_error_break to debug</font></div></div><div><br></div><div>This occurs inside the destructor of ClangTidyDiagnosticConsumer. Here's my callstack:</div>
<div><br></div><div><div><font face="courier new, monospace">#4<span style="white-space:pre-wrap"> </span>0x000000010058e3e2 in ~ClangTidyDiagnosticConsumer at /Users/awishnick/clang-tidy/llvm/tools/clang/tools/extra/clang-tidy/ClangTidyDiagnosticConsumer.h:190</font></div>
<div><font face="courier new, monospace">#5<span style="white-space:pre-wrap"> </span>0x0000000100656a73 in std::__1::default_delete<clang::DiagnosticConsumer>::operator()(clang::DiagnosticConsumer*) const [inlined] at /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../lib/c++/v1/memory:2426</font></div>
<div><font face="courier new, monospace">#6<span style="white-space:pre-wrap"> </span>0x0000000100656a4b in std::__1::unique_ptr<clang::DiagnosticConsumer, std::__1::default_delete<clang::DiagnosticConsumer> >::reset(clang::DiagnosticConsumer*) [inlined] at /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../lib/c++/v1/memory:2625</font></div>
<div><font face="courier new, monospace">#7<span style="white-space:pre-wrap"> </span>0x00000001006569f5 in ~unique_ptr [inlined] at /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../lib/c++/v1/memory:2593</font></div>
<div><font face="courier new, monospace">#8<span style="white-space:pre-wrap"> </span>0x00000001006569f5 in ~unique_ptr [inlined] at /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../lib/c++/v1/memory:2593</font></div>
<div><font face="courier new, monospace">#9<span style="white-space:pre-wrap"> </span>0x00000001006569f5 in ~ChainedDiagnosticConsumer at /Users/awishnick/clang-tidy/llvm/tools/clang/include/clang/Frontend/ChainedDiagnosticConsumer.h:23</font></div>
<div><font face="courier new, monospace">#10<span style="white-space:pre-wrap"> </span>0x0000000100656595 in ~ChainedDiagnosticConsumer at /Users/awishnick/clang-tidy/llvm/tools/clang/include/clang/Frontend/ChainedDiagnosticConsumer.h:23</font></div>
<div><font face="courier new, monospace">#11<span style="white-space:pre-wrap"> </span>0x00000001006565b9 in ~ChainedDiagnosticConsumer at /Users/awishnick/clang-tidy/llvm/tools/clang/include/clang/Frontend/ChainedDiagnosticConsumer.h:23</font></div>
<div><font face="courier new, monospace">#12<span style="white-space:pre-wrap"> </span>0x00000001015eec84 in ~DiagnosticsEngine at /Users/awishnick/clang-tidy/llvm/tools/clang/lib/Basic/Diagnostic.cpp:68</font></div><div>
<font face="courier new, monospace">#13<span style="white-space:pre-wrap"> </span>0x00000001015eec35 in ~DiagnosticsEngine at /Users/awishnick/clang-tidy/llvm/tools/clang/lib/Basic/Diagnostic.cpp:66</font></div><div><font face="courier new, monospace">#14<span style="white-space:pre-wrap"> </span>0x00000001006bd3d3 in llvm::RefCountedBase<clang::DiagnosticsEngine>::Release() const at /Users/awishnick/clang-tidy/llvm/include/llvm/ADT/IntrusiveRefCntPtr.h:55</font></div>
<div><font face="courier new, monospace">#15<span style="white-space:pre-wrap"> </span>0x00000001006bd325 in llvm::IntrusiveRefCntPtrInfo<clang::DiagnosticsEngine>::release(clang::DiagnosticsEngine*) at /Users/awishnick/clang-tidy/llvm/include/llvm/ADT/IntrusiveRefCntPtr.h:90</font></div>
<div><font face="courier new, monospace">#16<span style="white-space:pre-wrap"> </span>0x00000001006bd2fd in llvm::IntrusiveRefCntPtr<clang::DiagnosticsEngine>::release() at /Users/awishnick/clang-tidy/llvm/include/llvm/ADT/IntrusiveRefCntPtr.h:199</font></div>
<div><font face="courier new, monospace">#17<span style="white-space:pre-wrap"> </span>0x00000001006bd2c5 in ~IntrusiveRefCntPtr at /Users/awishnick/clang-tidy/llvm/include/llvm/ADT/IntrusiveRefCntPtr.h:172</font></div>
<div><font face="courier new, monospace">#18<span style="white-space:pre-wrap"> </span>0x00000001006bbe15 in ~IntrusiveRefCntPtr at /Users/awishnick/clang-tidy/llvm/include/llvm/ADT/IntrusiveRefCntPtr.h:172</font></div>
<div><font face="courier new, monospace">#19<span style="white-space:pre-wrap"> </span>0x000000010065cbc1 in ~CompilerInstance at /Users/awishnick/clang-tidy/llvm/tools/clang/lib/Frontend/CompilerInstance.cpp:63</font></div>
<div><font face="courier new, monospace">#20<span style="white-space:pre-wrap"> </span>0x000000010065c505 in ~CompilerInstance at /Users/awishnick/clang-tidy/llvm/tools/clang/lib/Frontend/CompilerInstance.cpp:61</font></div>
<div><font face="courier new, monospace">#21<span style="white-space:pre-wrap"> </span>0x00000001005d6474 in clang::tooling::FrontendActionFactory::runInvocation(clang::CompilerInvocation*, clang::FileManager*, clang::DiagnosticConsumer*) at /Users/awishnick/clang-tidy/llvm/tools/clang/lib/Tooling/Tooling.cpp:270</font></div>
<div><font face="courier new, monospace">#22<span style="white-space:pre-wrap"> </span>0x00000001005d614f in clang::tooling::ToolInvocation::runInvocation(char const*, clang::driver::Compilation*, clang::CompilerInvocation*) at /Users/awishnick/clang-tidy/llvm/tools/clang/lib/Tooling/Tooling.cpp:243</font></div>
<div><font face="courier new, monospace">#23<span style="white-space:pre-wrap"> </span>0x00000001005d5290 in clang::tooling::ToolInvocation::run() at /Users/awishnick/clang-tidy/llvm/tools/clang/lib/Tooling/Tooling.cpp:229</font></div>
<div><font face="courier new, monospace">#24<span style="white-space:pre-wrap"> </span>0x00000001005d7b29 in clang::tooling::ClangTool::run(clang::tooling::ToolAction*) at /Users/awishnick/clang-tidy/llvm/tools/clang/lib/Tooling/Tooling.cpp:360</font></div>
<div><font face="courier new, monospace">#25<span style="white-space:pre-wrap"> </span>0x0000000100566cd2 in clang::tidy::runClangTidy(clang::tidy::ClangTidyOptionsProvider*, clang::tooling::CompilationDatabase const&, llvm::ArrayRef<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, std::__1::vector<clang::tidy::ClangTidyError, std::__1::allocator<clang::tidy::ClangTidyError> >*) at /Users/awishnick/clang-tidy/llvm/tools/clang/tools/extra/clang-tidy/ClangTidy.cpp:345</font></div>
<div><font face="courier new, monospace">#26<span style="white-space:pre-wrap"> </span>0x0000000100002a96 in main at /Users/awishnick/clang-tidy/llvm/tools/clang/tools/extra/clang-tidy/tool/ClangTidyMain.cpp:145</font></div>
</div><div><br></div><div>In short, it appears that ClangTool takes ownership of the diagnostic consumer, but it's being allocated on the stack. My fix is to allocate it on the heap instead. I've attached my patch. Please let me know if this assessment is incorrect, or if you'd like me to go about this differently.</div>
</div></blockquote><div><br></div></div></div><div>Well, the ownership of the diagnostic consumer shouldn't be transferred, and I don't see any evidence ClangTool::setDiagnosticConsumer expects this to happen. This all looks strange, and I'm investigating this.</div>
</div></div></div></blockquote><div><br></div></div></div><div>I wasn't able to reproduce this crash. Your stack trace has ChainedDiagnosticConsumer in it, which afaiu, it is only used twice in Clang, and both places don't seem to be unrelated to clang-tidy. Could you set a breakpoint in ChainedDiagnosticConsumer constructor and send me the stack trace where it gets called in clang-tidy? (or add an "assert(false);" there to get the stack trace on the console in the assertions-enabled build)</div><span>
<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">
<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr">
<div><br></div><div>Thanks!<span><font color="#888888"><br>Aaron</font></span></div><div><br></div></div></blockquote></div></div></div></blockquote></span></div></div></div></blockquote></div></div></div></div></blockquote></div></div></div></div></blockquote></div>
</div></div></div>
______________________________<u></u>_________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/<u></u>mailman/listinfo/cfe-commits</a><br>
</blockquote></div></div></div></div>
</blockquote></div></div></div><br></div></div>
</blockquote></div></div></div><br>
</div></div>
</blockquote></div><br></div></div>