NoQ created this revision. NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet. Herald added subscribers: cfe-commits, rnkovacs.
Default global `operator new()`, like `malloc()`, should return heap pointers, which in the analyzer are represented by `SymbolicRegion`s with `HeapSpaceRegion` as their parent. In the `-analyzer-config c++-allocator-inlining` mode, this was broken, and regular `SymbolicRegion`s were returned instead, which have `UnknownSpaceRegion` as their parent. This patch fixes this straightforwardly on `ExprEngine` side. We may want to delegate this job to the checkers though `evalCall`, but for now this mode doesn't support `evalCall` for `operator new()`, and i'm not sure if it'd be used much. With this patch going on top of previous patches, enabling `c++-allocator-inlining` by default causes no regressions on tests (causes some improvements though). It doesn't mean it works (we still have callbacks broken, path diagnostic pieces unsupported, and i've just noticed one more void element region crash), just a psychological checkpoint. Repository: rC Clang https://reviews.llvm.org/D41266 Files: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h lib/StaticAnalyzer/Core/ExprEngineCXX.cpp lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp test/Analysis/NewDelete-checker-test.cpp
Index: test/Analysis/NewDelete-checker-test.cpp =================================================================== --- test/Analysis/NewDelete-checker-test.cpp +++ test/Analysis/NewDelete-checker-test.cpp @@ -1,5 +1,7 @@ // RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDelete -std=c++11 -fblocks -verify %s // RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDeleteLeaks -DLEAKS -std=c++11 -fblocks -verify %s +// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDelete -std=c++11 -fblocks -analyzer-config c++-allocator-inlining=true -verify %s +// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDeleteLeaks -DLEAKS -std=c++11 -fblocks -analyzer-config c++-allocator-inlining=true -verify %s #include "Inputs/system-header-simulator-cxx.h" typedef __typeof__(sizeof(int)) size_t; Index: lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp =================================================================== --- lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -561,7 +561,19 @@ QualType ResultTy = Call.getResultType(); SValBuilder &SVB = getSValBuilder(); unsigned Count = currBldrCtx->blockCount(); - SVal R = SVB.conjureSymbolVal(nullptr, E, LCtx, ResultTy, Count); + + // See if we need to conjure a heap pointer instead of + // a regular unknown pointer. + bool IsHeapPointer = false; + if (const auto *CNE = dyn_cast<CXXNewExpr>(E)) + if (isStandardGlobalOperatorNewFunction(CNE)) { + // FIXME: Delegate this to evalCall in MallocChecker? + IsHeapPointer = true; + } + + SVal R = IsHeapPointer + ? SVB.getConjuredHeapSymbolVal(E, LCtx, Count) + : SVB.conjureSymbolVal(nullptr, E, LCtx, ResultTy, Count); return State->BindExpr(E, LCtx, R); } Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp =================================================================== --- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -458,8 +458,10 @@ ExplodedNodeSet DstPostCall; StmtNodeBuilder CallBldr(DstPreCall, DstPostCall, *currBldrCtx); - for (auto I: DstPreCall) + for (auto I: DstPreCall) { + // FIXME: Provide evalCall for checkers? defaultEvalCall(CallBldr, I, *Call); + } // Store return value of operator new() for future use, until the actual // CXXNewExpr gets processed. @@ -475,6 +477,22 @@ *Call, *this); } +bool ExprEngine::isStandardGlobalOperatorNewFunction(const CXXNewExpr *CNE) { + const FunctionDecl *FD = CNE->getOperatorNew(); + if (FD && !isa<CXXMethodDecl>(FD) && !FD->isVariadic()) { + if (FD->getNumParams() == 2) { + QualType T = FD->getParamDecl(1)->getType(); + if (const IdentifierInfo *II = T.getBaseTypeIdentifier()) { + // NoThrow placement new behaves as a standard new. + return II->getName().equals("nothrow_t"); + } + } else { + // Placement forms are considered non-standard. + return (FD->getNumParams() == 1); + } + } + return false; +} void ExprEngine::VisitCXXNewExpr(const CXXNewExpr *CNE, ExplodedNode *Pred, ExplodedNodeSet &Dst) { @@ -488,18 +506,7 @@ SVal symVal = UnknownVal(); FunctionDecl *FD = CNE->getOperatorNew(); - bool IsStandardGlobalOpNewFunction = false; - if (FD && !isa<CXXMethodDecl>(FD) && !FD->isVariadic()) { - if (FD->getNumParams() == 2) { - QualType T = FD->getParamDecl(1)->getType(); - if (const IdentifierInfo *II = T.getBaseTypeIdentifier()) - // NoThrow placement new behaves as a standard new. - IsStandardGlobalOpNewFunction = II->getName().equals("nothrow_t"); - } - else - // Placement forms are considered non-standard. - IsStandardGlobalOpNewFunction = (FD->getNumParams() == 1); - } + bool IsStandardGlobalOpNewFunction = isStandardGlobalOperatorNewFunction(CNE); ProgramStateRef State = Pred->getState(); Index: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h =================================================================== --- include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h +++ include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h @@ -605,6 +605,8 @@ bool inlineCall(const CallEvent &Call, const Decl *D, NodeBuilder &Bldr, ExplodedNode *Pred, ProgramStateRef State); + static bool isStandardGlobalOperatorNewFunction(const CXXNewExpr *CNE); + /// \brief Conservatively evaluate call by invalidating regions and binding /// a conjured return value. void conservativeEvalCall(const CallEvent &Call, NodeBuilder &Bldr,
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits