Index: include/clang/Analysis/ConstructionContext.h =================================================================== --- include/clang/Analysis/ConstructionContext.h +++ include/clang/Analysis/ConstructionContext.h @@ -98,6 +98,13 @@ ConstructionContextItem(const ObjCMessageExpr *ME, unsigned Index) : Data(ME), Kind(ArgumentKind), Index(Index) {} + // A polymorphic version of the previous calls with dynamic type check. + ConstructionContextItem(const Expr *E, unsigned Index) + : Data(E), Kind(ArgumentKind), Index(Index) { + assert(isa(E) || isa(E) || + isa(E)); + } + ConstructionContextItem(const CXXCtorInitializer *Init) : Data(Init), Kind(InitializerKind), Index(0) {} Index: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h =================================================================== --- include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h +++ include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h @@ -654,6 +654,11 @@ const EvalCallOptions &CallOpts = {}); private: + ProgramStateRef finishArgumentConstruction(ProgramStateRef State, + const CallEvent &Call); + void finishArgumentConstruction(ExplodedNodeSet &Dst, ExplodedNode *Pred, + const CallEvent &Call); + void evalLoadCommon(ExplodedNodeSet &Dst, const Expr *NodeEx, /* Eventually will be a CFGStmt */ const Expr *BoundEx, Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp =================================================================== --- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -12,6 +12,7 @@ //===----------------------------------------------------------------------===// #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h" +#include "clang/Analysis/CFGStmtMap.h" #include "clang/Analysis/ConstructionContext.h" #include "clang/AST/DeclCXX.h" #include "clang/AST/StmtCXX.h" @@ -292,8 +293,106 @@ return std::make_pair(State, V); } case ConstructionContext::ArgumentKind: { - // Function argument constructors. Not implemented yet. - break; + // Arguments are technically temporaries. + CallOpts.IsTemporaryCtorOrDtor = true; + + const auto *ACC = cast(CC); + const Expr *E = ACC->getCallLikeExpr(); + unsigned Idx = ACC->getIndex(); + const CXXBindTemporaryExpr *BTE = ACC->getCXXBindTemporaryExpr(); + + CallEventManager &CEMgr = getStateManager().getCallEventManager(); + MemRegionManager &MRMgr = getStateManager().getRegionManager(); + AnalysisDeclContextManager *ADCMgr = + LCtx->getAnalysisDeclContext()->getManager(); + + SVal V = UnknownVal(); + auto getArgLoc = [&](CallEventRef<> Caller) -> Optional { + const Decl *CalleeD = Caller->getDecl(); + // If we don't know which function are we calling, it'll be hard to + // express parameter regions. + if (!CalleeD) + return None; + + // FIXME: Skip variadic calls for now. + if (CallEvent::isVariadic(CalleeD)) + return None; + + // FIXME: Skip virtual methods for now. There's no fancy method to find + // parameters, especially when we have no definition and there's + // no method on CallEvent to find the runtime non-definition. + if (const Decl *RD = Caller->getRuntimeDefinition().getDecl()) + if (RD != CalleeD) + return None; + + // Operator arguments do not correspond to operator parameters + // because this-argument is implemented as a normal argument in + // operator call expressions but not in operator declarations. + unsigned Off = 0; + if (isa(E) && isa(CalleeD)) { + assert(Idx > 0 && + "'this'-argument should not have argument construction context!"); + Off = 1; + } + + const ParmVarDecl *PVD = Caller->parameters()[Idx - Off]; + assert(PVD); + AnalysisDeclContext *FutureADC = ADCMgr->getContext(CalleeD); + + // FIXME: This is ugly. We should provide Block and Index as part of + // the ConstructionContext instead. + CFGStmtMap *Map = LCtx->getAnalysisDeclContext()->getCFGStmtMap(); + const CFGBlock *Block = Map->getBlock(E); + assert(Block); + unsigned Index; + for (Index = 0; Index < Block->size(); ++Index) { + CFGElement Elem = (*Block)[Index]; + if (auto StmtElem = Elem.getAs()) + if (StmtElem->getStmt() == E) + break; + } + assert(Index < Block->size()); + + const LocationContext *FutureSFC = ADCMgr->getStackFrame( + FutureADC, LCtx, E, Block, Index); + assert(FutureSFC); + const VarRegion *VR = MRMgr.getVarRegion(PVD, FutureSFC); + assert(VR->getStackFrame() == FutureSFC); + return loc::MemRegionVal(VR); + }; + + if (const auto *CE = dyn_cast(E)) { + CallEventRef<> Caller = CEMgr.getSimpleCall(CE, State, LCtx); + if (auto OptV = getArgLoc(Caller)) + V = *OptV; + 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; + 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; + else + break; + State = addObjectUnderConstruction(State, {ME, Idx}, LCtx, V); + } + + assert(!V.isUnknown()); + + if (BTE) + State = addObjectUnderConstruction(State, BTE, LCtx, V); + + return std::make_pair(State, V); } } } @@ -442,6 +541,10 @@ State = State->bindDefaultZero(Target, LCtx); } + // If there were other constructors called for object-type arguments + // of this constructor, clean them up. + State = finishArgumentConstruction(State, *Call); + Bldr.generateNode(CE, *I, State, /*tag=*/nullptr, ProgramPoint::PreStmtKind); } Index: lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp =================================================================== --- lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -507,6 +507,48 @@ *this); } +ProgramStateRef ExprEngine::finishArgumentConstruction(ProgramStateRef State, + const CallEvent &Call) { + const Expr *E = Call.getOriginExpr(); + // FIXME: Constructors to placement arguments of operator new + // are not supported yet. + if (!E || isa(E)) + return State; + + const LocationContext *LC = Call.getLocationContext(); + for (unsigned I = 0, N = Call.parameters().size(); I != N; ++I) { + if (Optional V = + getObjectUnderConstruction(State, {E, I}, LC)) { + SVal VV = *V; + assert(cast(VV.castAs().getRegion()) + ->getStackFrame()->getParent() + ->getCurrentStackFrame() == LC->getCurrentStackFrame()); + State = finishObjectConstruction(State, {E, I}, LC); + } + } + + return State; +} + +void ExprEngine::finishArgumentConstruction(ExplodedNodeSet &Dst, + ExplodedNode *Pred, + const CallEvent &Call) { + ProgramStateRef State = Pred->getState(); + ProgramStateRef CleanedState = finishArgumentConstruction(State, Call); + if (CleanedState == State) { + Dst.insert(Pred); + return; + } + + const Expr *E = Call.getOriginExpr(); + const LocationContext *LC = Call.getLocationContext(); + NodeBuilder B(Pred, Dst, *currBldrCtx); + static SimpleProgramPointTag Tag("ExprEngine", + "Finish argument construction"); + PreStmt PP(E, LC, &Tag); + B.generateNode(PP, CleanedState, Pred); +} + void ExprEngine::evalCall(ExplodedNodeSet &Dst, ExplodedNode *Pred, const CallEvent &Call) { // WARNING: At this time, the state attached to 'Call' may be older than the @@ -516,9 +558,14 @@ // the most recent state. (It is probably not worth doing the work here since // for some callers this will not be necessary.) + // If there were constructors called for object-type arguments, clean them up. + ExplodedNodeSet dstPrePreVisit; + finishArgumentConstruction(dstPrePreVisit, Pred, Call); + // Run any pre-call checks using the generic call interface. ExplodedNodeSet dstPreVisit; - getCheckerManager().runCheckersForPreCall(dstPreVisit, Pred, Call, *this); + getCheckerManager().runCheckersForPreCall(dstPreVisit, dstPrePreVisit, Call, + *this); // Actually evaluate the function call. We try each of the checkers // to see if the can evaluate the function call, and get a callback at Index: lib/StaticAnalyzer/Core/ExprEngineObjC.cpp =================================================================== --- lib/StaticAnalyzer/Core/ExprEngineObjC.cpp +++ lib/StaticAnalyzer/Core/ExprEngineObjC.cpp @@ -225,9 +225,13 @@ } } + // If there were constructors called for object-type arguments, clean them up. + ExplodedNodeSet dstPrePrevisit; + finishArgumentConstruction(dstPrePrevisit, Pred, *Msg); + // Handle the previsits checks. ExplodedNodeSet dstPrevisit; - getCheckerManager().runCheckersForPreObjCMessage(dstPrevisit, Pred, + getCheckerManager().runCheckersForPreObjCMessage(dstPrevisit, dstPrePrevisit, *Msg, *this); ExplodedNodeSet dstGenericPrevisit; getCheckerManager().runCheckersForPreCall(dstGenericPrevisit, dstPrevisit, Index: lib/StaticAnalyzer/Core/MemRegion.cpp =================================================================== --- lib/StaticAnalyzer/Core/MemRegion.cpp +++ lib/StaticAnalyzer/Core/MemRegion.cpp @@ -516,7 +516,8 @@ if (const IdentifierInfo *ID = VD->getIdentifier()) os << ID->getName(); else - os << "VarRegion{" << static_cast(this) << '}'; + os << "VarRegion{" << static_cast(this) << ',' + << static_cast(getStackFrame()) << '}'; } LLVM_DUMP_METHOD void RegionRawOffset::dump() const { Index: test/Analysis/explain-svals.cpp =================================================================== --- test/Analysis/explain-svals.cpp +++ test/Analysis/explain-svals.cpp @@ -93,6 +93,6 @@ } // end of anonymous namespace void test_6() { - clang_analyzer_explain(conjure_S()); // expected-warning-re{{{{^lazily frozen compound value of temporary object constructed at statement 'conjure_S\(\)'$}}}} + clang_analyzer_explain(conjure_S()); // expected-warning-re{{{{^lazily frozen compound value of parameter ''}}}} clang_analyzer_explain(conjure_S().z); // expected-warning-re{{{{^value derived from \(symbol of type 'int' conjured at statement 'conjure_S\(\)'\) for field 'z' of temporary object constructed at statement 'conjure_S\(\)'$}}}} } Index: test/Analysis/temporaries.cpp =================================================================== --- test/Analysis/temporaries.cpp +++ test/Analysis/temporaries.cpp @@ -1,7 +1,7 @@ -// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus,debug.ExprInspection -analyzer-config cfg-temporary-dtors=false -verify -w -std=c++03 %s -// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus,debug.ExprInspection -analyzer-config cfg-temporary-dtors=false -verify -w -std=c++11 %s -// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus,debug.ExprInspection -DTEMPORARY_DTORS -verify -w -analyzer-config cfg-temporary-dtors=true,c++-temp-dtor-inlining=true %s -std=c++11 -// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus,debug.ExprInspection -DTEMPORARY_DTORS -w -analyzer-config cfg-temporary-dtors=true,c++-temp-dtor-inlining=true %s -std=c++17 +// RUN: %clang_analyze_cc1 -Wno-non-pod-varargs -analyzer-checker=core,cplusplus,debug.ExprInspection -analyzer-config cfg-temporary-dtors=false -verify -w -std=c++03 %s +// RUN: %clang_analyze_cc1 -Wno-non-pod-varargs -analyzer-checker=core,cplusplus,debug.ExprInspection -analyzer-config cfg-temporary-dtors=false -verify -w -std=c++11 %s +// RUN: %clang_analyze_cc1 -Wno-non-pod-varargs -analyzer-checker=core,cplusplus,debug.ExprInspection -DTEMPORARY_DTORS -verify -w -analyzer-config cfg-temporary-dtors=true,c++-temp-dtor-inlining=true %s -std=c++11 +// RUN: %clang_analyze_cc1 -Wno-non-pod-varargs -analyzer-checker=core,cplusplus,debug.ExprInspection -DTEMPORARY_DTORS -w -analyzer-config cfg-temporary-dtors=true,c++-temp-dtor-inlining=true %s -std=c++17 // Note: The C++17 run-line doesn't -verify yet - it is a no-crash test. @@ -962,6 +962,87 @@ } // end namespace pass_references_through +namespace arguments { +int glob; + +struct S { + int x; + S(int x): x(x) {} + S(const S &s) : x(s.x) {} + ~S() {} + + S &operator+(S s) { + glob = s.x; + x += s.x; + return *this; + } +}; + +class C { +public: + virtual void bar3(S s) {} +}; + +class D: public C { +public: + D() {} + virtual void bar3(S s) override { glob = s.x; } +}; + +void bar1(S s) { + glob = s.x; +} + +// Record-typed calls are a different CFGStmt, let's see if we handle that +// as well. +S bar2(S s) { + glob = s.x; + return S(3); +} + +void bar5(int, ...); + +void foo(void (*bar4)(S)) { + bar1(S(1)); + clang_analyzer_eval(glob == 1); +#ifdef TEMPORARY_DTORS + // expected-warning@-2{{TRUE}} +#else + // expected-warning@-4{{UNKNOWN}} +#endif + + bar2(S(2)); + clang_analyzer_eval(glob == 2); +#ifdef TEMPORARY_DTORS + // expected-warning@-2{{TRUE}} +#else + // expected-warning@-4{{UNKNOWN}} +#endif + + C *c = new D(); + c->bar3(S(3)); + // FIXME: Should be TRUE. + clang_analyzer_eval(glob == 3); // expected-warning{{UNKNOWN}} + delete c; + + // What if we've no idea what we're calling? + bar4(S(4)); // no-crash + + S(5) + S(6); + clang_analyzer_eval(glob == 6); +#ifdef TEMPORARY_DTORS + // expected-warning@-2{{TRUE}} +#else + // expected-warning@-4{{UNKNOWN}} +#endif + + // Variadic functions. This will __builtin_trap() because you cannot pass + // an object as a variadic argument. + bar5(7, S(7)); // no-crash + clang_analyzer_warnIfReached(); // no-warning +} +} // namespace arguments + namespace ctor_argument { // Stripped down unique_ptr struct IntPtr { @@ -983,6 +1064,10 @@ IntPtr ptr; int *i = ptr.i; Foo f(static_cast(ptr)); - *i = 99; // no-warning + *i = 99; +#ifdef TEMPORARY_DTORS + // FIXME: Should not warn. + // expected-warning@-3{{Use of memory after it is freed}} +#endif } } // namespace ctor_argument Index: test/Analysis/temporaries.mm =================================================================== --- test/Analysis/temporaries.mm +++ test/Analysis/temporaries.mm @@ -1,7 +1,5 @@ // RUN: %clang_analyze_cc1 -analyzer-checker core,cplusplus -verify %s -// expected-no-diagnostics - // Stripped down unique_ptr struct IntPtr { IntPtr(): i(new int) {} @@ -19,5 +17,6 @@ IntPtr ptr; int *i = ptr.i; [f foo: static_cast(ptr)]; - *i = 99; // no-warning + // FIXME: Should not warn. + *i = 99; // expected-warning{{Use of memory after it is freed}} }