From: Artem Dergachev Date: Thu, 18 Jan 2018 00:53:50 +0000 (+0000) Subject: [analyzer] operator new: Fix callback order for CXXNewExpr. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=fdba1f2ef691d682d045df8793c65fea9b4f8b97;p=clang [analyzer] operator new: Fix callback order for CXXNewExpr. PreStmt was never called. Additionally, under c++-allocator-inlining=true, PostStmt was called twice when the allocator was inlined: once after evaluating the new-expression itself, once after evaluating the allocator call which, for the lack of better options, uses the new-expression as the call site. This patch fixes both problems. Differential Revision: https://reviews.llvm.org/D41934 rdar://problem/12180598 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@322797 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp b/lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp index 90d5c0e36a..e2a35c0426 100644 --- a/lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp @@ -15,8 +15,10 @@ //===----------------------------------------------------------------------===// #include "ClangSACheckers.h" +#include "clang/AST/ExprCXX.h" #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/CheckerManager.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" using namespace clang; @@ -29,6 +31,11 @@ class AnalysisOrderChecker check::PostStmt, check::PreStmt, check::PostStmt, + check::PreStmt, + check::PostStmt, + check::PreCall, + check::PostCall, + check::NewAllocator, check::Bind, check::RegionChanges> { bool isCallbackEnabled(AnalyzerOptions &Opts, StringRef CallbackName) const { @@ -72,6 +79,40 @@ public: llvm::errs() << "PostStmt\n"; } + void checkPreStmt(const CXXNewExpr *NE, CheckerContext &C) const { + if (isCallbackEnabled(C, "PreStmtCXXNewExpr")) + llvm::errs() << "PreStmt\n"; + } + + void checkPostStmt(const CXXNewExpr *NE, CheckerContext &C) const { + if (isCallbackEnabled(C, "PostStmtCXXNewExpr")) + llvm::errs() << "PostStmt\n"; + } + + void checkPreCall(const CallEvent &Call, CheckerContext &C) const { + if (isCallbackEnabled(C, "PreCall")) { + llvm::errs() << "PreCall"; + if (const NamedDecl *ND = dyn_cast_or_null(Call.getDecl())) + llvm::errs() << " (" << ND->getQualifiedNameAsString() << ')'; + llvm::errs() << '\n'; + } + } + + void checkPostCall(const CallEvent &Call, CheckerContext &C) const { + if (isCallbackEnabled(C, "PostCall")) { + llvm::errs() << "PostCall"; + if (const NamedDecl *ND = dyn_cast_or_null(Call.getDecl())) + llvm::errs() << " (" << ND->getQualifiedNameAsString() << ')'; + llvm::errs() << '\n'; + } + } + + void checkNewAllocator(const CXXNewExpr *CNE, SVal Target, + CheckerContext &C) const { + if (isCallbackEnabled(C, "NewAllocator")) + llvm::errs() << "NewAllocator\n"; + } + void checkBind(SVal Loc, SVal Val, const Stmt *S, CheckerContext &C) const { if (isCallbackEnabled(C, "Bind")) llvm::errs() << "Bind\n"; diff --git a/lib/StaticAnalyzer/Core/ExprEngine.cpp b/lib/StaticAnalyzer/Core/ExprEngine.cpp index a01f362de3..4a179c5cc4 100644 --- a/lib/StaticAnalyzer/Core/ExprEngine.cpp +++ b/lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -1324,8 +1324,16 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred, case Stmt::CXXNewExprClass: { Bldr.takeNodes(Pred); + + ExplodedNodeSet PreVisit; + getCheckerManager().runCheckersForPreStmt(PreVisit, Pred, S, *this); + ExplodedNodeSet PostVisit; - VisitCXXNewExpr(cast(S), Pred, PostVisit); + for (ExplodedNodeSet::iterator i = PreVisit.begin(), + e = PreVisit.end(); i != e ; ++i) { + VisitCXXNewExpr(cast(S), *i, PostVisit); + } + getCheckerManager().runCheckersForPostStmt(Dst, PostVisit, S, *this); Bldr.addNodes(Dst); break; diff --git a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp index 3526e48174..3c6949786e 100644 --- a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -372,7 +372,9 @@ void ExprEngine::processCallExit(ExplodedNode *CEBNode) { getCheckerManager().runCheckersForPostObjCMessage(Dst, DstPostCall, *Msg, *this, /*WasInlined=*/true); - } else if (CE) { + } else if (CE && + !(isa(CE) && // Called when visiting CXXNewExpr. + AMgr.getAnalyzerOptions().mayInlineCXXAllocator())) { getCheckerManager().runCheckersForPostStmt(Dst, DstPostCall, CE, *this, /*WasInlined=*/true); } else { diff --git a/test/Analysis/cxxnewexpr-callback-inline.cpp b/test/Analysis/cxxnewexpr-callback-inline.cpp new file mode 100644 index 0000000000..c823de8582 --- /dev/null +++ b/test/Analysis/cxxnewexpr-callback-inline.cpp @@ -0,0 +1,32 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=debug.AnalysisOrder -analyzer-config c++-allocator-inlining=true,debug.AnalysisOrder:PreStmtCXXNewExpr=true,debug.AnalysisOrder:PostStmtCXXNewExpr=true,debug.AnalysisOrder:PreCall=true,debug.AnalysisOrder:PostCall=true,debug.AnalysisOrder:NewAllocator=true %s 2>&1 | FileCheck %s + +#include "Inputs/system-header-simulator-cxx.h" + +namespace std { + void *malloc(size_t); +} + +void *operator new(size_t size) { return std::malloc(size); } + +struct S { + S() {} +}; + +void foo(); + +void test() { + S *s = new S(); + foo(); +} + +// CHECK: PreCall (operator new) +// CHECK-NEXT: PreCall (std::malloc) +// CHECK-NEXT: PostCall (std::malloc) +// CHECK-NEXT: PostCall (operator new) +// CHECK-NEXT: NewAllocator +// CHECK-NEXT: PreCall (S::S) +// CHECK-NEXT: PostCall (S::S) +// CHECK-NEXT: PreStmt +// CHECK-NEXT: PostStmt +// CHECK-NEXT: PreCall (foo) +// CHECK-NEXT: PostCall (foo) diff --git a/test/Analysis/cxxnewexpr-callback-noinline.cpp b/test/Analysis/cxxnewexpr-callback-noinline.cpp new file mode 100644 index 0000000000..87edeac3fd --- /dev/null +++ b/test/Analysis/cxxnewexpr-callback-noinline.cpp @@ -0,0 +1,29 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=debug.AnalysisOrder -analyzer-config c++-allocator-inlining=false,debug.AnalysisOrder:PreStmtCXXNewExpr=true,debug.AnalysisOrder:PostStmtCXXNewExpr=true,debug.AnalysisOrder:PreCall=true,debug.AnalysisOrder:PostCall=true,debug.AnalysisOrder:NewAllocator=true %s 2>&1 | FileCheck %s + +#include "Inputs/system-header-simulator-cxx.h" + +namespace std { + void *malloc(size_t); +} + +void *operator new(size_t size) { return std::malloc(size); } + +struct S { + S() {} +}; + +void foo(); + +void test() { + S *s = new S(); + foo(); +} + +// CHECK: PreCall (S::S) +// CHECK-NEXT: PostCall (S::S) +// CHECK-NEXT: PreStmt +// CHECK-NEXT: PostStmt +// CHECK-NEXT: PreCall (foo) +// CHECK-NEXT: PostCall (foo) +// CHECK-NEXT: PreCall (std::malloc) +// CHECK-NEXT: PostCall (std::malloc)