baloghadamsoftware updated this revision to Diff 263748. baloghadamsoftware added a comment. Herald added a subscriber: mgorny.
Unit test added. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79704/new/ https://reviews.llvm.org/D79704 Files: clang/include/clang/StaticAnalyzer/Checkers/SValExplainer.h clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h clang/include/clang/StaticAnalyzer/Core/PathSensitive/Regions.def clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp clang/lib/StaticAnalyzer/Checkers/ValistChecker.cpp clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp clang/lib/StaticAnalyzer/Core/CallEvent.cpp clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp clang/lib/StaticAnalyzer/Core/MemRegion.cpp clang/lib/StaticAnalyzer/Core/RegionStore.cpp clang/lib/StaticAnalyzer/Core/Store.cpp clang/lib/StaticAnalyzer/Core/SymbolManager.cpp clang/unittests/StaticAnalyzer/CMakeLists.txt clang/unittests/StaticAnalyzer/ParamRegionTest.cpp
Index: clang/unittests/StaticAnalyzer/ParamRegionTest.cpp =================================================================== --- /dev/null +++ clang/unittests/StaticAnalyzer/ParamRegionTest.cpp @@ -0,0 +1,83 @@ +//===- unittests/StaticAnalyzer/ParamRegionTest.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 "Reusables.h" + +#include "clang/Tooling/Tooling.h" +#include "gtest/gtest.h" + +namespace clang { +namespace ento { +namespace { + +class ParamRegionTestConsumer : public ExprEngineConsumer { + void performTest(const Decl *D) { + StoreManager &StMgr = Eng.getStoreManager(); + MemRegionManager &MRMgr = StMgr.getRegionManager(); + const StackFrameContext *SFC = + Eng.getAnalysisDeclContextManager().getStackFrame(D); + + if (isa<BlockDecl>(D)) { + const auto *ParamN = findDeclByName<ParmVarDecl>(D, "n"); + const auto *ParamM = findDeclByName<ParmVarDecl>(D, "m"); + const TypedValueRegion *RegN = MRMgr.getRegionForParam(ParamN, SFC); + const TypedValueRegion *RegM = MRMgr.getRegionForParam(ParamM, SFC); + assert(isa<VarRegion>(RegN)); + if (SFC->inTopFrame()) + assert(isa<VarRegion>(RegM)); + else + assert(isa<ParamRegion>(RegM)); + } + + if (const auto *FD = dyn_cast<FunctionDecl>(D)) { + if (FD->getNameAsString() == "foo") { + const auto *ParamN = findDeclByName<ParmVarDecl>(D, "n"); + const TypedValueRegion *RegN = MRMgr.getRegionForParam(ParamN, SFC); + if (SFC->inTopFrame()) + assert(isa<VarRegion>(RegN)); + else + assert(isa<ParamRegion>(RegN)); + } else if (FD->getNameAsString() == "bar") { + const auto *ParamL = findDeclByName<ParmVarDecl>(D, "l"); + const TypedValueRegion *RegL = MRMgr.getRegionForParam(ParamL, SFC); + assert(isa<VarRegion>(RegL)); + } + } + } + +public: + ParamRegionTestConsumer(CompilerInstance &C) : ExprEngineConsumer(C) {} + + bool HandleTopLevelDecl(DeclGroupRef DG) override { + for (const auto *D : DG) { + performTest(D); + } + return true; + } +}; + +class ParamRegionTestAction : public ASTFrontendAction { +public: + std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &Compiler, + StringRef File) override { + return std::make_unique<ParamRegionTestConsumer>(Compiler); + } +}; + +TEST(ParamRegion, ParamRegionTest) { + EXPECT_TRUE(tooling::runToolOnCode( + std::make_unique<ParamRegionTestAction>(), + "void foo(int n) { " + "auto lambda = [n](int m) { return n + m; }; " + "int k = lambda(2); } " + "void bar(int l) { foo(l); }")); +} + +} // namespace +} // namespace ento +} // namespace clang Index: clang/unittests/StaticAnalyzer/CMakeLists.txt =================================================================== --- clang/unittests/StaticAnalyzer/CMakeLists.txt +++ clang/unittests/StaticAnalyzer/CMakeLists.txt @@ -6,6 +6,7 @@ AnalyzerOptionsTest.cpp CallDescriptionTest.cpp StoreTest.cpp + ParamRegionTest.cpp RegisterCustomCheckersTest.cpp SymbolReaperTest.cpp ) Index: clang/lib/StaticAnalyzer/Core/SymbolManager.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/SymbolManager.cpp +++ clang/lib/StaticAnalyzer/Core/SymbolManager.cpp @@ -440,6 +440,9 @@ if (const auto *VR = dyn_cast<VarRegion>(MR)) return isLive(VR, true); + if (const auto *PR = dyn_cast<ParamRegion>(MR)) + return isLive(PR, true); + // FIXME: This is a gross over-approximation. What we really need is a way to // tell if anything still refers to this region. Unlike SymbolicRegions, // AllocaRegions don't have associated symbols, though, so we don't actually @@ -573,3 +576,53 @@ return VarContext->isParentOf(CurrentContext); } + +bool SymbolReaper::isLive(const ParamRegion *PR, + bool includeStoreBindings) const{ + const StackFrameContext *ParamContext = PR->getStackFrame(); + + if (!ParamContext) + return true; + + if (!LCtx) + return false; + const StackFrameContext *CurrentContext = LCtx->getStackFrame(); + + if (ParamContext == CurrentContext) { + // If no statement is provided, everything is live. + if (!Loc) + return true; + + // Anonymous parameters of an inheriting constructor are live for the entire + // duration of the constructor. + if (isa<CXXInheritedCtorInitExpr>(Loc)) + return true; + + if (const ParmVarDecl *PVD = PR->getDecl()) { + if (LCtx->getAnalysis<RelaxedLiveVariables>()->isLive(Loc, PVD)) + return true; + } + + if (!includeStoreBindings) + return false; + + unsigned &cachedQuery = + const_cast<SymbolReaper *>(this)->includedRegionCache[PR]; + + if (cachedQuery) { + return cachedQuery == 1; + } + + // Query the store to see if the region occurs in any live bindings. + if (Store store = reapedStore.getStore()) { + bool hasRegion = + reapedStore.getStoreManager().includedInBindings(store, PR); + cachedQuery = hasRegion ? 1 : 2; + return hasRegion; + } + + return false; + } + + return ParamContext->isParentOf(CurrentContext); +} Index: clang/lib/StaticAnalyzer/Core/Store.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/Store.cpp +++ clang/lib/StaticAnalyzer/Core/Store.cpp @@ -138,6 +138,7 @@ case MemRegion::CXXTempObjectRegionKind: case MemRegion::CXXBaseObjectRegionKind: case MemRegion::CXXDerivedObjectRegionKind: + case MemRegion::ParamRegionKind: return MakeElementRegion(cast<SubRegion>(R), PointeeTy); case MemRegion::ElementRegionKind: { @@ -435,6 +436,13 @@ return svalBuilder.dispatchCast(V, castTy); } +Loc StoreManager::getLValueVar(const VarDecl *VD, const LocationContext *LC) { + if (const auto *PVD = dyn_cast<ParmVarDecl>(VD)) + return svalBuilder.makeLoc(MRMgr.getRegionForParam(PVD, LC)); + + return svalBuilder.makeLoc(MRMgr.getVarRegion(VD, LC)); +} + SVal StoreManager::getLValueFieldOrIvar(const Decl *D, SVal Base) { if (Base.isUnknownOrUndef()) return Base; Index: clang/lib/StaticAnalyzer/Core/RegionStore.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/RegionStore.cpp +++ clang/lib/StaticAnalyzer/Core/RegionStore.cpp @@ -569,6 +569,8 @@ SVal getBindingForVar(RegionBindingsConstRef B, const VarRegion *R); + SVal getBindingForParam(RegionBindingsConstRef B, const ParamRegion *R); + SVal getBindingForLazySymbol(const TypedValueRegion *R); SVal getBindingForFieldOrElementCommon(RegionBindingsConstRef B, @@ -1510,6 +1512,16 @@ return CastRetrievedVal(getBindingForVar(B, VR), VR, T); } + if (const ParamRegion *PR = dyn_cast<ParamRegion>(R)) { + // FIXME: Here we actually perform an implicit conversion from the loaded + // value to the variable type. What we should model is stores to variables + // that blow past the extent of the variable. If the address of the + // variable is reinterpretted, it is possible we stored a different value + // that could fit within the variable. Either we need to cast these when + // storing them or reinterpret them lazily (as we do here). + return CastRetrievedVal(getBindingForParam(B, PR), PR, T); + } + const SVal *V = B.lookup(R, BindingKey::Direct); // Check if the region has a binding. @@ -2004,6 +2016,24 @@ return UndefinedVal(); } +SVal RegionStoreManager::getBindingForParam(RegionBindingsConstRef B, + const ParamRegion *R) { + // Check if the region has a binding. + if (Optional<SVal> V = B.getDirectBinding(R)) + return *V; + + if (Optional<SVal> V = B.getDefaultBinding(R)) + return *V; + + // Lazily derive a value for the ParamRegion. + const MemSpaceRegion *MS = R->getMemorySpace(); + + assert (isa<StackArgumentsSpaceRegion>(MS)); + + SVal V = svalBuilder.getRegionValueSymbolVal(R); + return V; +} + SVal RegionStoreManager::getBindingForLazySymbol(const TypedValueRegion *R) { // All other values are symbolic. return svalBuilder.getRegionValueSymbolVal(R); @@ -2505,6 +2535,13 @@ return; } + if (const ParamRegion *PR = dyn_cast<ParamRegion>(baseR)) { + if (SymReaper.isLive(PR)) + AddToWorkList(baseR, &C); + + return; + } + if (const SymbolicRegion *SR = dyn_cast<SymbolicRegion>(baseR)) { if (SymReaper.isLive(SR->getSymbol())) AddToWorkList(SR, &C); Index: clang/lib/StaticAnalyzer/Core/MemRegion.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/MemRegion.cpp +++ clang/lib/StaticAnalyzer/Core/MemRegion.cpp @@ -159,6 +159,11 @@ return SSR ? SSR->getStackFrame() : nullptr; } +const StackFrameContext *ParamRegion::getStackFrame() const { + const auto *SSR = cast<StackSpaceRegion>(getMemorySpace()); + return SSR->getStackFrame(); +} + ObjCIvarRegion::ObjCIvarRegion(const ObjCIvarDecl *ivd, const SubRegion *sReg) : DeclRegion(ivd, sReg, ObjCIvarRegionKind) {} @@ -178,6 +183,32 @@ return QualType(getDecl()->getTypeForDecl(), 0); } +QualType ParamRegion::getValueType() const { + return getDecl()->getType(); +} + +const ParmVarDecl *ParamRegion::getDecl() const { + const Decl *D = getStackFrame()->getDecl(); + + if (const auto *FD = dyn_cast<FunctionDecl>(D)) { + if (Index >= FD->param_size()) + return nullptr; + + return FD->parameters()[Index]; + } else if (const auto *BD = dyn_cast<BlockDecl>(D)) { + assert (Index < BD->param_size()); + return BD->parameters()[Index]; + } else if (const auto *MD = dyn_cast<ObjCMethodDecl>(D)) { + assert (Index < MD->param_size()); + return MD->parameters()[Index]; + } else if (const auto *CD = dyn_cast<CXXConstructorDecl>(D)) { + assert (Index < CD->param_size()); + return CD->parameters()[Index]; + } else { + llvm_unreachable("Unexpected Decl kind!"); + } +} + //===----------------------------------------------------------------------===// // FoldingSet profiling. //===----------------------------------------------------------------------===// @@ -368,6 +399,18 @@ ProfileRegion(ID, getDecl(), superRegion); } +void ParamRegion::ProfileRegion(llvm::FoldingSetNodeID &ID, const Expr *OE, + unsigned Idx, const MemRegion *SReg) { + ID.AddInteger(static_cast<unsigned>(ParamRegionKind)); + ID.AddPointer(OE); + ID.AddInteger(Idx); + ID.AddPointer(SReg); +} + +void ParamRegion::Profile(llvm::FoldingSetNodeID &ID) const { + ProfileRegion(ID, getOriginExpr(), getIndex(), superRegion); +} + //===----------------------------------------------------------------------===// // Region anchors. //===----------------------------------------------------------------------===// @@ -531,6 +574,15 @@ os << "StackLocalsSpaceRegion"; } +void ParamRegion::dumpToStream(raw_ostream &os) const { + const ParmVarDecl *PVD = getDecl(); + if (const IdentifierInfo *ID = PVD->getIdentifier()) { + os << ID->getName(); + } else { + os << "ParamRegion{P" << PVD->getID() << '}'; + } +} + bool MemRegion::canPrintPretty() const { return canPrintPrettyAsExpr(); } @@ -606,6 +658,14 @@ superRegion->printPrettyAsExpr(os); } +bool ParamRegion::canPrintPrettyAsExpr() const { + return true; +} + +void ParamRegion::printPrettyAsExpr(raw_ostream &os) const { + os << getDecl()->getName(); +} + std::string MemRegion::getDescriptiveName(bool UseQuotes) const { std::string VariableName; std::string ArrayIndices; @@ -695,7 +755,8 @@ case MemRegion::ObjCIvarRegionKind: case MemRegion::VarRegionKind: case MemRegion::ElementRegionKind: - case MemRegion::ObjCStringRegionKind: { + case MemRegion::ObjCStringRegionKind: + case MemRegion::ParamRegionKind: { QualType Ty = cast<TypedValueRegion>(SR)->getDesugaredValueType(Ctx); if (isa<VariableArrayType>(Ty)) return nonloc::SymbolVal(SymMgr.getExtentSymbol(SR)); @@ -847,9 +908,14 @@ for (BlockDataRegion::referenced_vars_iterator I = BR->referenced_vars_begin(), E = BR->referenced_vars_end(); I != E; ++I) { - const VarRegion *VR = I.getOriginalRegion(); - if (VR->getDecl() == VD) - return cast<VarRegion>(I.getCapturedRegion()); + const TypedValueRegion *OrigR = I.getOriginalRegion(); + if (const auto *VR = dyn_cast<VarRegion>(OrigR)) { + if (VR->getDecl() == VD) + return cast<VarRegion>(I.getCapturedRegion()); + } else if (const auto *PR = dyn_cast<ParamRegion>(OrigR)) { + if (PR->getDecl() == VD) + return cast<VarRegion>(I.getCapturedRegion()); + } } } @@ -1153,6 +1219,35 @@ return dyn_cast<MemSpaceRegion>(R); } +const ParamRegion* +MemRegionManager::getParamRegion(const Expr *OE, unsigned Index, + const LocationContext *LC) { + const StackFrameContext *STC = LC->getStackFrame(); + assert(STC); + return getSubRegion<ParamRegion>(OE, Index, getStackArgumentsRegion(STC)); +} + +const TypedValueRegion* +MemRegionManager::getRegionForParam(const ParmVarDecl *PVD, + const LocationContext *LC) { + unsigned Index = PVD->getFunctionScopeIndex(); + const StackFrameContext *SFC = LC->getStackFrame(); + const Stmt *CallSite = SFC->getCallSite(); + if (!CallSite) + return getVarRegion(PVD, LC); + + const Decl *D = SFC->getDecl(); + if (const auto *FD = dyn_cast<FunctionDecl>(D)) { + if (Index >= FD->param_size() || FD->parameters()[Index] != PVD) + return getVarRegion(PVD, LC); + } else if (const auto *BD = dyn_cast<BlockDecl>(D)) { + if (Index >= BD->param_size() || BD->parameters()[Index] != PVD) + return getVarRegion(PVD, LC); + } + + return getParamRegion(cast<Expr>(CallSite), Index, LC); +} + bool MemRegion::hasStackStorage() const { return isa<StackSpaceRegion>(getMemorySpace()); } @@ -1343,6 +1438,7 @@ case MemRegion::ObjCStringRegionKind: case MemRegion::VarRegionKind: case MemRegion::CXXTempObjectRegionKind: + case MemRegion::ParamRegionKind: // Usual base regions. goto Finish; @@ -1490,15 +1586,18 @@ // BlockDataRegion //===----------------------------------------------------------------------===// -std::pair<const VarRegion *, const VarRegion *> +std::pair<const VarRegion *, const TypedValueRegion *> BlockDataRegion::getCaptureRegions(const VarDecl *VD) { MemRegionManager &MemMgr = getMemRegionManager(); const VarRegion *VR = nullptr; - const VarRegion *OriginalVR = nullptr; + const TypedValueRegion *OriginalVR = nullptr; if (!VD->hasAttr<BlocksAttr>() && VD->hasLocalStorage()) { VR = MemMgr.getVarRegion(VD, this); - OriginalVR = MemMgr.getVarRegion(VD, LC); + if (const auto *PVD = dyn_cast<ParmVarDecl>(VD)) + OriginalVR = MemMgr.getRegionForParam(PVD, LC); + else + OriginalVR = MemMgr.getVarRegion(VD, LC); } else { if (LC) { @@ -1540,7 +1639,7 @@ for (const auto *VD : ReferencedBlockVars) { const VarRegion *VR = nullptr; - const VarRegion *OriginalVR = nullptr; + const TypedValueRegion *OriginalVR = nullptr; std::tie(VR, OriginalVR) = getCaptureRegions(VD); assert(VR); assert(OriginalVR); @@ -1584,7 +1683,8 @@ VecOriginal->end()); } -const VarRegion *BlockDataRegion::getOriginalRegion(const VarRegion *R) const { +const TypedValueRegion +*BlockDataRegion::getOriginalRegion(const VarRegion *R) const { for (referenced_vars_iterator I = referenced_vars_begin(), E = referenced_vars_end(); I != E; ++I) { Index: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -537,9 +537,11 @@ getObjectUnderConstruction(State, {E, I}, LC)) { SVal VV = *V; (void)VV; - assert(cast<VarRegion>(VV.castAs<loc::MemRegionVal>().getRegion()) - ->getStackFrame()->getParent() - ->getStackFrame() == LC->getStackFrame()); + if (const auto *VR = + dyn_cast<VarRegion>(VV.castAs<loc::MemRegionVal>().getRegion())) { + assert(VR->getStackFrame()->getParent() + ->getStackFrame() == LC->getStackFrame()); + } State = finishObjectConstruction(State, {E, I}, LC); } } Index: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -342,12 +342,12 @@ // 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. - const VarRegion *VR = Caller->getParameterLocation( + const TypedValueRegion *TVR = Caller->getParameterLocation( *Caller->getAdjustedParameterIndex(Idx), currBldrCtx->blockCount()); - if (!VR) + if (!TVR) return None; - return loc::MemRegionVal(VR); + return loc::MemRegionVal(TVR); }; if (const auto *CE = dyn_cast<CallExpr>(E)) { Index: clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp +++ clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp @@ -218,7 +218,7 @@ auto CE = BD->capture_end(); for (; I != E; ++I) { const VarRegion *capturedR = I.getCapturedRegion(); - const VarRegion *originalR = I.getOriginalRegion(); + const TypedValueRegion *originalR = I.getOriginalRegion(); // If the capture had a copy expression, use the result of evaluating // that expression, otherwise use the original value. Index: clang/lib/StaticAnalyzer/Core/CallEvent.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/CallEvent.cpp +++ clang/lib/StaticAnalyzer/Core/CallEvent.cpp @@ -222,39 +222,17 @@ return ADC->getManager()->getStackFrame(ADC, LCtx, E, B, BlockCount, Idx); } -const VarRegion *CallEvent::getParameterLocation(unsigned Index, - unsigned BlockCount) const { +const ParamRegion *CallEvent::getParameterLocation(unsigned Index, + unsigned BlockCount) const { const StackFrameContext *SFC = getCalleeStackFrame(BlockCount); // We cannot construct a VarRegion without a stack frame. if (!SFC) return nullptr; - // Retrieve parameters of the definition, which are different from - // CallEvent's parameters() because getDecl() isn't necessarily - // the definition. SFC contains the definition that would be used - // during analysis. - const Decl *D = SFC->getDecl(); - - // TODO: Refactor into a virtual method of CallEvent, like parameters(). - const ParmVarDecl *PVD = nullptr; - if (const auto *FD = dyn_cast<FunctionDecl>(D)) - PVD = FD->parameters()[Index]; - else if (const auto *BD = dyn_cast<BlockDecl>(D)) - PVD = BD->parameters()[Index]; - else if (const auto *MD = dyn_cast<ObjCMethodDecl>(D)) - PVD = MD->parameters()[Index]; - else if (const auto *CD = dyn_cast<CXXConstructorDecl>(D)) - PVD = CD->parameters()[Index]; - assert(PVD && "Unexpected Decl kind!"); - - const VarRegion *VR = - State->getStateManager().getRegionManager().getVarRegion(PVD, SFC); - - // This sanity check would fail if our parameter declaration doesn't - // correspond to the stack frame's function declaration. - assert(VR->getStackFrame() == SFC); - - return VR; + const ParamRegion *PR = + State->getStateManager().getRegionManager().getParamRegion(getOriginExpr(), + Index, SFC); + return PR; } /// Returns true if a type is a pointer-to-const or reference-to-const @@ -325,8 +303,9 @@ if (getKind() != CE_CXXAllocator) if (isArgumentConstructedDirectly(Idx)) if (auto AdjIdx = getAdjustedParameterIndex(Idx)) - if (const VarRegion *VR = getParameterLocation(*AdjIdx, BlockCount)) - ValuesToInvalidate.push_back(loc::MemRegionVal(VR)); + if (const TypedValueRegion *TVR = + getParameterLocation(*AdjIdx, BlockCount)) + ValuesToInvalidate.push_back(loc::MemRegionVal(TVR)); } // Invalidate designated regions using the batch invalidation API. @@ -528,7 +507,8 @@ // which makes getArgSVal() fail and return UnknownVal. SVal ArgVal = Call.getArgSVal(Idx); if (!ArgVal.isUnknown()) { - Loc ParamLoc = SVB.makeLoc(MRMgr.getVarRegion(ParamDecl, CalleeCtx)); + Loc ParamLoc = + SVB.makeLoc(MRMgr.getParamRegion(Call.getOriginExpr(), Idx, CalleeCtx)); Bindings.push_back(std::make_pair(ParamLoc, ArgVal)); } } Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp +++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp @@ -742,13 +742,20 @@ os << Sep; - // Can only reasonably pretty-print DeclRegions. - if (!isa<DeclRegion>(R)) - return false; + // Can only reasonably pretty-print DeclRegions and ParamRegions. + if (const auto *DR = dyn_cast<DeclRegion>(R)) { + Sep = DR->getValueType()->isAnyPointerType() ? "->" : "."; + DR->getDecl()->getDeclName().print(os, PP); + } else if (const auto *PR = dyn_cast<ParamRegion>(R)) { + const ParmVarDecl *D = PR->getDecl(); + if (!D) + return false; - const auto *DR = cast<DeclRegion>(R); - Sep = DR->getValueType()->isAnyPointerType() ? "->" : "."; - DR->getDecl()->getDeclName().print(os, PP); + Sep = PR->getValueType()->isAnyPointerType() ? "->" : "."; + D->getDeclName().print(os, PP); + } else { + return false; + } } if (Sep.empty()) @@ -1240,6 +1247,32 @@ return FrameSpace->getStackFrame() == LCtx->getStackFrame(); } +/// Returns true if \p N represents the DeclStmt declaring and initializing +/// \p PR. +static bool isInitializationOfParam(const ExplodedNode *N, + const ParamRegion *PR) { + Optional<PostStmt> P = N->getLocationAs<PostStmt>(); + if (!P) + return false; + + const DeclStmt *DS = P->getStmtAs<DeclStmt>(); + if (!DS) + return false; + + const ParmVarDecl *PVD = PR->getDecl(); + if (!PVD) + return false; + + if (DS->getSingleDecl() != PVD) + return false; + + const MemSpaceRegion *ParamSpace = PR->getMemorySpace(); + const auto *FrameSpace = dyn_cast<StackSpaceRegion>(ParamSpace); + + const LocationContext *LCtx = N->getLocationContext(); + return FrameSpace->getStackFrame() == LCtx->getStackFrame(); +} + /// Show diagnostics for initializing or declaring a region \p R with a bad value. static void showBRDiagnostics(const char *action, llvm::raw_svector_ostream &os, const MemRegion *R, SVal V, const DeclStmt *DS) { @@ -1284,14 +1317,11 @@ /// Display diagnostics for passing bad region as a parameter. static void showBRParamDiagnostics(llvm::raw_svector_ostream& os, - const VarRegion *VR, - SVal V) { - const auto *Param = cast<ParmVarDecl>(VR->getDecl()); - + const ParamRegion *PR, SVal V) { os << "Passing "; if (V.getAs<loc::ConcreteInt>()) { - if (Param->getType()->isObjCObjectPointerType()) + if (PR->getValueType()->isObjCObjectPointerType()) os << "nil object reference"; else os << "null pointer value"; @@ -1304,11 +1334,11 @@ } // Printed parameter indexes are 1-based, not 0-based. - unsigned Idx = Param->getFunctionScopeIndex() + 1; + unsigned Idx = PR->getIndex() + 1; os << " via " << Idx << llvm::getOrdinalSuffix(Idx) << " parameter"; - if (VR->canPrintPretty()) { + if (PR->canPrintPretty()) { os << " "; - VR->printPretty(os); + PR->printPretty(os); } } @@ -1377,6 +1407,14 @@ } } + // First see if we reached the declaration of the region. + if (const auto *PR = dyn_cast<ParamRegion>(R)) { + if (isInitializationOfParam(Pred, PR)) { + StoreSite = Pred; + InitE = PR->getDecl()->getInit(); + } + } + // If this is a post initializer expression, initializing the region, we // should track the initializer expression. if (Optional<PostInitializer> PIP = Pred->getLocationAs<PostInitializer>()) { @@ -1416,7 +1454,14 @@ // 'this' should never be NULL, but this visitor isn't just for NULL and // UndefinedVal.) if (Optional<CallEnter> CE = Succ->getLocationAs<CallEnter>()) { - if (const auto *VR = dyn_cast<VarRegion>(R)) { + if (const auto *PR = dyn_cast<ParamRegion>(R)) { + ProgramStateManager &StateMgr = BRC.getStateManager(); + CallEventManager &CallMgr = StateMgr.getCallEventManager(); + + CallEventRef<> Call = CallMgr.getCaller(CE->getCalleeContext(), + Succ->getState()); + InitE = Call->getArgExpr(PR->getIndex()); + } else if (const auto *VR = dyn_cast<VarRegion>(R)) { if (const auto *Param = dyn_cast<ParmVarDecl>(VR->getDecl())) { ProgramStateManager &StateMgr = BRC.getStateManager(); @@ -1482,7 +1527,7 @@ SVal V = StoreSite->getSVal(S); if (const auto *BDR = dyn_cast_or_null<BlockDataRegion>(V.getAsRegion())) { - if (const VarRegion *OriginalR = BDR->getOriginalRegion(VR)) { + if (const TypedValueRegion *OriginalR = BDR->getOriginalRegion(VR)) { if (auto KV = State->getSVal(OriginalR).getAs<KnownSVal>()) BR.addVisitor(std::make_unique<FindLastStoreBRVisitor>( *KV, OriginalR, EnableNullFPSuppression, TKind, OriginSFC)); @@ -1494,8 +1539,9 @@ showBRDiagnostics(action, os, R, V, DS); } else if (StoreSite->getLocation().getAs<CallEnter>()) { - if (const auto *VR = dyn_cast<VarRegion>(R)) - showBRParamDiagnostics(os, VR, V); + if (const auto *PR = dyn_cast<ParamRegion>(R)) { + showBRParamDiagnostics(os, PR, V); + } } if (os.str().empty()) @@ -1834,7 +1880,11 @@ return nullptr; ProgramStateManager &StateMgr = N->getState()->getStateManager(); MemRegionManager &MRMgr = StateMgr.getRegionManager(); - return MRMgr.getVarRegion(VD, N->getLocationContext()); + const LocationContext *LCtx = N->getLocationContext(); + if (const auto *PVD = dyn_cast<ParmVarDecl>(VD)) + return MRMgr.getRegionForParam(PVD, LCtx); + + return MRMgr.getVarRegion(VD, LCtx); } } Index: clang/lib/StaticAnalyzer/Checkers/ValistChecker.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/ValistChecker.cpp +++ clang/lib/StaticAnalyzer/Checkers/ValistChecker.cpp @@ -176,6 +176,8 @@ if (const auto *DeclReg = Reg->getAs<DeclRegion>()) { if (isa<ParmVarDecl>(DeclReg->getDecl())) Reg = C.getState()->getSVal(SV.castAs<Loc>()).getAsRegion(); + } else if (Reg->getAs<ParamRegion>()) { + Reg = C.getState()->getSVal(SV.castAs<Loc>()).getAsRegion(); } IsSymbolic = Reg && Reg->getAs<SymbolicRegion>(); // Some VarRegion based VA lists reach here as ElementRegions. Index: clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp +++ clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp @@ -432,13 +432,22 @@ getRefBinding(N->getFirstPred()->getState(), Sym)) return nullptr; - const auto *VR = cast<VarRegion>(cast<SymbolRegionValue>(Sym)->getRegion()); - const auto *PVD = cast<ParmVarDecl>(VR->getDecl()); - PathDiagnosticLocation L = PathDiagnosticLocation(PVD, SM); + const VarDecl *VD; + if (const auto *VR = + dyn_cast<VarRegion>(cast<SymbolRegionValue>(Sym)->getRegion())) { + VD = cast<VarDecl>(VR->getDecl()); + } else if (const auto *PR = + dyn_cast<ParamRegion>(cast<SymbolRegionValue>(Sym)->getRegion())) { + VD = cast<ParmVarDecl>(PR->getDecl()); + } + + assert(VD); + + PathDiagnosticLocation L = PathDiagnosticLocation(VD, SM); std::string s; llvm::raw_string_ostream os(s); - os << "Parameter '" << PVD->getNameAsString() << "' starts at +"; + os << "Parameter '" << VD->getNameAsString() << "' starts at +"; if (CurrT->getCount() == 1) { os << "1, as it is marked as consuming"; } else { @@ -606,6 +615,8 @@ static Optional<std::string> describeRegion(const MemRegion *MR) { if (const auto *VR = dyn_cast_or_null<VarRegion>(MR)) return std::string(VR->getDecl()->getName()); + if (const auto *PR = dyn_cast_or_null<ParamRegion>(MR)) + return std::string(PR->getDecl()->getName()); // Once we support more storage locations for bindings, // this would need to be improved. return None; Index: clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp +++ clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp @@ -507,13 +507,20 @@ return false; const auto *VR = dyn_cast<VarRegion>(R); + const auto *PR = dyn_cast<ParamRegion>(R); - if (!R->hasStackStorage() || !VR) + if (!R->hasStackStorage() || (!VR && !PR)) return true; - const VarDecl *VD = VR->getDecl(); - if (!VD->hasAttr<CleanupAttr>()) - return false; // CleanupAttr attaches destructors, which cause escaping. + if (VR) { + const VarDecl *VD = VR->getDecl(); + if (!VD->hasAttr<CleanupAttr>()) + return false; // CleanupAttr attaches destructors, which cause escaping. + } else { + const ParmVarDecl *PVD = PR->getDecl(); + if (!PVD->hasAttr<CleanupAttr>()) + return false; // CleanupAttr attaches destructors, which cause escaping. + } return true; } Index: clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp +++ clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp @@ -193,6 +193,12 @@ stackReg = dyn_cast<StackArgumentsSpaceRegion>(VR->getMemorySpace())) if (stackReg->getStackFrame() == SFC) return VR->getValueType(); + if (const ParamRegion *PR = R->getAs<ParamRegion>()) { + const StackArgumentsSpaceRegion *stackReg = + cast<StackArgumentsSpaceRegion>(PR->getMemorySpace()); + if (stackReg->getStackFrame() == SFC) + return PR->getValueType(); + } } return QualType(); Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h =================================================================== --- clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h +++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h @@ -584,6 +584,7 @@ bool isLiveRegion(const MemRegion *region); bool isLive(const Stmt *ExprVal, const LocationContext *LCtx) const; bool isLive(const VarRegion *VR, bool includeStoreBindings = false) const; + bool isLive(const ParamRegion *VR, bool includeStoreBindings = false) const; /// Unconditionally marks a symbol as live. /// Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h =================================================================== --- clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h +++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h @@ -131,9 +131,7 @@ SValBuilder& getSValBuilder() { return svalBuilder; } - virtual Loc getLValueVar(const VarDecl *VD, const LocationContext *LC) { - return svalBuilder.makeLoc(MRMgr.getVarRegion(VD, LC)); - } + virtual Loc getLValueVar(const VarDecl *VD, const LocationContext *LC); Loc getLValueCompoundLiteral(const CompoundLiteralExpr *CL, const LocationContext *LC) { Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/Regions.def =================================================================== --- clang/include/clang/StaticAnalyzer/Core/PathSensitive/Regions.def +++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/Regions.def @@ -79,10 +79,11 @@ REGION(ElementRegion, TypedValueRegion) REGION(ObjCStringRegion, TypedValueRegion) REGION(StringRegion, TypedValueRegion) + REGION(ParamRegion, TypedValueRegion) REGION_RANGE(TYPED_VALUE_REGIONS, CompoundLiteralRegionKind, - StringRegionKind) + ParamRegionKind) REGION_RANGE(TYPED_REGIONS, BlockDataRegionKind, - StringRegionKind) + ParamRegionKind) #undef REGION_RANGE #undef ABSTRACT_REGION Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h =================================================================== --- clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h +++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h @@ -305,6 +305,10 @@ Loc getLValue(const CXXRecordDecl *BaseClass, const SubRegion *Super, bool IsVirtual) const; + /// Get the lvalue for a parameter. + Loc getLValue(const Expr *Call, unsigned Index, + const LocationContext *LC) const; + /// Get the lvalue for a variable reference. Loc getLValue(const VarDecl *D, const LocationContext *LC) const; Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h =================================================================== --- clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h +++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h @@ -704,8 +704,8 @@ return cast<VarRegion>(*R); } - const VarRegion *getOriginalRegion() const { - return cast<VarRegion>(*OriginalR); + const TypedValueRegion *getOriginalRegion() const { + return cast<TypedValueRegion>(*OriginalR); } bool operator==(const referenced_vars_iterator &I) const { @@ -727,7 +727,7 @@ /// Return the original region for a captured region, if /// one exists. - const VarRegion *getOriginalRegion(const VarRegion *VR) const; + const TypedValueRegion *getOriginalRegion(const VarRegion *VR) const; referenced_vars_iterator referenced_vars_begin() const; referenced_vars_iterator referenced_vars_end() const; @@ -742,7 +742,7 @@ private: void LazyInitializeReferencedVars(); - std::pair<const VarRegion *, const VarRegion *> + std::pair<const VarRegion *, const TypedValueRegion *> getCaptureRegions(const VarDecl *VD); }; @@ -1041,6 +1041,46 @@ } }; +/// ParamRegion - Represents a region for paremters. Only parameters of the +/// function in the current stack frame are represented as `ParamRegion`s. +/// Parameters of top-level analyzed functions as well as captured paremeters +/// by lambdas and blocks are repesented as `VarRegion`s. +class ParamRegion : public TypedValueRegion { + friend class MemRegionManager; + + const Expr *OriginExpr; + unsigned Index; + + ParamRegion(const Expr *OE, unsigned Idx, const MemRegion *SReg) + : TypedValueRegion(SReg, ParamRegionKind), OriginExpr(OE), Index(Idx) { + assert(isa<StackSpaceRegion>(SReg)); + assert(!cast<StackSpaceRegion>(SReg)->getStackFrame()->inTopFrame()); + } + + static void ProfileRegion(llvm::FoldingSetNodeID &ID, const Expr *OE, + unsigned Idx, const MemRegion *SReg); + +public: + const Expr *getOriginExpr() const { return OriginExpr; } + unsigned getIndex() const { return Index; } + + void Profile(llvm::FoldingSetNodeID& ID) const override; + + void dumpToStream(raw_ostream &os) const override; + + const StackFrameContext *getStackFrame() const; + + QualType getValueType() const override; + const ParmVarDecl* getDecl() const; + + bool canPrintPrettyAsExpr() const override; + void printPrettyAsExpr(raw_ostream &os) const override; + + static bool classof(const MemRegion* R) { + return R->getKind() == ParamRegionKind; + } +}; + //===----------------------------------------------------------------------===// // Auxiliary data classes for use with MemRegions. //===----------------------------------------------------------------------===// @@ -1395,6 +1435,13 @@ /// super-region used. const CXXTempObjectRegion *getCXXStaticTempObjectRegion(const Expr *Ex); + /// getParamRegion - Retrieve or create the memory region + /// associated with a specified CallExpr, Index and LocationContext. + const ParamRegion *getParamRegion(const Expr *OriginExpr, + unsigned Index, const LocationContext *LC); + + const TypedValueRegion *getRegionForParam(const ParmVarDecl *PVD, + const LocationContext *LC); private: template <typename RegionTy, typename SuperTy, typename Arg1Ty> Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h =================================================================== --- clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h +++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h @@ -400,8 +400,8 @@ /// Returns memory location for a parameter variable within the callee stack /// frame. May fail; returns null on failure. - const VarRegion *getParameterLocation(unsigned Index, - unsigned BlockCount) const; + const ParamRegion *getParameterLocation(unsigned Index, + unsigned BlockCount) const; /// Returns true if on the current path, the argument was constructed by /// calling a C++ constructor over it. This is an internal detail of the Index: clang/include/clang/StaticAnalyzer/Checkers/SValExplainer.h =================================================================== --- clang/include/clang/StaticAnalyzer/Checkers/SValExplainer.h +++ clang/include/clang/StaticAnalyzer/Checkers/SValExplainer.h @@ -216,6 +216,13 @@ "' inside " + Visit(R->getSuperRegion()); } + std::string VisitParamRegion(const ParamRegion *R) { + const ParmVarDecl *PVD = R->getDecl(); + std::string Name = PVD->getQualifiedNameAsString(); + + return "parameter '" + Name + "'"; + } + std::string VisitSVal(SVal V) { std::string Str; llvm::raw_string_ostream OS(Str);
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
