diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp index 84adbf318e9f8..15971168934b5 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp @@ -132,11 +132,6 @@ bool tryToFindPtrOrigin( } } - if (call->isCallToStdMove() && call->getNumArgs() == 1) { - E = call->getArg(0)->IgnoreParenCasts(); - continue; - } - if (auto *callee = call->getDirectCallee()) { if (isCtorOfSafePtr(callee)) { if (StopAtFirstRefCountedObj) @@ -146,6 +141,11 @@ bool tryToFindPtrOrigin( continue; } + if (isStdOrWTFMove(callee) && call->getNumArgs() == 1) { + E = call->getArg(0)->IgnoreParenCasts(); + continue; + } + if (isSafePtrType(callee->getReturnType())) return callback(E, true); @@ -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(E); + if (auto *POE = dyn_cast(E)) { + if (unsigned ExprCount = POE->getNumSemanticExprs()) { + auto *Expr = POE->getSemanticExpr(ExprCount - 1)->IgnoreParenCasts(); + ObjCMsgExpr = dyn_cast(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(Receiver)) { + if (InnerExpr) + *InnerExpr = Inner; + auto InnerSelector = Inner->getSelector(); + return InnerSelector.getNameForSlot(0).starts_with("alloc"); + } else if (auto *CE = dyn_cast(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 { public: diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h index 9fff456b7e8b8..d0a3e471365e2 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/ForwardDeclChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ForwardDeclChecker.cpp index 1d4e6dd572749..05a83dbfcb7d2 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ForwardDeclChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ForwardDeclChecker.cpp @@ -266,11 +266,11 @@ class ForwardDeclChecker : public Checker> { while (ArgExpr) { ArgExpr = ArgExpr->IgnoreParenCasts(); if (auto *InnerCE = dyn_cast(ArgExpr)) { - auto *InnerCallee = InnerCE->getDirectCallee(); - if (InnerCallee && InnerCallee->isInStdNamespace() && - safeGetName(InnerCallee) == "move" && InnerCE->getNumArgs() == 1) { - ArgExpr = InnerCE->getArg(0); - continue; + if (auto *InnerCallee = InnerCE->getDirectCallee()) { + if (isStdOrWTFMove(InnerCallee) && InnerCE->getNumArgs() == 1) { + ArgExpr = InnerCE->getArg(0); + continue; + } } } if (auto *UO = dyn_cast(ArgExpr)) { diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp index a72ffa25e33a0..cf88813c8b840 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp @@ -185,6 +185,18 @@ bool isCtorOfSafePtr(const clang::FunctionDecl *F) { isCtorOfRetainPtrOrOSPtr(F); } +bool isStdOrWTFMove(const clang::FunctionDecl *F) { + auto FnName = safeGetName(F); + auto *Namespace = F->getParent(); + if (!Namespace) + return false; + auto *TUDeck = Namespace->getParent(); + if (!isa_and_nonnull(TUDeck)) + return false; + auto NsName = safeGetName(Namespace); + return (NsName == "WTF" || NsName == "std") && FnName == "move"; +} + template static bool isPtrOfType(const clang::QualType T, Predicate Pred) { QualType type = T; @@ -288,37 +300,6 @@ bool RetainTypeChecker::isUnretained(const QualType QT, bool ignoreARC) { return RT && CFPointees.contains(RT); } -std::optional isUnretained(const QualType T, bool IsARCEnabled) { - if (auto *Subst = dyn_cast(T)) { - if (auto *Decl = Subst->getAssociatedDecl()) { - if (isRetainPtrOrOSPtr(safeGetName(Decl))) - return false; - } - } - if ((ento::cocoa::isCocoaObjectRef(T) && !IsARCEnabled) || - ento::coreFoundation::isCFObjectRef(T)) - return true; - - // RetainPtr strips typedef for CF*Ref. Manually check for struct __CF* types. - auto CanonicalType = T.getCanonicalType(); - auto *Type = CanonicalType.getTypePtrOrNull(); - if (!Type) - return false; - auto Pointee = Type->getPointeeType(); - auto *PointeeType = Pointee.getTypePtrOrNull(); - if (!PointeeType) - return false; - auto *Record = PointeeType->getAsStructureType(); - if (!Record) - return false; - auto *Decl = Record->getDecl(); - if (!Decl) - return false; - auto TypeName = Decl->getName(); - return TypeName.starts_with("__CF") || TypeName.starts_with("__CG") || - TypeName.starts_with("__CM"); -} - std::optional isUncounted(const CXXRecordDecl* Class) { // Keep isRefCounted first as it's cheaper. @@ -354,25 +335,6 @@ std::optional isUncheckedPtr(const QualType T) { return false; } -std::optional isUnsafePtr(const QualType T, bool IsArcEnabled) { - if (T->isPointerType() || T->isReferenceType()) { - if (auto *CXXRD = T->getPointeeCXXRecordDecl()) { - auto isUncountedPtr = isUncounted(CXXRD); - auto isUncheckedPtr = isUnchecked(CXXRD); - auto isUnretainedPtr = isUnretained(T, IsArcEnabled); - std::optional result; - if (isUncountedPtr) - result = *isUncountedPtr; - if (isUncheckedPtr) - result = result ? *result || *isUncheckedPtr : *isUncheckedPtr; - if (isUnretainedPtr) - result = result ? *result || *isUnretainedPtr : *isUnretainedPtr; - return result; - } - } - return false; -} - std::optional isGetterOfSafePtr(const CXXMethodDecl *M) { assert(M); @@ -574,6 +536,15 @@ class TrivialFunctionAnalysisVisitor }); } + bool IsStatementTrivial(const Stmt *S) { + auto CacheIt = Cache.find(S); + if (CacheIt != Cache.end()) + return CacheIt->second; + bool Result = Visit(S); + Cache[S] = Result; + return Result; + } + bool VisitStmt(const Stmt *S) { // All statements are non-trivial unless overriden later. // Don't even recurse into children by default. @@ -877,9 +848,7 @@ bool TrivialFunctionAnalysis::isTrivialImpl( bool TrivialFunctionAnalysis::isTrivialImpl( const Stmt *S, TrivialFunctionAnalysis::CacheTy &Cache) { TrivialFunctionAnalysisVisitor V(Cache); - bool Result = V.Visit(S); - assert(Cache.contains(S) && "Top-level statement not properly cached!"); - return Result; + return V.IsStatementTrivial(S); } } // namespace clang diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h index 12e2e2d75b75d..d5ced4d857241 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h @@ -87,10 +87,6 @@ class RetainTypeChecker { bool defaultSynthProperties() const { return DefaultSynthProperties; } }; -/// \returns true if \p Class is NS or CF objects AND not retained, false if -/// not, std::nullopt if inconclusive. -std::optional isUnretained(const clang::QualType T, bool IsARCEnabled); - /// \returns true if \p Class is ref-countable AND not ref-counted, false if /// not, std::nullopt if inconclusive. std::optional isUncounted(const clang::CXXRecordDecl* Class); @@ -107,10 +103,6 @@ std::optional isUncountedPtr(const clang::QualType T); /// class, false if not, std::nullopt if inconclusive. std::optional isUncheckedPtr(const clang::QualType T); -/// \returns true if \p T is either a raw pointer or reference to an uncounted -/// or unchecked class, false if not, std::nullopt if inconclusive. -std::optional isUnsafePtr(const QualType T, bool IsArcEnabled); - /// \returns true if \p T is a RefPtr, Ref, CheckedPtr, CheckedRef, or its /// variant, false if not. bool isRefOrCheckedPtrType(const clang::QualType T); @@ -134,6 +126,9 @@ bool isCtorOfCheckedPtr(const clang::FunctionDecl *F); /// uncounted parameter, false if not. bool isCtorOfSafePtr(const clang::FunctionDecl *F); +/// \returns true if \p F is std::move or WTF::move. +bool isStdOrWTFMove(const clang::FunctionDecl *F); + /// \returns true if \p Name is RefPtr, Ref, or its variant, false if not. bool isRefType(const std::string &Name); diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp index 791e70998477f..dcc14a0aecdf7 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp @@ -177,16 +177,11 @@ class RawPtrRefCallArgsChecker if (BR->getSourceManager().isInSystemHeader(E->getExprLoc())) return; - auto Selector = E->getSelector(); if (auto *Receiver = E->getInstanceReceiver()) { std::optional IsUnsafe = isUnsafePtr(E->getReceiverType()); if (IsUnsafe && *IsUnsafe && !isPtrOriginSafe(Receiver)) { - if (auto *InnerMsg = dyn_cast(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 f60d1936b7584..b5b03624a5601 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp @@ -343,6 +343,17 @@ class RawPtrRefLambdaCapturesChecker auto *Callee = CE->getCallee(); if (!Callee) return; + Callee = Callee->IgnoreParenCasts(); + if (auto *MTE = dyn_cast(Callee)) { + Callee = MTE->getSubExpr(); + if (!Callee) + return; + Callee = Callee->IgnoreParenCasts(); + } + if (auto *L = dyn_cast(Callee)) { + LambdasToIgnore.insert(L); // Calling a lambda upon creation is safe. + return; + } auto *DRE = dyn_cast(Callee->IgnoreParenCasts()); if (!DRE) return; @@ -416,12 +427,9 @@ class RawPtrRefLambdaCapturesChecker return false; } if (auto *CE = dyn_cast(Arg)) { - if (CE->isCallToStdMove() && CE->getNumArgs() == 1) { - Arg = CE->getArg(0)->IgnoreParenCasts(); - continue; - } if (auto *Callee = CE->getDirectCallee()) { - if (isCtorOfSafePtr(Callee) && CE->getNumArgs() == 1) { + if ((isStdOrWTFMove(Callee) || isCtorOfSafePtr(Callee)) && + CE->getNumArgs() == 1) { Arg = CE->getArg(0)->IgnoreParenCasts(); continue; } @@ -587,6 +595,8 @@ class UnretainedLambdaCapturesChecker : public RawPtrRefLambdaCapturesChecker { } std::optional 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 c13df47920f72..f720f8e15ed03 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp @@ -234,6 +234,14 @@ class RawPtrRefLocalVarsChecker } bool TraverseIfStmt(IfStmt *IS) override { + if (IS->getConditionVariable()) { + // This code currently does not explicitly check the "else" statement + // since getConditionVariable returns nullptr when there is a + // condition defined after ";" as in "if (auto foo = ~; !foo)". If + // this semantics change, we should add an explicit check for "else". + if (auto *Then = IS->getThen(); !Then || TFA.isTrivial(Then)) + return true; + } if (!TFA.isTrivial(IS)) return DynamicRecursiveASTVisitor::TraverseIfStmt(IS); return true; @@ -433,6 +441,8 @@ class UnretainedLocalVarsChecker final : public RawPtrRefLocalVarsChecker { RTC = RetainTypeChecker(); } std::optional 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 ace639ce7ab18..0e23ae34ea212 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp @@ -231,8 +231,11 @@ class RawPtrRefMemberChecker // "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 @@ class NoUnretainedMemberChecker final : public RawPtrRefMemberChecker { } std::optional 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 955b8d19a820c..2af9067f8f808 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp @@ -355,15 +355,37 @@ class RetainPtrCtorAdoptChecker void visitBinaryOperator(const BinaryOperator *BO) const { if (!BO->isAssignmentOp()) return; - if (!isa(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(LHS)) { + const Expr *Inner = nullptr; + if (isAllocInit(RHS, &Inner)) { + CreateOrCopyFnCall.insert(RHS); + if (Inner) + CreateOrCopyFnCall.insert(Inner); + } + return; } + auto *UO = dyn_cast(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(DerefTarget); + if (!DRE) + return; + auto *Decl = DRE->getDecl(); + if (!Decl) + return; + if (!isa(Decl) || !isCreateOrCopy(RHS)) + return; + if (Decl->hasAttr()) + CreateOrCopyFnCall.insert(RHS); } void visitReturnStmt(const ReturnStmt *RS, const Decl *DeclWithIssue) const { @@ -423,50 +445,6 @@ class RetainPtrCtorAdoptChecker return std::nullopt; } - bool isAllocInit(const Expr *E, const Expr **InnerExpr = nullptr) const { - auto *ObjCMsgExpr = dyn_cast(E); - if (auto *POE = dyn_cast(E)) { - if (unsigned ExprCount = POE->getNumSemanticExprs()) { - auto *Expr = POE->getSemanticExpr(ExprCount - 1)->IgnoreParenCasts(); - ObjCMsgExpr = dyn_cast(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(Receiver)) { - if (InnerExpr) - *InnerExpr = Inner; - auto InnerSelector = Inner->getSelector(); - return InnerSelector.getNameForSlot(0) == "alloc"; - } else if (auto *CE = dyn_cast(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(E); if (!CE) diff --git a/clang/test/Analysis/Checkers/WebKit/call-args-checked.cpp b/clang/test/Analysis/Checkers/WebKit/call-args-checked.cpp index e9f01c8ed7540..b257d09236c07 100644 --- a/clang/test/Analysis/Checkers/WebKit/call-args-checked.cpp +++ b/clang/test/Analysis/Checkers/WebKit/call-args-checked.cpp @@ -80,6 +80,7 @@ namespace call_with_std_move { void consume(CheckedObj&&); void foo(CheckedObj&& obj) { consume(std::move(obj)); + consume(WTF::move(obj)); } } diff --git a/clang/test/Analysis/Checkers/WebKit/forward-decl-checker.mm b/clang/test/Analysis/Checkers/WebKit/forward-decl-checker.mm index 8aad838b71b35..96c4d66379360 100644 --- a/clang/test/Analysis/Checkers/WebKit/forward-decl-checker.mm +++ b/clang/test/Analysis/Checkers/WebKit/forward-decl-checker.mm @@ -44,6 +44,7 @@ void opaque_call_arg(Obj* obj, Obj&& otherObj, const RefPtr& safeObj, WeakP receive_obj_ref(*obj); receive_obj_ptr(&*obj); receive_obj_rref(std::move(otherObj)); + receive_obj_rref(WTF::move(otherObj)); receive_obj_ref(*safeObj.get()); receive_obj_ptr(weakObj.get()); // expected-warning@-1{{Call argument for parameter 'p' uses a forward declared type 'Obj *'}} @@ -59,6 +60,7 @@ void rval(Obj&& arg) { auto &&obj = provide_obj_rval(); // expected-warning@-1{{Local variable 'obj' uses a forward declared type 'Obj &&'}} receive_obj_rval(std::move(arg)); + receive_obj_rval(WTF::move(arg)); } ObjCObj *provide_objcobj(); @@ -69,6 +71,10 @@ void rval(Obj&& arg) { return objcobj; } +void obj_ptr_null_callee(ObjCObj* (*cb)()) { + receive_objcobj(cb()); +} + struct WrapperObj { Obj* ptr { nullptr }; // expected-warning@-1{{Member variable 'ptr' uses a forward declared type 'Obj *'}} @@ -84,6 +90,7 @@ void construct_ptr(Obj&& arg) { WrapperObj wrapper2(provide_obj_ref()); // expected-warning@-1{{Call argument for parameter 'obj' uses a forward declared type 'Obj &'}} WrapperObj wrapper3(std::move(arg)); + WrapperObj wrapper4(WTF::move(arg)); } JSStringRef provide_opaque_ptr(); diff --git a/clang/test/Analysis/Checkers/WebKit/local-vars-checked-const-member.cpp b/clang/test/Analysis/Checkers/WebKit/local-vars-checked-const-member.cpp index be04cf26be2e8..bf58e676f3bb0 100644 --- a/clang/test/Analysis/Checkers/WebKit/local-vars-checked-const-member.cpp +++ b/clang/test/Analysis/Checkers/WebKit/local-vars-checked-const-member.cpp @@ -21,10 +21,8 @@ class Foo { CheckedObj& ensureObj4() { if (!m_obj4) const_cast&>(m_obj4) = new CheckedObj; - if (auto* next = m_obj4->next()) { - // expected-warning@-1{{Local variable 'next' is unchecked and unsafe [alpha.webkit.UncheckedLocalVarsChecker]}} + if (auto* next = m_obj4->next()) return *next; - } return *m_obj4; } diff --git a/clang/test/Analysis/Checkers/WebKit/local-vars-counted-const-member.cpp b/clang/test/Analysis/Checkers/WebKit/local-vars-counted-const-member.cpp index e12c9b900a423..5f35535bde3aa 100644 --- a/clang/test/Analysis/Checkers/WebKit/local-vars-counted-const-member.cpp +++ b/clang/test/Analysis/Checkers/WebKit/local-vars-counted-const-member.cpp @@ -21,10 +21,8 @@ class Foo { RefCountable& ensureObj4() { if (!m_obj4) const_cast&>(m_obj4) = RefCountable::create(); - if (auto* next = m_obj4->next()) { - // expected-warning@-1{{Local variable 'next' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}} + if (auto* next = m_obj4->next()) return *next; - } return *m_obj4; } diff --git a/clang/test/Analysis/Checkers/WebKit/mock-types.h b/clang/test/Analysis/Checkers/WebKit/mock-types.h index 7055a94753a37..8a24a3c64e0e4 100644 --- a/clang/test/Analysis/Checkers/WebKit/mock-types.h +++ b/clang/test/Analysis/Checkers/WebKit/mock-types.h @@ -17,6 +17,12 @@ template typename remove_reference::type&& move(T&& t); #endif +namespace WTF { + +template typename std::remove_reference::type&& move(T&& t); + +} + #ifndef mock_types_1103988513531 #define mock_types_1103988513531 diff --git a/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h b/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h index edf40115afa19..124821a8ded55 100644 --- a/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h +++ b/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h @@ -17,6 +17,12 @@ template typename remove_reference::type&& move(T&& t); #endif +namespace WTF { + +template typename std::remove_reference::type&& move(T&& t); + +} + namespace std { template struct enable_if { @@ -453,7 +459,7 @@ template class OSObjectPtr { } OSObjectPtr(T ptr) - : m_ptr(std::move(ptr)) + : m_ptr(WTF::move(ptr)) { if (m_ptr) retainOSObject(m_ptr); @@ -483,7 +489,7 @@ template class OSObjectPtr { OSObjectPtr& operator=(T other) { - OSObjectPtr ptr = std::move(other); + OSObjectPtr ptr = WTF::move(other); swap(ptr); return *this; } diff --git a/clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use.mm b/clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use.mm index 45705615f3196..427affdbbd601 100644 --- a/clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use.mm +++ b/clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use.mm @@ -190,6 +190,14 @@ void adopt_retainptr() { auto bar = adoptNS([allocSomeObj() init]); } +CFTypeRef make_cf_obj() CF_RETURNS_RETAINED { + return CFArrayCreateMutable(kCFAllocatorDefault, 1); +} + +void get_cf_obj(CFTypeRef* CF_RETURNS_RETAINED result) { + *result = CFArrayCreateMutable(kCFAllocatorDefault, 1); +} + RetainPtr return_arg(CFArrayRef arg) { return arg; } diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp index fd1eecdda64fd..c06466d258c7d 100644 --- a/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp +++ b/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp @@ -437,6 +437,14 @@ struct RefCountableWithLambdaCapturingThis { }); }); } + + void method_nested_lambda4() { + callAsync([this, protectedThis = RefPtr { this }] { + callAsync([this, protectedThis = WTF::move(*protectedThis)] { + nonTrivial(); + }); + }); + } }; struct NonRefCountableWithLambdaCapturingThis { @@ -512,6 +520,23 @@ void capture_copy_in_lambda(CheckedObj& checked) { }); } +struct TemplateFunctionCallsLambda { + void ref() const; + void deref() const; + + RefCountable* obj(); + + template + RefPtr method(T* t) { + auto ret = ([&]() -> RefPtr { + if constexpr (T::isEncodable) + return t; + return obj() ? t : nullptr; + })(); + return ret; + } +}; + class Iterator { public: Iterator(void* array, unsigned long sizeOfElement, unsigned int index); diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp index 3364637369799..e8022b7fe8ba0 100644 --- a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp +++ b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp @@ -492,4 +492,49 @@ namespace virtual_function { auto* baz = &*obj; // expected-warning@-1{{Local variable 'baz' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}} } -} \ No newline at end of file +} + +namespace vardecl_in_if_condition { + RefCountable* provide(); + + RefCountable* get() { + if (auto* obj = provide()) + return obj; // no warning + return nullptr; + } + + RefCountable* get_non_trivial_then() { + if (auto* obj = provide()) // expected-warning{{Local variable 'obj' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}} + return obj->next(); + return nullptr; + } + + RefCountable* get_non_trivial_else() { + if (auto* obj = provide()) + return obj; + else + return provide()->next(); + return nullptr; + } + + RefCountable& get_ref() { + if (auto* obj = provide()) + return *obj; // no warning + static Ref empty = RefCountable::create(); + return empty.get(); + } + + RefCountable* get_non_trivial_condition() { + if (auto* obj = provide(); obj && obj->next()) + return obj; // expected-warning@-1{{Local variable 'obj' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}} + return nullptr; + } + + RefCountable* get_non_trivial_else2() { + if (auto* obj = provide(); !obj) // expected-warning{{Local variable 'obj' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}} + return nullptr; + else + return obj->next(); + } + +} diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp index a9cd77c066f6f..b83eaedf264e4 100644 --- a/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp +++ b/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp @@ -387,6 +387,7 @@ class RefCounted { unsigned trivial69() { return offsetof(OtherObj, children); } DerivedNumber* trivial70() { [[clang::suppress]] return static_cast(number); } unsigned trivial71() { return std::bit_cast(nullptr); } + unsigned trivial72() { Number n { 5 }; return WTF::move(n).value(); } static RefCounted& singleton() { static RefCounted s_RefCounted; diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm b/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm index 8bef24f93ceed..cfc214fae33e5 100644 --- a/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm +++ b/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm @@ -625,6 +625,8 @@ void foo() { } // namespace template_function +SomeObj *allocObj(); + @interface TestObject : NSObject - (void)doWork:(NSString *)msg, ...; - (void)doWorkOnSelf; @@ -647,6 +649,7 @@ - (void)doWorkOnSelf { [self doWork:__null]; [self doWork:nil]; [NSApp run]; + adoptNS([allocObj() init]); } - (SomeObj *)getSomeObj { diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-lambda-captures-weak-arc.mm b/clang/test/Analysis/Checkers/WebKit/unretained-lambda-captures-weak-arc.mm new file mode 100644 index 0000000000000..a52bc7c9a5572 --- /dev/null +++ b/clang/test/Analysis/Checkers/WebKit/unretained-lambda-captures-weak-arc.mm @@ -0,0 +1,22 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UnretainedLambdaCapturesChecker -fobjc-runtime-has-weak -fobjc-weak -fobjc-arc -verify %s +// expected-no-diagnostics + +#include "objc-mock-types.h" + +void someFunction(); +template void call(Callback callback) { + someFunction(); + callback(); +} + +NSString *provideStr(); +SomeObj *provideSomeObj(); + +void foo() { + __weak NSString *weakStr = provideStr(); + __weak SomeObj *weakObj = provideSomeObj(); + auto lambda = [weakStr, weakObj]() { + return [weakStr length] + [weakObj value]; + }; + call(lambda); +} diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-lambda-captures-weak.mm b/clang/test/Analysis/Checkers/WebKit/unretained-lambda-captures-weak.mm new file mode 100644 index 0000000000000..7439d7f8bb93b --- /dev/null +++ b/clang/test/Analysis/Checkers/WebKit/unretained-lambda-captures-weak.mm @@ -0,0 +1,22 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UnretainedLambdaCapturesChecker -fobjc-runtime-has-weak -fobjc-weak -verify %s +// expected-no-diagnostics + +#include "objc-mock-types.h" + +void someFunction(); +template void call(Callback callback) { + someFunction(); + callback(); +} + +NSString *provideStr(); +SomeObj *provideSomeObj(); + +void foo() { + __weak NSString *weakStr = provideStr(); + __weak SomeObj *weakObj = provideSomeObj(); + auto lambda = [weakStr, weakObj]() { + return [weakStr length] + [weakObj value]; + }; + call(lambda); +} diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-local-vars-weak-arc.mm b/clang/test/Analysis/Checkers/WebKit/unretained-local-vars-weak-arc.mm new file mode 100644 index 0000000000000..8c709b5921227 --- /dev/null +++ b/clang/test/Analysis/Checkers/WebKit/unretained-local-vars-weak-arc.mm @@ -0,0 +1,13 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UnretainedLocalVarsChecker -fobjc-runtime-has-weak -fobjc-weak -fobjc-arc -verify %s +// expected-no-diagnostics + +#include "objc-mock-types.h" + +NSString *provideStr(); +SomeObj *provideSomeObj(); + +int foo() { + __weak NSString *weakStr = provideStr(); + __weak SomeObj *weakObj = provideSomeObj(); + return [weakStr length] + [weakObj value]; +} diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-local-vars-weak.mm b/clang/test/Analysis/Checkers/WebKit/unretained-local-vars-weak.mm new file mode 100644 index 0000000000000..3ac4ff9d1e4cb --- /dev/null +++ b/clang/test/Analysis/Checkers/WebKit/unretained-local-vars-weak.mm @@ -0,0 +1,13 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UnretainedLocalVarsChecker -fobjc-runtime-has-weak -fobjc-weak -verify %s +// expected-no-diagnostics + +#include "objc-mock-types.h" + +NSString *provideStr(); +SomeObj *provideSomeObj(); + +int foo() { + __weak NSString *weakStr = provideStr(); + __weak SomeObj *weakObj = provideSomeObj(); + return [weakStr length] + [weakObj value]; +} diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-members-weak-arc.mm b/clang/test/Analysis/Checkers/WebKit/unretained-members-weak-arc.mm new file mode 100644 index 0000000000000..c0aaac09e68d8 --- /dev/null +++ b/clang/test/Analysis/Checkers/WebKit/unretained-members-weak-arc.mm @@ -0,0 +1,29 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.NoUnretainedMemberChecker -fobjc-runtime-has-weak -fobjc-weak -fobjc-arc -verify %s +// expected-no-diagnostics + +#include "objc-mock-types.h" + +struct Foo { + __weak NSString *weakPtr = nullptr; + Foo(); + ~Foo(); + void bar(); +}; + +@interface ObjectWithWeakProperty : NSObject +@property(nonatomic, weak) NSString *weak_prop; +@end + +@implementation ObjectWithWeakProperty +@end + +NS_REQUIRES_PROPERTY_DEFINITIONS +@interface NoSynthesisObjectWithWeakProperty : NSObject +@property(nonatomic, readonly, weak) NSString *weak_prop; +@end + +@implementation NoSynthesisObjectWithWeakProperty +- (NSString *)weak_prop { + return nil; +} +@end diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-members-weak.mm b/clang/test/Analysis/Checkers/WebKit/unretained-members-weak.mm new file mode 100644 index 0000000000000..422cf6189446d --- /dev/null +++ b/clang/test/Analysis/Checkers/WebKit/unretained-members-weak.mm @@ -0,0 +1,31 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.NoUnretainedMemberChecker -fobjc-runtime-has-weak -fobjc-weak -verify %s +// expected-no-diagnostics + +#include "objc-mock-types.h" + +struct Foo { + __weak NSString *weakPtr = nullptr; + Foo(); + ~Foo(); + void bar(); +}; + +@interface ObjectWithWeakProperty : NSObject +@property(nonatomic, weak) NSString *weak_prop; +@end + +@implementation ObjectWithWeakProperty +@end + +NS_REQUIRES_PROPERTY_DEFINITIONS +@interface NoSynthesisObjectWithWeakProperty : NSObject +@property(nonatomic, readonly, weak) NSString *weak_prop; +@end + +@implementation NoSynthesisObjectWithWeakProperty { + __weak NSNumber *weak_ivar; +} +- (NSString *)weak_prop { + return nil; +} +@end