[PATCH] Fix clang-tidy delete of stack object

David Blaikie dblaikie at gmail.com
Fri Nov 14 10:40:09 PST 2014 

On Fri, Nov 14, 2014 at 10:32 AM, Alexander Kornienko <alexfh at google.com>
wrote:

>
>
> On Thu, Nov 13, 2014 at 6:39 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>>
>>
>> On Thu, Nov 13, 2014 at 1:20 AM, Manuel Klimek <klimek at google.com> wrote:
>>
>>> +dblakie
>>>
>>> On Thu Nov 13 2014 at 1:53:39 AM Alexander Kornienko <alexfh at google.com>
>>> 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 <
>>>> aaron.s.wishnick at gmail.com> 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 <
>>>>> aaron.s.wishnick at gmail.com> 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 <
>>>>>> alexfh at google.com> wrote:
>>>>>>
>>>>>>>
>>>>>>> On Mon, Jun 23, 2014 at 2:51 PM, Alexander Kornienko <
>>>>>>> alexfh at google.com> wrote:
>>>>>>>
>>>>>>>>
>>>>>>>> On Thu, Jun 19, 2014 at 5:59 PM, Aaron Wishnick <
>>>>>>>> aaron.s.wishnick at gmail.com> 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
>>>>>>>>> cfe-commits at cs.uiuc.edu
>>>>>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>>>>>>
>>>>>>>>
>>>>>>
>>>>>> On Mon, Jun 23, 2014 at 9:51 AM, Alexander Kornienko <
>>>>>> alexfh at google.com> wrote:
>>>>>>
>>>>>>>
>>>>>>> On Mon, Jun 23, 2014 at 2:51 PM, Alexander Kornienko <
>>>>>>> alexfh at google.com> wrote:
>>>>>>>
>>>>>>>>
>>>>>>>> On Thu, Jun 19, 2014 at 5:59 PM, Aaron Wishnick <
>>>>>>>> aaron.s.wishnick at gmail.com> 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
>>>> cfe-commits at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141114/5aa84cf6/attachment.html>

More information about the cfe-commits mailing list