search

Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

Crash in static analyzer when analyzing some code with destructors #41786

Open Quuxplusone opened 5 years ago

Quuxplusone commented 5 years ago
Bugzilla Link PR42816
Status CONFIRMED
Importance P normal
Reported by Philip Chimento (philip.chimento@gmail.com)
Reported on 2019-07-29 13:07:38 -0700
Last modified on 2019-08-09 19:49:21 -0700
Version 8.0
Hardware Macintosh MacOS X
CC dcoughlin@apple.com, llvm-bugs@lists.llvm.org, noqnoqneo@gmail.com
Fixed by commit(s)
Attachments bug.cpp (2310 bytes, text/x-csrc)
Blocks
Blocked by
See also 
Created attachment 22312
Program that reproduces the problem

I am trying to develop a static analyzer plugin and encountered a crash when
analyzing some particular code. A minimal program that reproduces the crash
(including as well a minimal version of the code being analyzed) is attached.

I am quite sure it's not a bug in my analyzer code since I can reproduce it
with a completely empty analyzer with no code at all.

In the code being analyzed, the crash seems to depend on all of these things
being present:
- A record (B) with a destructor;
- A record member (A) of B with a destructor;
- An explicit call to B's destructor in a path where it's known that the
pointer to B is non-null.

The stack trace of the crash is below:

$ lldb -- ./bug
(lldb) target create "./bug"
Current executable set to './bug' (x86_64).
(lldb) r
Process 26969 launched: '/path/to/bug' (x86_64)
Process 26969 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS
(code=1, address=0x50)
    frame #0: 0x0000000100ba4f66 bug`(anonymous namespace)::RegionStoreManager::invalidateRegions(void const*, llvm::ArrayRef<clang::ento::SVal>, clang::Expr const*, unsigned int, clang::LocationContext const*, clang::ento::CallEvent const*, llvm::DenseSet<clang::ento::SymExpr const*, llvm::DenseMapInfo<clang::ento::SymExpr const*> >&, clang::ento::RegionAndSymbolInvalidationTraits&, llvm::SmallVector<clang::ento::MemRegion const*, 8u>*, llvm::SmallVector<clang::ento::MemRegion const*, 8u>*) + 3192
bug`(anonymous namespace)::RegionStoreManager::invalidateRegions:
->  0x100ba4f66 <+3192>: callq  *0x50(%rcx)
    0x100ba4f69 <+3195>: testb  %al, %al
    0x100ba4f6b <+3197>: je     0x100ba4f94               ; <+3238>
    0x100ba4f6d <+3199>: movq   -0x1d8(%rbp), %rdi
Target 0: (bug) stopped.
(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS
(code=1, address=0x50)
  * frame #0: 0x0000000100ba4f66 bug`(anonymous namespace)::RegionStoreManager::invalidateRegions(void const*, llvm::ArrayRef<clang::ento::SVal>, clang::Expr const*, unsigned int, clang::LocationContext const*, clang::ento::CallEvent const*, llvm::DenseSet<clang::ento::SymExpr const*, llvm::DenseMapInfo<clang::ento::SymExpr const*> >&, clang::ento::RegionAndSymbolInvalidationTraits&, llvm::SmallVector<clang::ento::MemRegion const*, 8u>*, llvm::SmallVector<clang::ento::MemRegion const*, 8u>*) + 3192
    frame #1: 0x0000000100b92c74 bug`clang::ento::ProgramState::invalidateRegionsImpl(llvm::ArrayRef<clang::ento::SVal>, clang::Expr const*, unsigned int, clang::LocationContext const*, bool, llvm::DenseSet<clang::ento::SymExpr const*, llvm::DenseMapInfo<clang::ento::SymExpr const*> >*, clang::ento::RegionAndSymbolInvalidationTraits*, clang::ento::CallEvent const*) const + 258
    frame #2: 0x0000000100b92ecb bug`clang::ento::ProgramState::invalidateRegions(llvm::ArrayRef<clang::ento::SVal>, clang::Expr const*, unsigned int, clang::LocationContext const*, bool, llvm::DenseSet<clang::ento::SymExpr const*, llvm::DenseMapInfo<clang::ento::SymExpr const*> >*, clang::ento::CallEvent const*, clang::ento::RegionAndSymbolInvalidationTraits*) const + 37
    frame #3: 0x0000000100b35f97 bug`clang::ento::CallEvent::invalidateRegions(unsigned int, llvm::IntrusiveRefCntPtr<clang::ento::ProgramState const>) const + 759
    frame #4: 0x0000000100b67722 bug`clang::ento::ExprEngine::conservativeEvalCall(clang::ento::CallEvent const&, clang::ento::NodeBuilder&, clang::ento::ExplodedNode*, llvm::IntrusiveRefCntPtr<clang::ento::ProgramState const>) + 84
    frame #5: 0x0000000100b68182 bug`clang::ento::ExprEngine::defaultEvalCall(clang::ento::NodeBuilder&, clang::ento::ExplodedNode*, clang::ento::CallEvent const&, clang::ento::ExprEngine::EvalCallOptions const&) + 466
    frame #6: 0x0000000100b62b84 bug`clang::ento::ExprEngine::VisitCXXDestructor(clang::QualType, clang::ento::MemRegion const*, clang::Stmt const*, bool, clang::ento::ExplodedNode*, clang::ento::ExplodedNodeSet&, clang::ento::ExprEngine::EvalCallOptions const&) + 706
    frame #7: 0x0000000100b50350 bug`clang::ento::ExprEngine::ProcessMemberDtor(clang::CFGMemberDtor, clang::ento::ExplodedNode*, clang::ento::ExplodedNodeSet&) + 262
    frame #8: 0x0000000100b4c035 bug`clang::ento::ExprEngine::ProcessImplicitDtor(clang::CFGImplicitDtor, clang::ento::ExplodedNode*) + 237
    frame #9: 0x0000000100b4ae93 bug`clang::ento::ExprEngine::processCFGElement(clang::CFGElement, clang::ento::ExplodedNode*, unsigned int, clang::ento::NodeBuilderContext*) + 155
    frame #10: 0x0000000100b42fb3 bug`clang::ento::CoreEngine::HandleBlockEntrance(clang::BlockEntrance const&, clang::ento::ExplodedNode*) + 155
    frame #11: 0x0000000100b42b0b bug`clang::ento::CoreEngine::dispatchWorkItem(clang::ento::ExplodedNode*, clang::ProgramPoint, clang::ento::WorkListUnit const&) + 235
    frame #12: 0x0000000100b42890 bug`clang::ento::CoreEngine::ExecuteWorkList(clang::LocationContext const*, unsigned int, llvm::IntrusiveRefCntPtr<clang::ento::ProgramState const>) + 754
    frame #13: 0x0000000100bd2130 bug`clang::ento::ExprEngine::ExecuteWorkList(clang::LocationContext const*, unsigned int) + 26
    frame #14: 0x0000000100bd1ff4 bug`(anonymous namespace)::AnalysisConsumer::HandleCode(clang::Decl*, unsigned int, clang::ento::ExprEngine::InliningModes, llvm::DenseSet<clang::Decl const*, llvm::DenseMapInfo<clang::Decl const*> >*) + 1122
    frame #15: 0x0000000100bcd404 bug`(anonymous namespace)::AnalysisConsumer::HandleTranslationUnit(clang::ASTContext&) + 754
    frame #16: 0x000000010045d1a6 bug`clang::ParseAST(clang::Sema&, bool, bool) + 457
    frame #17: 0x00000001003c909d bug`clang::FrontendAction::Execute() + 71
    frame #18: 0x0000000100394e90 bug`clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) + 734
    frame #19: 0x0000000100bf3061 bug`clang::tooling::FrontendActionFactory::runInvocation(std::__1::shared_ptr<clang::CompilerInvocation>, clang::FileManager*, std::__1::shared_ptr<clang::PCHContainerOperations>, clang::DiagnosticConsumer*) + 267
    frame #20: 0x0000000100bf2de0 bug`clang::tooling::ToolInvocation::runInvocation(char const*, clang::driver::Compilation*, std::__1::shared_ptr<clang::CompilerInvocation>, std::__1::shared_ptr<clang::PCHContainerOperations>) + 186
    frame #21: 0x0000000100bf210a bug`clang::tooling::ToolInvocation::run() + 1908
    frame #22: 0x0000000100bf169e bug`clang::tooling::runToolOnCodeWithArgs(clang::FrontendAction*, llvm::Twine const&, llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem>, std::__1::vector<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::allocator<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > > > const&, llvm::Twine const&, llvm::Twine const&, std::__1::shared_ptr<clang::PCHContainerOperations>) + 443
    frame #23: 0x0000000100bf128e bug`clang::tooling::runToolOnCodeWithArgs(clang::FrontendAction*, llvm::Twine const&, std::__1::vector<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::allocator<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > > > const&, llvm::Twine const&, llvm::Twine const&, std::__1::shared_ptr<clang::PCHContainerOperations>, std::__1::vector<std::__1::pair<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, std::__1::allocator<std::__1::pair<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > > > > const&) + 744
    frame #24: 0x0000000100bf0f14 bug`clang::tooling::runToolOnCode(clang::FrontendAction*, llvm::Twine const&, llvm::Twine const&, std::__1::shared_ptr<clang::PCHContainerOperations>) + 83
    frame #25: 0x00000001000026d8 bug`main + 136
    frame #26: 0x00007fff7c07e3d5 libdyld.dylib`start + 1
Quuxplusone commented 5 years ago

Attached bug.cpp (2310 bytes, text/x-csrc): Program that reproduces the problem

Quuxplusone commented 5 years ago

I think i accidentally almost fixed this in https://reviews.llvm.org/D65349.

Quuxplusone commented 5 years ago

Also note that running path-sensitive analysis with core checkers disabled is technically unsupported. In my case (beyond the link above) the problem was caused by exposing the destructor handling code to a situation that core checkers were not supposed to prevent, but in your case it's a pattern that core checkers were supposed to prevent. I.e., in reality the destructor would not have been called on this execution path, because it's preceded by a null pointer dereference. The null pointer dereference checker would have caught this and emitted a warning before even getting to the destructor.

Quuxplusone commented 5 years ago

If I enable the core checkers as well as bug.Analyzer, by adding the following code

        {"core.CallAndMessage", true},
        {"core.DivideZero", true},
        {"core.DynamicTypePropagation", true},
        {"core.NonNullParamChecker", true},
        {"core.NonnilStringConstants", true},
        {"core.NullDereference", true},
        {"core.StackAddressEscape", true},
        {"core.UndefinedBinaryOperatorResult", true},
        {"core.VLASize", true},
        {"core.builtin.BuiltinFunctions", true},
        {"core.builtin.NoReturnFunctions", true},
        {"core.uninitialized.ArraySubscript", true},
        {"core.uninitialized.Assign", true},
        {"core.uninitialized.Branch", true},
        {"core.uninitialized.CapturedBlockVariable", true},
        {"core.uninitialized.UndefReturn", true},

then I still get a crash, but a different one:

bug(27545,0x10eefd5c0) malloc: error for object 0x7ffee3f4b850: pointer being freed was not allocated bug(27545,0x10eefd5c0) malloc: set a breakpoint in malloc_error_break to debug Abort trap: 6

Debugger stack trace:

$ lldb -- ./bug (lldb) target create "./bug" Current executable set to './bug' (x86_64). (lldb) b malloc_error_break Breakpoint 1: where = libsystem_malloc.dylib`malloc_error_break, address = 0x0000000000017a67 (lldb) r Process 27565 launched: '/Users/ptomato/Documents/Coding/floatmage/bug' (x86_64) bug(27565,0x10154e5c0) malloc: error for object 0x7ffeefbff6f0: pointer being freed was not allocated bug(27565,0x10154e5c0) malloc: set a breakpoint in malloc_error_break to debug Process 27565 stopped

As an aside, I'd rather not enable the null pointer checker on my real code, because an external library that I use unfortunately defines an assert macro that crashes by doing something like *((volatile int*)NULL) = __LINE__ which makes the null pointer checker go bananas on every assertion in that library's inline functions. It's something I can work around eventually, but it would be nice to not enable it, or failing that somehow tell the null pointer checker to ignore a certain macro.

Quuxplusone commented 5 years ago

Yup, i've seen this use-after-free as well, but it doesn't look like it's a Static Analyzer bug; it's happening when you're destroying your action variable. If you allocate it on the heap and leak the problem doesn't happen (but you get a leak instead). I guess somebody does something wonky with the unique pointer.

Unfortunately there's currently no way to disable core checker warnings but keep doing their modeling work. It'll take some time to implement and unfortunately there aren't any easy hacks around it. You'll run into a lot more crashes due to disabling uninitialized variable checkers because a lot of other checkers aren't prepared to dealing with undefined values propagating their way :(

Quuxplusone commented 5 years ago 
I see. Well, anyway, I worked around the problem by making an analyzer-friendly
shim for the offending header file and inserting it with
tooling::ClangTool::mapVirtualFile and
tooling::ClangTool::appendArgumentsAdjuster. In any case the crash is gone when
including all the core.* analyzers, thanks for the help.

I guess I should leave this open because there does seem to be an actual bug to
fix here?
Quuxplusone commented 5 years ago

Unfortunately there's currently no way to disable core checker warnings but keep doing their modeling work.

Hmm, actually, https://reviews.llvm.org/D66042.