On Fri, Nov 14, 2014 at 10:32 AM, Alexander Kornienko <[email protected]> wrote:
> > > On Thu, Nov 13, 2014 at 6:39 PM, David Blaikie <[email protected]> wrote: > >> >> >> On Thu, Nov 13, 2014 at 1:20 AM, Manuel Klimek <[email protected]> wrote: >> >>> +dblakie >>> >>> On Thu Nov 13 2014 at 1:53:39 AM Alexander Kornienko <[email protected]> >>> wrote: >>> >>>> Thank you for the analysis and the proposed solution! >>>> >>>> I can reproduce the issue (with any q.cpp that is not clang-tidy clean): >>>> >>>> $ clang-tidy q.cpp -- --serialize-diagnostics test.dia >>>> *** Error in `clang_tidy': free(): invalid pointer: 0x00007fffa65bb4d8 >>>> *** >>>> Aborted (core dumped) >>>> >>>> >>>> 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. >>>> >>> >>> 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. >>> >> >> 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) >> >> 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. >> >> 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. >> >> 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). >> >> 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). >> > > 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. > Yep :s > As for this patch, it should use the already existing getClient() function > in the non-owning branch. I can commit a fix. > Yeah, I think that'd help a lot. (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)?) > > >> >> >> - David >> >> >>> >>> >>>> >>>> -- Alex >>>> >>>> On Wed, Nov 12, 2014 at 9:32 PM, Aaron Wishnick < >>>> [email protected]> wrote: >>>> >>>>> 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 >>>>> 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. >>>>> >>>>> Thanks, >>>>> Aaron >>>>> >>>>> On Wed, Nov 12, 2014 at 3:14 PM, Aaron Wishnick < >>>>> [email protected]> wrote: >>>>> >>>>>> 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. >>>>>> >>>>>> 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(): >>>>>> >>>>>> static void SetupSerializedDiagnostics(DiagnosticOptions *DiagOpts, >>>>>> DiagnosticsEngine &Diags, >>>>>> StringRef OutputFile) { >>>>>> auto SerializedConsumer = >>>>>> clang::serialized_diags::create(OutputFile, DiagOpts); >>>>>> >>>>>> assert(Diags.ownsClient()); >>>>>> Diags.setClient(new ChainedDiagnosticConsumer( >>>>>> std::unique_ptr<DiagnosticConsumer>(Diags.takeClient()), >>>>>> std::move(SerializedConsumer))); >>>>>> } >>>>>> >>>>>> 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. >>>>>> >>>>>> 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. >>>>>> >>>>>> 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. >>>>>> >>>>>> 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. >>>>>> >>>>>> Hope this helps! >>>>>> >>>>>> Best, >>>>>> Aaron >>>>>> >>>>>> >>>>>> On Mon, Jun 23, 2014 at 9:51 AM, Alexander Kornienko < >>>>>> [email protected]> wrote: >>>>>> >>>>>>> >>>>>>> On Mon, Jun 23, 2014 at 2:51 PM, Alexander Kornienko < >>>>>>> [email protected]> wrote: >>>>>>> >>>>>>>> >>>>>>>> On Thu, Jun 19, 2014 at 5:59 PM, Aaron Wishnick < >>>>>>>> [email protected]> wrote: >>>>>>>> >>>>>>>>> When I run clang-tidy on OS X 10.9.3, I immediately get this >>>>>>>>> output: >>>>>>>>> >>>>>>>>> clang-tidy(97903,0x7fff782fb310) malloc: *** error for object >>>>>>>>> 0x7fff5fbfecd0: pointer being freed was not allocated >>>>>>>>> *** set a breakpoint in malloc_error_break to debug >>>>>>>>> >>>>>>>>> This occurs inside the destructor of ClangTidyDiagnosticConsumer. >>>>>>>>> Here's my callstack: >>>>>>>>> >>>>>>>>> #4 0x000000010058e3e2 in ~ClangTidyDiagnosticConsumer at >>>>>>>>> /Users/awishnick/clang-tidy/llvm/tools/clang/tools/extra/clang-tidy/ClangTidyDiagnosticConsumer.h:190 >>>>>>>>> #5 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 >>>>>>>>> #6 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 >>>>>>>>> #7 0x00000001006569f5 in ~unique_ptr [inlined] at >>>>>>>>> /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../lib/c++/v1/memory:2593 >>>>>>>>> #8 0x00000001006569f5 in ~unique_ptr [inlined] at >>>>>>>>> /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../lib/c++/v1/memory:2593 >>>>>>>>> #9 0x00000001006569f5 in ~ChainedDiagnosticConsumer at >>>>>>>>> /Users/awishnick/clang-tidy/llvm/tools/clang/include/clang/Frontend/ChainedDiagnosticConsumer.h:23 >>>>>>>>> #10 0x0000000100656595 in ~ChainedDiagnosticConsumer at >>>>>>>>> /Users/awishnick/clang-tidy/llvm/tools/clang/include/clang/Frontend/ChainedDiagnosticConsumer.h:23 >>>>>>>>> #11 0x00000001006565b9 in ~ChainedDiagnosticConsumer at >>>>>>>>> /Users/awishnick/clang-tidy/llvm/tools/clang/include/clang/Frontend/ChainedDiagnosticConsumer.h:23 >>>>>>>>> #12 0x00000001015eec84 in ~DiagnosticsEngine at >>>>>>>>> /Users/awishnick/clang-tidy/llvm/tools/clang/lib/Basic/Diagnostic.cpp:68 >>>>>>>>> #13 0x00000001015eec35 in ~DiagnosticsEngine at >>>>>>>>> /Users/awishnick/clang-tidy/llvm/tools/clang/lib/Basic/Diagnostic.cpp:66 >>>>>>>>> #14 0x00000001006bd3d3 in >>>>>>>>> llvm::RefCountedBase<clang::DiagnosticsEngine>::Release() const at >>>>>>>>> /Users/awishnick/clang-tidy/llvm/include/llvm/ADT/IntrusiveRefCntPtr.h:55 >>>>>>>>> #15 0x00000001006bd325 in >>>>>>>>> llvm::IntrusiveRefCntPtrInfo<clang::DiagnosticsEngine>::release(clang::DiagnosticsEngine*) >>>>>>>>> at >>>>>>>>> /Users/awishnick/clang-tidy/llvm/include/llvm/ADT/IntrusiveRefCntPtr.h:90 >>>>>>>>> #16 0x00000001006bd2fd in >>>>>>>>> llvm::IntrusiveRefCntPtr<clang::DiagnosticsEngine>::release() at >>>>>>>>> /Users/awishnick/clang-tidy/llvm/include/llvm/ADT/IntrusiveRefCntPtr.h:199 >>>>>>>>> #17 0x00000001006bd2c5 in ~IntrusiveRefCntPtr at >>>>>>>>> /Users/awishnick/clang-tidy/llvm/include/llvm/ADT/IntrusiveRefCntPtr.h:172 >>>>>>>>> #18 0x00000001006bbe15 in ~IntrusiveRefCntPtr at >>>>>>>>> /Users/awishnick/clang-tidy/llvm/include/llvm/ADT/IntrusiveRefCntPtr.h:172 >>>>>>>>> #19 0x000000010065cbc1 in ~CompilerInstance at >>>>>>>>> /Users/awishnick/clang-tidy/llvm/tools/clang/lib/Frontend/CompilerInstance.cpp:63 >>>>>>>>> #20 0x000000010065c505 in ~CompilerInstance at >>>>>>>>> /Users/awishnick/clang-tidy/llvm/tools/clang/lib/Frontend/CompilerInstance.cpp:61 >>>>>>>>> #21 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 >>>>>>>>> #22 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 >>>>>>>>> #23 0x00000001005d5290 in clang::tooling::ToolInvocation::run() >>>>>>>>> at >>>>>>>>> /Users/awishnick/clang-tidy/llvm/tools/clang/lib/Tooling/Tooling.cpp:229 >>>>>>>>> #24 0x00000001005d7b29 in >>>>>>>>> clang::tooling::ClangTool::run(clang::tooling::ToolAction*) at >>>>>>>>> /Users/awishnick/clang-tidy/llvm/tools/clang/lib/Tooling/Tooling.cpp:360 >>>>>>>>> #25 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 >>>>>>>>> #26 0x0000000100002a96 in main at >>>>>>>>> /Users/awishnick/clang-tidy/llvm/tools/clang/tools/extra/clang-tidy/tool/ClangTidyMain.cpp:145 >>>>>>>>> >>>>>>>>> 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. >>>>>>>>> >>>>>>>> >>>>>>>> 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. >>>>>>>> >>>>>>> >>>>>>> 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) >>>>>>> >>>>>>> >>>>>>>> >>>>>>>> >>>>>>>>> >>>>>>>>> Thanks! >>>>>>>>> Aaron >>>>>>>>> >>>>>>>>> >>>>>>>>> _______________________________________________ >>>>>>>>> cfe-commits mailing list >>>>>>>>> [email protected] >>>>>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >>>>>>>> >>>>>>>> >>>>>> >>>>>> On Mon, Jun 23, 2014 at 9:51 AM, Alexander Kornienko < >>>>>> [email protected]> wrote: >>>>>> >>>>>>> >>>>>>> On Mon, Jun 23, 2014 at 2:51 PM, Alexander Kornienko < >>>>>>> [email protected]> wrote: >>>>>>> >>>>>>>> >>>>>>>> On Thu, Jun 19, 2014 at 5:59 PM, Aaron Wishnick < >>>>>>>> [email protected]> wrote: >>>>>>>> >>>>>>>>> When I run clang-tidy on OS X 10.9.3, I immediately get this >>>>>>>>> output: >>>>>>>>> >>>>>>>>> clang-tidy(97903,0x7fff782fb310) malloc: *** error for object >>>>>>>>> 0x7fff5fbfecd0: pointer being freed was not allocated >>>>>>>>> *** set a breakpoint in malloc_error_break to debug >>>>>>>>> >>>>>>>>> This occurs inside the destructor of ClangTidyDiagnosticConsumer. >>>>>>>>> Here's my callstack: >>>>>>>>> >>>>>>>>> #4 0x000000010058e3e2 in ~ClangTidyDiagnosticConsumer at >>>>>>>>> /Users/awishnick/clang-tidy/llvm/tools/clang/tools/extra/clang-tidy/ClangTidyDiagnosticConsumer.h:190 >>>>>>>>> #5 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 >>>>>>>>> #6 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 >>>>>>>>> #7 0x00000001006569f5 in ~unique_ptr [inlined] at >>>>>>>>> /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../lib/c++/v1/memory:2593 >>>>>>>>> #8 0x00000001006569f5 in ~unique_ptr [inlined] at >>>>>>>>> /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../lib/c++/v1/memory:2593 >>>>>>>>> #9 0x00000001006569f5 in ~ChainedDiagnosticConsumer at >>>>>>>>> /Users/awishnick/clang-tidy/llvm/tools/clang/include/clang/Frontend/ChainedDiagnosticConsumer.h:23 >>>>>>>>> #10 0x0000000100656595 in ~ChainedDiagnosticConsumer at >>>>>>>>> /Users/awishnick/clang-tidy/llvm/tools/clang/include/clang/Frontend/ChainedDiagnosticConsumer.h:23 >>>>>>>>> #11 0x00000001006565b9 in ~ChainedDiagnosticConsumer at >>>>>>>>> /Users/awishnick/clang-tidy/llvm/tools/clang/include/clang/Frontend/ChainedDiagnosticConsumer.h:23 >>>>>>>>> #12 0x00000001015eec84 in ~DiagnosticsEngine at >>>>>>>>> /Users/awishnick/clang-tidy/llvm/tools/clang/lib/Basic/Diagnostic.cpp:68 >>>>>>>>> #13 0x00000001015eec35 in ~DiagnosticsEngine at >>>>>>>>> /Users/awishnick/clang-tidy/llvm/tools/clang/lib/Basic/Diagnostic.cpp:66 >>>>>>>>> #14 0x00000001006bd3d3 in >>>>>>>>> llvm::RefCountedBase<clang::DiagnosticsEngine>::Release() const at >>>>>>>>> /Users/awishnick/clang-tidy/llvm/include/llvm/ADT/IntrusiveRefCntPtr.h:55 >>>>>>>>> #15 0x00000001006bd325 in >>>>>>>>> llvm::IntrusiveRefCntPtrInfo<clang::DiagnosticsEngine>::release(clang::DiagnosticsEngine*) >>>>>>>>> at >>>>>>>>> /Users/awishnick/clang-tidy/llvm/include/llvm/ADT/IntrusiveRefCntPtr.h:90 >>>>>>>>> #16 0x00000001006bd2fd in >>>>>>>>> llvm::IntrusiveRefCntPtr<clang::DiagnosticsEngine>::release() at >>>>>>>>> /Users/awishnick/clang-tidy/llvm/include/llvm/ADT/IntrusiveRefCntPtr.h:199 >>>>>>>>> #17 0x00000001006bd2c5 in ~IntrusiveRefCntPtr at >>>>>>>>> /Users/awishnick/clang-tidy/llvm/include/llvm/ADT/IntrusiveRefCntPtr.h:172 >>>>>>>>> #18 0x00000001006bbe15 in ~IntrusiveRefCntPtr at >>>>>>>>> /Users/awishnick/clang-tidy/llvm/include/llvm/ADT/IntrusiveRefCntPtr.h:172 >>>>>>>>> #19 0x000000010065cbc1 in ~CompilerInstance at >>>>>>>>> /Users/awishnick/clang-tidy/llvm/tools/clang/lib/Frontend/CompilerInstance.cpp:63 >>>>>>>>> #20 0x000000010065c505 in ~CompilerInstance at >>>>>>>>> /Users/awishnick/clang-tidy/llvm/tools/clang/lib/Frontend/CompilerInstance.cpp:61 >>>>>>>>> #21 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 >>>>>>>>> #22 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 >>>>>>>>> #23 0x00000001005d5290 in clang::tooling::ToolInvocation::run() >>>>>>>>> at >>>>>>>>> /Users/awishnick/clang-tidy/llvm/tools/clang/lib/Tooling/Tooling.cpp:229 >>>>>>>>> #24 0x00000001005d7b29 in >>>>>>>>> clang::tooling::ClangTool::run(clang::tooling::ToolAction*) at >>>>>>>>> /Users/awishnick/clang-tidy/llvm/tools/clang/lib/Tooling/Tooling.cpp:360 >>>>>>>>> #25 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 >>>>>>>>> #26 0x0000000100002a96 in main at >>>>>>>>> /Users/awishnick/clang-tidy/llvm/tools/clang/tools/extra/clang-tidy/tool/ClangTidyMain.cpp:145 >>>>>>>>> >>>>>>>>> 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. >>>>>>>>> >>>>>>>> >>>>>>>> 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. >>>>>>>> >>>>>>> >>>>>>> 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) >>>>>>> >>>>>>> >>>>>>>> >>>>>>>> >>>>>>>>> >>>>>>>>> Thanks! >>>>>>>>> Aaron >>>>>>>>> >>>>>>>>> _______________________________________________ >>>> cfe-commits mailing list >>>> [email protected] >>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >>>> >>> >> >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
