diff --git a/clang/include/clang/Analysis/AnalysisDeclContext.h b/clang/include/clang/Analysis/AnalysisDeclContext.h --- a/clang/include/clang/Analysis/AnalysisDeclContext.h +++ b/clang/include/clang/Analysis/AnalysisDeclContext.h @@ -324,6 +324,8 @@ unsigned getIndex() const { return Index; } + CFGElement getCallSiteCFGElement() const { return (*Block)[Index]; } + void Profile(llvm::FoldingSetNodeID &ID) override; static void Profile(llvm::FoldingSetNodeID &ID, AnalysisDeclContext *ADC, diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h --- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h +++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h @@ -434,6 +434,15 @@ return CallArgumentIndex; } + /// Returns the construction context of the call, if it is a C++ constructor + /// call or a call of a function returning a C++ class instance. Otherwise + /// return nullptr. + const ConstructionContext *getConstructionContext() const; + + /// If the call returns a C++ record type then the region of its return value + /// can be retrieved from its construction context. + Optional getReturnValueUnderConstruction() const; + // Iterator access to formal parameters and their types. private: struct GetTypeFn { diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h --- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h +++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h @@ -126,6 +126,13 @@ /// for example 'A { const C &c; }; A a = { C() };' bool IsTemporaryLifetimeExtendedViaAggregate = false; + /// This call is a pre-C++17 elidable constructor that we failed to elide + /// because we failed to compute the target region into which + /// this constructor would have been ultimately elided. Analysis that + /// we perform in this case is still correct but it behaves differently, + /// as if copy elision is disabled. + bool IsElidableCtorThatHasNotBeenElided = false; + EvalCallOptions() {} }; @@ -709,6 +716,35 @@ const CallEvent &Call, const EvalCallOptions &CallOpts = {}); + /// Find location of the object that is being constructed by a given + /// constructor. This should ideally always succeed but due to not being + /// fully implemented it sometimes indicates that it failed via its + /// out-parameter CallOpts; in such cases a fake temporary region is + /// returned, which is better than nothing but does not represent + /// the actual behavior of the program. + SVal computeObjectUnderConstruction( + const Expr *E, ProgramStateRef State, const LocationContext *LCtx, + const ConstructionContext *CC, EvalCallOptions &CallOpts); + + /// Update the program state with all the path-sensitive information + /// that's necessary to perform construction of an object with a given + /// syntactic construction context. V and CallOpts have to be obtained from + /// computeObjectUnderConstruction() invoked with the same set of + /// the remaining arguments (E, State, LCtx, CC). + ProgramStateRef updateObjectsUnderConstruction( + SVal V, const Expr *E, ProgramStateRef State, const LocationContext *LCtx, + const ConstructionContext *CC, const EvalCallOptions &CallOpts); + + /// A convenient wrapper around computeObjectUnderConstruction + /// and updateObjectsUnderConstruction. + std::pair handleConstructionContext( + const Expr *E, ProgramStateRef State, const LocationContext *LCtx, + const ConstructionContext *CC, EvalCallOptions &CallOpts) { + SVal V = computeObjectUnderConstruction(E, State, LCtx, CC, CallOpts); + return std::make_pair( + updateObjectsUnderConstruction(V, E, State, LCtx, CC, CallOpts), V); + } + private: ProgramStateRef finishArgumentConstruction(ProgramStateRef State, const CallEvent &Call); @@ -827,16 +863,6 @@ /// constructing into an existing region. const CXXConstructExpr *findDirectConstructorForCurrentCFGElement(); - /// Update the program state with all the path-sensitive information - /// that's necessary to perform construction of an object with a given - /// syntactic construction context. If the construction context is unavailable - /// or unusable for any reason, a dummy temporary region is returned, and the - /// IsConstructorWithImproperlyModeledTargetRegion flag is set in \p CallOpts. - /// Returns the updated program state and the new object's this-region. - std::pair handleConstructionContext( - const Expr *E, ProgramStateRef State, const LocationContext *LCtx, - const ConstructionContext *CC, EvalCallOptions &CallOpts); - /// Common code that handles either a CXXConstructExpr or a /// CXXInheritedCtorInitExpr. void handleConstructor(const Expr *E, ExplodedNode *Pred, diff --git a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp --- a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp +++ b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp @@ -535,6 +535,37 @@ // FIXME: Variadic arguments are not handled at all right now. } +const ConstructionContext *CallEvent::getConstructionContext() const { + const StackFrameContext *StackFrame = getCalleeStackFrame(0); + if (!StackFrame) + return nullptr; + + const CFGElement Element = StackFrame->getCallSiteCFGElement(); + if (const auto Ctor = Element.getAs()) { + return Ctor->getConstructionContext(); + } + + if (const auto RecCall = Element.getAs()) { + return RecCall->getConstructionContext(); + } + + return nullptr; +} + +Optional +CallEvent::getReturnValueUnderConstruction() const { + const auto *CC = getConstructionContext(); + if (!CC) + return None; + + ExprEngine::EvalCallOptions CallOpts; + ExprEngine &Engine = getState()->getStateManager().getOwningEngine(); + SVal RetVal = + Engine.computeObjectUnderConstruction(getOriginExpr(), getState(), + getLocationContext(), CC, CallOpts); + return RetVal; +} + ArrayRef AnyFunctionCall::parameters() const { const FunctionDecl *D = getDecl(); if (!D) diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -109,15 +109,14 @@ return LValue; } -std::pair ExprEngine::handleConstructionContext( +SVal ExprEngine::computeObjectUnderConstruction( const Expr *E, ProgramStateRef State, const LocationContext *LCtx, const ConstructionContext *CC, EvalCallOptions &CallOpts) { SValBuilder &SVB = getSValBuilder(); MemRegionManager &MRMgr = SVB.getRegionManager(); ASTContext &ACtx = SVB.getContext(); - // See if we're constructing an existing region by looking at the - // current construction context. + // Compute the target region by exploring the construction context. if (CC) { switch (CC->getKind()) { case ConstructionContext::CXX17ElidedCopyVariableKind: @@ -125,13 +124,9 @@ const auto *DSCC = cast(CC); const auto *DS = DSCC->getDeclStmt(); const auto *Var = cast(DS->getSingleDecl()); - SVal LValue = State->getLValue(Var, LCtx); QualType Ty = Var->getType(); - LValue = - makeZeroElementRegion(State, LValue, Ty, CallOpts.IsArrayCtorOrDtor); - State = - addObjectUnderConstruction(State, DSCC->getDeclStmt(), LCtx, LValue); - return std::make_pair(State, LValue); + return makeZeroElementRegion(State, State->getLValue(Var, LCtx), Ty, + CallOpts.IsArrayCtorOrDtor); } case ConstructionContext::CXX17ElidedCopyConstructorInitializerKind: case ConstructionContext::SimpleConstructorInitializerKind: { @@ -139,8 +134,7 @@ const auto *Init = ICC->getCXXCtorInitializer(); assert(Init->isAnyMemberInitializer()); const CXXMethodDecl *CurCtor = cast(LCtx->getDecl()); - Loc ThisPtr = - SVB.getCXXThis(CurCtor, LCtx->getStackFrame()); + Loc ThisPtr = SVB.getCXXThis(CurCtor, LCtx->getStackFrame()); SVal ThisVal = State->getSVal(ThisPtr); const ValueDecl *Field; @@ -154,10 +148,8 @@ } QualType Ty = Field->getType(); - FieldVal = makeZeroElementRegion(State, FieldVal, Ty, - CallOpts.IsArrayCtorOrDtor); - State = addObjectUnderConstruction(State, Init, LCtx, FieldVal); - return std::make_pair(State, FieldVal); + return makeZeroElementRegion(State, FieldVal, Ty, + CallOpts.IsArrayCtorOrDtor); } case ConstructionContext::NewAllocatedObjectKind: { if (AMgr.getAnalyzerOptions().MayInlineCXXAllocator) { @@ -170,11 +162,10 @@ // TODO: In fact, we need to call the constructor for every // allocated element, not just the first one! CallOpts.IsArrayCtorOrDtor = true; - return std::make_pair( - State, loc::MemRegionVal(getStoreManager().GetElementZeroRegion( - MR, NE->getType()->getPointeeType()))); + return loc::MemRegionVal(getStoreManager().GetElementZeroRegion( + MR, NE->getType()->getPointeeType())); } - return std::make_pair(State, V); + return V; } // TODO: Detect when the allocator returns a null pointer. // Constructor shall not be called in this case. @@ -202,7 +193,7 @@ CallerLCtx = CallerLCtx->getParent(); assert(!isa(CallerLCtx)); } - return handleConstructionContext( + return computeObjectUnderConstruction( cast(SFC->getCallSite()), State, CallerLCtx, RTC->getConstructionContext(), CallOpts); } else { @@ -223,64 +214,46 @@ assert(RetE && "Void returns should not have a construction context"); QualType ReturnTy = RetE->getType(); QualType RegionTy = ACtx.getPointerType(ReturnTy); - SVal V = SVB.conjureSymbolVal(&TopLevelSymRegionTag, RetE, SFC, - RegionTy, currBldrCtx->blockCount()); - return std::make_pair(State, V); + return SVB.conjureSymbolVal(&TopLevelSymRegionTag, RetE, SFC, RegionTy, + currBldrCtx->blockCount()); } llvm_unreachable("Unhandled return value construction context!"); } case ConstructionContext::ElidedTemporaryObjectKind: { assert(AMgr.getAnalyzerOptions().ShouldElideConstructors); const auto *TCC = cast(CC); - const CXXBindTemporaryExpr *BTE = TCC->getCXXBindTemporaryExpr(); - const MaterializeTemporaryExpr *MTE = TCC->getMaterializedTemporaryExpr(); - const CXXConstructExpr *CE = TCC->getConstructorAfterElision(); // Support pre-C++17 copy elision. We'll have the elidable copy // constructor in the AST and in the CFG, but we'll skip it // and construct directly into the final object. This call // also sets the CallOpts flags for us. - SVal V; // If the elided copy/move constructor is not supported, there's still // benefit in trying to model the non-elided constructor. // Stash our state before trying to elide, as it'll get overwritten. ProgramStateRef PreElideState = State; EvalCallOptions PreElideCallOpts = CallOpts; - std::tie(State, V) = handleConstructionContext( - CE, State, LCtx, TCC->getConstructionContextAfterElision(), CallOpts); + SVal V = computeObjectUnderConstruction( + TCC->getConstructorAfterElision(), State, LCtx, + TCC->getConstructionContextAfterElision(), CallOpts); // FIXME: This definition of "copy elision has not failed" is unreliable. // It doesn't indicate that the constructor will actually be inlined - // later; it is still up to evalCall() to decide. - if (!CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion) { - // Remember that we've elided the constructor. - State = addObjectUnderConstruction(State, CE, LCtx, V); - - // Remember that we've elided the destructor. - if (BTE) - State = elideDestructor(State, BTE, LCtx); - - // Instead of materialization, shamelessly return - // the final object destination. - if (MTE) - State = addObjectUnderConstruction(State, MTE, LCtx, V); - - return std::make_pair(State, V); - } else { - // Copy elision failed. Revert the changes and proceed as if we have - // a simple temporary. - State = PreElideState; - CallOpts = PreElideCallOpts; - } + // later; this is still up to evalCall() to decide. + if (!CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion) + return V; + + // Copy elision failed. Revert the changes and proceed as if we have + // a simple temporary. + CallOpts = PreElideCallOpts; + CallOpts.IsElidableCtorThatHasNotBeenElided = true; LLVM_FALLTHROUGH; } case ConstructionContext::SimpleTemporaryObjectKind: { const auto *TCC = cast(CC); - const CXXBindTemporaryExpr *BTE = TCC->getCXXBindTemporaryExpr(); const MaterializeTemporaryExpr *MTE = TCC->getMaterializedTemporaryExpr(); - SVal V = UnknownVal(); + CallOpts.IsTemporaryCtorOrDtor = true; if (MTE) { if (const ValueDecl *VD = MTE->getExtendingDecl()) { assert(MTE->getStorageDuration() != SD_FullExpression); @@ -296,20 +269,10 @@ if (MTE->getStorageDuration() == SD_Static || MTE->getStorageDuration() == SD_Thread) - V = loc::MemRegionVal(MRMgr.getCXXStaticTempObjectRegion(E)); + return loc::MemRegionVal(MRMgr.getCXXStaticTempObjectRegion(E)); } - if (V.isUnknown()) - V = loc::MemRegionVal(MRMgr.getCXXTempObjectRegion(E, LCtx)); - - if (BTE) - State = addObjectUnderConstruction(State, BTE, LCtx, V); - - if (MTE) - State = addObjectUnderConstruction(State, MTE, LCtx, V); - - CallOpts.IsTemporaryCtorOrDtor = true; - return std::make_pair(State, V); + return loc::MemRegionVal(MRMgr.getCXXTempObjectRegion(E, LCtx)); } case ConstructionContext::ArgumentKind: { // Arguments are technically temporaries. @@ -318,10 +281,8 @@ const auto *ACC = cast(CC); const Expr *E = ACC->getCallLikeExpr(); unsigned Idx = ACC->getIndex(); - const CXXBindTemporaryExpr *BTE = ACC->getCXXBindTemporaryExpr(); CallEventManager &CEMgr = getStateManager().getCallEventManager(); - SVal V = UnknownVal(); auto getArgLoc = [&](CallEventRef<> Caller) -> Optional { const LocationContext *FutureSFC = Caller->getCalleeStackFrame(currBldrCtx->blockCount()); @@ -352,44 +313,133 @@ if (const auto *CE = dyn_cast(E)) { CallEventRef<> Caller = CEMgr.getSimpleCall(CE, State, LCtx); - if (auto OptV = getArgLoc(Caller)) - V = *OptV; + if (Optional V = getArgLoc(Caller)) + return *V; else break; - State = addObjectUnderConstruction(State, {CE, Idx}, LCtx, V); } else if (const auto *CCE = dyn_cast(E)) { // Don't bother figuring out the target region for the future // constructor because we won't need it. CallEventRef<> Caller = CEMgr.getCXXConstructorCall(CCE, /*Target=*/nullptr, State, LCtx); - if (auto OptV = getArgLoc(Caller)) - V = *OptV; + if (Optional V = getArgLoc(Caller)) + return *V; else break; - State = addObjectUnderConstruction(State, {CCE, Idx}, LCtx, V); } else if (const auto *ME = dyn_cast(E)) { CallEventRef<> Caller = CEMgr.getObjCMethodCall(ME, State, LCtx); - if (auto OptV = getArgLoc(Caller)) - V = *OptV; + if (Optional V = getArgLoc(Caller)) + return *V; else break; - State = addObjectUnderConstruction(State, {ME, Idx}, LCtx, V); } + } + } // switch (CC->getKind()) + } + + // If we couldn't find an existing region to construct into, assume we're + // constructing a temporary. Notify the caller of our failure. + CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion = true; + return loc::MemRegionVal(MRMgr.getCXXTempObjectRegion(E, LCtx)); +} + +ProgramStateRef ExprEngine::updateObjectsUnderConstruction( + SVal V, const Expr *E, ProgramStateRef State, const LocationContext *LCtx, + const ConstructionContext *CC, const EvalCallOptions &CallOpts) { + if (CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion) { + // Sounds like we failed to find the target region and therefore + // copy elision failed. There's nothing we can do about it here. + return State; + } + + // See if we're constructing an existing region by looking at the + // current construction context. + assert(CC && "Computed target region without construction context?"); + switch (CC->getKind()) { + case ConstructionContext::CXX17ElidedCopyVariableKind: + case ConstructionContext::SimpleVariableKind: { + const auto *DSCC = cast(CC); + return addObjectUnderConstruction(State, DSCC->getDeclStmt(), LCtx, V); + } + case ConstructionContext::CXX17ElidedCopyConstructorInitializerKind: + case ConstructionContext::SimpleConstructorInitializerKind: { + const auto *ICC = cast(CC); + return addObjectUnderConstruction(State, ICC->getCXXCtorInitializer(), + LCtx, V); + } + case ConstructionContext::NewAllocatedObjectKind: { + return State; + } + case ConstructionContext::SimpleReturnedValueKind: + case ConstructionContext::CXX17ElidedCopyReturnedValueKind: { + const StackFrameContext *SFC = LCtx->getStackFrame(); + const LocationContext *CallerLCtx = SFC->getParent(); + if (!CallerLCtx) { + // No extra work is necessary in top frame. + return State; + } + + auto RTC = (*SFC->getCallSiteBlock())[SFC->getIndex()] + .getAs(); + assert(RTC && "Could not have had a target region without it"); + if (isa(CallerLCtx)) { + // Unwrap block invocation contexts. They're mostly part of + // the current stack frame. + CallerLCtx = CallerLCtx->getParent(); + assert(!isa(CallerLCtx)); + } + + return updateObjectsUnderConstruction(V, + cast(SFC->getCallSite()), State, CallerLCtx, + RTC->getConstructionContext(), CallOpts); + } + case ConstructionContext::ElidedTemporaryObjectKind: { + assert(AMgr.getAnalyzerOptions().ShouldElideConstructors); + if (!CallOpts.IsElidableCtorThatHasNotBeenElided) { + const auto *TCC = cast(CC); + State = updateObjectsUnderConstruction( + V, TCC->getConstructorAfterElision(), State, LCtx, + TCC->getConstructionContextAfterElision(), CallOpts); + + // Remember that we've elided the constructor. + State = addObjectUnderConstruction( + State, TCC->getConstructorAfterElision(), LCtx, V); + + // Remember that we've elided the destructor. + if (const auto *BTE = TCC->getCXXBindTemporaryExpr()) + State = elideDestructor(State, BTE, LCtx); - assert(!V.isUnknown()); + // Instead of materialization, shamelessly return + // the final object destination. + if (const auto *MTE = TCC->getMaterializedTemporaryExpr()) + State = addObjectUnderConstruction(State, MTE, LCtx, V); - if (BTE) + return State; + } + // If we decided not to elide the constructor, proceed as if + // it's a simple temporary. + LLVM_FALLTHROUGH; + } + case ConstructionContext::SimpleTemporaryObjectKind: { + const auto *TCC = cast(CC); + if (const auto *BTE = TCC->getCXXBindTemporaryExpr()) State = addObjectUnderConstruction(State, BTE, LCtx, V); - return std::make_pair(State, V); + if (const auto *MTE = TCC->getMaterializedTemporaryExpr()) + State = addObjectUnderConstruction(State, MTE, LCtx, V); + + return State; } + case ConstructionContext::ArgumentKind: { + const auto *ACC = cast(CC); + if (const auto *BTE = ACC->getCXXBindTemporaryExpr()) + State = addObjectUnderConstruction(State, BTE, LCtx, V); + + return addObjectUnderConstruction( + State, {ACC->getCallLikeExpr(), ACC->getIndex()}, LCtx, V); } } - // If we couldn't find an existing region to construct into, assume we're - // constructing a temporary. Notify the caller of our failure. - CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion = true; - return std::make_pair( - State, loc::MemRegionVal(MRMgr.getCXXTempObjectRegion(E, LCtx))); + llvm_unreachable("Unhandled construction context!"); } void ExprEngine::handleConstructor(const Expr *E, diff --git a/clang/unittests/StaticAnalyzer/CMakeLists.txt b/clang/unittests/StaticAnalyzer/CMakeLists.txt --- a/clang/unittests/StaticAnalyzer/CMakeLists.txt +++ b/clang/unittests/StaticAnalyzer/CMakeLists.txt @@ -7,10 +7,11 @@ AnalyzerOptionsTest.cpp CallDescriptionTest.cpp CallEventTest.cpp - StoreTest.cpp + RangeSetTest.cpp RegisterCustomCheckersTest.cpp + StoreTest.cpp SymbolReaperTest.cpp - RangeSetTest.cpp + TestReturnValueUnderConstruction.cpp ) clang_target_link_libraries(StaticAnalysisTests diff --git a/clang/unittests/StaticAnalyzer/TestReturnValueUnderConstruction.cpp b/clang/unittests/StaticAnalyzer/TestReturnValueUnderConstruction.cpp new file mode 100644 --- /dev/null +++ b/clang/unittests/StaticAnalyzer/TestReturnValueUnderConstruction.cpp @@ -0,0 +1,75 @@ +//===- unittests/StaticAnalyzer/TestReturnValueUnderConstruction.cpp ------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "CheckerRegistration.h" +#include "clang/StaticAnalyzer/Core/Checker.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" +#include "clang/StaticAnalyzer/Frontend/AnalysisConsumer.h" +#include "clang/StaticAnalyzer/Frontend/CheckerRegistry.h" +#include "clang/Tooling/Tooling.h" +#include "gtest/gtest.h" + +namespace clang { +namespace ento { +namespace { + +class TestReturnValueUnderConstructionChecker + : public Checker { +public: + void checkPostCall(const CallEvent &Call, CheckerContext &C) const { + // We are checking the invocation of `returnC` which returns an object + // by value. + const IdentifierInfo *ID = Call.getCalleeIdentifier(); + if (ID->getName() != "returnC") + return; + + // Since `returnC` returns an object by value, the invocation results + // in an object of type `C` constructed into variable `c`. Thus the + // return value of `CallEvent::getReturnValueUnderConstruction()` must + // be non-empty and has to be a `MemRegion`. + Optional RetVal = Call.getReturnValueUnderConstruction(); + ASSERT_TRUE(RetVal); + ASSERT_TRUE(RetVal->getAsRegion()); + } +}; + +void addTestReturnValueUnderConstructionChecker( + AnalysisASTConsumer &AnalysisConsumer, AnalyzerOptions &AnOpts) { + AnOpts.CheckersAndPackages = + {{"test.TestReturnValueUnderConstruction", true}}; + AnalysisConsumer.AddCheckerRegistrationFn([](CheckerRegistry &Registry) { + Registry.addChecker( + "test.TestReturnValueUnderConstruction", "", ""); + }); +} + +TEST(TestReturnValueUnderConstructionChecker, + ReturnValueUnderConstructionChecker) { + EXPECT_TRUE(runCheckerOnCode( + R"(class C { + public: + C(int nn): n(nn) {} + virtual ~C() {} + private: + int n; + }; + + C returnC(int m) { + C c(m); + return c; + } + + void foo() { + C c = returnC(1); + })")); +} + +} // namespace +} // namespace ento +} // namespace clang