diff options
Diffstat (limited to 'clang/lib/StaticAnalyzer/Checkers')
14 files changed, 170 insertions, 108 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) |