diff options
Diffstat (limited to 'clang/lib/StaticAnalyzer/Checkers')
38 files changed, 1042 insertions, 374 deletions
diff --git a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp index 59163c1f31fa..605b11874ef5 100644 --- a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp @@ -16,7 +16,7 @@ #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/CheckerManager.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" -#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h" using namespace clang; diff --git a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp index 7c264bba4b6a..2a5fe9d8ed92 100644 --- a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp @@ -19,7 +19,7 @@ #include "clang/StaticAnalyzer/Core/CheckerManager.h" #include "clang/StaticAnalyzer/Core/PathSensitive/APSIntType.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" -#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h" #include "llvm/ADT/SmallString.h" #include "llvm/Support/raw_ostream.h" @@ -179,7 +179,7 @@ void ArrayBoundCheckerV2::checkLocation(SVal location, bool isLoad, // CHECK UPPER BOUND: Is byteOffset >= size(baseRegion)? If so, // we are doing a load/store after the last valid offset. const MemRegion *MR = rawOffset.getRegion(); - DefinedOrUnknownSVal Size = getDynamicSize(state, MR, svalBuilder); + DefinedOrUnknownSVal Size = getDynamicExtent(state, MR, svalBuilder); if (!Size.getAs<NonLoc>()) break; diff --git a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp index 233ce57c3ac9..13781b336426 100644 --- a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp @@ -16,7 +16,7 @@ #include "clang/StaticAnalyzer/Core/CheckerManager.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" -#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h" using namespace clang; using namespace ento; @@ -92,12 +92,8 @@ bool BuiltinFunctionChecker::evalCall(const CallEvent &Call, if (Size.isUndef()) return true; // Return true to model purity. - SValBuilder& svalBuilder = C.getSValBuilder(); - DefinedOrUnknownSVal DynSize = getDynamicSize(state, R, svalBuilder); - DefinedOrUnknownSVal DynSizeMatchesSizeArg = - svalBuilder.evalEQ(state, DynSize, Size.castAs<DefinedOrUnknownSVal>()); - state = state->assume(DynSizeMatchesSizeArg, true); - assert(state && "The region should not have any previous constraints"); + state = setDynamicExtent(state, R, Size.castAs<DefinedOrUnknownSVal>(), + C.getSValBuilder()); C.addTransition(state->BindExpr(CE, LCtx, loc::MemRegionVal(R))); return true; diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp index 30fd62f887c4..69b90be9aa7e 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp @@ -19,7 +19,7 @@ #include "clang/StaticAnalyzer/Core/CheckerManager.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" -#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h" #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/SmallString.h" @@ -346,7 +346,7 @@ ProgramStateRef CStringChecker::CheckLocation(CheckerContext &C, // Get the size of the array. const auto *superReg = cast<SubRegion>(ER->getSuperRegion()); DefinedOrUnknownSVal Size = - getDynamicSize(state, superReg, C.getSValBuilder()); + getDynamicExtent(state, superReg, C.getSValBuilder()); // Get the index of the accessed element. DefinedOrUnknownSVal Idx = ER->getIndex().castAs<DefinedOrUnknownSVal>(); @@ -923,7 +923,7 @@ bool CStringChecker::IsFirstBufInBound(CheckerContext &C, // Get the size of the array. const SubRegion *superReg = cast<SubRegion>(ER->getSuperRegion()); - DefinedOrUnknownSVal SizeDV = getDynamicSize(state, superReg, svalBuilder); + DefinedOrUnknownSVal SizeDV = getDynamicExtent(state, superReg, svalBuilder); // Get the index of the accessed element. DefinedOrUnknownSVal Idx = ER->getIndex().castAs<DefinedOrUnknownSVal>(); @@ -1060,7 +1060,7 @@ bool CStringChecker::memsetAux(const Expr *DstBuffer, SVal CharVal, if (Offset.isValid() && !Offset.hasSymbolicOffset() && Offset.getOffset() == 0) { // Get the base region's size. - DefinedOrUnknownSVal SizeDV = getDynamicSize(State, BR, svalBuilder); + DefinedOrUnknownSVal SizeDV = getDynamicExtent(State, BR, svalBuilder); ProgramStateRef StateWholeReg, StateNotWholeReg; std::tie(StateWholeReg, StateNotWholeReg) = @@ -2039,7 +2039,7 @@ void CStringChecker::evalStrcmpCommon(CheckerContext &C, const CallExpr *CE, RightStrRef = RightStrRef.substr(0, s2Term); // Use StringRef's comparison methods to compute the actual result. - int compareRes = IgnoreCase ? LeftStrRef.compare_lower(RightStrRef) + int compareRes = IgnoreCase ? LeftStrRef.compare_insensitive(RightStrRef) : LeftStrRef.compare(RightStrRef); // The strcmp function returns an integer greater than, equal to, or less diff --git a/clang/lib/StaticAnalyzer/Checkers/CastSizeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CastSizeChecker.cpp index a498f252e693..2d2e14de3f2b 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CastSizeChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/CastSizeChecker.cpp @@ -17,7 +17,7 @@ #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/CheckerManager.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" -#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h" using namespace clang; using namespace ento; @@ -112,7 +112,7 @@ void CastSizeChecker::checkPreStmt(const CastExpr *CE,CheckerContext &C) const { SValBuilder &svalBuilder = C.getSValBuilder(); - DefinedOrUnknownSVal Size = getDynamicSize(state, SR, svalBuilder); + DefinedOrUnknownSVal Size = getDynamicExtent(state, SR, svalBuilder); const llvm::APSInt *SizeInt = svalBuilder.getKnownValue(state, Size); if (!SizeInt) return; diff --git a/clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp b/clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp index dc9cd717be9e..99e11a15c08d 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp @@ -13,7 +13,7 @@ #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" -#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h" #include "llvm/Support/FormatVariadic.h" using namespace clang; @@ -64,7 +64,7 @@ private: SVal PlacementNewChecker::getExtentSizeOfPlace(const CXXNewExpr *NE, CheckerContext &C) const { const Expr *Place = NE->getPlacementArg(0); - return getDynamicSizeWithOffset(C.getState(), C.getSVal(Place)); + return getDynamicExtentWithOffset(C.getState(), C.getSVal(Place)); } SVal PlacementNewChecker::getExtentSizeOfNewTarget(const CXXNewExpr *NE, diff --git a/clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp index 73c6517fd0eb..1a7f0d5ab74c 100644 --- a/clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp @@ -763,14 +763,14 @@ bool isBeginCall(const FunctionDecl *Func) { const auto *IdInfo = Func->getIdentifier(); if (!IdInfo) return false; - return IdInfo->getName().endswith_lower("begin"); + return IdInfo->getName().endswith_insensitive("begin"); } bool isEndCall(const FunctionDecl *Func) { const auto *IdInfo = Func->getIdentifier(); if (!IdInfo) return false; - return IdInfo->getName().endswith_lower("end"); + return IdInfo->getName().endswith_insensitive("end"); } const CXXRecordDecl *getCXXRecordDecl(ProgramStateRef State, diff --git a/clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp index 6bc186aa2755..8070d869f678 100644 --- a/clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp @@ -11,17 +11,18 @@ // //===----------------------------------------------------------------------===// -#include "clang/Lex/Lexer.h" -#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" #include "clang/AST/ASTContext.h" #include "clang/AST/Attr.h" #include "clang/AST/ParentMap.h" #include "clang/AST/RecursiveASTVisitor.h" #include "clang/Analysis/Analyses/LiveVariables.h" +#include "clang/Lex/Lexer.h" +#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h" #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h" #include "llvm/ADT/BitVector.h" +#include "llvm/ADT/STLExtras.h" #include "llvm/ADT/SmallString.h" #include "llvm/Support/SaveAndRestore.h" @@ -260,8 +261,8 @@ public: break; } - BR.EmitBasicReport(AC->getDecl(), Checker, BugType, "Dead store", os.str(), - L, R, Fixits); + BR.EmitBasicReport(AC->getDecl(), Checker, BugType, categories::UnusedCode, + os.str(), L, R, Fixits); } void CheckVarDecl(const VarDecl *VD, const Expr *Ex, const Expr *Val, @@ -408,15 +409,17 @@ public: // Special case: check for initializations with constants. // // e.g. : int x = 0; + // struct A = {0, 1}; + // struct B = {{0}, {1, 2}}; // // If x is EVER assigned a new value later, don't issue // a warning. This is because such initialization can be // due to defensive programming. - if (E->isEvaluatable(Ctx)) + if (isConstant(E)) return; if (const DeclRefExpr *DRE = - dyn_cast<DeclRefExpr>(E->IgnoreParenCasts())) + dyn_cast<DeclRefExpr>(E->IgnoreParenCasts())) if (const VarDecl *VD = dyn_cast<VarDecl>(DRE->getDecl())) { // Special case: check for initialization from constant // variables. @@ -444,6 +447,29 @@ public: } } } + +private: + /// Return true if the given init list can be interpreted as constant + bool isConstant(const InitListExpr *Candidate) const { + // We consider init list to be constant if each member of the list can be + // interpreted as constant. + return llvm::all_of(Candidate->inits(), + [this](const Expr *Init) { return isConstant(Init); }); + } + + /// Return true if the given expression can be interpreted as constant + bool isConstant(const Expr *E) const { + // It looks like E itself is a constant + if (E->isEvaluatable(Ctx)) + return true; + + // We should also allow defensive initialization of structs, i.e. { 0 } + if (const auto *ILE = dyn_cast<InitListExpr>(E->IgnoreParenCasts())) { + return isConstant(ILE); + } + + return false; + } }; } // end anonymous namespace diff --git a/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp index adfc2f8cb8fe..4a9c7ce3c66d 100644 --- a/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp @@ -193,7 +193,7 @@ void DereferenceChecker::reportBug(DerefKind K, ProgramStateRef State, } auto report = std::make_unique<PathSensitiveBugReport>( - *BT, buf.empty() ? BT->getDescription() : StringRef(buf), N); + *BT, buf.empty() ? BT->getDescription() : buf.str(), N); bugreporter::trackExpressionValue(N, bugreporter::getDerefExpr(S), *report); diff --git a/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp index c0167b53ae26..2ce1bef6d228 100644 --- a/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp @@ -14,7 +14,7 @@ #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" -#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h" #include "llvm/ADT/StringSwitch.h" #include "llvm/Support/ScopedPrinter.h" @@ -22,8 +22,8 @@ using namespace clang; using namespace ento; namespace { -class ExprInspectionChecker : public Checker<eval::Call, check::DeadSymbols, - check::EndAnalysis> { +class ExprInspectionChecker + : public Checker<eval::Call, check::DeadSymbols, check::EndAnalysis> { mutable std::unique_ptr<BugType> BT; // These stats are per-analysis, not per-branch, hence they shouldn't @@ -44,6 +44,8 @@ class ExprInspectionChecker : public Checker<eval::Call, check::DeadSymbols, void analyzerExplain(const CallExpr *CE, CheckerContext &C) const; void analyzerPrintState(const CallExpr *CE, CheckerContext &C) const; void analyzerGetExtent(const CallExpr *CE, CheckerContext &C) const; + void analyzerDumpExtent(const CallExpr *CE, CheckerContext &C) const; + void analyzerDumpElementCount(const CallExpr *CE, CheckerContext &C) const; void analyzerHashDump(const CallExpr *CE, CheckerContext &C) const; void analyzerDenote(const CallExpr *CE, CheckerContext &C) const; void analyzerExpress(const CallExpr *CE, CheckerContext &C) const; @@ -55,17 +57,19 @@ class ExprInspectionChecker : public Checker<eval::Call, check::DeadSymbols, // Optional parameter `ExprVal` for expression value to be marked interesting. ExplodedNode *reportBug(llvm::StringRef Msg, CheckerContext &C, Optional<SVal> ExprVal = None) const; - ExplodedNode *reportBug(llvm::StringRef Msg, BugReporter &BR, - ExplodedNode *N, + ExplodedNode *reportBug(llvm::StringRef Msg, BugReporter &BR, ExplodedNode *N, Optional<SVal> ExprVal = None) const; + const Expr *getArgExpr(const CallExpr *CE, CheckerContext &C) const; + const MemRegion *getArgRegion(const CallExpr *CE, CheckerContext &C) const; + public: bool evalCall(const CallEvent &Call, CheckerContext &C) const; void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const; void checkEndAnalysis(ExplodedGraph &G, BugReporter &BR, ExprEngine &Eng) const; }; -} +} // namespace REGISTER_SET_WITH_PROGRAMSTATE(MarkedSymbols, SymbolRef) REGISTER_MAP_WITH_PROGRAMSTATE(DenotedSymbols, SymbolRef, const StringLiteral *) @@ -90,6 +94,10 @@ bool ExprInspectionChecker::evalCall(const CallEvent &Call, &ExprInspectionChecker::analyzerWarnOnDeadSymbol) .StartsWith("clang_analyzer_explain", &ExprInspectionChecker::analyzerExplain) + .Case("clang_analyzer_dumpExtent", + &ExprInspectionChecker::analyzerDumpExtent) + .Case("clang_analyzer_dumpElementCount", + &ExprInspectionChecker::analyzerDumpElementCount) .StartsWith("clang_analyzer_dump", &ExprInspectionChecker::analyzerDump) .Case("clang_analyzer_getExtent", @@ -131,7 +139,7 @@ static const char *getArgumentValueString(const CallExpr *CE, ProgramStateRef StTrue, StFalse; std::tie(StTrue, StFalse) = - State->assume(AssertionVal.castAs<DefinedOrUnknownSVal>()); + State->assume(AssertionVal.castAs<DefinedOrUnknownSVal>()); if (StTrue) { if (StFalse) @@ -155,8 +163,7 @@ ExplodedNode *ExprInspectionChecker::reportBug(llvm::StringRef Msg, } ExplodedNode *ExprInspectionChecker::reportBug(llvm::StringRef Msg, - BugReporter &BR, - ExplodedNode *N, + BugReporter &BR, ExplodedNode *N, Optional<SVal> ExprVal) const { if (!N) return nullptr; @@ -172,6 +179,30 @@ ExplodedNode *ExprInspectionChecker::reportBug(llvm::StringRef Msg, return N; } +const Expr *ExprInspectionChecker::getArgExpr(const CallExpr *CE, + CheckerContext &C) const { + if (CE->getNumArgs() == 0) { + reportBug("Missing argument", C); + return nullptr; + } + return CE->getArg(0); +} + +const MemRegion *ExprInspectionChecker::getArgRegion(const CallExpr *CE, + CheckerContext &C) const { + const Expr *Arg = getArgExpr(CE, C); + if (!Arg) + return nullptr; + + const MemRegion *MR = C.getSVal(Arg).getAsRegion(); + if (!MR) { + reportBug("Cannot obtain the region", C); + return nullptr; + } + + return MR; +} + void ExprInspectionChecker::analyzerEval(const CallExpr *CE, CheckerContext &C) const { const LocationContext *LC = C.getPredecessor()->getLocationContext(); @@ -215,24 +246,22 @@ void ExprInspectionChecker::analyzerCheckInlined(const CallExpr *CE, void ExprInspectionChecker::analyzerExplain(const CallExpr *CE, CheckerContext &C) const { - if (CE->getNumArgs() == 0) { - reportBug("Missing argument for explaining", C); + const Expr *Arg = getArgExpr(CE, C); + if (!Arg) return; - } - SVal V = C.getSVal(CE->getArg(0)); + SVal V = C.getSVal(Arg); SValExplainer Ex(C.getASTContext()); reportBug(Ex.Visit(V), C); } void ExprInspectionChecker::analyzerDump(const CallExpr *CE, CheckerContext &C) const { - if (CE->getNumArgs() == 0) { - reportBug("Missing argument for dumping", C); + const Expr *Arg = getArgExpr(CE, C); + if (!Arg) return; - } - SVal V = C.getSVal(CE->getArg(0)); + SVal V = C.getSVal(Arg); llvm::SmallString<32> Str; llvm::raw_svector_ostream OS(Str); @@ -242,24 +271,57 @@ void ExprInspectionChecker::analyzerDump(const CallExpr *CE, void ExprInspectionChecker::analyzerGetExtent(const CallExpr *CE, CheckerContext &C) const { - if (CE->getNumArgs() == 0) { - reportBug("Missing region for obtaining extent", C); + const MemRegion *MR = getArgRegion(CE, C); + if (!MR) return; - } - - auto MR = dyn_cast_or_null<SubRegion>(C.getSVal(CE->getArg(0)).getAsRegion()); - if (!MR) { - reportBug("Obtaining extent of a non-region", C); - return; - } ProgramStateRef State = C.getState(); - DefinedOrUnknownSVal Size = getDynamicSize(State, MR, C.getSValBuilder()); + DefinedOrUnknownSVal Size = getDynamicExtent(State, MR, C.getSValBuilder()); State = State->BindExpr(CE, C.getLocationContext(), Size); C.addTransition(State); } +void ExprInspectionChecker::analyzerDumpExtent(const CallExpr *CE, + CheckerContext &C) const { + const MemRegion *MR = getArgRegion(CE, C); + if (!MR) + return; + + DefinedOrUnknownSVal Size = + getDynamicExtent(C.getState(), MR, C.getSValBuilder()); + + SmallString<64> Msg; + llvm::raw_svector_ostream Out(Msg); + Out << Size; + reportBug(Out.str(), C); +} + +void ExprInspectionChecker::analyzerDumpElementCount(const CallExpr *CE, + CheckerContext &C) const { + const MemRegion *MR = getArgRegion(CE, C); + if (!MR) + return; + + QualType ElementTy; + if (const auto *TVR = MR->getAs<TypedValueRegion>()) { + ElementTy = TVR->getValueType(); + } else { + ElementTy = + MR->castAs<SymbolicRegion>()->getSymbol()->getType()->getPointeeType(); + } + + assert(!ElementTy->isPointerType()); + + DefinedOrUnknownSVal ElementCount = + getDynamicElementCount(C.getState(), MR, C.getSValBuilder(), ElementTy); + + SmallString<128> Msg; + llvm::raw_svector_ostream Out(Msg); + Out << ElementCount; + reportBug(Out.str(), C); +} + void ExprInspectionChecker::analyzerPrintState(const CallExpr *CE, CheckerContext &C) const { C.getState()->dump(); @@ -267,9 +329,11 @@ void ExprInspectionChecker::analyzerPrintState(const CallExpr *CE, void ExprInspectionChecker::analyzerWarnOnDeadSymbol(const CallExpr *CE, CheckerContext &C) const { - if (CE->getNumArgs() == 0) + const Expr *Arg = getArgExpr(CE, C); + if (!Arg) return; - SVal Val = C.getSVal(CE->getArg(0)); + + SVal Val = C.getSVal(Arg); SymbolRef Sym = Val.getAsSymbol(); if (!Sym) return; @@ -306,7 +370,7 @@ void ExprInspectionChecker::checkDeadSymbols(SymbolReaper &SymReaper, void ExprInspectionChecker::checkEndAnalysis(ExplodedGraph &G, BugReporter &BR, ExprEngine &Eng) const { - for (auto Item: ReachedStats) { + for (auto Item : ReachedStats) { unsigned NumTimesReached = Item.second.NumTimesReached; ExplodedNode *N = Item.second.ExampleNode; @@ -373,9 +437,7 @@ public: return None; } - Optional<std::string> VisitSymExpr(const SymExpr *S) { - return lookup(S); - } + Optional<std::string> VisitSymExpr(const SymExpr *S) { return lookup(S); } Optional<std::string> VisitSymIntExpr(const SymIntExpr *S) { if (Optional<std::string> Str = lookup(S)) @@ -394,7 +456,8 @@ public: if (Optional<std::string> Str1 = Visit(S->getLHS())) if (Optional<std::string> Str2 = Visit(S->getRHS())) return (*Str1 + " " + BinaryOperator::getOpcodeStr(S->getOpcode()) + - " " + *Str2).str(); + " " + *Str2) + .str(); return None; } @@ -410,10 +473,9 @@ public: void ExprInspectionChecker::analyzerExpress(const CallExpr *CE, CheckerContext &C) const { - if (CE->getNumArgs() == 0) { - reportBug("clang_analyzer_express() requires a symbol", C); + const Expr *Arg = getArgExpr(CE, C); + if (!Arg) return; - } SVal ArgVal = C.getSVal(CE->getArg(0)); SymbolRef Sym = ArgVal.getAsSymbol(); diff --git a/clang/lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp index 63fbe75fd498..8e02ef74c668 100644 --- a/clang/lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp @@ -80,7 +80,7 @@ static bool isTest(const Decl *D) { if (const auto *CD = dyn_cast<ObjCContainerDecl>(OD->getParent())) { std::string ContainerName = CD->getNameAsString(); StringRef CN(ContainerName); - if (CN.contains_lower("test") || CN.contains_lower("mock")) + if (CN.contains_insensitive("test") || CN.contains_insensitive("mock")) return true; } } diff --git a/clang/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp index 65e52e139ee4..bcae73378028 100644 --- a/clang/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp @@ -34,9 +34,9 @@ namespace { class InnerPointerChecker : public Checker<check::DeadSymbols, check::PostCall> { - CallDescription AppendFn, AssignFn, ClearFn, CStrFn, DataFn, EraseFn, - InsertFn, PopBackFn, PushBackFn, ReplaceFn, ReserveFn, ResizeFn, - ShrinkToFitFn, SwapFn; + CallDescription AppendFn, AssignFn, AddressofFn, ClearFn, CStrFn, DataFn, + DataMemberFn, EraseFn, InsertFn, PopBackFn, PushBackFn, ReplaceFn, + ReserveFn, ResizeFn, ShrinkToFitFn, SwapFn; public: class InnerPointerBRVisitor : public BugReporterVisitor { @@ -73,9 +73,10 @@ public: InnerPointerChecker() : AppendFn({"std", "basic_string", "append"}), AssignFn({"std", "basic_string", "assign"}), + AddressofFn({"std", "addressof"}), ClearFn({"std", "basic_string", "clear"}), - CStrFn({"std", "basic_string", "c_str"}), - DataFn({"std", "basic_string", "data"}), + CStrFn({"std", "basic_string", "c_str"}), DataFn({"std", "data"}, 1), + DataMemberFn({"std", "basic_string", "data"}), EraseFn({"std", "basic_string", "erase"}), InsertFn({"std", "basic_string", "insert"}), PopBackFn({"std", "basic_string", "pop_back"}), @@ -90,6 +91,9 @@ public: /// pointers referring to the container object's inner buffer. bool isInvalidatingMemberFunction(const CallEvent &Call) const; + /// Check whether the called function returns a raw inner pointer. + bool isInnerPointerAccessFunction(const CallEvent &Call) const; + /// Mark pointer symbols associated with the given memory region released /// in the program state. void markPtrSymbolsReleased(const CallEvent &Call, ProgramStateRef State, @@ -130,6 +134,12 @@ bool InnerPointerChecker::isInvalidatingMemberFunction( Call.isCalled(SwapFn)); } +bool InnerPointerChecker::isInnerPointerAccessFunction( + const CallEvent &Call) const { + return (Call.isCalled(CStrFn) || Call.isCalled(DataFn) || + Call.isCalled(DataMemberFn)); +} + void InnerPointerChecker::markPtrSymbolsReleased(const CallEvent &Call, ProgramStateRef State, const MemRegion *MR, @@ -172,6 +182,11 @@ void InnerPointerChecker::checkFunctionArguments(const CallEvent &Call, if (!ArgRegion) continue; + // std::addressof function accepts a non-const reference as an argument, + // but doesn't modify it. + if (Call.isCalled(AddressofFn)) + continue; + markPtrSymbolsReleased(Call, State, ArgRegion, C); } } @@ -195,36 +210,49 @@ void InnerPointerChecker::checkPostCall(const CallEvent &Call, CheckerContext &C) const { ProgramStateRef State = C.getState(); + // TODO: Do we need these to be typed? + const TypedValueRegion *ObjRegion = nullptr; + if (const auto *ICall = dyn_cast<CXXInstanceCall>(&Call)) { - // TODO: Do we need these to be typed? - const auto *ObjRegion = dyn_cast_or_null<TypedValueRegion>( + ObjRegion = dyn_cast_or_null<TypedValueRegion>( ICall->getCXXThisVal().getAsRegion()); - if (!ObjRegion) - return; - if (Call.isCalled(CStrFn) || Call.isCalled(DataFn)) { - SVal RawPtr = Call.getReturnValue(); - if (SymbolRef Sym = RawPtr.getAsSymbol(/*IncludeBaseRegions=*/true)) { - // Start tracking this raw pointer by adding it to the set of symbols - // associated with this container object in the program state map. + // Check [string.require] / second point. + if (isInvalidatingMemberFunction(Call)) { + markPtrSymbolsReleased(Call, State, ObjRegion, C); + return; + } + } - PtrSet::Factory &F = State->getStateManager().get_context<PtrSet>(); - const PtrSet *SetPtr = State->get<RawPtrMap>(ObjRegion); - PtrSet Set = SetPtr ? *SetPtr : F.getEmptySet(); - assert(C.wasInlined || !Set.contains(Sym)); - Set = F.add(Set, Sym); + if (isInnerPointerAccessFunction(Call)) { - State = State->set<RawPtrMap>(ObjRegion, Set); - C.addTransition(State); - } - return; + if (isa<SimpleFunctionCall>(Call)) { + // NOTE: As of now, we only have one free access function: std::data. + // If we add more functions like this in the list, hardcoded + // argument index should be changed. + ObjRegion = + dyn_cast_or_null<TypedValueRegion>(Call.getArgSVal(0).getAsRegion()); } - // Check [string.require] / second point. - if (isInvalidatingMemberFunction(Call)) { - markPtrSymbolsReleased(Call, State, ObjRegion, C); + if (!ObjRegion) return; + + SVal RawPtr = Call.getReturnValue(); + if (SymbolRef Sym = RawPtr.getAsSymbol(/*IncludeBaseRegions=*/true)) { + // Start tracking this raw pointer by adding it to the set of symbols + // associated with this container object in the program state map. + + PtrSet::Factory &F = State->getStateManager().get_context<PtrSet>(); + const PtrSet *SetPtr = State->get<RawPtrMap>(ObjRegion); + PtrSet Set = SetPtr ? *SetPtr : F.getEmptySet(); + assert(C.wasInlined || !Set.contains(Sym)); + Set = F.add(Set, Sym); + + State = State->set<RawPtrMap>(ObjRegion, Set); + C.addTransition(State); } + + return; } // Check [string.require] / first point. diff --git a/clang/lib/StaticAnalyzer/Checkers/Iterator.cpp b/clang/lib/StaticAnalyzer/Checkers/Iterator.cpp index ac0f24603dd9..496190149991 100644 --- a/clang/lib/StaticAnalyzer/Checkers/Iterator.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/Iterator.cpp @@ -29,8 +29,8 @@ bool isIterator(const CXXRecordDecl *CRD) { return false; const auto Name = CRD->getName(); - if (!(Name.endswith_lower("iterator") || Name.endswith_lower("iter") || - Name.endswith_lower("it"))) + if (!(Name.endswith_insensitive("iterator") || + Name.endswith_insensitive("iter") || Name.endswith_insensitive("it"))) return false; bool HasCopyCtor = false, HasCopyAssign = true, HasDtor = false, diff --git a/clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp index dd014648eb6f..a47484497771 100644 --- a/clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp @@ -228,7 +228,7 @@ void IteratorRangeChecker::verifyRandomIncrOrDecr(CheckerContext &C, Value = State->getRawSVal(*ValAsLoc); } - if (Value.isUnknown()) + if (Value.isUnknownOrUndef()) return; // Incremention or decremention by 0 is never a bug. diff --git a/clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp index 837213875a60..b72d72580c28 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp @@ -284,8 +284,9 @@ void MIGChecker::checkReturnAux(const ReturnStmt *RS, CheckerContext &C) const { N); R->addRange(RS->getSourceRange()); - bugreporter::trackExpressionValue(N, RS->getRetValue(), *R, - bugreporter::TrackingKind::Thorough, false); + bugreporter::trackExpressionValue( + N, RS->getRetValue(), *R, + {bugreporter::TrackingKind::Thorough, /*EnableNullFPSuppression=*/false}); C.emitReport(std::move(R)); } diff --git a/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp index 7ac7a38dacf3..5d6bd381d3cc 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp @@ -16,7 +16,7 @@ #include "MPIChecker.h" #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" -#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h" namespace clang { namespace ento { diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp index f117d5505ecb..a6470da09c45 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -63,7 +63,7 @@ #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h" -#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState_Fwd.h" @@ -509,10 +509,6 @@ private: ProgramStateRef State, AllocationFamily Family); - LLVM_NODISCARD - static ProgramStateRef addExtentSize(CheckerContext &C, const CXXNewExpr *NE, - ProgramStateRef State, SVal Target); - // Check if this malloc() for special flags. At present that means M_ZERO or // __GFP_ZERO (in which case, treat it like calloc). LLVM_NODISCARD @@ -1424,7 +1420,6 @@ MallocChecker::processNewAllocation(const CXXAllocatorCall &Call, // existing binding. SVal Target = Call.getObjectUnderConstruction(); State = MallocUpdateRefState(C, NE, State, Family, Target); - State = addExtentSize(C, NE, State, Target); State = ProcessZeroAllocCheck(Call, 0, State, Target); return State; } @@ -1439,52 +1434,6 @@ void MallocChecker::checkNewAllocator(const CXXAllocatorCall &Call, } } -// Sets the extent value of the MemRegion allocated by -// new expression NE to its size in Bytes. -// -ProgramStateRef MallocChecker::addExtentSize(CheckerContext &C, - const CXXNewExpr *NE, - ProgramStateRef State, - SVal Target) { - if (!State) - return nullptr; - SValBuilder &svalBuilder = C.getSValBuilder(); - SVal ElementCount; - const SubRegion *Region; - if (NE->isArray()) { - const Expr *SizeExpr = *NE->getArraySize(); - ElementCount = C.getSVal(SizeExpr); - // Store the extent size for the (symbolic)region - // containing the elements. - Region = Target.getAsRegion() - ->castAs<SubRegion>() - ->StripCasts() - ->castAs<SubRegion>(); - } else { - ElementCount = svalBuilder.makeIntVal(1, true); - Region = Target.getAsRegion()->castAs<SubRegion>(); - } - - // Set the region's extent equal to the Size in Bytes. - QualType ElementType = NE->getAllocatedType(); - ASTContext &AstContext = C.getASTContext(); - CharUnits TypeSize = AstContext.getTypeSizeInChars(ElementType); - - if (ElementCount.getAs<NonLoc>()) { - DefinedOrUnknownSVal DynSize = getDynamicSize(State, Region, svalBuilder); - - // size in Bytes = ElementCount*TypeSize - SVal SizeInBytes = svalBuilder.evalBinOpNN( - State, BO_Mul, ElementCount.castAs<NonLoc>(), - svalBuilder.makeArrayIndex(TypeSize.getQuantity()), - svalBuilder.getArrayIndexType()); - DefinedOrUnknownSVal DynSizeMatchesSize = svalBuilder.evalEQ( - State, DynSize, SizeInBytes.castAs<DefinedOrUnknownSVal>()); - State = State->assume(DynSizeMatchesSize, true); - } - return State; -} - static bool isKnownDeallocObjCMethodName(const ObjCMethodCall &Call) { // If the first selector piece is one of the names below, assume that the // object takes ownership of the memory, promising to eventually deallocate it @@ -1588,21 +1537,9 @@ ProgramStateRef MallocChecker::MallocMemAux(CheckerContext &C, // Fill the region with the initialization value. State = State->bindDefaultInitial(RetVal, Init, LCtx); - // Set the region's extent equal to the Size parameter. - const SymbolicRegion *R = - dyn_cast_or_null<SymbolicRegion>(RetVal.getAsRegion()); - if (!R) - return nullptr; - if (Optional<DefinedOrUnknownSVal> DefinedSize = - Size.getAs<DefinedOrUnknownSVal>()) { - DefinedOrUnknownSVal DynSize = getDynamicSize(State, R, svalBuilder); - - DefinedOrUnknownSVal DynSizeMatchesSize = - svalBuilder.evalEQ(State, DynSize, *DefinedSize); - - State = State->assume(DynSizeMatchesSize, true); - assert(State); - } + // Set the region's extent. + State = setDynamicExtent(State, RetVal.getAsRegion(), + Size.castAs<DefinedOrUnknownSVal>(), svalBuilder); return MallocUpdateRefState(C, CE, State, Family); } @@ -2186,7 +2123,7 @@ void MallocChecker::HandleMismatchedDealloc(CheckerContext &C, os.str(), N); R->markInteresting(Sym); R->addRange(Range); - R->addVisitor(std::make_unique<MallocBugVisitor>(Sym)); + R->addVisitor<MallocBugVisitor>(Sym); C.emitReport(std::move(R)); } } @@ -2279,7 +2216,7 @@ void MallocChecker::HandleUseAfterFree(CheckerContext &C, SourceRange Range, R->markInteresting(Sym); R->addRange(Range); - R->addVisitor(std::make_unique<MallocBugVisitor>(Sym)); + R->addVisitor<MallocBugVisitor>(Sym); if (AF == AF_InnerBuffer) R->addVisitor(allocation_state::getInnerPointerBRVisitor(Sym)); @@ -2315,7 +2252,7 @@ void MallocChecker::HandleDoubleFree(CheckerContext &C, SourceRange Range, R->markInteresting(Sym); if (PrevSym) R->markInteresting(PrevSym); - R->addVisitor(std::make_unique<MallocBugVisitor>(Sym)); + R->addVisitor<MallocBugVisitor>(Sym); C.emitReport(std::move(R)); } } @@ -2341,7 +2278,7 @@ void MallocChecker::HandleDoubleDelete(CheckerContext &C, SymbolRef Sym) const { *BT_DoubleDelete, "Attempt to delete released memory", N); R->markInteresting(Sym); - R->addVisitor(std::make_unique<MallocBugVisitor>(Sym)); + R->addVisitor<MallocBugVisitor>(Sym); C.emitReport(std::move(R)); } } @@ -2371,7 +2308,7 @@ void MallocChecker::HandleUseZeroAlloc(CheckerContext &C, SourceRange Range, R->addRange(Range); if (Sym) { R->markInteresting(Sym); - R->addVisitor(std::make_unique<MallocBugVisitor>(Sym)); + R->addVisitor<MallocBugVisitor>(Sym); } C.emitReport(std::move(R)); } @@ -2641,7 +2578,7 @@ void MallocChecker::HandleLeak(SymbolRef Sym, ExplodedNode *N, *BT_Leak[*CheckKind], os.str(), N, LocUsedForUniqueing, AllocNode->getLocationContext()->getDecl()); R->markInteresting(Sym); - R->addVisitor(std::make_unique<MallocBugVisitor>(Sym, true)); + R->addVisitor<MallocBugVisitor>(Sym, true); C.emitReport(std::move(R)); } @@ -3208,9 +3145,10 @@ static SymbolRef findFailedReallocSymbol(ProgramStateRef currState, static bool isReferenceCountingPointerDestructor(const CXXDestructorDecl *DD) { if (const IdentifierInfo *II = DD->getParent()->getIdentifier()) { StringRef N = II->getName(); - if (N.contains_lower("ptr") || N.contains_lower("pointer")) { - if (N.contains_lower("ref") || N.contains_lower("cnt") || - N.contains_lower("intrusive") || N.contains_lower("shared")) { + if (N.contains_insensitive("ptr") || N.contains_insensitive("pointer")) { + if (N.contains_insensitive("ref") || N.contains_insensitive("cnt") || + N.contains_insensitive("intrusive") || + N.contains_insensitive("shared")) { return true; } } diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp index 71f593cb2b56..4b5206a102b8 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp @@ -139,6 +139,10 @@ static bool typesCompatible(ASTContext &C, QualType A, QualType B) { if (B->isVoidPointerType() && A->getAs<PointerType>()) return true; + // sizeof(pointer type) is compatible with void* + if (A->isVoidPointerType() && B->getAs<PointerType>()) + return true; + while (true) { A = A.getCanonicalType(); B = B.getCanonicalType(); diff --git a/clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp index a38298a7abed..cbe938982000 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp @@ -202,7 +202,7 @@ public: }; private: - mutable std::unique_ptr<BugType> BT; + BugType BT{this, "Use-after-move", categories::CXXMoveSemantics}; // Check if the given form of potential misuse of a given object // should be reported. If so, get it reported. The callback from which @@ -393,11 +393,6 @@ ExplodedNode *MoveChecker::reportBug(const MemRegion *Region, MisuseKind MK) const { if (ExplodedNode *N = misuseCausesCrash(MK) ? C.generateErrorNode() : C.generateNonFatalErrorNode()) { - - if (!BT) - BT.reset(new BugType(this, "Use-after-move", - "C++ move semantics")); - // Uniqueing report to the same object. PathDiagnosticLocation LocUsedForUniqueing; const ExplodedNode *MoveNode = getMoveLocation(N, Region, C); @@ -431,7 +426,7 @@ ExplodedNode *MoveChecker::reportBug(const MemRegion *Region, } auto R = std::make_unique<PathSensitiveBugReport>( - *BT, OS.str(), N, LocUsedForUniqueing, + BT, OS.str(), N, LocUsedForUniqueing, MoveNode->getLocationContext()->getDecl()); R->addVisitor(std::make_unique<MovedBugVisitor>(*this, Region, RD, MK)); C.emitReport(std::move(R)); @@ -477,7 +472,7 @@ void MoveChecker::checkPostCall(const CallEvent &Call, const MemRegion *BaseRegion = ArgRegion->getBaseRegion(); // Skip temp objects because of their short lifetime. if (BaseRegion->getAs<CXXTempObjectRegion>() || - AFC->getArgExpr(0)->isRValue()) + AFC->getArgExpr(0)->isPRValue()) return; // If it has already been reported do not need to modify the state. diff --git a/clang/lib/StaticAnalyzer/Checkers/NonnullGlobalConstantsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/NonnullGlobalConstantsChecker.cpp index 80b705fb7392..c5437b16c688 100644 --- a/clang/lib/StaticAnalyzer/Checkers/NonnullGlobalConstantsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/NonnullGlobalConstantsChecker.cpp @@ -89,7 +89,7 @@ void NonnullGlobalConstantsChecker::checkLocation(SVal location, bool isLoad, } /// \param V loaded lvalue. -/// \return whether {@code val} is a string-like const global. +/// \return whether @c val is a string-like const global. bool NonnullGlobalConstantsChecker::isGlobalConstString(SVal V) const { Optional<loc::MemRegionVal> RegionVal = V.getAs<loc::MemRegionVal>(); if (!RegionVal) @@ -127,7 +127,7 @@ bool NonnullGlobalConstantsChecker::isGlobalConstString(SVal V) const { return false; } -/// \return whether {@code type} is extremely unlikely to be null +/// \return whether @c type is extremely unlikely to be null bool NonnullGlobalConstantsChecker::isNonnullType(QualType Ty) const { if (Ty->isPointerType() && Ty->getPointeeType()->isCharType()) diff --git a/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp index bc7a8a3b12a1..fe8f7e7bf69e 100644 --- a/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp @@ -170,7 +170,7 @@ private: auto R = std::make_unique<PathSensitiveBugReport>(*BT, Msg, N); if (Region) { R->markInteresting(Region); - R->addVisitor(std::make_unique<NullabilityBugVisitor>(Region)); + R->addVisitor<NullabilityBugVisitor>(Region); } if (ValueExpr) { R->addRange(ValueExpr->getSourceRange()); diff --git a/clang/lib/StaticAnalyzer/Checkers/OSObjectCStyleCast.cpp b/clang/lib/StaticAnalyzer/Checkers/OSObjectCStyleCast.cpp index 270b66dab020..0a8379d9ab99 100644 --- a/clang/lib/StaticAnalyzer/Checkers/OSObjectCStyleCast.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/OSObjectCStyleCast.cpp @@ -32,7 +32,21 @@ public: void checkASTCodeBody(const Decl *D, AnalysisManager &AM, BugReporter &BR) const; }; +} // namespace + +namespace clang { +namespace ast_matchers { +AST_MATCHER_P(StringLiteral, mentionsBoundType, std::string, BindingID) { + return Builder->removeBindings([this, &Node](const BoundNodesMap &Nodes) { + const auto &BN = Nodes.getNode(this->BindingID); + if (const auto *ND = BN.get<NamedDecl>()) { + return ND->getName() != Node.getString(); + } + return true; + }); } +} // end namespace ast_matchers +} // end namespace clang static void emitDiagnostics(const BoundNodes &Nodes, BugReporter &BR, @@ -63,22 +77,41 @@ static decltype(auto) hasTypePointingTo(DeclarationMatcher DeclM) { return hasType(pointerType(pointee(hasDeclaration(DeclM)))); } -void OSObjectCStyleCastChecker::checkASTCodeBody(const Decl *D, AnalysisManager &AM, +void OSObjectCStyleCastChecker::checkASTCodeBody(const Decl *D, + AnalysisManager &AM, BugReporter &BR) const { AnalysisDeclContext *ADC = AM.getAnalysisDeclContext(D); auto DynamicCastM = callExpr(callee(functionDecl(hasName("safeMetaCast")))); - - auto OSObjTypeM = hasTypePointingTo(cxxRecordDecl(isDerivedFrom("OSMetaClassBase"))); + // 'allocClassWithName' allocates an object with the given type. + // The type is actually provided as a string argument (type's name). + // This makes the following pattern possible: + // + // Foo *object = (Foo *)allocClassWithName("Foo"); + // + // While OSRequiredCast can be used here, it is still not a useful warning. + auto AllocClassWithNameM = callExpr( + callee(functionDecl(hasName("allocClassWithName"))), + // Here we want to make sure that the string argument matches the + // type in the cast expression. + hasArgument(0, stringLiteral(mentionsBoundType(WarnRecordDecl)))); + + auto OSObjTypeM = + hasTypePointingTo(cxxRecordDecl(isDerivedFrom("OSMetaClassBase"))); auto OSObjSubclassM = hasTypePointingTo( - cxxRecordDecl(isDerivedFrom("OSObject")).bind(WarnRecordDecl)); - - auto CastM = cStyleCastExpr( - allOf(hasSourceExpression(allOf(OSObjTypeM, unless(DynamicCastM))), - OSObjSubclassM)).bind(WarnAtNode); - - auto Matches = match(stmt(forEachDescendant(CastM)), *D->getBody(), AM.getASTContext()); + cxxRecordDecl(isDerivedFrom("OSObject")).bind(WarnRecordDecl)); + + auto CastM = + cStyleCastExpr( + allOf(OSObjSubclassM, + hasSourceExpression( + allOf(OSObjTypeM, + unless(anyOf(DynamicCastM, AllocClassWithNameM)))))) + .bind(WarnAtNode); + + auto Matches = + match(stmt(forEachDescendant(CastM)), *D->getBody(), AM.getASTContext()); for (BoundNodes Match : Matches) emitDiagnostics(Match, BR, ADC, this); } diff --git a/clang/lib/StaticAnalyzer/Checkers/ObjCAutoreleaseWriteChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ObjCAutoreleaseWriteChecker.cpp index 7fd6e2abef4c..c8eab3288094 100644 --- a/clang/lib/StaticAnalyzer/Checkers/ObjCAutoreleaseWriteChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/ObjCAutoreleaseWriteChecker.cpp @@ -8,7 +8,7 @@ // // This file defines ObjCAutoreleaseWriteChecker which warns against writes // into autoreleased out parameters which cause crashes. -// An example of a problematic write is a write to {@code error} in the example +// An example of a problematic write is a write to @c error in the example // below: // // - (BOOL) mymethod:(NSError *__autoreleasing *)error list:(NSArray*) list { diff --git a/clang/lib/StaticAnalyzer/Checkers/ObjCContainersChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ObjCContainersChecker.cpp index 8c2008a7ceb4..13985af76b00 100644 --- a/clang/lib/StaticAnalyzer/Checkers/ObjCContainersChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/ObjCContainersChecker.cpp @@ -147,8 +147,9 @@ void ObjCContainersChecker::checkPreStmt(const CallExpr *CE, auto R = std::make_unique<PathSensitiveBugReport>( *BT, "Index is out of bounds", N); R->addRange(IdxExpr->getSourceRange()); - bugreporter::trackExpressionValue( - N, IdxExpr, *R, bugreporter::TrackingKind::Thorough, false); + bugreporter::trackExpressionValue(N, IdxExpr, *R, + {bugreporter::TrackingKind::Thorough, + /*EnableNullFPSuppression=*/false}); C.emitReport(std::move(R)); return; } diff --git a/clang/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp index 96f0d9bb3c3d..40472ccfe7e6 100644 --- a/clang/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp @@ -193,6 +193,11 @@ public: CharUnits PaddingSum; CharUnits Offset = ASTContext.toCharUnitsFromBits(RL.getFieldOffset(0)); for (const FieldDecl *FD : RD->fields()) { + // Skip field that is a subobject of zero size, marked with + // [[no_unique_address]] or an empty bitfield, because its address can be + // set the same as the other fields addresses. + if (FD->isZeroSize(ASTContext)) + continue; // This checker only cares about the padded size of the // field, and not the data size. If the field is a record // with tail padding, then we won't put that number in our @@ -249,7 +254,7 @@ public: RetVal.Field = FD; auto &Ctx = FD->getASTContext(); auto Info = Ctx.getTypeInfoInChars(FD->getType()); - RetVal.Size = Info.Width; + RetVal.Size = FD->isZeroSize(Ctx) ? CharUnits::Zero() : Info.Width; RetVal.Align = Info.Align; assert(llvm::isPowerOf2_64(RetVal.Align.getQuantity())); if (auto Max = FD->getMaxAlignment()) diff --git a/clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp index 88e80c481a5a..ee71b55a39e6 100644 --- a/clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp @@ -339,7 +339,16 @@ void PthreadLockChecker::printState(raw_ostream &Out, ProgramStateRef State, } } - // TODO: Dump destroyed mutex symbols? + DestroyRetValTy DRV = State->get<DestroyRetVal>(); + if (!DRV.isEmpty()) { + Out << Sep << "Mutexes in unresolved possibly destroyed state:" << NL; + for (auto I : DRV) { + I.first->dumpToStream(Out); + Out << ": "; + I.second->dumpToStream(Out); + Out << NL; + } + } } void PthreadLockChecker::AcquirePthreadLock(const CallEvent &Call, @@ -638,8 +647,10 @@ void PthreadLockChecker::checkDeadSymbols(SymbolReaper &SymReaper, for (auto I : State->get<LockMap>()) { // Stop tracking dead mutex regions as well. - if (!SymReaper.isLiveRegion(I.first)) + if (!SymReaper.isLiveRegion(I.first)) { State = State->remove<LockMap>(I.first); + State = State->remove<DestroyRetVal>(I.first); + } } // TODO: We probably need to clean up the lock stack as well. diff --git a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp index 1d903530201f..64ac6bc4c06b 100644 --- a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp @@ -13,6 +13,8 @@ #include "RetainCountDiagnostics.h" #include "RetainCountChecker.h" +#include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/SmallVector.h" using namespace clang; using namespace ento; @@ -89,7 +91,7 @@ static std::string getPrettyTypeName(QualType QT) { return QT.getAsString(); } -/// Write information about the type state change to {@code os}, +/// Write information about the type state change to @c os, /// return whether the note should be generated. static bool shouldGenerateNote(llvm::raw_string_ostream &os, const RefVal *PrevT, @@ -164,8 +166,8 @@ static bool shouldGenerateNote(llvm::raw_string_ostream &os, return true; } -/// Finds argument index of the out paramter in the call {@code S} -/// corresponding to the symbol {@code Sym}. +/// Finds argument index of the out paramter in the call @c S +/// corresponding to the symbol @c Sym. /// If none found, returns None. static Optional<unsigned> findArgIdxOfSymbol(ProgramStateRef CurrSt, const LocationContext *LCtx, @@ -337,11 +339,15 @@ public: class RefLeakReportVisitor : public RefCountReportVisitor { public: - RefLeakReportVisitor(SymbolRef sym) : RefCountReportVisitor(sym) {} + RefLeakReportVisitor(SymbolRef Sym, const MemRegion *LastBinding) + : RefCountReportVisitor(Sym), LastBinding(LastBinding) {} PathDiagnosticPieceRef getEndPath(BugReporterContext &BRC, const ExplodedNode *N, PathSensitiveBugReport &BR) override; + +private: + const MemRegion *LastBinding; }; } // end namespace retaincountchecker @@ -610,6 +616,41 @@ static Optional<std::string> describeRegion(const MemRegion *MR) { return None; } +using Bindings = llvm::SmallVector<std::pair<const MemRegion *, SVal>, 4>; + +class VarBindingsCollector : public StoreManager::BindingsHandler { + SymbolRef Sym; + Bindings &Result; + +public: + VarBindingsCollector(SymbolRef Sym, Bindings &ToFill) + : Sym(Sym), Result(ToFill) {} + + bool HandleBinding(StoreManager &SMgr, Store Store, const MemRegion *R, + SVal Val) override { + SymbolRef SymV = Val.getAsLocSymbol(); + if (!SymV || SymV != Sym) + return true; + + if (isa<NonParamVarRegion>(R)) + Result.emplace_back(R, Val); + + return true; + } +}; + +Bindings getAllVarBindingsForSymbol(ProgramStateManager &Manager, + const ExplodedNode *Node, SymbolRef Sym) { + Bindings Result; + VarBindingsCollector Collector{Sym, Result}; + while (Result.empty() && Node) { + Manager.iterBindings(Node->getState(), Collector); + Node = Node->getFirstPred(); + } + + return Result; +} + namespace { // Find the first node in the current function context that referred to the // tracked symbol and the memory location that value was stored to. Note, the @@ -729,14 +770,6 @@ RefLeakReportVisitor::getEndPath(BugReporterContext &BRC, // assigned to different variables, etc. BR.markInteresting(Sym); - // We are reporting a leak. Walk up the graph to get to the first node where - // the symbol appeared, and also get the first VarDecl that tracked object - // is stored to. - AllocationInfo AllocI = GetAllocationSite(BRC.getStateManager(), EndN, Sym); - - const MemRegion* FirstBinding = AllocI.R; - BR.markInteresting(AllocI.InterestingMethodContext); - PathDiagnosticLocation L = cast<RefLeakReport>(BR).getEndOfPath(); std::string sbuf; @@ -744,7 +777,7 @@ RefLeakReportVisitor::getEndPath(BugReporterContext &BRC, os << "Object leaked: "; - Optional<std::string> RegionDescription = describeRegion(FirstBinding); + Optional<std::string> RegionDescription = describeRegion(LastBinding); if (RegionDescription) { os << "object allocated and stored into '" << *RegionDescription << '\''; } else { @@ -753,7 +786,7 @@ RefLeakReportVisitor::getEndPath(BugReporterContext &BRC, } // Get the retain count. - const RefVal* RV = getRefBinding(EndN->getState(), Sym); + const RefVal *RV = getRefBinding(EndN->getState(), Sym); assert(RV); if (RV->getKind() == RefVal::ErrorLeakReturned) { @@ -794,14 +827,15 @@ RefLeakReportVisitor::getEndPath(BugReporterContext &BRC, " Foundation"; } else if (RV->getObjKind() == ObjKind::OS) { std::string FuncName = FD->getNameAsString(); - os << "whose name ('" << FuncName - << "') starts with '" << StringRef(FuncName).substr(0, 3) << "'"; + os << "whose name ('" << FuncName << "') starts with '" + << StringRef(FuncName).substr(0, 3) << "'"; } } } } else { os << " is not referenced later in this execution path and has a retain " - "count of +" << RV->getCount(); + "count of +" + << RV->getCount(); } return std::make_shared<PathDiagnosticEventPiece>(L, os.str()); @@ -812,7 +846,7 @@ RefCountReport::RefCountReport(const RefCountBug &D, const LangOptions &LOpts, : PathSensitiveBugReport(D, D.getDescription(), n), Sym(sym), isLeak(isLeak) { if (!isLeak) - addVisitor(std::make_unique<RefCountReportVisitor>(sym)); + addVisitor<RefCountReportVisitor>(sym); } RefCountReport::RefCountReport(const RefCountBug &D, const LangOptions &LOpts, @@ -820,19 +854,19 @@ RefCountReport::RefCountReport(const RefCountBug &D, const LangOptions &LOpts, StringRef endText) : PathSensitiveBugReport(D, D.getDescription(), endText, n) { - addVisitor(std::make_unique<RefCountReportVisitor>(sym)); + addVisitor<RefCountReportVisitor>(sym); } -void RefLeakReport::deriveParamLocation(CheckerContext &Ctx, SymbolRef sym) { - const SourceManager& SMgr = Ctx.getSourceManager(); +void RefLeakReport::deriveParamLocation(CheckerContext &Ctx) { + const SourceManager &SMgr = Ctx.getSourceManager(); - if (!sym->getOriginRegion()) + if (!Sym->getOriginRegion()) return; - auto *Region = dyn_cast<DeclRegion>(sym->getOriginRegion()); + auto *Region = dyn_cast<DeclRegion>(Sym->getOriginRegion()); if (Region) { const Decl *PDecl = Region->getDecl(); - if (PDecl && isa<ParmVarDecl>(PDecl)) { + if (isa_and_nonnull<ParmVarDecl>(PDecl)) { PathDiagnosticLocation ParamLocation = PathDiagnosticLocation::create(PDecl, SMgr); Location = ParamLocation; @@ -842,8 +876,7 @@ void RefLeakReport::deriveParamLocation(CheckerContext &Ctx, SymbolRef sym) { } } -void RefLeakReport::deriveAllocLocation(CheckerContext &Ctx, - SymbolRef sym) { +void RefLeakReport::deriveAllocLocation(CheckerContext &Ctx) { // Most bug reports are cached at the location where they occurred. // With leaks, we want to unique them by the location where they were // allocated, and only report a single path. To do this, we need to find @@ -854,13 +887,13 @@ void RefLeakReport::deriveAllocLocation(CheckerContext &Ctx, // same SourceLocation. const ExplodedNode *AllocNode = nullptr; - const SourceManager& SMgr = Ctx.getSourceManager(); + const SourceManager &SMgr = Ctx.getSourceManager(); AllocationInfo AllocI = - GetAllocationSite(Ctx.getStateManager(), getErrorNode(), sym); + GetAllocationSite(Ctx.getStateManager(), getErrorNode(), Sym); AllocNode = AllocI.N; - AllocBinding = AllocI.R; + AllocFirstBinding = AllocI.R; markInteresting(AllocI.InterestingMethodContext); // Get the SourceLocation for the allocation site. @@ -870,13 +903,12 @@ void RefLeakReport::deriveAllocLocation(CheckerContext &Ctx, AllocStmt = AllocNode->getStmtForDiagnostics(); if (!AllocStmt) { - AllocBinding = nullptr; + AllocFirstBinding = nullptr; return; } - PathDiagnosticLocation AllocLocation = - PathDiagnosticLocation::createBegin(AllocStmt, SMgr, - AllocNode->getLocationContext()); + PathDiagnosticLocation AllocLocation = PathDiagnosticLocation::createBegin( + AllocStmt, SMgr, AllocNode->getLocationContext()); Location = AllocLocation; // Set uniqieing info, which will be used for unique the bug reports. The @@ -891,7 +923,8 @@ void RefLeakReport::createDescription(CheckerContext &Ctx) { llvm::raw_string_ostream os(Description); os << "Potential leak of an object"; - Optional<std::string> RegionDescription = describeRegion(AllocBinding); + Optional<std::string> RegionDescription = + describeRegion(AllocBindingToReport); if (RegionDescription) { os << " stored into '" << *RegionDescription << '\''; } else { @@ -901,16 +934,75 @@ void RefLeakReport::createDescription(CheckerContext &Ctx) { } } +void RefLeakReport::findBindingToReport(CheckerContext &Ctx, + ExplodedNode *Node) { + if (!AllocFirstBinding) + // If we don't have any bindings, we won't be able to find any + // better binding to report. + return; + + // If the original region still contains the leaking symbol... + if (Node->getState()->getSVal(AllocFirstBinding).getAsSymbol() == Sym) { + // ...it is the best binding to report. + AllocBindingToReport = AllocFirstBinding; + return; + } + + // At this point, we know that the original region doesn't contain the leaking + // when the actual leak happens. It means that it can be confusing for the + // user to see such description in the message. + // + // Let's consider the following example: + // Object *Original = allocate(...); + // Object *New = Original; + // Original = allocate(...); + // Original->release(); + // + // Complaining about a leaking object "stored into Original" might cause a + // rightful confusion because 'Original' is actually released. + // We should complain about 'New' instead. + Bindings AllVarBindings = + getAllVarBindingsForSymbol(Ctx.getStateManager(), Node, Sym); + + // While looking for the last var bindings, we can still find + // `AllocFirstBinding` to be one of them. In situations like this, + // it would still be the easiest case to explain to our users. + if (!AllVarBindings.empty() && + llvm::count_if(AllVarBindings, + [this](const std::pair<const MemRegion *, SVal> Binding) { + return Binding.first == AllocFirstBinding; + }) == 0) { + // Let's pick one of them at random (if there is something to pick from). + AllocBindingToReport = AllVarBindings[0].first; + + // Because 'AllocBindingToReport' is not the the same as + // 'AllocFirstBinding', we need to explain how the leaking object + // got from one to another. + // + // NOTE: We use the actual SVal stored in AllocBindingToReport here because + // trackStoredValue compares SVal's and it can get trickier for + // something like derived regions if we want to construct SVal from + // Sym. Instead, we take the value that is definitely stored in that + // region, thus guaranteeing that trackStoredValue will work. + bugreporter::trackStoredValue(AllVarBindings[0].second.castAs<KnownSVal>(), + AllocBindingToReport, *this); + } else { + AllocBindingToReport = AllocFirstBinding; + } +} + RefLeakReport::RefLeakReport(const RefCountBug &D, const LangOptions &LOpts, - ExplodedNode *n, SymbolRef sym, + ExplodedNode *N, SymbolRef Sym, CheckerContext &Ctx) - : RefCountReport(D, LOpts, n, sym, /*isLeak=*/true) { + : RefCountReport(D, LOpts, N, Sym, /*isLeak=*/true) { + + deriveAllocLocation(Ctx); + findBindingToReport(Ctx, N); - deriveAllocLocation(Ctx, sym); - if (!AllocBinding) - deriveParamLocation(Ctx, sym); + if (!AllocFirstBinding) + deriveParamLocation(Ctx); createDescription(Ctx); - addVisitor(std::make_unique<RefLeakReportVisitor>(sym)); + addVisitor<RefLeakReportVisitor>(Sym, AllocBindingToReport); } diff --git a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h index 286a8ae2ef7d..d05900895c6a 100644 --- a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h +++ b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h @@ -68,17 +68,20 @@ public: }; class RefLeakReport : public RefCountReport { - const MemRegion* AllocBinding; - const Stmt *AllocStmt; + const MemRegion *AllocFirstBinding = nullptr; + const MemRegion *AllocBindingToReport = nullptr; + const Stmt *AllocStmt = nullptr; PathDiagnosticLocation Location; // Finds the function declaration where a leak warning for the parameter // 'sym' should be raised. - void deriveParamLocation(CheckerContext &Ctx, SymbolRef sym); - // Finds the location where a leak warning for 'sym' should be raised. - void deriveAllocLocation(CheckerContext &Ctx, SymbolRef sym); + void deriveParamLocation(CheckerContext &Ctx); + // Finds the location where the leaking object is allocated. + void deriveAllocLocation(CheckerContext &Ctx); // Produces description of a leak warning which is printed on the console. void createDescription(CheckerContext &Ctx); + // Finds the binding that we should use in a leak warning. + void findBindingToReport(CheckerContext &Ctx, ExplodedNode *Node); public: RefLeakReport(const RefCountBug &D, const LangOptions &LOpts, ExplodedNode *n, diff --git a/clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp index 1a94ccdc2825..885750218b9e 100644 --- a/clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp @@ -16,7 +16,7 @@ #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/CheckerManager.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" -#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h" using namespace clang; diff --git a/clang/lib/StaticAnalyzer/Checkers/RunLoopAutoreleaseLeakChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/RunLoopAutoreleaseLeakChecker.cpp index d9dc72ddaa21..2cf6c6ff47f1 100644 --- a/clang/lib/StaticAnalyzer/Checkers/RunLoopAutoreleaseLeakChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/RunLoopAutoreleaseLeakChecker.cpp @@ -57,8 +57,8 @@ public: } // end anonymous namespace -/// \return Whether {@code A} occurs before {@code B} in traversal of -/// {@code Parent}. +/// \return Whether @c A occurs before @c B in traversal of +/// @c Parent. /// Conceptually a very incomplete/unsound approximation of happens-before /// relationship (A is likely to be evaluated before B), /// but useful enough in this case. diff --git a/clang/lib/StaticAnalyzer/Checkers/SmartPtr.h b/clang/lib/StaticAnalyzer/Checkers/SmartPtr.h index 92c386bbb2b0..6a40f8eda5fa 100644 --- a/clang/lib/StaticAnalyzer/Checkers/SmartPtr.h +++ b/clang/lib/StaticAnalyzer/Checkers/SmartPtr.h @@ -22,6 +22,10 @@ namespace smartptr { /// Returns true if the event call is on smart pointer. bool isStdSmartPtrCall(const CallEvent &Call); +bool isStdSmartPtr(const CXXRecordDecl *RD); +bool isStdSmartPtr(const Expr *E); + +bool isStdSmartPtr(const CXXRecordDecl *RD); /// Returns whether the smart pointer is null or not. bool isNullSmartPtr(const ProgramStateRef State, const MemRegion *ThisRegion); diff --git a/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp index 6ee7bd9252b3..09e885e8133f 100644 --- a/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp @@ -25,16 +25,20 @@ #include "clang/StaticAnalyzer/Core/CheckerManager.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h" #include "clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h" #include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h" #include "clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h" #include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h" +#include "llvm/ADT/StringMap.h" +#include "llvm/Support/ErrorHandling.h" #include <string> using namespace clang; using namespace ento; namespace { + class SmartPtrModeling : public Checker<eval::Call, check::DeadSymbols, check::RegionChanges, check::LiveSymbols> { @@ -60,7 +64,7 @@ public: private: void handleReset(const CallEvent &Call, CheckerContext &C) const; void handleRelease(const CallEvent &Call, CheckerContext &C) const; - void handleSwap(const CallEvent &Call, CheckerContext &C) const; + void handleSwapMethod(const CallEvent &Call, CheckerContext &C) const; void handleGet(const CallEvent &Call, CheckerContext &C) const; bool handleAssignOp(const CallEvent &Call, CheckerContext &C) const; bool handleMoveCtr(const CallEvent &Call, CheckerContext &C, @@ -68,19 +72,56 @@ private: bool updateMovedSmartPointers(CheckerContext &C, const MemRegion *ThisRegion, const MemRegion *OtherSmartPtrRegion) const; void handleBoolConversion(const CallEvent &Call, CheckerContext &C) const; + bool handleComparisionOp(const CallEvent &Call, CheckerContext &C) const; + bool handleOstreamOperator(const CallEvent &Call, CheckerContext &C) const; + bool handleSwap(ProgramStateRef State, SVal First, SVal Second, + CheckerContext &C) const; + std::pair<SVal, ProgramStateRef> + retrieveOrConjureInnerPtrVal(ProgramStateRef State, + const MemRegion *ThisRegion, const Expr *E, + QualType Type, CheckerContext &C) const; using SmartPtrMethodHandlerFn = void (SmartPtrModeling::*)(const CallEvent &Call, CheckerContext &) const; CallDescriptionMap<SmartPtrMethodHandlerFn> SmartPtrMethodHandlers{ {{"reset"}, &SmartPtrModeling::handleReset}, {{"release"}, &SmartPtrModeling::handleRelease}, - {{"swap", 1}, &SmartPtrModeling::handleSwap}, + {{"swap", 1}, &SmartPtrModeling::handleSwapMethod}, {{"get"}, &SmartPtrModeling::handleGet}}; + const CallDescription StdSwapCall{{"std", "swap"}, 2}; + const CallDescription StdMakeUniqueCall{{"std", "make_unique"}}; + const CallDescription StdMakeUniqueForOverwriteCall{ + {"std", "make_unique_for_overwrite"}}; }; } // end of anonymous namespace REGISTER_MAP_WITH_PROGRAMSTATE(TrackedRegionMap, const MemRegion *, SVal) +// Checks if RD has name in Names and is in std namespace +static bool hasStdClassWithName(const CXXRecordDecl *RD, + ArrayRef<llvm::StringLiteral> Names) { + if (!RD || !RD->getDeclContext()->isStdNamespace()) + return false; + if (RD->getDeclName().isIdentifier()) { + StringRef Name = RD->getName(); + return llvm::any_of(Names, [&Name](StringRef GivenName) -> bool { + return Name == GivenName; + }); + } + return false; +} + +constexpr llvm::StringLiteral STD_PTR_NAMES[] = {"shared_ptr", "unique_ptr", + "weak_ptr"}; + +static bool isStdSmartPtr(const CXXRecordDecl *RD) { + return hasStdClassWithName(RD, STD_PTR_NAMES); +} + +static bool isStdSmartPtr(const Expr *E) { + return isStdSmartPtr(E->getType()->getAsCXXRecordDecl()); +} + // Define the inter-checker API. namespace clang { namespace ento { @@ -89,18 +130,24 @@ bool isStdSmartPtrCall(const CallEvent &Call) { const auto *MethodDecl = dyn_cast_or_null<CXXMethodDecl>(Call.getDecl()); if (!MethodDecl || !MethodDecl->getParent()) return false; + return isStdSmartPtr(MethodDecl->getParent()); +} - const auto *RecordDecl = MethodDecl->getParent(); - if (!RecordDecl || !RecordDecl->getDeclContext()->isStdNamespace()) +bool isStdSmartPtr(const CXXRecordDecl *RD) { + if (!RD || !RD->getDeclContext()->isStdNamespace()) return false; - if (RecordDecl->getDeclName().isIdentifier()) { - StringRef Name = RecordDecl->getName(); + if (RD->getDeclName().isIdentifier()) { + StringRef Name = RD->getName(); return Name == "shared_ptr" || Name == "unique_ptr" || Name == "weak_ptr"; } return false; } +bool isStdSmartPtr(const Expr *E) { + return isStdSmartPtr(E->getType()->getAsCXXRecordDecl()); +} + bool isNullSmartPtr(const ProgramStateRef State, const MemRegion *ThisRegion) { const auto *InnerPointVal = State->get<TrackedRegionMap>(ThisRegion); return InnerPointVal && @@ -135,28 +182,47 @@ static ProgramStateRef updateSwappedRegion(ProgramStateRef State, return State; } -// Helper method to get the inner pointer type of specialized smart pointer -// Returns empty type if not found valid inner pointer type. -static QualType getInnerPointerType(const CallEvent &Call, CheckerContext &C) { - const auto *MethodDecl = dyn_cast_or_null<CXXMethodDecl>(Call.getDecl()); - if (!MethodDecl || !MethodDecl->getParent()) - return {}; - - const auto *RecordDecl = MethodDecl->getParent(); - if (!RecordDecl || !RecordDecl->isInStdNamespace()) +static QualType getInnerPointerType(CheckerContext C, const CXXRecordDecl *RD) { + if (!RD || !RD->isInStdNamespace()) return {}; - const auto *TSD = dyn_cast<ClassTemplateSpecializationDecl>(RecordDecl); + const auto *TSD = dyn_cast<ClassTemplateSpecializationDecl>(RD); if (!TSD) return {}; auto TemplateArgs = TSD->getTemplateArgs().asArray(); - if (TemplateArgs.size() == 0) + if (TemplateArgs.empty()) return {}; auto InnerValueType = TemplateArgs[0].getAsType(); return C.getASTContext().getPointerType(InnerValueType.getCanonicalType()); } +// This is for use with standalone-functions like std::make_unique, +// std::make_unique_for_overwrite, etc. It reads the template parameter and +// returns the pointer type corresponding to it, +static QualType getPointerTypeFromTemplateArg(const CallEvent &Call, + CheckerContext &C) { + const auto *FD = dyn_cast_or_null<FunctionDecl>(Call.getDecl()); + if (!FD || !FD->isFunctionTemplateSpecialization()) + return {}; + const auto &TemplateArgs = FD->getTemplateSpecializationArgs()->asArray(); + if (TemplateArgs.size() == 0) + return {}; + auto ValueType = TemplateArgs[0].getAsType(); + return C.getASTContext().getPointerType(ValueType.getCanonicalType()); +} + +// Helper method to get the inner pointer type of specialized smart pointer +// Returns empty type if not found valid inner pointer type. +static QualType getInnerPointerType(const CallEvent &Call, CheckerContext &C) { + const auto *MethodDecl = dyn_cast_or_null<CXXMethodDecl>(Call.getDecl()); + if (!MethodDecl || !MethodDecl->getParent()) + return {}; + + const auto *RecordDecl = MethodDecl->getParent(); + return getInnerPointerType(C, RecordDecl); +} + // Helper method to pretty print region and avoid extra spacing. static void checkAndPrettyPrintRegion(llvm::raw_ostream &OS, const MemRegion *Region) { @@ -175,9 +241,107 @@ bool SmartPtrModeling::isBoolConversionMethod(const CallEvent &Call) const { return CD && CD->getConversionType()->isBooleanType(); } +constexpr llvm::StringLiteral BASIC_OSTREAM_NAMES[] = {"basic_ostream"}; + +bool isStdBasicOstream(const Expr *E) { + const auto *RD = E->getType()->getAsCXXRecordDecl(); + return hasStdClassWithName(RD, BASIC_OSTREAM_NAMES); +} + +static bool isStdFunctionCall(const CallEvent &Call) { + return Call.getDecl() && Call.getDecl()->getDeclContext()->isStdNamespace(); +} + +bool isStdOstreamOperatorCall(const CallEvent &Call) { + if (Call.getNumArgs() != 2 || !isStdFunctionCall(Call)) + return false; + const auto *FC = dyn_cast<SimpleFunctionCall>(&Call); + if (!FC) + return false; + const FunctionDecl *FD = FC->getDecl(); + if (!FD->isOverloadedOperator()) + return false; + const OverloadedOperatorKind OOK = FD->getOverloadedOperator(); + if (OOK != clang::OO_LessLess) + return false; + return isStdSmartPtr(Call.getArgExpr(1)) && + isStdBasicOstream(Call.getArgExpr(0)); +} + +static bool isPotentiallyComparisionOpCall(const CallEvent &Call) { + if (Call.getNumArgs() != 2 || !isStdFunctionCall(Call)) + return false; + return smartptr::isStdSmartPtr(Call.getArgExpr(0)) || + smartptr::isStdSmartPtr(Call.getArgExpr(1)); +} + bool SmartPtrModeling::evalCall(const CallEvent &Call, CheckerContext &C) const { + ProgramStateRef State = C.getState(); + + // If any one of the arg is a unique_ptr, then + // we can try this function + if (ModelSmartPtrDereference && isPotentiallyComparisionOpCall(Call)) + if (handleComparisionOp(Call, C)) + return true; + + if (ModelSmartPtrDereference && isStdOstreamOperatorCall(Call)) + return handleOstreamOperator(Call, C); + + if (Call.isCalled(StdSwapCall)) { + // Check the first arg, if it is of std::unique_ptr type. + assert(Call.getNumArgs() == 2 && "std::swap should have two arguments"); + const Expr *FirstArg = Call.getArgExpr(0); + if (!smartptr::isStdSmartPtr(FirstArg->getType()->getAsCXXRecordDecl())) + return false; + return handleSwap(State, Call.getArgSVal(0), Call.getArgSVal(1), C); + } + + if (Call.isCalled(StdMakeUniqueCall) || + Call.isCalled(StdMakeUniqueForOverwriteCall)) { + if (!ModelSmartPtrDereference) + return false; + + const Optional<SVal> ThisRegionOpt = Call.getReturnValueUnderConstruction(); + if (!ThisRegionOpt) + return false; + + const auto PtrVal = C.getSValBuilder().getConjuredHeapSymbolVal( + Call.getOriginExpr(), C.getLocationContext(), + getPointerTypeFromTemplateArg(Call, C), C.blockCount()); + + const MemRegion *ThisRegion = ThisRegionOpt->getAsRegion(); + State = State->set<TrackedRegionMap>(ThisRegion, PtrVal); + State = State->assume(PtrVal, true); + + // TODO: ExprEngine should do this for us. + // For a bit more context: + // 1) Why do we need this? Since we are modelling a "function" + // that returns a constructed object we need to store this information in + // the program state. + // + // 2) Why does this work? + // `updateObjectsUnderConstruction` does exactly as it sounds. + // + // 3) How should it look like when moved to the Engine? + // It would be nice if we can just + // pretend we don't need to know about this - ie, completely automatic work. + // However, realistically speaking, I think we would need to "signal" the + // ExprEngine evalCall handler that we are constructing an object with this + // function call (constructors obviously construct, hence can be + // automatically deduced). + auto &Engine = State->getStateManager().getOwningEngine(); + State = Engine.updateObjectsUnderConstruction( + *ThisRegionOpt, nullptr, State, C.getLocationContext(), + Call.getConstructionContext(), {}); + + // We don't leave a note here since it is guaranteed the + // unique_ptr from this call is non-null (hence is safe to de-reference). + C.addTransition(State); + return true; + } + if (!smartptr::isStdSmartPtrCall(Call)) return false; @@ -272,6 +436,108 @@ bool SmartPtrModeling::evalCall(const CallEvent &Call, return C.isDifferent(); } +std::pair<SVal, ProgramStateRef> SmartPtrModeling::retrieveOrConjureInnerPtrVal( + ProgramStateRef State, const MemRegion *ThisRegion, const Expr *E, + QualType Type, CheckerContext &C) const { + const auto *Ptr = State->get<TrackedRegionMap>(ThisRegion); + if (Ptr) + return {*Ptr, State}; + auto Val = C.getSValBuilder().conjureSymbolVal(E, C.getLocationContext(), + Type, C.blockCount()); + State = State->set<TrackedRegionMap>(ThisRegion, Val); + return {Val, State}; +} + +bool SmartPtrModeling::handleComparisionOp(const CallEvent &Call, + CheckerContext &C) const { + const auto *FC = dyn_cast<SimpleFunctionCall>(&Call); + if (!FC) + return false; + const FunctionDecl *FD = FC->getDecl(); + if (!FD->isOverloadedOperator()) + return false; + const OverloadedOperatorKind OOK = FD->getOverloadedOperator(); + if (!(OOK == OO_EqualEqual || OOK == OO_ExclaimEqual || OOK == OO_Less || + OOK == OO_LessEqual || OOK == OO_Greater || OOK == OO_GreaterEqual || + OOK == OO_Spaceship)) + return false; + + // There are some special cases about which we can infer about + // the resulting answer. + // For reference, there is a discussion at https://reviews.llvm.org/D104616. + // Also, the cppreference page is good to look at + // https://en.cppreference.com/w/cpp/memory/unique_ptr/operator_cmp. + + auto makeSValFor = [&C, this](ProgramStateRef State, const Expr *E, + SVal S) -> std::pair<SVal, ProgramStateRef> { + if (S.isZeroConstant()) { + return {S, State}; + } + const MemRegion *Reg = S.getAsRegion(); + assert(Reg && + "this pointer of std::unique_ptr should be obtainable as MemRegion"); + QualType Type = getInnerPointerType(C, E->getType()->getAsCXXRecordDecl()); + return retrieveOrConjureInnerPtrVal(State, Reg, E, Type, C); + }; + + SVal First = Call.getArgSVal(0); + SVal Second = Call.getArgSVal(1); + const auto *FirstExpr = Call.getArgExpr(0); + const auto *SecondExpr = Call.getArgExpr(1); + + const auto *ResultExpr = Call.getOriginExpr(); + const auto *LCtx = C.getLocationContext(); + auto &Bldr = C.getSValBuilder(); + ProgramStateRef State = C.getState(); + + SVal FirstPtrVal, SecondPtrVal; + std::tie(FirstPtrVal, State) = makeSValFor(State, FirstExpr, First); + std::tie(SecondPtrVal, State) = makeSValFor(State, SecondExpr, Second); + BinaryOperatorKind BOK = + operationKindFromOverloadedOperator(OOK, true).GetBinaryOpUnsafe(); + auto RetVal = Bldr.evalBinOp(State, BOK, FirstPtrVal, SecondPtrVal, + Call.getResultType()); + + if (OOK != OO_Spaceship) { + ProgramStateRef TrueState, FalseState; + std::tie(TrueState, FalseState) = + State->assume(*RetVal.getAs<DefinedOrUnknownSVal>()); + if (TrueState) + C.addTransition( + TrueState->BindExpr(ResultExpr, LCtx, Bldr.makeTruthVal(true))); + if (FalseState) + C.addTransition( + FalseState->BindExpr(ResultExpr, LCtx, Bldr.makeTruthVal(false))); + } else { + C.addTransition(State->BindExpr(ResultExpr, LCtx, RetVal)); + } + return true; +} + +bool SmartPtrModeling::handleOstreamOperator(const CallEvent &Call, + CheckerContext &C) const { + // operator<< does not modify the smart pointer. + // And we don't really have much of modelling of basic_ostream. + // So, we are better off: + // 1) Invalidating the mem-region of the ostream object at hand. + // 2) Setting the SVal of the basic_ostream as the return value. + // Not very satisfying, but it gets the job done, and is better + // than the default handling. :) + + ProgramStateRef State = C.getState(); + const auto StreamVal = Call.getArgSVal(0); + const MemRegion *StreamThisRegion = StreamVal.getAsRegion(); + if (!StreamThisRegion) + return false; + State = + State->invalidateRegions({StreamThisRegion}, Call.getOriginExpr(), + C.blockCount(), C.getLocationContext(), false); + State = + State->BindExpr(Call.getOriginExpr(), C.getLocationContext(), StreamVal); + C.addTransition(State); + return true; +} + void SmartPtrModeling::checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const { ProgramStateRef State = C.getState(); @@ -395,43 +661,52 @@ void SmartPtrModeling::handleRelease(const CallEvent &Call, // pointer. } -void SmartPtrModeling::handleSwap(const CallEvent &Call, - CheckerContext &C) const { +void SmartPtrModeling::handleSwapMethod(const CallEvent &Call, + CheckerContext &C) const { // To model unique_ptr::swap() method. const auto *IC = dyn_cast<CXXInstanceCall>(&Call); if (!IC) return; - const MemRegion *ThisRegion = IC->getCXXThisVal().getAsRegion(); - if (!ThisRegion) - return; + auto State = C.getState(); + handleSwap(State, IC->getCXXThisVal(), Call.getArgSVal(0), C); +} - const auto *ArgRegion = Call.getArgSVal(0).getAsRegion(); - if (!ArgRegion) - return; +bool SmartPtrModeling::handleSwap(ProgramStateRef State, SVal First, + SVal Second, CheckerContext &C) const { + const MemRegion *FirstThisRegion = First.getAsRegion(); + if (!FirstThisRegion) + return false; + const MemRegion *SecondThisRegion = Second.getAsRegion(); + if (!SecondThisRegion) + return false; - auto State = C.getState(); - const auto *ThisRegionInnerPointerVal = - State->get<TrackedRegionMap>(ThisRegion); - const auto *ArgRegionInnerPointerVal = - State->get<TrackedRegionMap>(ArgRegion); + const auto *FirstInnerPtrVal = State->get<TrackedRegionMap>(FirstThisRegion); + const auto *SecondInnerPtrVal = + State->get<TrackedRegionMap>(SecondThisRegion); - // Swap the tracked region values. - State = updateSwappedRegion(State, ThisRegion, ArgRegionInnerPointerVal); - State = updateSwappedRegion(State, ArgRegion, ThisRegionInnerPointerVal); + State = updateSwappedRegion(State, FirstThisRegion, SecondInnerPtrVal); + State = updateSwappedRegion(State, SecondThisRegion, FirstInnerPtrVal); - C.addTransition( - State, C.getNoteTag([ThisRegion, ArgRegion](PathSensitiveBugReport &BR, - llvm::raw_ostream &OS) { - if (&BR.getBugType() != smartptr::getNullDereferenceBugType() || - !BR.isInteresting(ThisRegion)) - return; - BR.markInteresting(ArgRegion); - OS << "Swapped null smart pointer"; - checkAndPrettyPrintRegion(OS, ArgRegion); - OS << " with smart pointer"; - checkAndPrettyPrintRegion(OS, ThisRegion); - })); + C.addTransition(State, C.getNoteTag([FirstThisRegion, SecondThisRegion]( + PathSensitiveBugReport &BR, + llvm::raw_ostream &OS) { + if (&BR.getBugType() != smartptr::getNullDereferenceBugType()) + return; + if (BR.isInteresting(FirstThisRegion) && + !BR.isInteresting(SecondThisRegion)) { + BR.markInteresting(SecondThisRegion); + BR.markNotInteresting(FirstThisRegion); + } + if (BR.isInteresting(SecondThisRegion) && + !BR.isInteresting(FirstThisRegion)) { + BR.markInteresting(FirstThisRegion); + BR.markNotInteresting(SecondThisRegion); + } + // TODO: We need to emit some note here probably!! + })); + + return true; } void SmartPtrModeling::handleGet(const CallEvent &Call, @@ -446,15 +721,8 @@ void SmartPtrModeling::handleGet(const CallEvent &Call, return; SVal InnerPointerVal; - if (const auto *InnerValPtr = State->get<TrackedRegionMap>(ThisRegion)) { - InnerPointerVal = *InnerValPtr; - } else { - const auto *CallExpr = Call.getOriginExpr(); - InnerPointerVal = C.getSValBuilder().conjureSymbolVal( - CallExpr, C.getLocationContext(), Call.getResultType(), C.blockCount()); - State = State->set<TrackedRegionMap>(ThisRegion, InnerPointerVal); - } - + std::tie(InnerPointerVal, State) = retrieveOrConjureInnerPtrVal( + State, ThisRegion, Call.getOriginExpr(), Call.getResultType(), C); State = State->BindExpr(Call.getOriginExpr(), C.getLocationContext(), InnerPointerVal); // TODO: Add NoteTag, for how the raw pointer got using 'get' method. diff --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp index d1c366a94fac..e758b465af1b 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp @@ -56,7 +56,11 @@ #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h" -#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h" +#include "llvm/ADT/SmallString.h" +#include "llvm/ADT/StringExtras.h" + +#include <string> using namespace clang; using namespace clang::ento; @@ -87,6 +91,10 @@ class StdLibraryFunctionsChecker typedef uint32_t ArgNo; static const ArgNo Ret; + /// Returns the string representation of an argument index. + /// E.g.: (1) -> '1st arg', (2) - > '2nd arg' + static SmallString<8> getArgDesc(ArgNo); + class ValueConstraint; // Pointer to the ValueConstraint. We need a copyable, polymorphic and @@ -126,8 +134,24 @@ class StdLibraryFunctionsChecker } ArgNo getArgNo() const { return ArgN; } + // Return those arguments that should be tracked when we report a bug. By + // default it is the argument that is constrained, however, in some special + // cases we need to track other arguments as well. E.g. a buffer size might + // be encoded in another argument. + virtual std::vector<ArgNo> getArgsToTrack() const { return {ArgN}; } + virtual StringRef getName() const = 0; + // Give a description that explains the constraint to the user. Used when + // the bug is reported. + virtual std::string describe(ProgramStateRef State, + const Summary &Summary) const { + // There are some descendant classes that are not used as argument + // constraints, e.g. ComparisonConstraint. In that case we can safely + // ignore the implementation of this function. + llvm_unreachable("Not implemented"); + } + protected: ArgNo ArgN; // Argument to which we apply the constraint. @@ -158,6 +182,9 @@ class StdLibraryFunctionsChecker RangeConstraint(ArgNo ArgN, RangeKind Kind, const IntRangeVector &Ranges) : ValueConstraint(ArgN), Kind(Kind), Ranges(Ranges) {} + std::string describe(ProgramStateRef State, + const Summary &Summary) const override; + const IntRangeVector &getRanges() const { return Ranges; } private: @@ -225,6 +252,8 @@ class StdLibraryFunctionsChecker bool CannotBeNull = true; public: + std::string describe(ProgramStateRef State, + const Summary &Summary) const override; StringRef getName() const override { return "NonNull"; } ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call, const Summary &Summary, @@ -286,6 +315,18 @@ class StdLibraryFunctionsChecker : ValueConstraint(Buffer), SizeArgN(BufSize), SizeMultiplierArgN(BufSizeMultiplier) {} + std::vector<ArgNo> getArgsToTrack() const override { + std::vector<ArgNo> Result{ArgN}; + if (SizeArgN) + Result.push_back(*SizeArgN); + if (SizeMultiplierArgN) + Result.push_back(*SizeMultiplierArgN); + return Result; + } + + std::string describe(ProgramStateRef State, + const Summary &Summary) const override; + ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call, const Summary &Summary, CheckerContext &C) const override { @@ -297,24 +338,22 @@ class StdLibraryFunctionsChecker const SVal SizeV = [this, &State, &Call, &Summary, &SvalBuilder]() { if (ConcreteSize) { return SVal(SvalBuilder.makeIntVal(*ConcreteSize)); - } else if (SizeArgN) { - // The size argument. - SVal SizeV = getArgSVal(Call, *SizeArgN); - // Multiply with another argument if given. - if (SizeMultiplierArgN) { - SVal SizeMulV = getArgSVal(Call, *SizeMultiplierArgN); - SizeV = SvalBuilder.evalBinOp(State, BO_Mul, SizeV, SizeMulV, - Summary.getArgType(*SizeArgN)); - } - return SizeV; - } else { - llvm_unreachable("The constraint must be either a concrete value or " - "encoded in an arguement."); } + assert(SizeArgN && "The constraint must be either a concrete value or " + "encoded in an argument."); + // The size argument. + SVal SizeV = getArgSVal(Call, *SizeArgN); + // Multiply with another argument if given. + if (SizeMultiplierArgN) { + SVal SizeMulV = getArgSVal(Call, *SizeMultiplierArgN); + SizeV = SvalBuilder.evalBinOp(State, BO_Mul, SizeV, SizeMulV, + Summary.getArgType(*SizeArgN)); + } + return SizeV; }(); // The dynamic size of the buffer argument, got from the analyzer engine. - SVal BufDynSize = getDynamicSizeWithOffset(State, BufV); + SVal BufDynSize = getDynamicExtentWithOffset(State, BufV); SVal Feasible = SvalBuilder.evalBinOp(State, Op, SizeV, BufDynSize, SvalBuilder.getContext().BoolTy); @@ -508,6 +547,7 @@ class StdLibraryFunctionsChecker mutable FunctionSummaryMapType FunctionSummaryMap; mutable std::unique_ptr<BugType> BT_InvalidArg; + mutable bool SummariesInitialized = false; static SVal getArgSVal(const CallEvent &Call, ArgNo ArgN) { return ArgN == Ret ? Call.getReturnValue() : Call.getArgSVal(ArgN); @@ -538,24 +578,30 @@ private: void initFunctionSummaries(CheckerContext &C) const; void reportBug(const CallEvent &Call, ExplodedNode *N, - const ValueConstraint *VC, CheckerContext &C) const { + const ValueConstraint *VC, const Summary &Summary, + CheckerContext &C) const { if (!ChecksEnabled[CK_StdCLibraryFunctionArgsChecker]) return; - // TODO Add more detailed diagnostic. std::string Msg = (Twine("Function argument constraint is not satisfied, constraint: ") + - VC->getName().data() + ", ArgN: " + Twine(VC->getArgNo())) + VC->getName().data()) .str(); if (!BT_InvalidArg) BT_InvalidArg = std::make_unique<BugType>( CheckNames[CK_StdCLibraryFunctionArgsChecker], "Unsatisfied argument constraints", categories::LogicError); auto R = std::make_unique<PathSensitiveBugReport>(*BT_InvalidArg, Msg, N); - bugreporter::trackExpressionValue(N, Call.getArgExpr(VC->getArgNo()), *R); + + for (ArgNo ArgN : VC->getArgsToTrack()) + bugreporter::trackExpressionValue(N, Call.getArgExpr(ArgN), *R); // Highlight the range of the argument that was violated. R->addRange(Call.getArgSourceRange(VC->getArgNo())); + // Describe the argument constraint in a note. + R->addNote(VC->describe(C.getState(), Summary), R->getLocation(), + Call.getArgSourceRange(VC->getArgNo())); + C.emitReport(std::move(R)); } }; @@ -565,6 +611,85 @@ const StdLibraryFunctionsChecker::ArgNo StdLibraryFunctionsChecker::Ret = } // end of anonymous namespace +static BasicValueFactory &getBVF(ProgramStateRef State) { + ProgramStateManager &Mgr = State->getStateManager(); + SValBuilder &SVB = Mgr.getSValBuilder(); + return SVB.getBasicValueFactory(); +} + +std::string StdLibraryFunctionsChecker::NotNullConstraint::describe( + ProgramStateRef State, const Summary &Summary) const { + SmallString<48> Result; + Result += "The "; + Result += getArgDesc(ArgN); + Result += " should not be NULL"; + return Result.c_str(); +} + +std::string StdLibraryFunctionsChecker::RangeConstraint::describe( + ProgramStateRef State, const Summary &Summary) const { + + BasicValueFactory &BVF = getBVF(State); + + QualType T = Summary.getArgType(getArgNo()); + SmallString<48> Result; + Result += "The "; + Result += getArgDesc(ArgN); + Result += " should be "; + + // Range kind as a string. + Kind == OutOfRange ? Result += "out of" : Result += "within"; + + // Get the range values as a string. + Result += " the range "; + if (Ranges.size() > 1) + Result += "["; + unsigned I = Ranges.size(); + for (const std::pair<RangeInt, RangeInt> &R : Ranges) { + Result += "["; + const llvm::APSInt &Min = BVF.getValue(R.first, T); + const llvm::APSInt &Max = BVF.getValue(R.second, T); + Min.toString(Result); + Result += ", "; + Max.toString(Result); + Result += "]"; + if (--I > 0) + Result += ", "; + } + if (Ranges.size() > 1) + Result += "]"; + + return Result.c_str(); +} + +SmallString<8> +StdLibraryFunctionsChecker::getArgDesc(StdLibraryFunctionsChecker::ArgNo ArgN) { + SmallString<8> Result; + Result += std::to_string(ArgN + 1); + Result += llvm::getOrdinalSuffix(ArgN + 1); + Result += " arg"; + return Result; +} + +std::string StdLibraryFunctionsChecker::BufferSizeConstraint::describe( + ProgramStateRef State, const Summary &Summary) const { + SmallString<96> Result; + Result += "The size of the "; + Result += getArgDesc(ArgN); + Result += " should be equal to or less than the value of "; + if (ConcreteSize) { + ConcreteSize->toString(Result); + } else if (SizeArgN) { + Result += "the "; + Result += getArgDesc(*SizeArgN); + if (SizeMultiplierArgN) { + Result += " times the "; + Result += getArgDesc(*SizeMultiplierArgN); + } + } + return Result.c_str(); +} + ProgramStateRef StdLibraryFunctionsChecker::RangeConstraint::applyAsOutOfRange( ProgramStateRef State, const CallEvent &Call, const Summary &Summary) const { @@ -692,7 +817,7 @@ void StdLibraryFunctionsChecker::checkPreCall(const CallEvent &Call, // The argument constraint is not satisfied. if (FailureSt && !SuccessSt) { if (ExplodedNode *N = C.generateErrorNode(NewState)) - reportBug(Call, N, Constraint.get(), C); + reportBug(Call, N, Constraint.get(), Summary, C); break; } else { // We will apply the constraint even if we cannot reason about the @@ -823,7 +948,7 @@ StdLibraryFunctionsChecker::findFunctionSummary(const CallEvent &Call, void StdLibraryFunctionsChecker::initFunctionSummaries( CheckerContext &C) const { - if (!FunctionSummaryMap.empty()) + if (SummariesInitialized) return; SValBuilder &SVB = C.getSValBuilder(); @@ -841,7 +966,7 @@ void StdLibraryFunctionsChecker::initFunctionSummaries( llvm::Optional<QualType> operator()(StringRef Name) { IdentifierInfo &II = ACtx.Idents.get(Name); auto LookupRes = ACtx.getTranslationUnitDecl()->lookup(&II); - if (LookupRes.size() == 0) + if (LookupRes.empty()) return None; // Prioritze typedef declarations. @@ -993,7 +1118,7 @@ void StdLibraryFunctionsChecker::initFunctionSummaries( return false; IdentifierInfo &II = ACtx.Idents.get(Name); auto LookupRes = ACtx.getTranslationUnitDecl()->lookup(&II); - if (LookupRes.size() == 0) + if (LookupRes.empty()) return false; for (Decl *D : LookupRes) { if (auto *FD = dyn_cast<FunctionDecl>(D)) { @@ -2441,6 +2566,35 @@ void StdLibraryFunctionsChecker::initFunctionSummaries( // Functions for testing. if (ChecksEnabled[CK_StdCLibraryFunctionsTesterChecker]) { addToFunctionSummaryMap( + "__not_null", Signature(ArgTypes{IntPtrTy}, RetType{IntTy}), + Summary(EvalCallAsPure).ArgConstraint(NotNull(ArgNo(0)))); + + // Test range values. + addToFunctionSummaryMap( + "__single_val_1", Signature(ArgTypes{IntTy}, RetType{IntTy}), + Summary(EvalCallAsPure) + .ArgConstraint(ArgumentCondition(0U, WithinRange, SingleValue(1)))); + addToFunctionSummaryMap( + "__range_1_2", Signature(ArgTypes{IntTy}, RetType{IntTy}), + Summary(EvalCallAsPure) + .ArgConstraint(ArgumentCondition(0U, WithinRange, Range(1, 2)))); + addToFunctionSummaryMap("__range_1_2__4_5", + Signature(ArgTypes{IntTy}, RetType{IntTy}), + Summary(EvalCallAsPure) + .ArgConstraint(ArgumentCondition( + 0U, WithinRange, Range({1, 2}, {4, 5})))); + + // Test range kind. + addToFunctionSummaryMap( + "__within", Signature(ArgTypes{IntTy}, RetType{IntTy}), + Summary(EvalCallAsPure) + .ArgConstraint(ArgumentCondition(0U, WithinRange, SingleValue(1)))); + addToFunctionSummaryMap( + "__out_of", Signature(ArgTypes{IntTy}, RetType{IntTy}), + Summary(EvalCallAsPure) + .ArgConstraint(ArgumentCondition(0U, OutOfRange, SingleValue(1)))); + + addToFunctionSummaryMap( "__two_constrained_args", Signature(ArgTypes{IntTy, IntTy}, RetType{IntTy}), Summary(EvalCallAsPure) @@ -2485,6 +2639,8 @@ void StdLibraryFunctionsChecker::initFunctionSummaries( Signature(ArgTypes{VoidPtrRestrictTy}, RetType{VoidTy}), Summary(EvalCallAsPure)); } + + SummariesInitialized = true; } void ento::registerStdCLibraryFunctionsChecker(CheckerManager &mgr) { diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp index 6b176b3c4e2b..dd65f8c035aa 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp @@ -25,6 +25,10 @@ using namespace clang; using namespace ento; using namespace std::placeholders; +//===----------------------------------------------------------------------===// +// Definition of state data structures. +//===----------------------------------------------------------------------===// + namespace { struct FnDescription; @@ -146,6 +150,14 @@ struct StreamState { } }; +} // namespace + +//===----------------------------------------------------------------------===// +// StreamChecker class and utility functions. +//===----------------------------------------------------------------------===// + +namespace { + class StreamChecker; using FnCheck = std::function<void(const StreamChecker *, const FnDescription *, const CallEvent &, CheckerContext &)>; @@ -219,6 +231,8 @@ public: /// If true, evaluate special testing stream functions. bool TestMode = false; + const BugType *getBT_StreamEof() const { return &BT_StreamEof; } + private: CallDescriptionMap<FnDescription> FnDescriptions = { {{"fopen"}, {nullptr, &StreamChecker::evalFopen, ArgNone}}, @@ -306,7 +320,8 @@ private: /// If it can only be NULL a fatal error is emitted and nullptr returned. /// Otherwise the return value is a new state where the stream is constrained /// to be non-null. - ProgramStateRef ensureStreamNonNull(SVal StreamVal, CheckerContext &C, + ProgramStateRef ensureStreamNonNull(SVal StreamVal, const Expr *StreamE, + CheckerContext &C, ProgramStateRef State) const; /// Check that the stream is the opened state. @@ -336,7 +351,8 @@ private: /// There will be always a state transition into the passed State, /// by the new non-fatal error node or (if failed) a normal transition, /// to ensure uniform handling. - void reportFEofWarning(CheckerContext &C, ProgramStateRef State) const; + void reportFEofWarning(SymbolRef StreamSym, CheckerContext &C, + ProgramStateRef State) const; /// Emit resource leak warnings for the given symbols. /// Createn a non-fatal error node for these, and returns it (if any warnings @@ -362,14 +378,14 @@ private: /// Generate a message for BugReporterVisitor if the stored symbol is /// marked as interesting by the actual bug report. + // FIXME: Use lambda instead. struct NoteFn { - const CheckerNameRef CheckerName; + const BugType *BT_ResourceLeak; SymbolRef StreamSym; std::string Message; std::string operator()(PathSensitiveBugReport &BR) const { - if (BR.isInteresting(StreamSym) && - CheckerName == BR.getBugType().getCheckerName()) + if (BR.isInteresting(StreamSym) && &BR.getBugType() == BT_ResourceLeak) return Message; return ""; @@ -378,7 +394,20 @@ private: const NoteTag *constructNoteTag(CheckerContext &C, SymbolRef StreamSym, const std::string &Message) const { - return C.getNoteTag(NoteFn{getCheckerName(), StreamSym, Message}); + return C.getNoteTag(NoteFn{&BT_ResourceLeak, StreamSym, Message}); + } + + const NoteTag *constructSetEofNoteTag(CheckerContext &C, + SymbolRef StreamSym) const { + return C.getNoteTag([this, StreamSym](PathSensitiveBugReport &BR) { + if (!BR.isInteresting(StreamSym) || + &BR.getBugType() != this->getBT_StreamEof()) + return ""; + + BR.markNotInteresting(StreamSym); + + return "Assuming stream reaches end-of-file here"; + }); } /// Searches for the ExplodedNode where the file descriptor was acquired for @@ -390,6 +419,9 @@ private: } // end anonymous namespace +// This map holds the state of a stream. +// The stream is identified with a SymbolRef that is created when a stream +// opening function is modeled by the checker. REGISTER_MAP_WITH_PROGRAMSTATE(StreamMap, SymbolRef, StreamState) inline void assertStreamStateOpened(const StreamState *SS) { @@ -418,6 +450,10 @@ const ExplodedNode *StreamChecker::getAcquisitionSite(const ExplodedNode *N, return nullptr; } +//===----------------------------------------------------------------------===// +// Methods of StreamChecker. +//===----------------------------------------------------------------------===// + void StreamChecker::checkPreCall(const CallEvent &Call, CheckerContext &C) const { const FnDescription *Desc = lookupFn(Call); @@ -472,7 +508,8 @@ void StreamChecker::preFreopen(const FnDescription *Desc, const CallEvent &Call, CheckerContext &C) const { // Do not allow NULL as passed stream pointer but allow a closed stream. ProgramStateRef State = C.getState(); - State = ensureStreamNonNull(getStreamArg(Desc, Call), C, State); + State = ensureStreamNonNull(getStreamArg(Desc, Call), + Call.getArgExpr(Desc->StreamArgNo), C, State); if (!State) return; @@ -549,7 +586,8 @@ void StreamChecker::preFread(const FnDescription *Desc, const CallEvent &Call, CheckerContext &C) const { ProgramStateRef State = C.getState(); SVal StreamVal = getStreamArg(Desc, Call); - State = ensureStreamNonNull(StreamVal, C, State); + State = ensureStreamNonNull(StreamVal, Call.getArgExpr(Desc->StreamArgNo), C, + State); if (!State) return; State = ensureStreamOpened(StreamVal, C, State); @@ -563,7 +601,7 @@ void StreamChecker::preFread(const FnDescription *Desc, const CallEvent &Call, if (Sym && State->get<StreamMap>(Sym)) { const StreamState *SS = State->get<StreamMap>(Sym); if (SS->ErrorState & ErrorFEof) - reportFEofWarning(C, State); + reportFEofWarning(Sym, C, State); } else { C.addTransition(State); } @@ -573,7 +611,8 @@ void StreamChecker::preFwrite(const FnDescription *Desc, const CallEvent &Call, CheckerContext &C) const { ProgramStateRef State = C.getState(); SVal StreamVal = getStreamArg(Desc, Call); - State = ensureStreamNonNull(StreamVal, C, State); + State = ensureStreamNonNull(StreamVal, Call.getArgExpr(Desc->StreamArgNo), C, + State); if (!State) return; State = ensureStreamOpened(StreamVal, C, State); @@ -605,11 +644,11 @@ void StreamChecker::evalFreadFwrite(const FnDescription *Desc, if (!NMembVal) return; - const StreamState *SS = State->get<StreamMap>(StreamSym); - if (!SS) + const StreamState *OldSS = State->get<StreamMap>(StreamSym); + if (!OldSS) return; - assertStreamStateOpened(SS); + assertStreamStateOpened(OldSS); // C'99 standard, ยง7.19.8.1.3, the return value of fread: // The fread function returns the number of elements successfully read, which @@ -628,7 +667,7 @@ void StreamChecker::evalFreadFwrite(const FnDescription *Desc, // Generate a transition for the success state. // If we know the state to be FEOF at fread, do not add a success state. - if (!IsFread || (SS->ErrorState != ErrorFEof)) { + if (!IsFread || (OldSS->ErrorState != ErrorFEof)) { ProgramStateRef StateNotFailed = State->BindExpr(CE, C.getLocationContext(), *NMembVal); if (StateNotFailed) { @@ -657,21 +696,26 @@ void StreamChecker::evalFreadFwrite(const FnDescription *Desc, StreamErrorState NewES; if (IsFread) - NewES = (SS->ErrorState == ErrorFEof) ? ErrorFEof : ErrorFEof | ErrorFError; + NewES = + (OldSS->ErrorState == ErrorFEof) ? ErrorFEof : ErrorFEof | ErrorFError; else NewES = ErrorFError; // If a (non-EOF) error occurs, the resulting value of the file position // indicator for the stream is indeterminate. - StreamState NewState = StreamState::getOpened(Desc, NewES, !NewES.isFEof()); - StateFailed = StateFailed->set<StreamMap>(StreamSym, NewState); - C.addTransition(StateFailed); + StreamState NewSS = StreamState::getOpened(Desc, NewES, !NewES.isFEof()); + StateFailed = StateFailed->set<StreamMap>(StreamSym, NewSS); + if (IsFread && OldSS->ErrorState != ErrorFEof) + C.addTransition(StateFailed, constructSetEofNoteTag(C, StreamSym)); + else + C.addTransition(StateFailed); } void StreamChecker::preFseek(const FnDescription *Desc, const CallEvent &Call, CheckerContext &C) const { ProgramStateRef State = C.getState(); SVal StreamVal = getStreamArg(Desc, Call); - State = ensureStreamNonNull(StreamVal, C, State); + State = ensureStreamNonNull(StreamVal, Call.getArgExpr(Desc->StreamArgNo), C, + State); if (!State) return; State = ensureStreamOpened(StreamVal, C, State); @@ -722,7 +766,7 @@ void StreamChecker::evalFseek(const FnDescription *Desc, const CallEvent &Call, StreamState::getOpened(Desc, ErrorNone | ErrorFEof | ErrorFError, true)); C.addTransition(StateNotFailed); - C.addTransition(StateFailed); + C.addTransition(StateFailed, constructSetEofNoteTag(C, StreamSym)); } void StreamChecker::evalClearerr(const FnDescription *Desc, @@ -790,7 +834,8 @@ void StreamChecker::preDefault(const FnDescription *Desc, const CallEvent &Call, CheckerContext &C) const { ProgramStateRef State = C.getState(); SVal StreamVal = getStreamArg(Desc, Call); - State = ensureStreamNonNull(StreamVal, C, State); + State = ensureStreamNonNull(StreamVal, Call.getArgExpr(Desc->StreamArgNo), C, + State); if (!State) return; State = ensureStreamOpened(StreamVal, C, State); @@ -814,7 +859,8 @@ void StreamChecker::evalSetFeofFerror(const FnDescription *Desc, } ProgramStateRef -StreamChecker::ensureStreamNonNull(SVal StreamVal, CheckerContext &C, +StreamChecker::ensureStreamNonNull(SVal StreamVal, const Expr *StreamE, + CheckerContext &C, ProgramStateRef State) const { auto Stream = StreamVal.getAs<DefinedSVal>(); if (!Stream) @@ -827,8 +873,11 @@ StreamChecker::ensureStreamNonNull(SVal StreamVal, CheckerContext &C, if (!StateNotNull && StateNull) { if (ExplodedNode *N = C.generateErrorNode(StateNull)) { - C.emitReport(std::make_unique<PathSensitiveBugReport>( - BT_FileNull, "Stream pointer might be NULL.", N)); + auto R = std::make_unique<PathSensitiveBugReport>( + BT_FileNull, "Stream pointer might be NULL.", N); + if (StreamE) + bugreporter::trackExpressionValue(N, StreamE, *R); + C.emitReport(std::move(R)); } return nullptr; } @@ -950,14 +999,16 @@ StreamChecker::ensureFseekWhenceCorrect(SVal WhenceVal, CheckerContext &C, return State; } -void StreamChecker::reportFEofWarning(CheckerContext &C, +void StreamChecker::reportFEofWarning(SymbolRef StreamSym, CheckerContext &C, ProgramStateRef State) const { if (ExplodedNode *N = C.generateNonFatalErrorNode(State)) { - C.emitReport(std::make_unique<PathSensitiveBugReport>( + auto R = std::make_unique<PathSensitiveBugReport>( BT_StreamEof, "Read function called when stream is in EOF state. " "Function has no effect.", - N)); + N); + R->markInteresting(StreamSym); + C.emitReport(std::move(R)); return; } C.addTransition(State); @@ -1048,6 +1099,10 @@ ProgramStateRef StreamChecker::checkPointerEscape( return State; } +//===----------------------------------------------------------------------===// +// Checker registration. +//===----------------------------------------------------------------------===// + void ento::registerStreamChecker(CheckerManager &Mgr) { Mgr.registerChecker<StreamChecker>(); } @@ -1063,4 +1118,4 @@ void ento::registerStreamTesterChecker(CheckerManager &Mgr) { bool ento::shouldRegisterStreamTesterChecker(const CheckerManager &Mgr) { return true; -} +}
\ No newline at end of file diff --git a/clang/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp index e457513d8de4..816a547cadc3 100644 --- a/clang/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp @@ -86,9 +86,9 @@ UndefCapturedBlockVarChecker::checkPostStmt(const BlockExpr *BE, auto R = std::make_unique<PathSensitiveBugReport>(*BT, os.str(), N); if (const Expr *Ex = FindBlockDeclRefExpr(BE->getBody(), VD)) R->addRange(Ex->getSourceRange()); - R->addVisitor(std::make_unique<FindLastStoreBRVisitor>( - *V, VR, /*EnableNullFPSuppression*/ false, - bugreporter::TrackingKind::Thorough)); + bugreporter::trackStoredValue(*V, VR, *R, + {bugreporter::TrackingKind::Thorough, + /*EnableNullFPSuppression*/ false}); R->disablePathPruning(); // need location of block C.emitReport(std::move(R)); diff --git a/clang/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp index 392da4818098..477d910bc653 100644 --- a/clang/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp @@ -16,7 +16,7 @@ #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/CheckerManager.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" -#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h" #include "llvm/ADT/SmallString.h" #include "llvm/Support/raw_ostream.h" diff --git a/clang/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp index 74eec81ffb3e..d231be64c2e1 100644 --- a/clang/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp @@ -169,7 +169,7 @@ void UnreachableCodeChecker::checkEndAnalysis(ExplodedGraph &G, if (SM.isInSystemHeader(SL) || SM.isInExternCSystemHeader(SL)) continue; - B.EmitBasicReport(D, this, "Unreachable code", "Dead code", + B.EmitBasicReport(D, this, "Unreachable code", categories::UnusedCode, "This statement is never executed", DL, SR); } } diff --git a/clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp index d76b2a06aba5..96501215c689 100644 --- a/clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp @@ -20,7 +20,7 @@ #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/CheckerManager.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" -#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h" #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/SmallString.h" #include "llvm/Support/raw_ostream.h" @@ -285,21 +285,11 @@ void VLASizeChecker::checkPreStmt(const DeclStmt *DS, CheckerContext &C) const { return; } - // VLASizeChecker is responsible for defining the extent of the array being - // declared. We do this by multiplying the array length by the element size, - // then matching that with the array region's extent symbol. - + // VLASizeChecker is responsible for defining the extent of the array. if (VD) { - // Assume that the array's size matches the region size. - const LocationContext *LC = C.getLocationContext(); - DefinedOrUnknownSVal DynSize = - getDynamicSize(State, State->getRegion(VD, LC), SVB); - - DefinedOrUnknownSVal SizeIsKnown = SVB.evalEQ(State, DynSize, *ArraySizeNL); - State = State->assume(SizeIsKnown, true); - - // Assume should not fail at this point. - assert(State); + State = + setDynamicExtent(State, State->getRegion(VD, C.getLocationContext()), + ArraySize.castAs<DefinedOrUnknownSVal>(), SVB); } // Remember our assumptions! |