diff options
Diffstat (limited to 'clang/lib/StaticAnalyzer')
28 files changed, 249 insertions, 210 deletions
diff --git a/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp index 3ddd659..2db5310 100644 --- a/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp @@ -115,7 +115,23 @@ class RAIIMutexDescriptor { return false; const IdentifierInfo *II = cast<CXXRecordDecl>(C->getDecl()->getParent())->getIdentifier(); - return II == Guard; + if (II != Guard) + return false; + + // For unique_lock, check if it's constructed with a ctor that takes the tag + // type defer_lock_t. In this case, the lock is not acquired. + if constexpr (std::is_same_v<T, CXXConstructorCall>) { + if (GuardName == "unique_lock" && C->getNumArgs() >= 2) { + const Expr *SecondArg = C->getArgExpr(1); + QualType ArgType = SecondArg->getType().getNonReferenceType(); + if (const auto *RD = ArgType->getAsRecordDecl(); + RD && RD->getName() == "defer_lock_t" && RD->isInStdNamespace()) { + return false; + } + } + } + + return true; } public: @@ -270,8 +286,7 @@ REGISTER_LIST_WITH_PROGRAMSTATE(ActiveCritSections, CritSectionMarker) // TODO: Move these to llvm::ImmutableList when overhauling immutable data // structures for proper iterator concept support. template <> -struct std::iterator_traits< - typename llvm::ImmutableList<CritSectionMarker>::iterator> { +struct std::iterator_traits<llvm::ImmutableList<CritSectionMarker>::iterator> { using iterator_category = std::forward_iterator_tag; using value_type = CritSectionMarker; using difference_type = std::ptrdiff_t; diff --git a/clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp index 7cc146e..e71fe47 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp @@ -322,13 +322,12 @@ bool CallAndMessageChecker::PreVisitProcessArg( else { os << " (e.g., via the field chain: '"; bool first = true; - for (SmallVectorImpl<const FieldDecl *>::iterator - DI = F.FieldChain.begin(), DE = F.FieldChain.end(); DI!=DE;++DI){ + for (const FieldDecl *FD : F.FieldChain) { if (first) first = false; else os << '.'; - os << **DI; + os << *FD; } os << "')"; } diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp index 70baab5..ec7ef23 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -6,41 +6,45 @@ // //===----------------------------------------------------------------------===// // -// This file defines a variety of memory management related checkers, such as +// This file defines checkers that report memory management errors such as // leak, double free, and use-after-free. // -// The following checkers are defined here: +// The logic for modeling memory allocations is implemented in the checker +// family which is called 'MallocChecker' for historical reasons. (This name is +// inaccurate, something like 'DynamicMemory' would be more precise.) // -// * MallocChecker -// Despite its name, it models all sorts of memory allocations and -// de- or reallocation, including but not limited to malloc, free, -// relloc, new, delete. It also reports on a variety of memory misuse -// errors. -// Many other checkers interact very closely with this checker, in fact, -// most are merely options to this one. Other checkers may register -// MallocChecker, but do not enable MallocChecker's reports (more details -// to follow around its field, ChecksEnabled). -// It also has a boolean "Optimistic" checker option, which if set to true -// will cause the checker to model user defined memory management related -// functions annotated via the attribute ownership_takes, ownership_holds -// and ownership_returns. +// The reports produced by this backend are exposed through several frontends: +// * MallocChecker: reports all misuse of dynamic memory allocated by +// malloc, related functions (like calloc, realloc etc.) and the functions +// annotated by ownership_returns. (Here the name "MallocChecker" is +// reasonably accurate; don't confuse this checker frontend with the whole +// misnamed family.) +// * NewDeleteChecker: reports most misuse (anything but memory leaks) of +// memory managed by the C++ operators new and new[]. +// * NewDeleteLeaksChecker: reports leaks of dynamic memory allocated by +// the C++ operators new and new[]. +// * MismatchedDeallocatorChecker: reports situations where the allocation +// and deallocation is mismatched, e.g. memory allocated via malloc is +// passed to operator delete. +// * InnerPointerChecker: reports use of pointers to the internal buffer of +// a std::string instance after operations that invalidate them. +// * TaintedAllocChecker: reports situations where the size argument of a +// memory allocation function or array new operator is tainted (i.e. comes +// from an untrusted source and can be controlled by an attacker). // -// * NewDeleteChecker -// Enables the modeling of new, new[], delete, delete[] in MallocChecker, -// and checks for related double-free and use-after-free errors. +// In addition to these frontends this file also defines the registration +// functions for "unix.DynamicMemoryModeling". This registers the callbacks of +// the checker family MallocChecker without enabling any of the frontends and +// and handle two checker options which are attached to this "modeling +// checker" because they affect multiple checker frontends. // -// * NewDeleteLeaksChecker -// Checks for leaks related to new, new[], delete, delete[]. -// Depends on NewDeleteChecker. -// -// * MismatchedDeallocatorChecker -// Enables checking whether memory is deallocated with the corresponding -// allocation function in MallocChecker, such as malloc() allocated -// regions are only freed by free(), new by delete, new[] by delete[]. -// -// InnerPointerChecker interacts very closely with MallocChecker, but unlike -// the above checkers, it has it's own file, hence the many InnerPointerChecker -// related headers and non-static functions. +// Note that what the users see as the checker "cplusplus.InnerPointer" is a +// combination of the frontend InnerPointerChecker (within this family) which +// emits the bug reports and a separate checker class (also named +// InnerPointerChecker) which is defined in InnerPointerChecker.cpp and does a +// significant part of the modeling. This cooperation is enabled by several +// non-static helper functions that are defined within this translation unit +// and used in InnerPointerChecker.cpp. // //===----------------------------------------------------------------------===// diff --git a/clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp index f5c3407..ba8281b 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp @@ -244,11 +244,12 @@ bool isMovedFrom(ProgramStateRef State, const MemRegion *Region) { // If a region is removed all of the subregions needs to be removed too. static ProgramStateRef removeFromState(ProgramStateRef State, - const MemRegion *Region) { + const MemRegion *Region, + bool Strict = false) { if (!Region) return State; for (auto &E : State->get<TrackedRegionMap>()) { - if (E.first->isSubRegionOf(Region)) + if ((!Strict || E.first != Region) && E.first->isSubRegionOf(Region)) State = State->remove<TrackedRegionMap>(E.first); } return State; @@ -709,18 +710,16 @@ ProgramStateRef MoveChecker::checkRegionChanges( // that are passed directly via non-const pointers or non-const references // or rvalue references. // In case of an InstanceCall don't invalidate the this-region since - // it is fully handled in checkPreCall and checkPostCall. - const MemRegion *ThisRegion = nullptr; - if (const auto *IC = dyn_cast<CXXInstanceCall>(Call)) - ThisRegion = IC->getCXXThisVal().getAsRegion(); + // it is fully handled in checkPreCall and checkPostCall, but do invalidate + // its strict subregions, as they are not handled. // Requested ("explicit") regions are the regions passed into the call // directly, but not all of them end up being invalidated. // But when they do, they appear in the InvalidatedRegions array as well. for (const auto *Region : RequestedRegions) { - if (ThisRegion != Region && - llvm::is_contained(InvalidatedRegions, Region)) - State = removeFromState(State, Region); + if (llvm::is_contained(InvalidatedRegions, Region)) + State = removeFromState(State, Region, + /*Strict=*/isa<CXXInstanceCall>(Call)); } } else { // For invalidations that aren't caused by calls, assume nothing. In diff --git a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp index 019e81f..027bf78 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp @@ -22,6 +22,7 @@ #include "clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h" #include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/SmallPtrSet.h" #include "llvm/Support/raw_ostream.h" using namespace clang; using namespace ento; @@ -247,6 +248,7 @@ class FindStackRegionsSymbolVisitor final : public SymbolVisitor { CheckerContext &Ctxt; const StackFrameContext *PoppedStackFrame; SmallVectorImpl<const MemRegion *> &EscapingStackRegions; + llvm::SmallPtrSet<const MemRegion *, 16> VisitedRegions; public: explicit FindStackRegionsSymbolVisitor( @@ -258,6 +260,9 @@ public: bool VisitSymbol(SymbolRef sym) override { return true; } bool VisitMemRegion(const MemRegion *MR) override { + if (!VisitedRegions.insert(MR).second) + return true; + SaveIfEscapes(MR); if (const BlockDataRegion *BDR = MR->getAs<BlockDataRegion>()) diff --git a/clang/lib/StaticAnalyzer/Checkers/StdVariantChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdVariantChecker.cpp index db8bbee..c5dad61 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StdVariantChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StdVariantChecker.cpp @@ -90,6 +90,9 @@ bool isStdVariant(const Type *Type) { static std::optional<ArrayRef<TemplateArgument>> getTemplateArgsFromVariant(const Type *VariantType) { const auto *TempSpecType = VariantType->getAs<TemplateSpecializationType>(); + while (TempSpecType && TempSpecType->isTypeAlias()) + TempSpecType = + TempSpecType->getAliasedType()->getAs<TemplateSpecializationType>(); if (!TempSpecType) return {}; @@ -219,10 +222,12 @@ private: bool handleStdGetCall(const CallEvent &Call, CheckerContext &C) const { ProgramStateRef State = C.getState(); - const auto &ArgType = Call.getArgSVal(0) - .getType(C.getASTContext()) - ->getPointeeType() - .getTypePtr(); + SVal ArgSVal = Call.getArgSVal(0); + if (ArgSVal.isUnknown()) + return false; + + const auto &ArgType = + ArgSVal.getType(C.getASTContext())->getPointeeType().getTypePtr(); // We have to make sure that the argument is an std::variant. // There is another std::get with std::pair argument if (!isStdVariant(ArgType)) diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp index 84adbf3..e46bba7 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp @@ -321,6 +321,51 @@ bool isExprToGetCheckedPtrCapableMember(const clang::Expr *E) { return result && *result; } +bool isAllocInit(const Expr *E, const Expr **InnerExpr) { + auto *ObjCMsgExpr = dyn_cast<ObjCMessageExpr>(E); + if (auto *POE = dyn_cast<PseudoObjectExpr>(E)) { + if (unsigned ExprCount = POE->getNumSemanticExprs()) { + auto *Expr = POE->getSemanticExpr(ExprCount - 1)->IgnoreParenCasts(); + ObjCMsgExpr = dyn_cast<ObjCMessageExpr>(Expr); + if (InnerExpr) + *InnerExpr = ObjCMsgExpr; + } + } + if (!ObjCMsgExpr) + return false; + auto Selector = ObjCMsgExpr->getSelector(); + auto NameForFirstSlot = Selector.getNameForSlot(0); + if (NameForFirstSlot.starts_with("alloc") || + NameForFirstSlot.starts_with("copy") || + NameForFirstSlot.starts_with("mutableCopy")) + return true; + if (!NameForFirstSlot.starts_with("init") && + !NameForFirstSlot.starts_with("_init")) + return false; + if (!ObjCMsgExpr->isInstanceMessage()) + return false; + auto *Receiver = ObjCMsgExpr->getInstanceReceiver(); + if (!Receiver) + return false; + Receiver = Receiver->IgnoreParenCasts(); + if (auto *Inner = dyn_cast<ObjCMessageExpr>(Receiver)) { + if (InnerExpr) + *InnerExpr = Inner; + auto InnerSelector = Inner->getSelector(); + return InnerSelector.getNameForSlot(0).starts_with("alloc"); + } else if (auto *CE = dyn_cast<CallExpr>(Receiver)) { + if (InnerExpr) + *InnerExpr = CE; + if (auto *Callee = CE->getDirectCallee()) { + if (Callee->getDeclName().isIdentifier()) { + auto CalleeName = Callee->getName(); + return CalleeName.starts_with("alloc"); + } + } + } + return false; +} + class EnsureFunctionVisitor : public ConstStmtVisitor<EnsureFunctionVisitor, bool> { public: diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h index 9fff456..d0a3e471 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h @@ -77,6 +77,10 @@ bool isConstOwnerPtrMemberExpr(const clang::Expr *E); /// supports CheckedPtr. bool isExprToGetCheckedPtrCapableMember(const clang::Expr *E); +/// \returns true if \p E is a [[alloc] init] pattern expression. +/// Sets \p InnerExpr to the inner function call or selector invocation. +bool isAllocInit(const Expr *E, const Expr **InnerExpr = nullptr); + /// \returns true if E is a CXXMemberCallExpr which returns a const smart /// pointer type. class EnsureFunctionAnalysis { diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp index d3d1f13..5cd894a 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp @@ -578,6 +578,10 @@ public: return WithCachedResult(CS, [&]() { return VisitChildren(CS); }); } + bool VisitCoroutineBodyStmt(const CoroutineBodyStmt *CBS) { + return WithCachedResult(CBS, [&]() { return VisitChildren(CBS); }); + } + bool VisitReturnStmt(const ReturnStmt *RS) { // A return statement is allowed as long as the return value is trivial. if (auto *RV = RS->getRetValue()) diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp index 791e7099..dcc14a0 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp @@ -177,16 +177,11 @@ public: if (BR->getSourceManager().isInSystemHeader(E->getExprLoc())) return; - auto Selector = E->getSelector(); if (auto *Receiver = E->getInstanceReceiver()) { std::optional<bool> IsUnsafe = isUnsafePtr(E->getReceiverType()); if (IsUnsafe && *IsUnsafe && !isPtrOriginSafe(Receiver)) { - if (auto *InnerMsg = dyn_cast<ObjCMessageExpr>(Receiver)) { - auto InnerSelector = InnerMsg->getSelector(); - if (InnerSelector.getNameForSlot(0) == "alloc" && - Selector.getNameForSlot(0).starts_with("init")) - return; - } + if (isAllocInit(E)) + return; reportBugOnReceiver(Receiver, D); } } diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp index f60d193..f3fadea 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp @@ -587,6 +587,8 @@ public: } std::optional<bool> isUnsafePtr(QualType QT) const final { + if (QT.hasStrongOrWeakObjCLifetime()) + return false; return RTC->isUnretained(QT); } diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp index c13df479..f2235e7c 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp @@ -433,6 +433,8 @@ public: RTC = RetainTypeChecker(); } std::optional<bool> isUnsafePtr(const QualType T) const final { + if (T.hasStrongOrWeakObjCLifetime()) + return false; return RTC->isUnretained(T); } bool isSafePtr(const CXXRecordDecl *Record) const final { diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp index ace639c..0e23ae3 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp @@ -231,8 +231,11 @@ public: // "assign" property doesn't retain even under ARC so treat it as unsafe. bool ignoreARC = !PD->isReadOnly() && PD->getSetterKind() == ObjCPropertyDecl::Assign; + bool IsWeak = + PD->getPropertyAttributes() & ObjCPropertyAttribute::kind_weak; + bool HasSafeAttr = PD->isRetaining() || IsWeak; auto IsUnsafePtr = isUnsafePtr(QT, ignoreARC); - return {IsUnsafePtr && *IsUnsafePtr && !PD->isRetaining(), PropType}; + return {IsUnsafePtr && *IsUnsafePtr && !HasSafeAttr, PropType}; } bool shouldSkipDecl(const RecordDecl *RD) const { @@ -363,6 +366,8 @@ public: } std::optional<bool> isUnsafePtr(QualType QT, bool ignoreARC) const final { + if (QT.hasStrongOrWeakObjCLifetime()) + return false; return RTC->isUnretained(QT, ignoreARC); } diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp index 955b8d1..2af9067 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp @@ -355,15 +355,37 @@ public: void visitBinaryOperator(const BinaryOperator *BO) const { if (!BO->isAssignmentOp()) return; - if (!isa<ObjCIvarRefExpr>(BO->getLHS())) - return; + auto *LHS = BO->getLHS(); auto *RHS = BO->getRHS()->IgnoreParenCasts(); - const Expr *Inner = nullptr; - if (isAllocInit(RHS, &Inner)) { - CreateOrCopyFnCall.insert(RHS); - if (Inner) - CreateOrCopyFnCall.insert(Inner); + if (isa<ObjCIvarRefExpr>(LHS)) { + const Expr *Inner = nullptr; + if (isAllocInit(RHS, &Inner)) { + CreateOrCopyFnCall.insert(RHS); + if (Inner) + CreateOrCopyFnCall.insert(Inner); + } + return; } + auto *UO = dyn_cast<UnaryOperator>(LHS); + if (!UO) + return; + auto OpCode = UO->getOpcode(); + if (OpCode != UO_Deref) + return; + auto *DerefTarget = UO->getSubExpr(); + if (!DerefTarget) + return; + DerefTarget = DerefTarget->IgnoreParenCasts(); + auto *DRE = dyn_cast<DeclRefExpr>(DerefTarget); + if (!DRE) + return; + auto *Decl = DRE->getDecl(); + if (!Decl) + return; + if (!isa<ParmVarDecl>(Decl) || !isCreateOrCopy(RHS)) + return; + if (Decl->hasAttr<CFReturnsRetainedAttr>()) + CreateOrCopyFnCall.insert(RHS); } void visitReturnStmt(const ReturnStmt *RS, const Decl *DeclWithIssue) const { @@ -423,50 +445,6 @@ public: return std::nullopt; } - bool isAllocInit(const Expr *E, const Expr **InnerExpr = nullptr) const { - auto *ObjCMsgExpr = dyn_cast<ObjCMessageExpr>(E); - if (auto *POE = dyn_cast<PseudoObjectExpr>(E)) { - if (unsigned ExprCount = POE->getNumSemanticExprs()) { - auto *Expr = POE->getSemanticExpr(ExprCount - 1)->IgnoreParenCasts(); - ObjCMsgExpr = dyn_cast<ObjCMessageExpr>(Expr); - if (InnerExpr) - *InnerExpr = ObjCMsgExpr; - } - } - if (!ObjCMsgExpr) - return false; - auto Selector = ObjCMsgExpr->getSelector(); - auto NameForFirstSlot = Selector.getNameForSlot(0); - if (NameForFirstSlot == "alloc" || NameForFirstSlot.starts_with("copy") || - NameForFirstSlot.starts_with("mutableCopy")) - return true; - if (!NameForFirstSlot.starts_with("init") && - !NameForFirstSlot.starts_with("_init")) - return false; - if (!ObjCMsgExpr->isInstanceMessage()) - return false; - auto *Receiver = ObjCMsgExpr->getInstanceReceiver(); - if (!Receiver) - return false; - Receiver = Receiver->IgnoreParenCasts(); - if (auto *Inner = dyn_cast<ObjCMessageExpr>(Receiver)) { - if (InnerExpr) - *InnerExpr = Inner; - auto InnerSelector = Inner->getSelector(); - return InnerSelector.getNameForSlot(0) == "alloc"; - } else if (auto *CE = dyn_cast<CallExpr>(Receiver)) { - if (InnerExpr) - *InnerExpr = CE; - if (auto *Callee = CE->getDirectCallee()) { - if (Callee->getDeclName().isIdentifier()) { - auto CalleeName = Callee->getName(); - return CalleeName.starts_with("alloc"); - } - } - } - return false; - } - bool isCreateOrCopy(const Expr *E) const { auto *CE = dyn_cast<CallExpr>(E); if (!CE) diff --git a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp b/clang/lib/StaticAnalyzer/Core/BugReporter.cpp index 5fe64dc..4c06652 100644 --- a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp +++ b/clang/lib/StaticAnalyzer/Core/BugReporter.cpp @@ -25,7 +25,6 @@ #include "clang/AST/StmtObjC.h" #include "clang/Analysis/AnalysisDeclContext.h" #include "clang/Analysis/CFG.h" -#include "clang/Analysis/CFGStmtMap.h" #include "clang/Analysis/PathDiagnostic.h" #include "clang/Analysis/ProgramPoint.h" #include "clang/Basic/LLVM.h" diff --git a/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp index 63f0d70..0ba3c05 100644 --- a/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp +++ b/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp @@ -3254,9 +3254,6 @@ bool ConditionBRVisitor::printValue(const Expr *CondVarExpr, raw_ostream &Out, return true; } -constexpr llvm::StringLiteral ConditionBRVisitor::GenericTrueMessage; -constexpr llvm::StringLiteral ConditionBRVisitor::GenericFalseMessage; - bool ConditionBRVisitor::isPieceMessageGeneric( const PathDiagnosticPiece *Piece) { return Piece->getString() == GenericTrueMessage || diff --git a/clang/lib/StaticAnalyzer/Core/BugSuppression.cpp b/clang/lib/StaticAnalyzer/Core/BugSuppression.cpp index 5b5f9df..a7300b7 100644 --- a/clang/lib/StaticAnalyzer/Core/BugSuppression.cpp +++ b/clang/lib/StaticAnalyzer/Core/BugSuppression.cpp @@ -117,7 +117,12 @@ private: } } - CacheInitializer(Ranges &R) : Result(R) {} + CacheInitializer(Ranges &R) : Result(R) { + ShouldVisitTemplateInstantiations = true; + ShouldWalkTypesOfTypeLocs = false; + ShouldVisitImplicitCode = false; + ShouldVisitLambdaBody = true; + } Ranges &Result; }; diff --git a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp index 62460cc..d04c827 100644 --- a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp +++ b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp @@ -230,13 +230,11 @@ static void findPtrToConstParams(llvm::SmallSet<unsigned, 4> &PreserveArgs, } ProgramStateRef CallEvent::invalidateRegions(unsigned BlockCount, - ProgramStateRef Orig) const { - ProgramStateRef Result = (Orig ? Orig : getState()); - + ProgramStateRef State) const { // Don't invalidate anything if the callee is marked pure/const. - if (const Decl *callee = getDecl()) - if (callee->hasAttr<PureAttr>() || callee->hasAttr<ConstAttr>()) - return Result; + if (const Decl *Callee = getDecl()) + if (Callee->hasAttr<PureAttr>() || Callee->hasAttr<ConstAttr>()) + return State; SmallVector<SVal, 8> ValuesToInvalidate; RegionAndSymbolInvalidationTraits ETraits; @@ -278,10 +276,10 @@ ProgramStateRef CallEvent::invalidateRegions(unsigned BlockCount, // Invalidate designated regions using the batch invalidation API. // NOTE: Even if RegionsToInvalidate is empty, we may still invalidate // global variables. - return Result->invalidateRegions(ValuesToInvalidate, getCFGElementRef(), - BlockCount, getLocationContext(), - /*CausedByPointerEscape*/ true, - /*Symbols=*/nullptr, this, &ETraits); + return State->invalidateRegions(ValuesToInvalidate, getCFGElementRef(), + BlockCount, getLocationContext(), + /*CausedByPointerEscape*/ true, + /*Symbols=*/nullptr, this, &ETraits); } ProgramPoint CallEvent::getProgramPoint(bool IsPreVisit, diff --git a/clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp b/clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp index 8b40437..29d55ac 100644 --- a/clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp +++ b/clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp @@ -142,19 +142,25 @@ std::optional<int> tryExpandAsInteger(StringRef Macro, const Preprocessor &PP) { if (InvalidSpelling) return std::nullopt; - llvm::APInt IntValue; + llvm::APSInt IntValue(/*BitWidth=*/0, /*isUnsigned=*/true); constexpr unsigned AutoSenseRadix = 0; - if (ValueStr.getAsInteger(AutoSenseRadix, IntValue)) + if (ValueStr.getAsInteger(AutoSenseRadix, + static_cast<llvm::APInt &>(IntValue))) return std::nullopt; // Parse an optional minus sign. size_t Size = FilteredTokens.size(); if (Size >= 2) { - if (FilteredTokens[Size - 2].is(tok::minus)) + if (FilteredTokens[Size - 2].is(tok::minus)) { + // Make sure there's space for a sign bit + if (IntValue.isSignBitSet()) + IntValue = IntValue.extend(IntValue.getBitWidth() + 1); + IntValue.setIsUnsigned(false); IntValue = -IntValue; + } } - return IntValue.getSExtValue(); + return IntValue.getExtValue(); } OperatorKind operationKindFromOverloadedOperator(OverloadedOperatorKind OOK, diff --git a/clang/lib/StaticAnalyzer/Core/DynamicExtent.cpp b/clang/lib/StaticAnalyzer/Core/DynamicExtent.cpp index 34078db..e5d5e01 100644 --- a/clang/lib/StaticAnalyzer/Core/DynamicExtent.cpp +++ b/clang/lib/StaticAnalyzer/Core/DynamicExtent.cpp @@ -128,5 +128,12 @@ ProgramStateRef setDynamicExtent(ProgramStateRef State, const MemRegion *MR, return State->set<DynamicExtentMap>(MR->StripCasts(), Size); } +void markAllDynamicExtentLive(ProgramStateRef State, SymbolReaper &SymReaper) { + for (const auto &I : State->get<DynamicExtentMap>()) + if (SymbolRef Sym = I.second.getAsSymbol()) + if (SymReaper.isLiveRegion(I.first)) + SymReaper.markLive(Sym); +} + } // namespace ento } // namespace clang diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp index 4e472b7..a759aee 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -1079,6 +1079,11 @@ void ExprEngine::removeDead(ExplodedNode *Pred, ExplodedNodeSet &Out, getCheckerManager().runCheckersForDeadSymbols(CheckedSet, Pred, SymReaper, DiagnosticStmt, *this, K); + // Extend lifetime of symbols used for dynamic extent while the parent region + // is live. In this way size information about memory allocations is not lost + // if the region remains live. + markAllDynamicExtentLive(CleanedState, SymReaper); + // For each node in CheckedSet, generate CleanedNodes that have the // environment, the store, and the constraints cleaned up but have the // user-supplied states as the predecessors. diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp index 4ddf8fd..db27c06 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp @@ -560,6 +560,7 @@ void ExprEngine::VisitCast(const CastExpr *CastE, const Expr *Ex, case CK_VectorSplat: case CK_HLSLElementwiseCast: case CK_HLSLAggregateSplatCast: + case CK_HLSLMatrixTruncation: case CK_HLSLVectorTruncation: { QualType resultType = CastE->getType(); if (CastE->isGLValue()) diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp index 75d7e26..00e3ef8 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -1013,7 +1013,7 @@ void ExprEngine::VisitCXXNewExpr(const CXXNewExpr *CNE, ExplodedNode *Pred, // FIXME: Once we figure out how we want allocators to work, // we should be using the usual pre-/(default-)eval-/post-call checkers // here. - State = Call->invalidateRegions(blockCount); + State = Call->invalidateRegions(blockCount, State); if (!State) return; diff --git a/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp b/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp index 01d87b0..5803cbd 100644 --- a/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp +++ b/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp @@ -23,6 +23,8 @@ using namespace clang; using namespace ento; using namespace clang::ast_matchers; +using ast_matchers::internal::Matcher; + static const int MAXIMUM_STEP_UNROLLED = 128; namespace { @@ -69,6 +71,11 @@ public: REGISTER_LIST_WITH_PROGRAMSTATE(LoopStack, LoopState) namespace clang { +namespace { +AST_MATCHER(QualType, isIntegralOrEnumerationType) { + return Node->isIntegralOrEnumerationType(); +} +} // namespace namespace ento { static bool isLoopStmt(const Stmt *S) { @@ -82,22 +89,23 @@ ProgramStateRef processLoopEnd(const Stmt *LoopStmt, ProgramStateRef State) { return State; } -static internal::Matcher<Stmt> simpleCondition(StringRef BindName, - StringRef RefName) { +static Matcher<Stmt> simpleCondition(StringRef BindName, StringRef RefName) { + auto LoopVariable = ignoringParenImpCasts( + declRefExpr(to(varDecl(hasType(isInteger())).bind(BindName))) + .bind(RefName)); + auto UpperBound = ignoringParenImpCasts( + expr(hasType(isIntegralOrEnumerationType())).bind("boundNum")); + return binaryOperator( anyOf(hasOperatorName("<"), hasOperatorName(">"), hasOperatorName("<="), hasOperatorName(">="), hasOperatorName("!=")), - hasEitherOperand(ignoringParenImpCasts( - declRefExpr(to(varDecl(hasType(isInteger())).bind(BindName))) - .bind(RefName))), - hasEitherOperand( - ignoringParenImpCasts(integerLiteral().bind("boundNum")))) + anyOf(binaryOperator(hasLHS(LoopVariable), hasRHS(UpperBound)), + binaryOperator(hasRHS(LoopVariable), hasLHS(UpperBound)))) .bind("conditionOperator"); } -static internal::Matcher<Stmt> -changeIntBoundNode(internal::Matcher<Decl> VarNodeMatcher) { +static Matcher<Stmt> changeIntBoundNode(Matcher<Decl> VarNodeMatcher) { return anyOf( unaryOperator(anyOf(hasOperatorName("--"), hasOperatorName("++")), hasUnaryOperand(ignoringParenImpCasts( @@ -107,15 +115,13 @@ changeIntBoundNode(internal::Matcher<Decl> VarNodeMatcher) { declRefExpr(to(varDecl(VarNodeMatcher))))))); } -static internal::Matcher<Stmt> -callByRef(internal::Matcher<Decl> VarNodeMatcher) { +static Matcher<Stmt> callByRef(Matcher<Decl> VarNodeMatcher) { return callExpr(forEachArgumentWithParam( declRefExpr(to(varDecl(VarNodeMatcher))), parmVarDecl(hasType(references(qualType(unless(isConstQualified()))))))); } -static internal::Matcher<Stmt> -assignedToRef(internal::Matcher<Decl> VarNodeMatcher) { +static Matcher<Stmt> assignedToRef(Matcher<Decl> VarNodeMatcher) { return declStmt(hasDescendant(varDecl( allOf(hasType(referenceType()), hasInitializer(anyOf( @@ -123,14 +129,13 @@ assignedToRef(internal::Matcher<Decl> VarNodeMatcher) { declRefExpr(to(varDecl(VarNodeMatcher))))))))); } -static internal::Matcher<Stmt> -getAddrTo(internal::Matcher<Decl> VarNodeMatcher) { +static Matcher<Stmt> getAddrTo(Matcher<Decl> VarNodeMatcher) { return unaryOperator( hasOperatorName("&"), hasUnaryOperand(declRefExpr(hasDeclaration(VarNodeMatcher)))); } -static internal::Matcher<Stmt> hasSuspiciousStmt(StringRef NodeName) { +static Matcher<Stmt> hasSuspiciousStmt(StringRef NodeName) { return hasDescendant(stmt( anyOf(gotoStmt(), switchStmt(), returnStmt(), // Escaping and not known mutation of the loop counter is handled @@ -142,7 +147,7 @@ static internal::Matcher<Stmt> hasSuspiciousStmt(StringRef NodeName) { assignedToRef(equalsBoundNode(std::string(NodeName)))))); } -static internal::Matcher<Stmt> forLoopMatcher() { +static Matcher<Stmt> forLoopMatcher() { return forStmt( hasCondition(simpleCondition("initVarName", "initVarRef")), // Initialization should match the form: 'int i = 6' or 'i = 42'. @@ -271,23 +276,26 @@ static bool shouldCompletelyUnroll(const Stmt *LoopStmt, ASTContext &ASTCtx, if (!isLoopStmt(LoopStmt)) return false; - // TODO: Match the cases where the bound is not a concrete literal but an - // integer with known value auto Matches = match(forLoopMatcher(), *LoopStmt, ASTCtx); if (Matches.empty()) return false; const auto *CounterVarRef = Matches[0].getNodeAs<DeclRefExpr>("initVarRef"); - llvm::APInt BoundNum = - Matches[0].getNodeAs<IntegerLiteral>("boundNum")->getValue(); + const Expr *BoundNumExpr = Matches[0].getNodeAs<Expr>("boundNum"); + + Expr::EvalResult BoundNumResult; + if (!BoundNumExpr || !BoundNumExpr->EvaluateAsInt(BoundNumResult, ASTCtx, + Expr::SE_NoSideEffects)) { + return false; + } llvm::APInt InitNum = Matches[0].getNodeAs<IntegerLiteral>("initNum")->getValue(); auto CondOp = Matches[0].getNodeAs<BinaryOperator>("conditionOperator"); - unsigned MaxWidth = std::max(InitNum.getBitWidth(), BoundNum.getBitWidth()); + unsigned MaxWidth = std::max(InitNum.getBitWidth(), + BoundNumResult.Val.getInt().getBitWidth()); InitNum = InitNum.zext(MaxWidth); - BoundNum = BoundNum.zext(MaxWidth); - + llvm::APInt BoundNum = BoundNumResult.Val.getInt().zext(MaxWidth); if (CondOp->getOpcode() == BO_GE || CondOp->getOpcode() == BO_LE) maxStep = (BoundNum - InitNum + 1).abs().getZExtValue(); else diff --git a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp index 2838533..4f4824a 100644 --- a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp +++ b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp @@ -714,11 +714,6 @@ public: // Part of public interface to class. return getBinding(getRegionBindings(S), L, T); } - std::optional<SVal> getUniqueDefaultBinding(RegionBindingsConstRef B, - const TypedValueRegion *R) const; - std::optional<SVal> - getUniqueDefaultBinding(nonloc::LazyCompoundVal LCV) const; - std::optional<SVal> getDefaultBinding(Store S, const MemRegion *R) override { RegionBindingsRef B = getRegionBindings(S); // Default bindings are always applied over a base region so look up the @@ -2465,11 +2460,6 @@ SVal RegionStoreManager::getBindingForStruct(RegionBindingsConstRef B, // behavior doesn't depend on the struct layout. // This way even an empty struct can carry taint, no matter if creduce drops // the last field member or not. - - // Try to avoid creating a LCV if it would anyways just refer to a single - // default binding. - if (std::optional<SVal> Val = getUniqueDefaultBinding(B, R)) - return *Val; return createLazyBinding(B, R); } @@ -2757,50 +2747,12 @@ RegionStoreManager::bindVector(LimitedRegionBindingsConstRef B, return NewB; } -std::optional<SVal> -RegionStoreManager::getUniqueDefaultBinding(RegionBindingsConstRef B, - const TypedValueRegion *R) const { - if (R != R->getBaseRegion()) - return std::nullopt; - - const auto *Cluster = B.lookup(R); - if (!Cluster || !llvm::hasSingleElement(*Cluster)) - return std::nullopt; - - const auto [Key, Value] = *Cluster->begin(); - return Key.isDirect() ? std::optional<SVal>{} : Value; -} - -std::optional<SVal> -RegionStoreManager::getUniqueDefaultBinding(nonloc::LazyCompoundVal LCV) const { - auto B = getRegionBindings(LCV.getStore()); - return getUniqueDefaultBinding(B, LCV.getRegion()); -} - std::optional<LimitedRegionBindingsRef> RegionStoreManager::tryBindSmallStruct( LimitedRegionBindingsConstRef B, const TypedValueRegion *R, const RecordDecl *RD, nonloc::LazyCompoundVal LCV) { if (B.hasExhaustedBindingLimit()) return B.withValuesEscaped(LCV); - // If we try to copy a Conjured value representing the value of the whole - // struct, don't try to element-wise copy each field. - // That would unnecessarily bind Derived symbols slicing off the subregion for - // the field from the whole Conjured symbol. - // - // struct Window { int width; int height; }; - // Window getWindow(); <-- opaque fn. - // Window w = getWindow(); <-- conjures a new Window. - // Window w2 = w; <-- trivial copy "w", calling "tryBindSmallStruct" - // - // We should not end up with a new Store for "w2" like this: - // Direct [ 0..31]: Derived{Conj{}, w.width} - // Direct [32..63]: Derived{Conj{}, w.height} - // Instead, we should just bind that Conjured value instead. - if (std::optional<SVal> Val = getUniqueDefaultBinding(LCV)) { - return B.addBinding(BindingKey::Make(R, BindingKey::Default), Val.value()); - } - FieldVector Fields; if (const CXXRecordDecl *Class = dyn_cast<CXXRecordDecl>(RD)) diff --git a/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp b/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp index 6673f2f..c9f5774 100644 --- a/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp +++ b/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp @@ -202,11 +202,10 @@ SarifDiagnostics::createResult(const PathDiagnostic *Diag, // Find the HTML report that was generated for this issue, if one exists. PDFileEntry::ConsumerFiles *Files = FM->getFiles(*Diag); if (Files) { - auto HtmlFile = - std::find_if(Files->cbegin(), Files->cend(), [](auto &File) { - return File.first == HTML_DIAGNOSTICS_NAME; - }); - if (HtmlFile != Files->cend()) { + auto HtmlFile = llvm::find_if(*Files, [](const auto &File) { + return File.first == HTML_DIAGNOSTICS_NAME; + }); + if (HtmlFile != Files->end()) { SmallString<128> HtmlReportPath = llvm::sys::path::parent_path(OutputFile); llvm::sys::path::append(HtmlReportPath, HtmlFile->second); @@ -219,7 +218,7 @@ SarifDiagnostics::createResult(const PathDiagnostic *Diag, .setRuleId(CheckName) .setDiagnosticMessage(Diag->getVerboseDescription()) .setDiagnosticLevel(SarifResultLevel::Warning) - .setLocations({Range}) + .addLocations({Range}) .addPartialFingerprint(IssueHashKey, IssueHash) .setHostedViewerURI(HtmlReportURL) .setThreadFlows(Flows); diff --git a/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp b/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp index e0deec1..827fcaa 100644 --- a/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp +++ b/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp @@ -364,7 +364,7 @@ private: void storeTopLevelDecls(DeclGroupRef DG); /// Check if we should skip (not analyze) the given function. - AnalysisMode getModeForDecl(Decl *D, AnalysisMode Mode); + AnalysisMode getModeForDecl(const Decl *D, AnalysisMode Mode) const; void runAnalysisOnTranslationUnit(ASTContext &C); /// Print \p S to stderr if \c Opts.AnalyzerDisplayProgress is set. @@ -677,7 +677,7 @@ void AnalysisConsumer::HandleTranslationUnit(ASTContext &C) { } AnalysisConsumer::AnalysisMode -AnalysisConsumer::getModeForDecl(Decl *D, AnalysisMode Mode) { +AnalysisConsumer::getModeForDecl(const Decl *D, AnalysisMode Mode) const { if (!Opts.AnalyzeSpecificFunction.empty() && AnalysisDeclContext::getFunctionName(D) != Opts.AnalyzeSpecificFunction && cross_tu::CrossTranslationUnitContext::getLookupName(D).value_or("") != @@ -695,7 +695,7 @@ AnalysisConsumer::getModeForDecl(Decl *D, AnalysisMode Mode) { const SourceManager &SM = Ctx->getSourceManager(); - const SourceLocation Loc = [&SM](Decl *D) -> SourceLocation { + const SourceLocation Loc = [&SM](const Decl *D) -> SourceLocation { const Stmt *Body = D->getBody(); SourceLocation SL = Body ? Body->getBeginLoc() : D->getLocation(); return SM.getExpansionLoc(SL); diff --git a/clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp b/clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp index aaf3b67..4c7ce29 100644 --- a/clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp +++ b/clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp @@ -8,9 +8,9 @@ #include "clang/StaticAnalyzer/Frontend/CheckerRegistry.h" #include "clang/Basic/Diagnostic.h" +#include "clang/Basic/DiagnosticFrontend.h" #include "clang/Basic/LLVM.h" #include "clang/Driver/DriverDiagnostic.h" -#include "clang/Frontend/FrontendDiagnostic.h" #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" #include "clang/StaticAnalyzer/Core/AnalyzerOptions.h" #include "clang/StaticAnalyzer/Core/CheckerManager.h" |