diff options
Diffstat (limited to 'clang/lib/StaticAnalyzer/Checkers')
13 files changed, 272 insertions, 78 deletions
diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp index 0ae784c..1444114 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp @@ -251,6 +251,8 @@ public: const Expr *Ex, const MemRegion *MR, bool hypothetical); + static const StringLiteral *getStringLiteralFromRegion(const MemRegion *MR); + SVal getCStringLength(CheckerContext &C, ProgramStateRef &state, const Expr *Ex, @@ -983,6 +985,21 @@ SVal CStringChecker::getCStringLengthForRegion(CheckerContext &C, return strLength; } +const StringLiteral * +CStringChecker::getStringLiteralFromRegion(const MemRegion *MR) { + switch (MR->getKind()) { + case MemRegion::StringRegionKind: + return cast<StringRegion>(MR)->getStringLiteral(); + case MemRegion::NonParamVarRegionKind: + if (const VarDecl *Decl = cast<NonParamVarRegion>(MR)->getDecl(); + Decl->getType().isConstQualified() && Decl->hasGlobalStorage()) + return dyn_cast_or_null<StringLiteral>(Decl->getInit()); + return nullptr; + default: + return nullptr; + } +} + SVal CStringChecker::getCStringLength(CheckerContext &C, ProgramStateRef &state, const Expr *Ex, SVal Buf, bool hypothetical) const { @@ -1013,30 +1030,19 @@ SVal CStringChecker::getCStringLength(CheckerContext &C, ProgramStateRef &state, // its length. For anything we can't figure out, just return UnknownVal. MR = MR->StripCasts(); - switch (MR->getKind()) { - case MemRegion::StringRegionKind: { - // Modifying the contents of string regions is undefined [C99 6.4.5p6], - // so we can assume that the byte length is the correct C string length. - SValBuilder &svalBuilder = C.getSValBuilder(); - QualType sizeTy = svalBuilder.getContext().getSizeType(); - const StringLiteral *strLit = cast<StringRegion>(MR)->getStringLiteral(); - return svalBuilder.makeIntVal(strLit->getLength(), sizeTy); - } - case MemRegion::NonParamVarRegionKind: { + if (const StringLiteral *StrLit = getStringLiteralFromRegion(MR)) { // If we have a global constant with a string literal initializer, // compute the initializer's length. - const VarDecl *Decl = cast<NonParamVarRegion>(MR)->getDecl(); - if (Decl->getType().isConstQualified() && Decl->hasGlobalStorage()) { - if (const Expr *Init = Decl->getInit()) { - if (auto *StrLit = dyn_cast<StringLiteral>(Init)) { - SValBuilder &SvalBuilder = C.getSValBuilder(); - QualType SizeTy = SvalBuilder.getContext().getSizeType(); - return SvalBuilder.makeIntVal(StrLit->getLength(), SizeTy); - } - } - } - [[fallthrough]]; + // Modifying the contents of string regions is undefined [C99 6.4.5p6], + // so we can assume that the byte length is the correct C string length. + // FIXME: Embedded null characters are not handled. + SValBuilder &SVB = C.getSValBuilder(); + return SVB.makeIntVal(StrLit->getLength(), SVB.getContext().getSizeType()); } + + switch (MR->getKind()) { + case MemRegion::StringRegionKind: + case MemRegion::NonParamVarRegionKind: case MemRegion::SymbolicRegionKind: case MemRegion::AllocaRegionKind: case MemRegion::ParamVarRegionKind: @@ -1046,10 +1052,28 @@ SVal CStringChecker::getCStringLength(CheckerContext &C, ProgramStateRef &state, case MemRegion::CompoundLiteralRegionKind: // FIXME: Can we track this? Is it necessary? return UnknownVal(); - case MemRegion::ElementRegionKind: - // FIXME: How can we handle this? It's not good enough to subtract the - // offset from the base string length; consider "123\x00567" and &a[5]. + case MemRegion::ElementRegionKind: { + // If an offset into the string literal is used, use the original length + // minus the offset. + // FIXME: Embedded null characters are not handled. + const ElementRegion *ER = cast<ElementRegion>(MR); + const SubRegion *SuperReg = + cast<SubRegion>(ER->getSuperRegion()->StripCasts()); + const StringLiteral *StrLit = getStringLiteralFromRegion(SuperReg); + if (!StrLit) + return UnknownVal(); + SValBuilder &SVB = C.getSValBuilder(); + NonLoc Idx = ER->getIndex(); + QualType SizeTy = SVB.getContext().getSizeType(); + NonLoc LengthVal = + SVB.makeIntVal(StrLit->getLength(), SizeTy).castAs<NonLoc>(); + if (state->assume(SVB.evalBinOpNN(state, BO_LE, Idx, LengthVal, + SVB.getConditionType()) + .castAs<DefinedOrUnknownSVal>(), + true)) + return SVB.evalBinOp(state, BO_Sub, LengthVal, Idx, SizeTy); return UnknownVal(); + } default: // Other regions (mostly non-data) can't have a reliable C string length. // In this case, an error is emitted and UndefinedVal is returned. @@ -1074,6 +1098,7 @@ SVal CStringChecker::getCStringLength(CheckerContext &C, ProgramStateRef &state, const StringLiteral *CStringChecker::getCStringLiteral(CheckerContext &C, ProgramStateRef &state, const Expr *expr, SVal val) const { + // FIXME: use getStringLiteralFromRegion (and remove unused parameters)? // Get the memory region pointed to by the val. const MemRegion *bufRegion = val.getAsRegion(); diff --git a/clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp index b304350..7cc146e 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp @@ -246,7 +246,7 @@ public: bool Find(const TypedValueRegion *R) { QualType T = R->getValueType(); if (const RecordType *RT = T->getAsStructureType()) { - const RecordDecl *RD = RT->getOriginalDecl()->getDefinition(); + const RecordDecl *RD = RT->getDecl()->getDefinition(); assert(RD && "Referred record has no definition"); for (const auto *I : RD->fields()) { if (I->isUnnamedBitField()) diff --git a/clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp b/clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp index 17af1ae..5e75c1c 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp @@ -154,15 +154,15 @@ void WalkAST::VisitCallExpr(CallExpr *CE) { .Case("mkstemp", &WalkAST::checkCall_mkstemp) .Case("mkdtemp", &WalkAST::checkCall_mkstemp) .Case("mkstemps", &WalkAST::checkCall_mkstemp) - .Cases("strcpy", "__strcpy_chk", &WalkAST::checkCall_strcpy) - .Cases("strcat", "__strcat_chk", &WalkAST::checkCall_strcat) - .Cases("sprintf", "vsprintf", "scanf", "wscanf", "fscanf", "fwscanf", - "vscanf", "vwscanf", "vfscanf", "vfwscanf", + .Cases({"strcpy", "__strcpy_chk"}, &WalkAST::checkCall_strcpy) + .Cases({"strcat", "__strcat_chk"}, &WalkAST::checkCall_strcat) + .Cases({"sprintf", "vsprintf", "scanf", "wscanf", "fscanf", "fwscanf", + "vscanf", "vwscanf", "vfscanf", "vfwscanf"}, &WalkAST::checkDeprecatedOrUnsafeBufferHandling) - .Cases("sscanf", "swscanf", "vsscanf", "vswscanf", "swprintf", - "snprintf", "vswprintf", "vsnprintf", "memcpy", "memmove", + .Cases({"sscanf", "swscanf", "vsscanf", "vswscanf", "swprintf", + "snprintf", "vswprintf", "vsnprintf", "memcpy", "memmove"}, &WalkAST::checkDeprecatedOrUnsafeBufferHandling) - .Cases("strncpy", "strncat", "memset", "fprintf", + .Cases({"strncpy", "strncat", "memset", "fprintf"}, &WalkAST::checkDeprecatedOrUnsafeBufferHandling) .Case("drand48", &WalkAST::checkCall_rand) .Case("erand48", &WalkAST::checkCall_rand) @@ -766,12 +766,14 @@ void WalkAST::checkDeprecatedOrUnsafeBufferHandling(const CallExpr *CE, int ArgIndex = llvm::StringSwitch<int>(Name) - .Cases("scanf", "wscanf", "vscanf", "vwscanf", 0) - .Cases("fscanf", "fwscanf", "vfscanf", "vfwscanf", "sscanf", - "swscanf", "vsscanf", "vswscanf", 1) - .Cases("sprintf", "vsprintf", "fprintf", 1) - .Cases("swprintf", "snprintf", "vswprintf", "vsnprintf", "memcpy", - "memmove", "memset", "strncpy", "strncat", DEPR_ONLY) + .Cases({"scanf", "wscanf", "vscanf", "vwscanf"}, 0) + .Cases({"fscanf", "fwscanf", "vfscanf", "vfwscanf", "sscanf", + "swscanf", "vsscanf", "vswscanf"}, + 1) + .Cases({"sprintf", "vsprintf", "fprintf"}, 1) + .Cases({"swprintf", "snprintf", "vswprintf", "vsnprintf", "memcpy", + "memmove", "memset", "strncpy", "strncat"}, + DEPR_ONLY) .Default(UNKNOWN_CALL); assert(ArgIndex != UNKNOWN_CALL && "Unsupported function"); diff --git a/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp index 395d724..37f5ec3 100644 --- a/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp @@ -19,6 +19,7 @@ #include "clang/StaticAnalyzer/Core/CheckerManager.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h" +#include "llvm/Support/FormatVariadic.h" #include "llvm/Support/raw_ostream.h" using namespace clang; @@ -39,9 +40,10 @@ public: class DereferenceChecker : public CheckerFamily<check::Location, check::Bind, + check::PreStmt<BinaryOperator>, EventDispatcher<ImplicitNullDerefEvent>> { - void reportBug(const DerefBugType &BT, ProgramStateRef State, const Stmt *S, - CheckerContext &C) const; + void reportDerefBug(const DerefBugType &BT, ProgramStateRef State, + const Stmt *S, CheckerContext &C) const; bool suppressReport(CheckerContext &C, const Expr *E) const; @@ -50,6 +52,7 @@ public: CheckerContext &C) const; void checkBind(SVal L, SVal V, const Stmt *S, bool AtDeclInit, CheckerContext &C) const; + void checkPreStmt(const BinaryOperator *Op, CheckerContext &C) const; static void AddDerefSource(raw_ostream &os, SmallVectorImpl<SourceRange> &Ranges, @@ -57,7 +60,7 @@ public: const LocationContext *LCtx, bool loadedFrom = false); - CheckerFrontend NullDerefChecker, FixedDerefChecker; + CheckerFrontend NullDerefChecker, FixedDerefChecker, NullPointerArithmChecker; const DerefBugType NullBug{&NullDerefChecker, "Dereference of null pointer", "a null pointer dereference", "a dereference of a null pointer"}; @@ -72,9 +75,22 @@ public: const DerefBugType FixedAddressBug{&FixedDerefChecker, "Dereference of a fixed address", "a dereference of a fixed address"}; + const BugType NullPointerArithmBug{ + &NullPointerArithmChecker, + "Possibly undefined arithmetic operation involving a null pointer"}; StringRef getDebugTag() const override { return "DereferenceChecker"; } }; + +struct ValueDescStr { + SmallVectorImpl<SourceRange> &Ranges; + const Expr *Ex; + const ProgramState *State; + const LocationContext *LCtx; + bool IsPointer; + ConditionTruthVal IsNull; +}; + } // end anonymous namespace void @@ -173,9 +189,9 @@ static bool isDeclRefExprToReference(const Expr *E) { return false; } -void DereferenceChecker::reportBug(const DerefBugType &BT, - ProgramStateRef State, const Stmt *S, - CheckerContext &C) const { +void DereferenceChecker::reportDerefBug(const DerefBugType &BT, + ProgramStateRef State, const Stmt *S, + CheckerContext &C) const { if (&BT == &FixedAddressBug) { if (!FixedDerefChecker.isEnabled()) // Deliberately don't add a sink node if check is disabled. @@ -249,9 +265,8 @@ void DereferenceChecker::reportBug(const DerefBugType &BT, bugreporter::trackExpressionValue(N, bugreporter::getDerefExpr(S), *BR); - for (SmallVectorImpl<SourceRange>::iterator - I = Ranges.begin(), E = Ranges.end(); I!=E; ++I) - BR->addRange(*I); + for (const auto &R : Ranges) + BR->addRange(R); C.emitReport(std::move(BR)); } @@ -262,7 +277,7 @@ void DereferenceChecker::checkLocation(SVal l, bool isLoad, const Stmt* S, if (l.isUndef()) { const Expr *DerefExpr = getDereferenceExpr(S); if (!suppressReport(C, DerefExpr)) - reportBug(UndefBug, C.getState(), DerefExpr, C); + reportDerefBug(UndefBug, C.getState(), DerefExpr, C); return; } @@ -283,7 +298,7 @@ void DereferenceChecker::checkLocation(SVal l, bool isLoad, const Stmt* S, // we call an "explicit" null dereference. const Expr *expr = getDereferenceExpr(S); if (!suppressReport(C, expr)) { - reportBug(NullBug, nullState, expr, C); + reportDerefBug(NullBug, nullState, expr, C); return; } } @@ -301,7 +316,7 @@ void DereferenceChecker::checkLocation(SVal l, bool isLoad, const Stmt* S, if (location.isConstant()) { const Expr *DerefExpr = getDereferenceExpr(S, isLoad); if (!suppressReport(C, DerefExpr)) - reportBug(FixedAddressBug, notNullState, DerefExpr, C); + reportDerefBug(FixedAddressBug, notNullState, DerefExpr, C); return; } @@ -317,7 +332,7 @@ void DereferenceChecker::checkBind(SVal L, SVal V, const Stmt *S, // One should never write to label addresses. if (auto Label = L.getAs<loc::GotoLabel>()) { - reportBug(LabelBug, C.getState(), S, C); + reportDerefBug(LabelBug, C.getState(), S, C); return; } @@ -338,7 +353,7 @@ void DereferenceChecker::checkBind(SVal L, SVal V, const Stmt *S, if (!StNonNull) { const Expr *expr = getDereferenceExpr(S, /*IsBind=*/true); if (!suppressReport(C, expr)) { - reportBug(NullBug, StNull, expr, C); + reportDerefBug(NullBug, StNull, expr, C); return; } } @@ -356,7 +371,7 @@ void DereferenceChecker::checkBind(SVal L, SVal V, const Stmt *S, if (V.isConstant()) { const Expr *DerefExpr = getDereferenceExpr(S, true); if (!suppressReport(C, DerefExpr)) - reportBug(FixedAddressBug, State, DerefExpr, C); + reportDerefBug(FixedAddressBug, State, DerefExpr, C); return; } @@ -379,6 +394,96 @@ void DereferenceChecker::checkBind(SVal L, SVal V, const Stmt *S, C.addTransition(State, this); } +namespace llvm { +template <> struct format_provider<ValueDescStr> { + static void format(const ValueDescStr &V, raw_ostream &Stream, + StringRef Style) { + static const char *ValueStr[2][3] = { + {"zero", "nonzero integer value", "probably nonzero integer value"}, + {"null pointer", "non-null pointer", "probably non-null pointer"}, + }; + Stream + << ValueStr[V.IsPointer][V.IsNull.isConstrainedTrue() + ? 0 + : (V.IsNull.isConstrainedFalse() ? 1 : 2)]; + DereferenceChecker::AddDerefSource(Stream, V.Ranges, V.Ex, V.State, V.LCtx, + false); + } +}; +} // namespace llvm + +void DereferenceChecker::checkPreStmt(const BinaryOperator *Op, + CheckerContext &C) const { + if (!Op->isAdditiveOp() || !NullPointerArithmChecker.isEnabled()) + return; + const Expr *E1 = Op->getLHS(); + const Expr *E2 = Op->getRHS(); + QualType T1 = E1->getType().getCanonicalType(); + QualType T2 = E2->getType().getCanonicalType(); + bool T1IsPointer = T1->isPointerType(); + bool T2IsPointer = T2->isPointerType(); + if (T1->isIntegerType() && T2->isIntegerType()) + return; + if (!T1IsPointer && !T1->isIntegerType() && !T2IsPointer && + !T2->isIntegerType()) + return; + + ProgramStateRef State = C.getState(); + ConditionTruthVal V1IsNull = State->isNull(C.getSVal(E1)); + ConditionTruthVal V2IsNull = State->isNull(C.getSVal(E2)); + bool IsConstrained = true; + + // Check cases 'NULL + x' and 'NULL - x' + if (T1IsPointer && !T2IsPointer) { + if (!V1IsNull.isConstrainedTrue() || V2IsNull.isConstrainedTrue()) + return; + IsConstrained = V2IsNull.isConstrainedFalse(); + } + + // Check case 'x + NULL' + if (!T1IsPointer && T2IsPointer) { + if (V1IsNull.isConstrainedTrue() || !V2IsNull.isConstrainedTrue()) + return; + IsConstrained = V1IsNull.isConstrainedFalse(); + } + + // Check case 'NULL - p' or 'p - NULL' + if (T1IsPointer && T2IsPointer) { + if (!V1IsNull.isConstrainedTrue() && !V2IsNull.isConstrainedTrue()) + return; + if (V1IsNull.isConstrainedTrue() && V2IsNull.isConstrainedTrue()) + return; + IsConstrained = + V1IsNull.isConstrainedFalse() || V2IsNull.isConstrainedFalse(); + } + + SmallVector<SourceRange, 2> Ranges; + const char *OpcodeStr = + Op->getOpcode() == BO_Add ? "Addition" : "Subtraction"; + const char *ResultStr = IsConstrained ? "results" : "may result"; + ValueDescStr DerefArg1{ + Ranges, E1, State.get(), C.getLocationContext(), T1IsPointer, V1IsNull}; + ValueDescStr DerefArg2{ + Ranges, E2, State.get(), C.getLocationContext(), T2IsPointer, V2IsNull}; + std::string Msg = + llvm::formatv("{0} of a {1} and a {2} {3} in undefined behavior", + OpcodeStr, DerefArg1, DerefArg2, ResultStr); + + ExplodedNode *N = C.generateErrorNode(State); + if (!N) + return; + auto BR = + std::make_unique<PathSensitiveBugReport>(NullPointerArithmBug, Msg, N); + if (V1IsNull.isConstrainedTrue()) + bugreporter::trackExpressionValue(N, E1, *BR); + if (V2IsNull.isConstrainedTrue()) + bugreporter::trackExpressionValue(N, E2, *BR); + for (const auto &R : Ranges) + BR->addRange(R); + + C.emitReport(std::move(BR)); +} + void ento::registerNullDereferenceChecker(CheckerManager &Mgr) { Mgr.getChecker<DereferenceChecker>()->NullDerefChecker.enable(Mgr); } @@ -395,3 +500,11 @@ bool ento::shouldRegisterFixedAddressDereferenceChecker( const CheckerManager &) { return true; } + +void ento::registerNullPointerArithmChecker(CheckerManager &Mgr) { + Mgr.getChecker<DereferenceChecker>()->NullPointerArithmChecker.enable(Mgr); +} + +bool ento::shouldRegisterNullPointerArithmChecker(const CheckerManager &) { + return true; +} diff --git a/clang/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp index b1a7cd7..bc67391 100644 --- a/clang/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp @@ -148,9 +148,8 @@ void NonNullParamChecker::checkPreCall(const CallEvent &Call, QualType T = ArgE->getType(); const RecordType *UT = T->getAsUnionType(); - if (!UT || !UT->getOriginalDecl() - ->getMostRecentDecl() - ->hasAttr<TransparentUnionAttr>()) + if (!UT || + !UT->getDecl()->getMostRecentDecl()->hasAttr<TransparentUnionAttr>()) continue; auto CSV = DV->getAs<nonloc::CompoundVal>(); diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp index 66cfccb..419d263 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp @@ -26,6 +26,7 @@ bool tryToFindPtrOrigin( const Expr *E, bool StopAtFirstRefCountedObj, std::function<bool(const clang::CXXRecordDecl *)> isSafePtr, std::function<bool(const clang::QualType)> isSafePtrType, + std::function<bool(const clang::Decl *)> isSafeGlobalDecl, std::function<bool(const clang::Expr *, bool)> callback) { while (E) { if (auto *DRE = dyn_cast<DeclRefExpr>(E)) { @@ -34,6 +35,8 @@ bool tryToFindPtrOrigin( auto IsImmortal = safeGetName(VD) == "NSApp"; if (VD->hasGlobalStorage() && (IsImmortal || QT.isConstQualified())) return callback(E, true); + if (VD->hasGlobalStorage() && isSafeGlobalDecl(VD)) + return callback(E, true); } } if (auto *tempExpr = dyn_cast<MaterializeTemporaryExpr>(E)) { @@ -71,9 +74,11 @@ bool tryToFindPtrOrigin( } if (auto *Expr = dyn_cast<ConditionalOperator>(E)) { return tryToFindPtrOrigin(Expr->getTrueExpr(), StopAtFirstRefCountedObj, - isSafePtr, isSafePtrType, callback) && + isSafePtr, isSafePtrType, isSafeGlobalDecl, + callback) && tryToFindPtrOrigin(Expr->getFalseExpr(), StopAtFirstRefCountedObj, - isSafePtr, isSafePtrType, callback); + isSafePtr, isSafePtrType, isSafeGlobalDecl, + callback); } if (auto *cast = dyn_cast<CastExpr>(E)) { if (StopAtFirstRefCountedObj) { @@ -93,7 +98,8 @@ bool tryToFindPtrOrigin( if (auto *call = dyn_cast<CallExpr>(E)) { if (auto *Callee = call->getCalleeDecl()) { if (Callee->hasAttr<CFReturnsRetainedAttr>() || - Callee->hasAttr<NSReturnsRetainedAttr>()) { + Callee->hasAttr<NSReturnsRetainedAttr>() || + Callee->hasAttr<NSReturnsAutoreleasedAttr>()) { return callback(E, true); } } @@ -158,7 +164,9 @@ bool tryToFindPtrOrigin( auto Name = safeGetName(callee); if (Name == "__builtin___CFStringMakeConstantString" || - Name == "NSClassFromString") + Name == "NSStringFromSelector" || Name == "NSSelectorFromString" || + Name == "NSStringFromClass" || Name == "NSClassFromString" || + Name == "NSStringFromProtocol" || Name == "NSProtocolFromString") return callback(E, true); } else if (auto *CalleeE = call->getCallee()) { if (auto *E = dyn_cast<DeclRefExpr>(CalleeE->IgnoreParenCasts())) { @@ -176,7 +184,7 @@ bool tryToFindPtrOrigin( if (auto *Subst = dyn_cast<SubstTemplateTypeParmType>(RetType)) { if (auto *SubstType = Subst->desugar().getTypePtr()) { if (auto *RD = dyn_cast<RecordType>(SubstType)) { - if (auto *CXX = dyn_cast<CXXRecordDecl>(RD->getOriginalDecl())) + if (auto *CXX = dyn_cast<CXXRecordDecl>(RD->getDecl())) if (isSafePtr(CXX)) return callback(E, true); } @@ -196,6 +204,8 @@ bool tryToFindPtrOrigin( !Selector.getNumArgs()) return callback(E, true); } + if (auto *ObjCProtocol = dyn_cast<ObjCProtocolExpr>(E)) + return callback(ObjCProtocol, true); if (auto *ObjCDict = dyn_cast<ObjCDictionaryLiteral>(E)) return callback(ObjCDict, true); if (auto *ObjCArray = dyn_cast<ObjCArrayLiteral>(E)) diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h index 3a009d6..9fff456 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h @@ -56,6 +56,7 @@ bool tryToFindPtrOrigin( const clang::Expr *E, bool StopAtFirstRefCountedObj, std::function<bool(const clang::CXXRecordDecl *)> isSafePtr, std::function<bool(const clang::QualType)> isSafePtrType, + std::function<bool(const clang::Decl *)> isSafeGlobalDecl, std::function<bool(const clang::Expr *, bool)> callback); /// For \p E referring to a ref-countable/-counted pointer/reference we return diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ForwardDeclChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ForwardDeclChecker.cpp index d8539ea..1d4e6dd 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ForwardDeclChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ForwardDeclChecker.cpp @@ -263,18 +263,43 @@ public: void visitCallArg(const Expr *Arg, const ParmVarDecl *Param, const Decl *DeclWithIssue) const { auto *ArgExpr = Arg->IgnoreParenCasts(); - if (auto *InnerCE = dyn_cast<CallExpr>(Arg)) { - auto *InnerCallee = InnerCE->getDirectCallee(); - if (InnerCallee && InnerCallee->isInStdNamespace() && - safeGetName(InnerCallee) == "move" && InnerCE->getNumArgs() == 1) { - ArgExpr = InnerCE->getArg(0); - if (ArgExpr) - ArgExpr = ArgExpr->IgnoreParenCasts(); + while (ArgExpr) { + ArgExpr = ArgExpr->IgnoreParenCasts(); + if (auto *InnerCE = dyn_cast<CallExpr>(ArgExpr)) { + auto *InnerCallee = InnerCE->getDirectCallee(); + if (InnerCallee && InnerCallee->isInStdNamespace() && + safeGetName(InnerCallee) == "move" && InnerCE->getNumArgs() == 1) { + ArgExpr = InnerCE->getArg(0); + continue; + } + } + if (auto *UO = dyn_cast<UnaryOperator>(ArgExpr)) { + auto OpCode = UO->getOpcode(); + if (OpCode == UO_Deref || OpCode == UO_AddrOf) { + ArgExpr = UO->getSubExpr(); + continue; + } } + break; } + + if (auto *MemberCallExpr = dyn_cast<CXXMemberCallExpr>(ArgExpr)) { + if (isOwnerPtrType(MemberCallExpr->getObjectType())) + return; + } + + if (auto *OpCE = dyn_cast<CXXOperatorCallExpr>(ArgExpr)) { + auto *Method = dyn_cast_or_null<CXXMethodDecl>(OpCE->getDirectCallee()); + if (Method && isOwnerPtr(safeGetName(Method->getParent()))) { + if (OpCE->getOperator() == OO_Star && OpCE->getNumArgs() == 1) + return; + } + } + if (isNullPtr(ArgExpr) || isa<IntegerLiteral>(ArgExpr) || isa<CXXDefaultArgExpr>(ArgExpr)) return; + if (auto *DRE = dyn_cast<DeclRefExpr>(ArgExpr)) { if (auto *ValDecl = DRE->getDecl()) { if (isa<ParmVarDecl>(ValDecl)) diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp index e5c74bb..d3d1f13 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp @@ -138,6 +138,11 @@ bool isCheckedPtr(const std::string &Name) { return Name == "CheckedPtr" || Name == "CheckedRef"; } +bool isOwnerPtr(const std::string &Name) { + return isRefType(Name) || isCheckedPtr(Name) || Name == "unique_ptr" || + Name == "UniqueRef" || Name == "LazyUniqueRef"; +} + bool isSmartPtrClass(const std::string &Name) { return isRefType(Name) || isCheckedPtr(Name) || isRetainPtrOrOSPtr(Name) || Name == "WeakPtr" || Name == "WeakPtrFactory" || @@ -206,10 +211,7 @@ bool isRetainPtrOrOSPtrType(const clang::QualType T) { } bool isOwnerPtrType(const clang::QualType T) { - return isPtrOfType(T, [](auto Name) { - return isRefType(Name) || isCheckedPtr(Name) || Name == "unique_ptr" || - Name == "UniqueRef" || Name == "LazyUniqueRef"; - }); + return isPtrOfType(T, [](auto Name) { return isOwnerPtr(Name); }); } std::optional<bool> isUncounted(const QualType T) { @@ -255,7 +257,7 @@ void RetainTypeChecker::visitTypedef(const TypedefDecl *TD) { return; } - for (auto *Redecl : RT->getOriginalDecl()->getMostRecentDecl()->redecls()) { + for (auto *Redecl : RT->getDecl()->getMostRecentDecl()->redecls()) { if (Redecl->getAttr<ObjCBridgeAttr>() || Redecl->getAttr<ObjCBridgeMutableAttr>()) { CFPointees.insert(RT); @@ -296,7 +298,7 @@ std::optional<bool> isUnretained(const QualType T, bool IsARCEnabled) { auto *Record = PointeeType->getAsStructureType(); if (!Record) return false; - auto *Decl = Record->getOriginalDecl(); + auto *Decl = Record->getDecl(); if (!Decl) return false; auto TypeName = Decl->getName(); diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h index 8300a6c..12e2e2d 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h @@ -143,6 +143,10 @@ bool isCheckedPtr(const std::string &Name); /// \returns true if \p Name is RetainPtr or its variant, false if not. bool isRetainPtrOrOSPtr(const std::string &Name); +/// \returns true if \p Name is an owning smar pointer such as Ref, CheckedPtr, +/// and unique_ptr. +bool isOwnerPtr(const std::string &Name); + /// \returns true if \p Name is a smart pointer type name, false if not. bool isSmartPtrClass(const std::string &Name); diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp index 9585ceb..791e709 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp @@ -29,12 +29,12 @@ namespace { class RawPtrRefCallArgsChecker : public Checker<check::ASTDecl<TranslationUnitDecl>> { BugType Bug; - mutable BugReporter *BR; TrivialFunctionAnalysis TFA; EnsureFunctionAnalysis EFA; protected: + mutable BugReporter *BR; mutable std::optional<RetainTypeChecker> RTC; public: @@ -46,6 +46,7 @@ public: virtual bool isSafePtr(const CXXRecordDecl *Record) const = 0; virtual bool isSafePtrType(const QualType type) const = 0; virtual bool isSafeExpr(const Expr *) const { return false; } + virtual bool isSafeDecl(const Decl *) const { return false; } virtual const char *ptrKind() const = 0; void checkASTDecl(const TranslationUnitDecl *TUD, AnalysisManager &MGR, @@ -214,6 +215,7 @@ public: Arg, /*StopAtFirstRefCountedObj=*/true, [&](const clang::CXXRecordDecl *Record) { return isSafePtr(Record); }, [&](const clang::QualType T) { return isSafePtrType(T); }, + [&](const clang::Decl *D) { return isSafeDecl(D); }, [&](const clang::Expr *ArgOrigin, bool IsSafe) { if (IsSafe) return true; @@ -479,6 +481,11 @@ public: isa<ObjCMessageExpr>(E); } + bool isSafeDecl(const Decl *D) const final { + // Treat NS/CF globals in system header as immortal. + return BR->getSourceManager().isInSystemHeader(D->getLocation()); + } + const char *ptrKind() const final { return "unretained"; } }; diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp index dd9701f..c13df479 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp @@ -166,10 +166,10 @@ bool isGuardedScopeEmbeddedInGuardianScope(const VarDecl *Guarded, class RawPtrRefLocalVarsChecker : public Checker<check::ASTDecl<TranslationUnitDecl>> { BugType Bug; - mutable BugReporter *BR; EnsureFunctionAnalysis EFA; protected: + mutable BugReporter *BR; mutable std::optional<RetainTypeChecker> RTC; public: @@ -180,6 +180,7 @@ public: virtual bool isSafePtr(const CXXRecordDecl *) const = 0; virtual bool isSafePtrType(const QualType) const = 0; virtual bool isSafeExpr(const Expr *) const { return false; } + virtual bool isSafeDecl(const Decl *) const { return false; } virtual const char *ptrKind() const = 0; void checkASTDecl(const TranslationUnitDecl *TUD, AnalysisManager &MGR, @@ -288,6 +289,7 @@ public: return isSafePtr(Record); }, [&](const clang::QualType Type) { return isSafePtrType(Type); }, + [&](const clang::Decl *D) { return isSafeDecl(D); }, [&](const clang::Expr *InitArgOrigin, bool IsSafe) { if (!InitArgOrigin || IsSafe) return true; @@ -443,6 +445,10 @@ public: return ento::cocoa::isCocoaObjectRef(E->getType()) && isa<ObjCMessageExpr>(E); } + bool isSafeDecl(const Decl *D) const final { + // Treat NS/CF globals in system header as immortal. + return BR->getSourceManager().isInSystemHeader(D->getLocation()); + } const char *ptrKind() const final { return "unretained"; } }; diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp index 6f3a280..c6421f8 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp @@ -121,13 +121,13 @@ public: return true; } } else if (auto *RD = dyn_cast<RecordType>(PointeeType)) { - if (declaresSameEntity(RD->getOriginalDecl(), ClassDecl)) + if (declaresSameEntity(RD->getDecl(), ClassDecl)) return true; } else if (auto *ST = dyn_cast<SubstTemplateTypeParmType>(PointeeType)) { auto Type = ST->getReplacementType(); if (auto *RD = dyn_cast<RecordType>(Type)) { - if (declaresSameEntity(RD->getOriginalDecl(), ClassDecl)) + if (declaresSameEntity(RD->getDecl(), ClassDecl)) return true; } } |