diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst index 459f4950d2a65..5364dc150195d 100644 --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -3433,6 +3433,24 @@ Check for non-determinism caused by sorting of pointers. alpha.WebKit ^^^^^^^^^^^^ +alpha.webkit.ForwardDeclChecker +""""""""""""""""""""""""""""""" +Check for local variables, member variables, and function arguments that are forward declared. + +.. code-block:: cpp + + struct Obj; + Obj* provide(); + + struct Foo { + Obj* ptr; // warn + }; + + void foo() { + Obj* obj = provide(); // warn + consume(obj); // warn + } + .. _alpha-webkit-NoUncheckedPtrMemberChecker: alpha.webkit.MemoryUnsafeCastChecker @@ -3479,6 +3497,31 @@ Raw pointers and references to an object which supports CheckedPtr or CheckedRef See `WebKit Guidelines for Safer C++ Programming `_ for details. +alpha.webkit.NoUnretainedMemberChecker +"""""""""""""""""""""""""""""""""""""""" +Raw pointers and references to a NS or CF object can't be used as class members or ivars. Only RetainPtr is allowed for CF types regardless of whether ARC is enabled or disabled. Only RetainPtr is allowed for NS types when ARC is disabled. + +.. code-block:: cpp + + struct Foo { + NSObject *ptr; // warn + // ... + }; + +See `WebKit Guidelines for Safer C++ Programming `_ for details. + +alpha.webkit.UnretainedLambdaCapturesChecker +"""""""""""""""""""""""""""""""""""""""""""" +Raw pointers and references to NS or CF types can't be captured in lambdas. Only RetainPtr is allowed for CF types regardless of whether ARC is enabled or disabled, and only RetainPtr is allowed for NS types when ARC is disabled. + +.. code-block:: cpp + + void foo(NSObject *a, NSObject *b) { + [&, a](){ // warn about 'a' + do_something(b); // warn about 'b' + }; + }; + .. _alpha-webkit-UncountedCallArgsChecker: alpha.webkit.UncountedCallArgsChecker @@ -3574,6 +3617,12 @@ The goal of this rule is to make sure that lifetime of any dynamically allocated The rules of when to use and not to use CheckedPtr / CheckedRef are same as alpha.webkit.UncountedCallArgsChecker for ref-counted objects. +alpha.webkit.UnretainedCallArgsChecker +"""""""""""""""""""""""""""""""""""""" +The goal of this rule is to make sure that lifetime of any dynamically allocated NS or CF objects passed as a call argument keeps its memory region past the end of the call. This applies to call to any function, method, lambda, function pointer or functor. NS or CF objects aren't supposed to be allocated on stack so we check arguments for parameters of raw pointers and references to unretained types. + +The rules of when to use and not to use RetainPtr are same as alpha.webkit.UncountedCallArgsChecker for ref-counted objects. + alpha.webkit.UncountedLocalVarsChecker """""""""""""""""""""""""""""""""""""" The goal of this rule is to make sure that any uncounted local variable is backed by a ref-counted object with lifetime that is strictly larger than the scope of the uncounted local variable. To be on the safe side we require the scope of an uncounted variable to be embedded in the scope of ref-counted object that backs it. @@ -3660,6 +3709,71 @@ Here are some examples of situations that we warn about as they *might* be poten RefCountable* uncounted = counted.get(); // warn } +alpha.webkit.UnretainedLocalVarsChecker +""""""""""""""""""""""""""""""""""""""" +The goal of this rule is to make sure that any NS or CF local variable is backed by a RetainPtr with lifetime that is strictly larger than the scope of the unretained local variable. To be on the safe side we require the scope of an unretained variable to be embedded in the scope of Retainptr object that backs it. + +The rules of when to use and not to use RetainPtr are same as alpha.webkit.UncountedCallArgsChecker for ref-counted objects. + +These are examples of cases that we consider safe: + + .. code-block:: cpp + + void foo1() { + RetainPtr retained; + // The scope of unretained is EMBEDDED in the scope of retained. + { + NSObject* unretained = retained.get(); // ok + } + } + + void foo2(RetainPtr retained_param) { + NSObject* unretained = retained_param.get(); // ok + } + + void FooClass::foo_method() { + NSObject* unretained = this; // ok + } + +Here are some examples of situations that we warn about as they *might* be potentially unsafe. The logic is that either we're able to guarantee that a local variable is safe or it's considered unsafe. + + .. code-block:: cpp + + void foo1() { + NSObject* unretained = [[NSObject alloc] init]; // warn + } + + NSObject* global_unretained; + void foo2() { + NSObject* unretained = global_unretained; // warn + } + + void foo3() { + RetainPtr retained; + // The scope of unretained is not EMBEDDED in the scope of retained. + NSObject* unretained = retained.get(); // warn + } + +webkit.RetainPtrCtorAdoptChecker +"""""""""""""""""""""""""""""""" +The goal of this rule is to make sure the constructor of RetainPtr as well as adoptNS and adoptCF are used correctly. +When creating a RetainPtr with +1 semantics, adoptNS or adoptCF should be used, and in +0 semantics, RetainPtr constructor should be used. +Warn otherwise. + +These are examples of cases that we consider correct: + + .. code-block:: cpp + + RetainPtr ptr = adoptNS([[NSObject alloc] init]); // ok + RetainPtr ptr = CGImageGetColorSpace(image); // ok + +Here are some examples of cases that we consider incorrect use of RetainPtr constructor and adoptCF + + .. code-block:: cpp + + RetainPtr ptr = [[NSObject alloc] init]; // warn + auto ptr = adoptCF(CGImageGetColorSpace(image)); // warn + Debug Checkers --------------- diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td index be94b7cd0d458..96dc11e2da0c2 100644 --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -1787,6 +1787,10 @@ def UncountedLambdaCapturesChecker : Checker<"UncountedLambdaCapturesChecker">, let ParentPackage = WebKitAlpha in { +def ForwardDeclChecker : Checker<"ForwardDeclChecker">, + HelpText<"Check for forward declared local or member variables and function arguments">, + Documentation; + def MemoryUnsafeCastChecker : Checker<"MemoryUnsafeCastChecker">, HelpText<"Check for memory unsafe casts from base type to derived type.">, Documentation; @@ -1795,6 +1799,14 @@ def NoUncheckedPtrMemberChecker : Checker<"NoUncheckedPtrMemberChecker">, HelpText<"Check for no unchecked member variables.">, Documentation; +def NoUnretainedMemberChecker : Checker<"NoUnretainedMemberChecker">, + HelpText<"Check for no unretained member variables.">, + Documentation; + +def UnretainedLambdaCapturesChecker : Checker<"UnretainedLambdaCapturesChecker">, + HelpText<"Check unretained lambda captures.">, + Documentation; + def UncountedCallArgsChecker : Checker<"UncountedCallArgsChecker">, HelpText<"Check uncounted call arguments.">, Documentation; @@ -1803,6 +1815,10 @@ def UncheckedCallArgsChecker : Checker<"UncheckedCallArgsChecker">, HelpText<"Check unchecked call arguments.">, Documentation; +def UnretainedCallArgsChecker : Checker<"UnretainedCallArgsChecker">, + HelpText<"Check unretained call arguments.">, + Documentation; + def UncountedLocalVarsChecker : Checker<"UncountedLocalVarsChecker">, HelpText<"Check uncounted local variables.">, Documentation; @@ -1811,4 +1827,12 @@ def UncheckedLocalVarsChecker : Checker<"UncheckedLocalVarsChecker">, HelpText<"Check unchecked local variables.">, Documentation; +def UnretainedLocalVarsChecker : Checker<"UnretainedLocalVarsChecker">, + HelpText<"Check unretained local variables.">, + Documentation; + +def RetainPtrCtorAdoptChecker : Checker<"RetainPtrCtorAdoptChecker">, + HelpText<"Check for correct use of RetainPtr constructor, adoptNS, and adoptCF">, + Documentation; + } // end alpha.webkit diff --git a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt index f1a24dc2b8f62..38f6c9e4f2c0e 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt +++ b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt @@ -133,14 +133,16 @@ add_clang_library(clangStaticAnalyzerCheckers VLASizeChecker.cpp ValistChecker.cpp VirtualCallChecker.cpp - WebKit/RawPtrRefMemberChecker.cpp WebKit/ASTUtils.cpp + WebKit/ForwardDeclChecker.cpp WebKit/MemoryUnsafeCastChecker.cpp WebKit/PtrTypesSemantics.cpp WebKit/RefCntblBaseVirtualDtorChecker.cpp + WebKit/RetainPtrCtorAdoptChecker.cpp WebKit/RawPtrRefCallArgsChecker.cpp - WebKit/UncountedLambdaCapturesChecker.cpp + WebKit/RawPtrRefLambdaCapturesChecker.cpp WebKit/RawPtrRefLocalVarsChecker.cpp + WebKit/RawPtrRefMemberChecker.cpp LINK_LIBS clangAST diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp index a12853d01819f..88b98756b2ca8 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp @@ -8,6 +8,7 @@ #include "ASTUtils.h" #include "PtrTypesSemantics.h" +#include "clang/AST/Attr.h" #include "clang/AST/CXXInheritance.h" #include "clang/AST/Decl.h" #include "clang/AST/DeclCXX.h" @@ -24,8 +25,18 @@ bool isSafePtr(clang::CXXRecordDecl *Decl) { bool tryToFindPtrOrigin( const Expr *E, bool StopAtFirstRefCountedObj, + std::function isSafePtr, + std::function isSafePtrType, std::function callback) { while (E) { + if (auto *DRE = dyn_cast(E)) { + if (auto *VD = dyn_cast_or_null(DRE->getDecl())) { + auto QT = VD->getType(); + if (VD->hasGlobalStorage() && QT.isConstQualified()) { + return callback(E, true); + } + } + } if (auto *tempExpr = dyn_cast(E)) { E = tempExpr->getSubExpr(); continue; @@ -41,6 +52,10 @@ bool tryToFindPtrOrigin( break; } } + if (auto *TempExpr = dyn_cast(E)) { + if (isSafePtrType(TempExpr->getTypeAsWritten())) + return callback(TempExpr, true); + } if (auto *POE = dyn_cast(E)) { if (auto *RF = POE->getResultExpr()) { E = RF; @@ -51,11 +66,15 @@ bool tryToFindPtrOrigin( E = tempExpr->getSubExpr(); continue; } + if (auto *OpaqueValue = dyn_cast(E)) { + E = OpaqueValue->getSourceExpr(); + continue; + } if (auto *Expr = dyn_cast(E)) { return tryToFindPtrOrigin(Expr->getTrueExpr(), StopAtFirstRefCountedObj, - callback) && + isSafePtr, isSafePtrType, callback) && tryToFindPtrOrigin(Expr->getFalseExpr(), StopAtFirstRefCountedObj, - callback); + isSafePtr, isSafePtrType, callback); } if (auto *cast = dyn_cast(E)) { if (StopAtFirstRefCountedObj) { @@ -64,6 +83,8 @@ bool tryToFindPtrOrigin( if (isCtorOfSafePtr(ConversionFunc)) return callback(E, true); } + if (isa(E) && isSafePtrType(cast->getType())) + return callback(E, true); } // FIXME: This can give false "origin" that would lead to false negatives // in checkers. See https://reviews.llvm.org/D37023 for reference. @@ -85,14 +106,22 @@ bool tryToFindPtrOrigin( } if (auto *operatorCall = dyn_cast(E)) { - if (operatorCall->getNumArgs() == 1) { - E = operatorCall->getArg(0); - continue; + if (auto *Callee = operatorCall->getDirectCallee()) { + auto ClsName = safeGetName(Callee->getParent()); + if (isRefType(ClsName) || isCheckedPtr(ClsName) || + isRetainPtr(ClsName) || ClsName == "unique_ptr" || + ClsName == "UniqueRef" || ClsName == "WeakPtr" || + ClsName == "WeakRef") { + if (operatorCall->getNumArgs() == 1) { + E = operatorCall->getArg(0); + continue; + } + } } } if (auto *callee = call->getDirectCallee()) { - if (isCtorOfRefCounted(callee) || isCtorOfCheckedPtr(callee)) { + if (isCtorOfSafePtr(callee)) { if (StopAtFirstRefCountedObj) return callback(E, true); @@ -115,6 +144,11 @@ bool tryToFindPtrOrigin( E = call->getArg(0); continue; } + + auto Name = safeGetName(callee); + if (Name == "__builtin___CFStringMakeConstantString" || + Name == "NSClassFromString") + return callback(E, true); } } if (auto *ObjCMsgExpr = dyn_cast(E)) { @@ -122,7 +156,18 @@ bool tryToFindPtrOrigin( if (isSafePtrType(Method->getReturnType())) return callback(E, true); } + auto Selector = ObjCMsgExpr->getSelector(); + auto NameForFirstSlot = Selector.getNameForSlot(0); + if ((NameForFirstSlot == "class" || NameForFirstSlot == "superclass") && + !Selector.getNumArgs()) + return callback(E, true); } + if (auto *ObjCDict = dyn_cast(E)) + return callback(ObjCDict, true); + if (auto *ObjCArray = dyn_cast(E)) + return callback(ObjCArray, true); + if (auto *ObjCStr = dyn_cast(E)) + return callback(ObjCStr, true); if (auto *unaryOp = dyn_cast(E)) { // FIXME: Currently accepts ANY unary operator. Is it OK? E = unaryOp->getSubExpr(); @@ -138,9 +183,22 @@ bool tryToFindPtrOrigin( bool isASafeCallArg(const Expr *E) { assert(E); if (auto *Ref = dyn_cast(E)) { - if (auto *D = dyn_cast_or_null(Ref->getFoundDecl())) { + auto *FoundDecl = Ref->getFoundDecl(); + if (auto *D = dyn_cast_or_null(FoundDecl)) { if (isa(D) || D->isLocalVarDecl()) return true; + if (auto *ImplicitP = dyn_cast(D)) { + auto Kind = ImplicitP->getParameterKind(); + if (Kind == ImplicitParamKind::ObjCSelf || + Kind == ImplicitParamKind::ObjCCmd || + Kind == ImplicitParamKind::CXXThis || + Kind == ImplicitParamKind::CXXVTT) + return true; + } + } else if (auto *BD = dyn_cast_or_null(FoundDecl)) { + VarDecl *VD = BD->getHoldingVar(); + if (VD && (isa(VD) || VD->isLocalVarDecl())) + return true; } } if (isConstOwnerPtrMemberExpr(E)) @@ -204,7 +262,7 @@ bool EnsureFunctionAnalysis::isACallToEnsureFn(const clang::Expr *E) const { if (!Callee) return false; auto *Body = Callee->getBody(); - if (!Body) + if (!Body || Callee->isVirtualAsWritten()) return false; auto [CacheIt, IsNew] = Cache.insert(std::make_pair(Callee, false)); if (IsNew) diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h index b508043d0f37f..e2cc7b976adfc 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h @@ -54,6 +54,8 @@ class Expr; /// Returns false if any of calls to callbacks returned false. Otherwise true. bool tryToFindPtrOrigin( const clang::Expr *E, bool StopAtFirstRefCountedObj, + std::function isSafePtr, + std::function isSafePtrType, std::function 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 new file mode 100644 index 0000000000000..73a0e9eda5b20 --- /dev/null +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ForwardDeclChecker.cpp @@ -0,0 +1,338 @@ +//=======- ForwardDeclChecker.cpp --------------------------------*- C++ -*-==// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "ASTUtils.h" +#include "DiagOutputUtils.h" +#include "PtrTypesSemantics.h" +#include "clang/AST/CXXInheritance.h" +#include "clang/AST/Decl.h" +#include "clang/AST/DeclCXX.h" +#include "clang/AST/RecursiveASTVisitor.h" +#include "clang/Analysis/DomainSpecific/CocoaConventions.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" +#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h" +#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" +#include "clang/StaticAnalyzer/Core/Checker.h" +#include "llvm/ADT/DenseSet.h" +#include "llvm/Support/SaveAndRestore.h" +#include + +using namespace clang; +using namespace ento; + +namespace { + +class ForwardDeclChecker : public Checker> { + BugType Bug; + mutable BugReporter *BR; + mutable RetainTypeChecker RTC; + mutable llvm::DenseSet SystemTypes; + +public: + ForwardDeclChecker() + : Bug(this, "Forward declared member or local variable or parameter", + "WebKit coding guidelines") {} + + void checkASTDecl(const TranslationUnitDecl *TUD, AnalysisManager &MGR, + BugReporter &BRArg) const { + BR = &BRArg; + + // The calls to checkAST* from AnalysisConsumer don't + // visit template instantiations or lambda classes. We + // want to visit those, so we make our own RecursiveASTVisitor. + struct LocalVisitor : public RecursiveASTVisitor { + using Base = RecursiveASTVisitor; + + const ForwardDeclChecker *Checker; + Decl *DeclWithIssue{nullptr}; + + explicit LocalVisitor(const ForwardDeclChecker *Checker) + : Checker(Checker) { + assert(Checker); + } + + bool shouldVisitTemplateInstantiations() const { return true; } + bool shouldVisitImplicitCode() const { return false; } + + bool VisitTypedefDecl(TypedefDecl *TD) { + Checker->visitTypedef(TD); + return true; + } + + bool VisitRecordDecl(const RecordDecl *RD) { + Checker->visitRecordDecl(RD, DeclWithIssue); + return true; + } + + bool TraverseDecl(Decl *D) { + llvm::SaveAndRestore SavedDecl(DeclWithIssue); + if (D && (isa(D) || isa(D))) + DeclWithIssue = D; + return Base::TraverseDecl(D); + } + + bool VisitVarDecl(VarDecl *V) { + if (V->isLocalVarDecl()) + Checker->visitVarDecl(V, DeclWithIssue); + return true; + } + + bool VisitCallExpr(const CallExpr *CE) { + Checker->visitCallExpr(CE, DeclWithIssue); + return true; + } + + bool VisitCXXConstructExpr(const CXXConstructExpr *CE) { + Checker->visitConstructExpr(CE, DeclWithIssue); + return true; + } + + bool VisitObjCMessageExpr(const ObjCMessageExpr *ObjCMsgExpr) { + Checker->visitObjCMessageExpr(ObjCMsgExpr, DeclWithIssue); + return true; + } + }; + + LocalVisitor visitor(this); + RTC.visitTranslationUnitDecl(TUD); + visitor.TraverseDecl(const_cast(TUD)); + } + + void visitTypedef(const TypedefDecl *TD) const { + RTC.visitTypedef(TD); + auto QT = TD->getUnderlyingType().getCanonicalType(); + if (BR->getSourceManager().isInSystemHeader(TD->getBeginLoc())) { + if (auto *Type = QT.getTypePtrOrNull()) + SystemTypes.insert(Type); + } + } + + bool isUnknownType(QualType QT) const { + auto *CanonicalType = QT.getCanonicalType().getTypePtrOrNull(); + if (!CanonicalType) + return false; + auto PointeeQT = CanonicalType->getPointeeType(); + auto *PointeeType = PointeeQT.getTypePtrOrNull(); + if (!PointeeType) + return false; + auto *R = PointeeType->getAsCXXRecordDecl(); + if (!R) // Forward declaration of a Objective-C interface is safe. + return false; + auto Name = R->getName(); + if (R->hasDefinition()) + return false; + // Find a definition amongst template declarations. + if (auto *Specialization = dyn_cast(R)) { + if (auto *S = Specialization->getSpecializedTemplate()) { + for (S = S->getMostRecentDecl(); S; S = S->getPreviousDecl()) { + if (S->isThisDeclarationADefinition()) + return false; + } + } + } + return !RTC.isUnretained(QT) && !SystemTypes.contains(CanonicalType) && + !SystemTypes.contains(PointeeType) && !Name.starts_with("Opaque") && + Name != "_NSZone"; + } + + void visitRecordDecl(const RecordDecl *RD, const Decl *DeclWithIssue) const { + if (!RD->isThisDeclarationADefinition()) + return; + + if (RD->isImplicit() || RD->isLambda()) + return; + + const auto RDLocation = RD->getLocation(); + if (!RDLocation.isValid()) + return; + + const auto Kind = RD->getTagKind(); + if (Kind != TagTypeKind::Struct && Kind != TagTypeKind::Class) + return; + + if (BR->getSourceManager().isInSystemHeader(RDLocation)) + return; + + // Ref-counted smartpointers actually have raw-pointer to uncounted type as + // a member but we trust them to handle it correctly. + auto R = llvm::dyn_cast_or_null(RD); + if (!R || isRefCounted(R) || isCheckedPtr(R) || isRetainPtr(R)) + return; + + for (auto *Member : RD->fields()) { + auto QT = Member->getType(); + if (isUnknownType(QT)) { + SmallString<100> Buf; + llvm::raw_svector_ostream Os(Buf); + + const std::string TypeName = QT.getAsString(); + Os << "Member variable "; + printQuotedName(Os, Member); + Os << " uses a forward declared type '" << TypeName << "'"; + + const SourceLocation SrcLocToReport = Member->getBeginLoc(); + PathDiagnosticLocation BSLoc(SrcLocToReport, BR->getSourceManager()); + auto Report = std::make_unique(Bug, Os.str(), BSLoc); + Report->addRange(Member->getSourceRange()); + Report->setDeclWithIssue(DeclWithIssue); + BR->emitReport(std::move(Report)); + } + } + } + + void visitVarDecl(const VarDecl *V, const Decl *DeclWithIssue) const { + if (BR->getSourceManager().isInSystemHeader(V->getBeginLoc())) + return; + + auto QT = V->getType(); + if (!isUnknownType(QT)) + return; + + SmallString<100> Buf; + llvm::raw_svector_ostream Os(Buf); + Os << "Local variable "; + printQuotedQualifiedName(Os, V); + + reportBug(V->getBeginLoc(), V->getSourceRange(), DeclWithIssue, Os.str(), + QT); + } + + void visitCallExpr(const CallExpr *CE, const Decl *DeclWithIssue) const { + if (BR->getSourceManager().isInSystemHeader(CE->getExprLoc())) + return; + + if (auto *F = CE->getDirectCallee()) { + // Skip the first argument for overloaded member operators (e. g. lambda + // or std::function call operator). + unsigned ArgIdx = + isa(CE) && isa_and_nonnull(F); + + for (auto P = F->param_begin(); + P < F->param_end() && ArgIdx < CE->getNumArgs(); ++P, ++ArgIdx) + visitCallArg(CE->getArg(ArgIdx), *P, DeclWithIssue); + } + } + + void visitConstructExpr(const CXXConstructExpr *CE, + const Decl *DeclWithIssue) const { + if (BR->getSourceManager().isInSystemHeader(CE->getExprLoc())) + return; + + if (auto *F = CE->getConstructor()) { + // Skip the first argument for overloaded member operators (e. g. lambda + // or std::function call operator). + unsigned ArgIdx = + isa(CE) && isa_and_nonnull(F); + + for (auto P = F->param_begin(); + P < F->param_end() && ArgIdx < CE->getNumArgs(); ++P, ++ArgIdx) + visitCallArg(CE->getArg(ArgIdx), *P, DeclWithIssue); + } + } + + void visitObjCMessageExpr(const ObjCMessageExpr *E, + const Decl *DeclWithIssue) const { + if (BR->getSourceManager().isInSystemHeader(E->getExprLoc())) + return; + + if (auto *Receiver = E->getInstanceReceiver()) { + Receiver = Receiver->IgnoreParenCasts(); + if (isUnknownType(E->getReceiverType())) + reportUnknownRecieverType(Receiver, DeclWithIssue); + } + + auto *MethodDecl = E->getMethodDecl(); + if (!MethodDecl) + return; + + auto ArgCount = E->getNumArgs(); + for (unsigned i = 0; i < ArgCount && i < MethodDecl->param_size(); ++i) + visitCallArg(E->getArg(i), MethodDecl->getParamDecl(i), DeclWithIssue); + } + + void visitCallArg(const Expr *Arg, const ParmVarDecl *Param, + const Decl *DeclWithIssue) const { + auto *ArgExpr = Arg->IgnoreParenCasts(); + if (auto *InnerCE = dyn_cast(Arg)) { + auto *InnerCallee = InnerCE->getDirectCallee(); + if (InnerCallee && InnerCallee->isInStdNamespace() && + safeGetName(InnerCallee) == "move" && InnerCE->getNumArgs() == 1) { + ArgExpr = InnerCE->getArg(0); + if (ArgExpr) + ArgExpr = ArgExpr->IgnoreParenCasts(); + } + } + if (isa(ArgExpr) || isa(ArgExpr) || + isa(ArgExpr)) + return; + if (auto *DRE = dyn_cast(ArgExpr)) { + if (auto *ValDecl = DRE->getDecl()) { + if (isa(ValDecl)) + return; + } + } + + QualType ArgType = Param->getType(); + if (!isUnknownType(ArgType)) + return; + + reportUnknownArgType(Arg, Param, DeclWithIssue); + } + + void reportUnknownArgType(const Expr *CA, const ParmVarDecl *Param, + const Decl *DeclWithIssue) const { + assert(CA); + + SmallString<100> Buf; + llvm::raw_svector_ostream Os(Buf); + + const std::string paramName = safeGetName(Param); + Os << "Call argument"; + if (!paramName.empty()) { + Os << " for parameter "; + printQuotedQualifiedName(Os, Param); + } + + reportBug(CA->getExprLoc(), CA->getSourceRange(), DeclWithIssue, Os.str(), + Param->getType()); + } + + void reportUnknownRecieverType(const Expr *Receiver, + const Decl *DeclWithIssue) const { + assert(Receiver); + reportBug(Receiver->getExprLoc(), Receiver->getSourceRange(), DeclWithIssue, + "Receiver", Receiver->getType()); + } + + void reportBug(const SourceLocation &SrcLoc, const SourceRange &SrcRange, + const Decl *DeclWithIssue, const StringRef &Description, + QualType Type) const { + SmallString<100> Buf; + llvm::raw_svector_ostream Os(Buf); + + const std::string TypeName = Type.getAsString(); + Os << Description << " uses a forward declared type '" << TypeName << "'"; + + PathDiagnosticLocation BSLoc(SrcLoc, BR->getSourceManager()); + auto Report = std::make_unique(Bug, Os.str(), BSLoc); + Report->addRange(SrcRange); + Report->setDeclWithIssue(DeclWithIssue); + BR->emitReport(std::move(Report)); + } +}; + +} // namespace + +void ento::registerForwardDeclChecker(CheckerManager &Mgr) { + Mgr.registerChecker(); +} + +bool ento::shouldRegisterForwardDeclChecker(const CheckerManager &) { + return true; +} diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp index 8340de9e5a7a9..d7111bcb35115 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp @@ -8,11 +8,13 @@ #include "PtrTypesSemantics.h" #include "ASTUtils.h" +#include "clang/AST/Attr.h" #include "clang/AST/CXXInheritance.h" #include "clang/AST/Decl.h" #include "clang/AST/DeclCXX.h" #include "clang/AST/ExprCXX.h" #include "clang/AST/StmtVisitor.h" +#include "clang/Analysis/DomainSpecific/CocoaConventions.h" #include using namespace clang; @@ -44,8 +46,18 @@ hasPublicMethodInBase(const CXXBaseSpecifier *Base, StringRef NameToMatch) { return std::nullopt; const CXXRecordDecl *R = T->getAsCXXRecordDecl(); - if (!R) - return std::nullopt; + if (!R) { + auto CT = Base->getType().getCanonicalType(); + if (auto *TST = dyn_cast(CT)) { + auto TmplName = TST->getTemplateName(); + if (!TmplName.isNull()) { + if (auto *TD = TmplName.getAsTemplateDecl()) + R = dyn_cast_or_null(TD->getTemplatedDecl()); + } + } + if (!R) + return std::nullopt; + } if (!R->hasDefinition()) return std::nullopt; @@ -117,10 +129,24 @@ bool isRefType(const std::string &Name) { Name == "RefPtr" || Name == "RefPtrAllowingPartiallyDestroyed"; } +bool isRetainPtr(const std::string &Name) { + return Name == "RetainPtr" || Name == "RetainPtrArc"; +} + bool isCheckedPtr(const std::string &Name) { return Name == "CheckedPtr" || Name == "CheckedRef"; } +bool isSmartPtrClass(const std::string &Name) { + return isRefType(Name) || isCheckedPtr(Name) || isRetainPtr(Name) || + Name == "WeakPtr" || Name == "WeakPtrFactory" || + Name == "WeakPtrFactoryWithBitField" || Name == "WeakPtrImplBase" || + Name == "WeakPtrImplBaseSingleThread" || Name == "ThreadSafeWeakPtr" || + Name == "ThreadSafeWeakOrStrongPtr" || + Name == "ThreadSafeWeakPtrControlBlock" || + Name == "ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr"; +} + bool isCtorOfRefCounted(const clang::FunctionDecl *F) { assert(F); const std::string &FunctionName = safeGetName(F); @@ -140,8 +166,15 @@ bool isCtorOfCheckedPtr(const clang::FunctionDecl *F) { return isCheckedPtr(safeGetName(F)); } +bool isCtorOfRetainPtr(const clang::FunctionDecl *F) { + const std::string &FunctionName = safeGetName(F); + return FunctionName == "RetainPtr" || FunctionName == "adoptNS" || + FunctionName == "adoptCF" || FunctionName == "retainPtr" || + FunctionName == "RetainPtrArc" || FunctionName == "adoptNSArc"; +} + bool isCtorOfSafePtr(const clang::FunctionDecl *F) { - return isCtorOfRefCounted(F) || isCtorOfCheckedPtr(F); + return isCtorOfRefCounted(F) || isCtorOfCheckedPtr(F) || isCtorOfRetainPtr(F); } template @@ -152,22 +185,27 @@ static bool isPtrOfType(const clang::QualType T, Predicate Pred) { type = elaboratedT->desugar(); continue; } - auto *SpecialT = type->getAs(); - if (!SpecialT) - return false; - auto *Decl = SpecialT->getTemplateName().getAsTemplateDecl(); - if (!Decl) - return false; - return Pred(Decl->getNameAsString()); + if (auto *SpecialT = type->getAs()) { + auto *Decl = SpecialT->getTemplateName().getAsTemplateDecl(); + return Decl && Pred(Decl->getNameAsString()); + } else if (auto *DTS = type->getAs()) { + auto *Decl = DTS->getTemplateName().getAsTemplateDecl(); + return Decl && Pred(Decl->getNameAsString()); + } else + break; } return false; } -bool isSafePtrType(const clang::QualType T) { +bool isRefOrCheckedPtrType(const clang::QualType T) { return isPtrOfType( T, [](auto Name) { return isRefType(Name) || isCheckedPtr(Name); }); } +bool isRetainPtrType(const clang::QualType T) { + return isPtrOfType(T, [](auto Name) { return isRetainPtr(Name); }); +} + bool isOwnerPtrType(const clang::QualType T) { return isPtrOfType(T, [](auto Name) { return isRefType(Name) || isCheckedPtr(Name) || Name == "unique_ptr" || @@ -195,6 +233,86 @@ std::optional isUnchecked(const QualType T) { return isUnchecked(T->getAsCXXRecordDecl()); } +void RetainTypeChecker::visitTranslationUnitDecl( + const TranslationUnitDecl *TUD) { + IsARCEnabled = TUD->getLangOpts().ObjCAutoRefCount; +} + +void RetainTypeChecker::visitTypedef(const TypedefDecl *TD) { + auto QT = TD->getUnderlyingType(); + if (!QT->isPointerType()) + return; + + auto PointeeQT = QT->getPointeeType(); + const RecordType *RT = PointeeQT->getAs(); + if (!RT) { + if (TD->hasAttr() || TD->hasAttr()) { + if (auto *Type = TD->getTypeForDecl()) + RecordlessTypes.insert(Type); + } + return; + } + + for (auto *Redecl : RT->getDecl()->getMostRecentDecl()->redecls()) { + if (Redecl->getAttr() || + Redecl->getAttr()) { + CFPointees.insert(RT); + return; + } + } +} + +bool RetainTypeChecker::isUnretained(const QualType QT, bool ignoreARC) { + if (ento::cocoa::isCocoaObjectRef(QT) && (!IsARCEnabled || ignoreARC)) + return true; + auto CanonicalType = QT.getCanonicalType(); + auto PointeeType = CanonicalType->getPointeeType(); + auto *RT = dyn_cast_or_null(PointeeType.getTypePtrOrNull()); + if (!RT) { + auto *Type = QT.getTypePtrOrNull(); + while (Type) { + if (RecordlessTypes.contains(Type)) + return true; + auto *ET = dyn_cast_or_null(Type); + if (!ET) + break; + Type = ET->desugar().getTypePtrOrNull(); + } + } + 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 (isRetainPtr(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. @@ -230,16 +348,20 @@ std::optional isUncheckedPtr(const QualType T) { return false; } -std::optional isUnsafePtr(const QualType T) { +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); - if (isUncountedPtr && isUncheckedPtr) - return *isUncountedPtr || *isUncheckedPtr; + auto isUnretainedPtr = isUnretained(T, IsArcEnabled); + std::optional result; if (isUncountedPtr) - return *isUncountedPtr; - return isUncheckedPtr; + result = *isUncountedPtr; + if (isUncheckedPtr) + result = result ? *result || *isUncheckedPtr : *isUncheckedPtr; + if (isUnretainedPtr) + result = result ? *result || *isUnretainedPtr : *isUnretainedPtr; + return result; } } return false; @@ -248,6 +370,8 @@ std::optional isUnsafePtr(const QualType T) { std::optional isGetterOfSafePtr(const CXXMethodDecl *M) { assert(M); + std::optional RTC; + if (isa(M)) { const CXXRecordDecl *calleeMethodsClass = M->getParent(); auto className = safeGetName(calleeMethodsClass); @@ -263,16 +387,34 @@ std::optional isGetterOfSafePtr(const CXXMethodDecl *M) { method == "impl")) return true; + if (isRetainPtr(className) && method == "get") + return true; + // Ref -> T conversion // FIXME: Currently allowing any Ref -> whatever cast. if (isRefType(className)) { - if (auto *maybeRefToRawOperator = dyn_cast(M)) - return isUnsafePtr(maybeRefToRawOperator->getConversionType()); + if (auto *maybeRefToRawOperator = dyn_cast(M)) { + auto QT = maybeRefToRawOperator->getConversionType(); + auto *T = QT.getTypePtrOrNull(); + return T && (T->isPointerType() || T->isReferenceType()); + } } if (isCheckedPtr(className)) { - if (auto *maybeRefToRawOperator = dyn_cast(M)) - return isUnsafePtr(maybeRefToRawOperator->getConversionType()); + if (auto *maybeRefToRawOperator = dyn_cast(M)) { + auto QT = maybeRefToRawOperator->getConversionType(); + auto *T = QT.getTypePtrOrNull(); + return T && (T->isPointerType() || T->isReferenceType()); + } + } + + if (isRetainPtr(className)) { + if (auto *maybeRefToRawOperator = dyn_cast(M)) { + auto QT = maybeRefToRawOperator->getConversionType(); + auto *T = QT.getTypePtrOrNull(); + return T && (T->isPointerType() || T->isReferenceType() || + T->isObjCObjectPointerType()); + } } } return false; @@ -297,6 +439,20 @@ bool isCheckedPtr(const CXXRecordDecl *R) { return false; } +bool isRetainPtr(const CXXRecordDecl *R) { + assert(R); + if (auto *TmplR = R->getTemplateInstantiationPattern()) + return isRetainPtr(safeGetName(TmplR)); + return false; +} + +bool isSmartPtr(const CXXRecordDecl *R) { + assert(R); + if (auto *TmplR = R->getTemplateInstantiationPattern()) + return isSmartPtrClass(safeGetName(TmplR)); + return false; +} + bool isPtrConversion(const FunctionDecl *F) { assert(F); if (isCtorOfRefCounted(F)) @@ -307,12 +463,24 @@ bool isPtrConversion(const FunctionDecl *F) { if (FunctionName == "getPtr" || FunctionName == "WeakPtr" || FunctionName == "dynamicDowncast" || FunctionName == "downcast" || FunctionName == "checkedDowncast" || - FunctionName == "uncheckedDowncast" || FunctionName == "bitwise_cast") + FunctionName == "uncheckedDowncast" || FunctionName == "bitwise_cast" || + FunctionName == "bridge_cast" || FunctionName == "bridge_id_cast" || + FunctionName == "dynamic_cf_cast" || FunctionName == "checked_cf_cast" || + FunctionName == "dynamic_objc_cast" || + FunctionName == "checked_objc_cast") return true; return false; } +bool isTrivialBuiltinFunction(const FunctionDecl *F) { + if (!F || !F->getDeclName().isIdentifier()) + return false; + auto Name = F->getName(); + return Name.starts_with("__builtin") || Name == "__libcpp_verbose_abort" || + Name.starts_with("os_log") || Name.starts_with("_os_log"); +} + bool isSingleton(const FunctionDecl *F) { assert(F); // FIXME: check # of params == 1 @@ -372,6 +540,10 @@ class TrivialFunctionAnalysisVisitor TrivialFunctionAnalysisVisitor(CacheTy &Cache) : Cache(Cache) {} bool IsFunctionTrivial(const Decl *D) { + if (auto *FnDecl = dyn_cast(D)) { + if (FnDecl->isVirtualAsWritten()) + return false; + } return WithCachedResult(D, [&]() { if (auto *CtorDecl = dyn_cast(D)) { for (auto *CtorInit : CtorDecl->inits()) { @@ -486,8 +658,7 @@ class TrivialFunctionAnalysisVisitor Name == "isMainThreadOrGCThread" || Name == "isMainRunLoop" || Name == "isWebThread" || Name == "isUIThread" || Name == "mayBeGCThread" || Name == "compilerFenceForCrash" || - Name == "bitwise_cast" || Name.find("__builtin") == 0 || - Name == "__libcpp_verbose_abort") + Name == "bitwise_cast" || isTrivialBuiltinFunction(Callee)) return true; return IsFunctionTrivial(Callee); diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h index 9adfc0f1f5726..f9fcfe9878d54 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h @@ -11,6 +11,7 @@ #include "llvm/ADT/APInt.h" #include "llvm/ADT/DenseMap.h" +#include "llvm/ADT/DenseSet.h" #include "llvm/ADT/PointerUnion.h" #include @@ -21,8 +22,11 @@ class CXXRecordDecl; class Decl; class FunctionDecl; class QualType; +class RecordType; class Stmt; +class TranslationUnitDecl; class Type; +class TypedefDecl; // Ref-countability of a type is implicitly defined by Ref and RefPtr // implementation. It can be modeled as: type T having public methods ref() and @@ -51,6 +55,13 @@ bool isRefCounted(const clang::CXXRecordDecl *Class); /// \returns true if \p Class is a CheckedPtr / CheckedRef, false if not. bool isCheckedPtr(const clang::CXXRecordDecl *Class); +/// \returns true if \p Class is a RetainPtr, false if not. +bool isRetainPtr(const clang::CXXRecordDecl *Class); + +/// \returns true if \p Class is a smart pointer (RefPtr, WeakPtr, etc...), +/// false if not. +bool isSmartPtr(const clang::CXXRecordDecl *Class); + /// \returns true if \p Class is ref-countable AND not ref-counted, false if /// not, std::nullopt if inconclusive. std::optional isUncounted(const clang::QualType T); @@ -59,6 +70,24 @@ std::optional isUncounted(const clang::QualType T); /// not, std::nullopt if inconclusive. std::optional isUnchecked(const clang::QualType T); +/// An inter-procedural analysis facility that detects CF types with the +/// underlying pointer type. +class RetainTypeChecker { + llvm::DenseSet CFPointees; + llvm::DenseSet RecordlessTypes; + bool IsARCEnabled{false}; + +public: + void visitTranslationUnitDecl(const TranslationUnitDecl *); + void visitTypedef(const TypedefDecl *); + bool isUnretained(const QualType, bool ignoreARC = false); + bool isARCEnabled() const { return IsARCEnabled; } +}; + +/// \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); @@ -77,11 +106,14 @@ 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); +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 isSafePtrType(const clang::QualType T); +bool isRefOrCheckedPtrType(const clang::QualType T); + +/// \returns true if \p T is a RetainPtr, false if not. +bool isRetainPtrType(const clang::QualType T); /// \returns true if \p T is a RefPtr, Ref, CheckedPtr, CheckedRef, or /// unique_ptr, false if not. @@ -105,6 +137,12 @@ bool isRefType(const std::string &Name); /// \returns true if \p Name is CheckedRef or CheckedPtr, false if not. bool isCheckedPtr(const std::string &Name); +/// \returns true if \p Name is RetainPtr or its variant, false if not. +bool isRetainPtr(const std::string &Name); + +/// \returns true if \p Name is a smart pointer type name, false if not. +bool isSmartPtrClass(const std::string &Name); + /// \returns true if \p M is getter of a ref-counted class, false if not. std::optional isGetterOfSafePtr(const clang::CXXMethodDecl *Method); @@ -112,6 +150,9 @@ std::optional isGetterOfSafePtr(const clang::CXXMethodDecl *Method); /// pointer types. bool isPtrConversion(const FunctionDecl *F); +/// \returns true if \p F is a builtin function which is considered trivial. +bool isTrivialBuiltinFunction(const FunctionDecl *F); + /// \returns true if \p F is a static singleton function. bool isSingleton(const FunctionDecl *F); diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp index f4ad549b8734a..e0fbee78a0fbc 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp @@ -13,6 +13,7 @@ #include "clang/AST/Decl.h" #include "clang/AST/DeclCXX.h" #include "clang/AST/RecursiveASTVisitor.h" +#include "clang/Analysis/DomainSpecific/CocoaConventions.h" #include "clang/Basic/SourceLocation.h" #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h" @@ -35,12 +36,18 @@ class RawPtrRefCallArgsChecker TrivialFunctionAnalysis TFA; EnsureFunctionAnalysis EFA; +protected: + mutable std::optional RTC; + public: RawPtrRefCallArgsChecker(const char *description) : Bug(this, description, "WebKit coding guidelines") {} virtual std::optional isUnsafeType(QualType) const = 0; virtual std::optional isUnsafePtr(QualType) const = 0; + 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 const char *ptrKind() const = 0; void checkASTDecl(const TranslationUnitDecl *TUD, AnalysisManager &MGR, @@ -65,7 +72,7 @@ class RawPtrRefCallArgsChecker bool shouldVisitImplicitCode() const { return false; } bool TraverseClassTemplateDecl(ClassTemplateDecl *Decl) { - if (isRefType(safeGetName(Decl))) + if (isSmartPtrClass(safeGetName(Decl))) return true; return Base::TraverseClassTemplateDecl(Decl); } @@ -81,9 +88,22 @@ class RawPtrRefCallArgsChecker Checker->visitCallExpr(CE, DeclWithIssue); return true; } + + bool VisitTypedefDecl(TypedefDecl *TD) { + if (Checker->RTC) + Checker->RTC->visitTypedef(TD); + return true; + } + + bool VisitObjCMessageExpr(ObjCMessageExpr *ObjCMsgExpr) { + Checker->visitObjCMessageExpr(ObjCMsgExpr, DeclWithIssue); + return true; + } }; LocalVisitor visitor(this); + if (RTC) + RTC->visitTranslationUnitDecl(TUD); visitor.TraverseDecl(const_cast(TUD)); } @@ -123,7 +143,7 @@ class RawPtrRefCallArgsChecker // if ((*P)->hasAttr()) // continue; - QualType ArgType = (*P)->getType().getCanonicalType(); + QualType ArgType = (*P)->getType(); // FIXME: more complex types (arrays, references to raw pointers, etc) std::optional IsUncounted = isUnsafePtr(ArgType); if (!IsUncounted || !(*IsUncounted)) @@ -139,29 +159,88 @@ class RawPtrRefCallArgsChecker reportBug(Arg, *P, D); } + for (; ArgIdx < CE->getNumArgs(); ++ArgIdx) { + const auto *Arg = CE->getArg(ArgIdx); + auto ArgType = Arg->getType(); + std::optional IsUncounted = isUnsafePtr(ArgType); + if (!IsUncounted || !(*IsUncounted)) + continue; + + if (auto *defaultArg = dyn_cast(Arg)) + Arg = defaultArg->getExpr(); + + if (isPtrOriginSafe(Arg)) + continue; + + reportBug(Arg, nullptr, D); + } + } + } + + void visitObjCMessageExpr(const ObjCMessageExpr *E, const Decl *D) const { + 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; + } + reportBugOnReceiver(Receiver, D); + } + } + + auto *MethodDecl = E->getMethodDecl(); + if (!MethodDecl) + return; + + auto ArgCount = E->getNumArgs(); + for (unsigned i = 0; i < ArgCount; ++i) { + auto *Arg = E->getArg(i); + bool hasParam = i < MethodDecl->param_size(); + auto *Param = hasParam ? MethodDecl->getParamDecl(i) : nullptr; + auto ArgType = Arg->getType(); + std::optional IsUnsafe = isUnsafePtr(ArgType); + if (!IsUnsafe || !(*IsUnsafe)) + continue; + if (isPtrOriginSafe(Arg)) + continue; + reportBug(Arg, Param, D); } } bool isPtrOriginSafe(const Expr *Arg) const { - return tryToFindPtrOrigin(Arg, /*StopAtFirstRefCountedObj=*/true, - [&](const clang::Expr *ArgOrigin, bool IsSafe) { - if (IsSafe) - return true; - if (isa(ArgOrigin)) { - // foo(nullptr) - return true; - } - if (isa(ArgOrigin)) { - // FIXME: Check the value. - // foo(NULL) - return true; - } - if (isASafeCallArg(ArgOrigin)) - return true; - if (EFA.isACallToEnsureFn(ArgOrigin)) - return true; - return false; - }); + return tryToFindPtrOrigin( + Arg, /*StopAtFirstRefCountedObj=*/true, + [&](const clang::CXXRecordDecl *Record) { return isSafePtr(Record); }, + [&](const clang::QualType T) { return isSafePtrType(T); }, + [&](const clang::Expr *ArgOrigin, bool IsSafe) { + if (IsSafe) + return true; + if (isa(ArgOrigin)) { + // foo(nullptr) + return true; + } + if (isa(ArgOrigin)) { + // FIXME: Check the value. + // foo(NULL) + return true; + } + if (isa(ArgOrigin)) + return true; + if (isASafeCallArg(ArgOrigin)) + return true; + if (EFA.isACallToEnsureFn(ArgOrigin)) + return true; + if (isSafeExpr(ArgOrigin)) + return true; + return false; + }); } bool shouldSkipCall(const CallExpr *CE) const { @@ -170,7 +249,10 @@ class RawPtrRefCallArgsChecker if (BR->getSourceManager().isInSystemHeader(CE->getExprLoc())) return true; - if (Callee && TFA.isTrivial(Callee)) + if (Callee && TFA.isTrivial(Callee) && !Callee->isVirtualAsWritten()) + return true; + + if (isTrivialBuiltinFunction(Callee)) return true; if (CE->getNumArgs() == 0) @@ -185,7 +267,7 @@ class RawPtrRefCallArgsChecker auto *callee = MemberOp->getDirectCallee(); if (auto *calleeDecl = dyn_cast(callee)) { if (const CXXRecordDecl *classDecl = calleeDecl->getParent()) { - if (isRefCounted(classDecl)) + if (isSafePtr(classDecl)) return true; } } @@ -210,15 +292,12 @@ class RawPtrRefCallArgsChecker overloadedOperatorType == OO_PipePipe) return true; - if (isCtorOfRefCounted(Callee)) + if (isCtorOfSafePtr(Callee) || isPtrConversion(Callee)) return true; auto name = safeGetName(Callee); if (name == "adoptRef" || name == "getPtr" || name == "WeakPtr" || - name == "dynamicDowncast" || name == "downcast" || - name == "checkedDowncast" || name == "uncheckedDowncast" || - name == "bitwise_cast" || name == "is" || name == "equal" || - name == "hash" || name == "isType" || + name == "is" || name == "equal" || name == "hash" || name == "isType" || // FIXME: Most/all of these should be implemented via attributes. name == "equalIgnoringASCIICase" || name == "equalIgnoringASCIICaseCommon" || @@ -275,9 +354,10 @@ class RawPtrRefCallArgsChecker } Os << " is " << ptrKind() << " and unsafe."; + bool usesDefaultArgValue = isa(CallArg) && Param; const SourceLocation SrcLocToReport = - isa(CallArg) ? Param->getDefaultArg()->getExprLoc() - : CallArg->getSourceRange().getBegin(); + usesDefaultArgValue ? Param->getDefaultArg()->getExprLoc() + : CallArg->getSourceRange().getBegin(); PathDiagnosticLocation BSLoc(SrcLocToReport, BR->getSourceManager()); auto Report = std::make_unique(Bug, Os.str(), BSLoc); @@ -302,6 +382,23 @@ class RawPtrRefCallArgsChecker Report->setDeclWithIssue(DeclWithIssue); BR->emitReport(std::move(Report)); } + + void reportBugOnReceiver(const Expr *CallArg, + const Decl *DeclWithIssue) const { + assert(CallArg); + + const SourceLocation SrcLocToReport = CallArg->getSourceRange().getBegin(); + + SmallString<100> Buf; + llvm::raw_svector_ostream Os(Buf); + Os << "Reciever is " << ptrKind() << " and unsafe."; + + PathDiagnosticLocation BSLoc(SrcLocToReport, BR->getSourceManager()); + auto Report = std::make_unique(Bug, Os.str(), BSLoc); + Report->addRange(CallArg->getSourceRange()); + Report->setDeclWithIssue(DeclWithIssue); + BR->emitReport(std::move(Report)); + } }; class UncountedCallArgsChecker final : public RawPtrRefCallArgsChecker { @@ -315,7 +412,15 @@ class UncountedCallArgsChecker final : public RawPtrRefCallArgsChecker { } std::optional isUnsafePtr(QualType QT) const final { - return isUncountedPtr(QT); + return isUncountedPtr(QT.getCanonicalType()); + } + + bool isSafePtr(const CXXRecordDecl *Record) const final { + return isRefCounted(Record) || isCheckedPtr(Record); + } + + bool isSafePtrType(const QualType type) const final { + return isRefOrCheckedPtrType(type); } const char *ptrKind() const final { return "uncounted"; } @@ -332,12 +437,52 @@ class UncheckedCallArgsChecker final : public RawPtrRefCallArgsChecker { } std::optional isUnsafePtr(QualType QT) const final { - return isUncheckedPtr(QT); + return isUncheckedPtr(QT.getCanonicalType()); + } + + bool isSafePtr(const CXXRecordDecl *Record) const final { + return isRefCounted(Record) || isCheckedPtr(Record); + } + + bool isSafePtrType(const QualType type) const final { + return isRefOrCheckedPtrType(type); } const char *ptrKind() const final { return "unchecked"; } }; +class UnretainedCallArgsChecker final : public RawPtrRefCallArgsChecker { +public: + UnretainedCallArgsChecker() + : RawPtrRefCallArgsChecker("Unretained call argument for a raw " + "pointer/reference parameter") { + RTC = RetainTypeChecker(); + } + + std::optional isUnsafeType(QualType QT) const final { + return RTC->isUnretained(QT); + } + + std::optional isUnsafePtr(QualType QT) const final { + return RTC->isUnretained(QT); + } + + bool isSafePtr(const CXXRecordDecl *Record) const final { + return isRetainPtr(Record); + } + + bool isSafePtrType(const QualType type) const final { + return isRetainPtrType(type); + } + + bool isSafeExpr(const Expr *E) const final { + return ento::cocoa::isCocoaObjectRef(E->getType()) && + isa(E); + } + + const char *ptrKind() const final { return "unretained"; } +}; + } // namespace void ento::registerUncountedCallArgsChecker(CheckerManager &Mgr) { @@ -355,3 +500,11 @@ void ento::registerUncheckedCallArgsChecker(CheckerManager &Mgr) { bool ento::shouldRegisterUncheckedCallArgsChecker(const CheckerManager &) { return true; } + +void ento::registerUnretainedCallArgsChecker(CheckerManager &Mgr) { + Mgr.registerChecker(); +} + +bool ento::shouldRegisterUnretainedCallArgsChecker(const CheckerManager &) { + return true; +} diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp similarity index 64% rename from clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp rename to clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp index 3fd722c6df70a..bca734c196e72 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp @@ -21,15 +21,24 @@ using namespace clang; using namespace ento; namespace { -class UncountedLambdaCapturesChecker +class RawPtrRefLambdaCapturesChecker : public Checker> { private: - BugType Bug{this, "Lambda capture of uncounted variable", - "WebKit coding guidelines"}; + BugType Bug; mutable BugReporter *BR = nullptr; TrivialFunctionAnalysis TFA; +protected: + mutable std::optional RTC; + public: + RawPtrRefLambdaCapturesChecker(const char *description) + : Bug(this, description, "WebKit coding guidelines") {} + + virtual std::optional isUnsafePtr(QualType) const = 0; + virtual bool isPtrType(const std::string &) const = 0; + virtual const char *ptrKind(QualType QT) const = 0; + void checkASTDecl(const TranslationUnitDecl *TUD, AnalysisManager &MGR, BugReporter &BRArg) const { BR = &BRArg; @@ -38,7 +47,7 @@ class UncountedLambdaCapturesChecker // visit template instantiations or lambda classes. We // want to visit those, so we make our own RecursiveASTVisitor. struct LocalVisitor : public RecursiveASTVisitor { - const UncountedLambdaCapturesChecker *Checker; + const RawPtrRefLambdaCapturesChecker *Checker; llvm::DenseSet DeclRefExprsToIgnore; llvm::DenseSet LambdasToIgnore; llvm::DenseSet ProtectedThisDecls; @@ -48,7 +57,7 @@ class UncountedLambdaCapturesChecker using Base = RecursiveASTVisitor; - explicit LocalVisitor(const UncountedLambdaCapturesChecker *Checker) + explicit LocalVisitor(const RawPtrRefLambdaCapturesChecker *Checker) : Checker(Checker) { assert(Checker); } @@ -63,15 +72,32 @@ class UncountedLambdaCapturesChecker return Base::TraverseCXXMethodDecl(CXXMD); } + bool TraverseObjCMethodDecl(ObjCMethodDecl *OCMD) { + llvm::SaveAndRestore SavedDecl(ClsType); + if (OCMD && OCMD->isInstanceMethod()) { + if (auto *ImplParamDecl = OCMD->getSelfDecl()) + ClsType = ImplParamDecl->getType(); + } + return Base::TraverseObjCMethodDecl(OCMD); + } + + bool VisitTypedefDecl(TypedefDecl *TD) { + if (Checker->RTC) + Checker->RTC->visitTypedef(TD); + return true; + } + bool shouldCheckThis() { - auto result = !ClsType.isNull() ? isUnsafePtr(ClsType) : std::nullopt; + auto result = + !ClsType.isNull() ? Checker->isUnsafePtr(ClsType) : std::nullopt; return result && *result; } bool VisitLambdaExpr(LambdaExpr *L) { if (LambdasToIgnore.contains(L)) return true; - Checker->visitLambdaExpr(L, shouldCheckThis() && !hasProtectedThis(L)); + Checker->visitLambdaExpr(L, shouldCheckThis() && !hasProtectedThis(L), + ClsType); return true; } @@ -99,7 +125,8 @@ class UncountedLambdaCapturesChecker if (!L) return true; LambdasToIgnore.insert(L); - Checker->visitLambdaExpr(L, shouldCheckThis() && !hasProtectedThis(L)); + Checker->visitLambdaExpr(L, shouldCheckThis() && !hasProtectedThis(L), + ClsType); return true; } @@ -124,8 +151,8 @@ class UncountedLambdaCapturesChecker if (auto *L = findLambdaInArg(Arg)) { LambdasToIgnore.insert(L); if (!Param->hasAttr()) - Checker->visitLambdaExpr(L, shouldCheckThis() && - !hasProtectedThis(L)); + Checker->visitLambdaExpr( + L, shouldCheckThis() && !hasProtectedThis(L), ClsType); } ++ArgIndex; } @@ -145,8 +172,8 @@ class UncountedLambdaCapturesChecker if (auto *L = findLambdaInArg(Arg)) { LambdasToIgnore.insert(L); if (!Param->hasAttr() && !TreatAllArgsAsNoEscape) - Checker->visitLambdaExpr(L, shouldCheckThis() && - !hasProtectedThis(L)); + Checker->visitLambdaExpr( + L, shouldCheckThis() && !hasProtectedThis(L), ClsType); } ++ArgIndex; } @@ -171,14 +198,22 @@ class UncountedLambdaCapturesChecker auto *CtorArg = CE->getArg(0)->IgnoreParenCasts(); if (!CtorArg) return nullptr; - if (auto *Lambda = dyn_cast(CtorArg)) { + auto *InnerCE = dyn_cast_or_null(CtorArg); + if (InnerCE && InnerCE->getNumArgs()) + CtorArg = InnerCE->getArg(0)->IgnoreParenCasts(); + auto updateIgnoreList = [&] { ConstructToIgnore.insert(CE); + if (InnerCE) + ConstructToIgnore.insert(InnerCE); + }; + if (auto *Lambda = dyn_cast(CtorArg)) { + updateIgnoreList(); return Lambda; } if (auto *TempExpr = dyn_cast(CtorArg)) { E = TempExpr->getSubExpr()->IgnoreParenCasts(); if (auto *Lambda = dyn_cast(E)) { - ConstructToIgnore.insert(CE); + updateIgnoreList(); return Lambda; } } @@ -191,10 +226,14 @@ class UncountedLambdaCapturesChecker auto *Init = VD->getInit(); if (!Init) return nullptr; + if (auto *Lambda = dyn_cast(Init)) { + updateIgnoreList(); + return Lambda; + } TempExpr = dyn_cast(Init->IgnoreParenCasts()); if (!TempExpr) return nullptr; - ConstructToIgnore.insert(CE); + updateIgnoreList(); return dyn_cast_or_null(TempExpr->getSubExpr()); } @@ -227,8 +266,6 @@ class UncountedLambdaCapturesChecker return; DeclRefExprsToIgnore.insert(ArgRef); LambdasToIgnore.insert(L); - Checker->visitLambdaExpr(L, shouldCheckThis() && !hasProtectedThis(L), - /* ignoreParamVarDecl */ true); } bool hasProtectedThis(LambdaExpr *L) { @@ -249,22 +286,44 @@ class UncountedLambdaCapturesChecker auto *VD = dyn_cast(ValueDecl); if (!VD) return false; - auto *Init = VD->getInit()->IgnoreParenCasts(); + auto *Init = VD->getInit(); if (!Init) return false; - const Expr *Arg = Init; + const Expr *Arg = Init->IgnoreParenCasts(); do { if (auto *BTE = dyn_cast(Arg)) Arg = BTE->getSubExpr()->IgnoreParenCasts(); - if (auto *CE = dyn_cast_or_null(Arg)) { + if (auto *CE = dyn_cast(Arg)) { auto *Ctor = CE->getConstructor(); if (!Ctor) return false; auto clsName = safeGetName(Ctor->getParent()); - if (!isRefType(clsName) || !CE->getNumArgs()) - return false; - Arg = CE->getArg(0)->IgnoreParenCasts(); - continue; + if (Checker->isPtrType(clsName) && CE->getNumArgs()) { + Arg = CE->getArg(0)->IgnoreParenCasts(); + continue; + } + if (auto *Type = ClsType.getTypePtrOrNull()) { + if (auto *CXXR = Type->getPointeeCXXRecordDecl()) { + if (CXXR == Ctor->getParent() && Ctor->isMoveConstructor() && + CE->getNumArgs() == 1) { + Arg = CE->getArg(0)->IgnoreParenCasts(); + continue; + } + } + } + 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) { + Arg = CE->getArg(0)->IgnoreParenCasts(); + continue; + } + } } if (auto *OpCE = dyn_cast(Arg)) { auto OpCode = OpCE->getOperator(); @@ -273,7 +332,7 @@ class UncountedLambdaCapturesChecker if (!Callee) return false; auto clsName = safeGetName(Callee->getParent()); - if (!isRefType(clsName) || !OpCE->getNumArgs()) + if (!Checker->isPtrType(clsName) || !OpCE->getNumArgs()) return false; Arg = OpCE->getArg(0)->IgnoreParenCasts(); continue; @@ -288,17 +347,26 @@ class UncountedLambdaCapturesChecker } break; } while (Arg); - if (auto *DRE = dyn_cast(Arg)) - return ProtectedThisDecls.contains(DRE->getDecl()); + if (auto *DRE = dyn_cast(Arg)) { + auto *Decl = DRE->getDecl(); + if (auto *ImplicitParam = dyn_cast(Decl)) { + auto kind = ImplicitParam->getParameterKind(); + return kind == ImplicitParamKind::ObjCSelf || + kind == ImplicitParamKind::CXXThis; + } + return ProtectedThisDecls.contains(Decl); + } return isa(Arg); } }; LocalVisitor visitor(this); + if (RTC) + RTC->visitTranslationUnitDecl(TUD); visitor.TraverseDecl(const_cast(TUD)); } - void visitLambdaExpr(LambdaExpr *L, bool shouldCheckThis, + void visitLambdaExpr(LambdaExpr *L, bool shouldCheckThis, const QualType T, bool ignoreParamVarDecl = false) const { if (TFA.isTrivial(L->getBody())) return; @@ -307,22 +375,33 @@ class UncountedLambdaCapturesChecker ValueDecl *CapturedVar = C.getCapturedVar(); if (ignoreParamVarDecl && isa(CapturedVar)) continue; + if (auto *ImplicitParam = dyn_cast(CapturedVar)) { + auto kind = ImplicitParam->getParameterKind(); + if ((kind == ImplicitParamKind::ObjCSelf || + kind == ImplicitParamKind::CXXThis) && + !shouldCheckThis) + continue; + } QualType CapturedVarQualType = CapturedVar->getType(); auto IsUncountedPtr = isUnsafePtr(CapturedVar->getType()); if (IsUncountedPtr && *IsUncountedPtr) - reportBug(C, CapturedVar, CapturedVarQualType); + reportBug(C, CapturedVar, CapturedVarQualType, L); } else if (C.capturesThis() && shouldCheckThis) { if (ignoreParamVarDecl) // this is always a parameter to this function. continue; - reportBugOnThisPtr(C); + reportBugOnThisPtr(C, T); } } } void reportBug(const LambdaCapture &Capture, ValueDecl *CapturedVar, - const QualType T) const { + const QualType T, LambdaExpr *L) const { assert(CapturedVar); + auto Location = Capture.getLocation(); + if (isa(CapturedVar) && !Location.isValid()) + Location = L->getBeginLoc(); + SmallString<100> Buf; llvm::raw_svector_ostream Os(Buf); @@ -331,22 +410,22 @@ class UncountedLambdaCapturesChecker } else { Os << "Implicitly captured "; } - if (T->isPointerType()) { + if (isa(T) || isa(T)) { Os << "raw-pointer "; } else { - assert(T->isReferenceType()); Os << "reference "; } - printQuotedQualifiedName(Os, Capture.getCapturedVar()); - Os << " to ref-counted type or CheckedPtr-capable type is unsafe."; + printQuotedQualifiedName(Os, CapturedVar); + Os << " to " << ptrKind(T) << " type is unsafe."; - PathDiagnosticLocation BSLoc(Capture.getLocation(), BR->getSourceManager()); + PathDiagnosticLocation BSLoc(Location, BR->getSourceManager()); auto Report = std::make_unique(Bug, Os.str(), BSLoc); BR->emitReport(std::move(Report)); } - void reportBugOnThisPtr(const LambdaCapture &Capture) const { + void reportBugOnThisPtr(const LambdaCapture &Capture, + const QualType T) const { SmallString<100> Buf; llvm::raw_svector_ostream Os(Buf); @@ -356,14 +435,62 @@ class UncountedLambdaCapturesChecker Os << "Implicitly captured "; } - Os << "raw-pointer 'this' to ref-counted type or CheckedPtr-capable type " - "is unsafe."; + Os << "raw-pointer 'this' to " << ptrKind(T) << " type is unsafe."; PathDiagnosticLocation BSLoc(Capture.getLocation(), BR->getSourceManager()); auto Report = std::make_unique(Bug, Os.str(), BSLoc); BR->emitReport(std::move(Report)); } }; + +class UncountedLambdaCapturesChecker : public RawPtrRefLambdaCapturesChecker { +public: + UncountedLambdaCapturesChecker() + : RawPtrRefLambdaCapturesChecker("Lambda capture of uncounted or " + "unchecked variable") {} + + std::optional isUnsafePtr(QualType QT) const final { + auto result1 = isUncountedPtr(QT); + auto result2 = isUncheckedPtr(QT); + if (result1 && *result1) + return true; + if (result2 && *result2) + return true; + if (result1) + return *result1; + return result2; + } + + virtual bool isPtrType(const std::string &Name) const final { + return isRefType(Name) || isCheckedPtr(Name); + } + + const char *ptrKind(QualType QT) const final { + if (isUncounted(QT)) + return "uncounted"; + return "unchecked"; + } +}; + +class UnretainedLambdaCapturesChecker : public RawPtrRefLambdaCapturesChecker { +public: + UnretainedLambdaCapturesChecker() + : RawPtrRefLambdaCapturesChecker("Lambda capture of unretained " + "variables") { + RTC = RetainTypeChecker(); + } + + std::optional isUnsafePtr(QualType QT) const final { + return RTC->isUnretained(QT); + } + + virtual bool isPtrType(const std::string &Name) const final { + return isRetainPtr(Name); + } + + const char *ptrKind(QualType QT) const final { return "unretained"; } +}; + } // namespace void ento::registerUncountedLambdaCapturesChecker(CheckerManager &Mgr) { @@ -374,3 +501,12 @@ bool ento::shouldRegisterUncountedLambdaCapturesChecker( const CheckerManager &mgr) { return true; } + +void ento::registerUnretainedLambdaCapturesChecker(CheckerManager &Mgr) { + Mgr.registerChecker(); +} + +bool ento::shouldRegisterUnretainedLambdaCapturesChecker( + const CheckerManager &mgr) { + return true; +} diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp index 6e9c8f1217a91..bdb2d102f96a1 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLocalVarsChecker.cpp @@ -14,6 +14,7 @@ #include "clang/AST/DeclCXX.h" #include "clang/AST/ParentMapContext.h" #include "clang/AST/RecursiveASTVisitor.h" +#include "clang/Analysis/DomainSpecific/CocoaConventions.h" #include "clang/Basic/SourceLocation.h" #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h" @@ -84,7 +85,7 @@ struct GuardianVisitor : public RecursiveASTVisitor { bool VisitCXXMemberCallExpr(const CXXMemberCallExpr *MCE) { auto MethodName = safeGetName(MCE->getMethodDecl()); if (MethodName == "swap" || MethodName == "leakRef" || - MethodName == "releaseNonNull") { + MethodName == "releaseNonNull" || MethodName == "clear") { auto *ThisArg = MCE->getImplicitObjectArgument()->IgnoreParenCasts(); if (auto *VarRef = dyn_cast(ThisArg)) { if (VarRef->getDecl() == Guardian) @@ -171,11 +172,17 @@ class RawPtrRefLocalVarsChecker mutable BugReporter *BR; EnsureFunctionAnalysis EFA; +protected: + mutable std::optional RTC; + public: RawPtrRefLocalVarsChecker(const char *description) : Bug(this, description, "WebKit coding guidelines") {} virtual std::optional isUnsafePtr(const QualType T) const = 0; + virtual bool isSafePtr(const CXXRecordDecl *) const = 0; + virtual bool isSafePtrType(const QualType) const = 0; + virtual bool isSafeExpr(const Expr *) const { return false; } virtual const char *ptrKind() const = 0; void checkASTDecl(const TranslationUnitDecl *TUD, AnalysisManager &MGR, @@ -208,6 +215,12 @@ class RawPtrRefLocalVarsChecker return Base::TraverseDecl(D); } + bool VisitTypedefDecl(TypedefDecl *TD) { + if (Checker->RTC) + Checker->RTC->visitTypedef(TD); + return true; + } + bool VisitVarDecl(VarDecl *V) { auto *Init = V->getInit(); if (Init && V->isLocalVarDecl()) @@ -254,9 +267,17 @@ class RawPtrRefLocalVarsChecker return Base::TraverseCompoundStmt(CS); return true; } + + bool TraverseClassTemplateDecl(ClassTemplateDecl *Decl) { + if (isSmartPtrClass(safeGetName(Decl))) + return true; + return Base::TraverseClassTemplateDecl(Decl); + } }; LocalVisitor visitor(this); + if (RTC) + RTC->visitTranslationUnitDecl(TUD); visitor.TraverseDecl(const_cast(TUD)); } @@ -269,6 +290,10 @@ class RawPtrRefLocalVarsChecker if (IsUncountedPtr && *IsUncountedPtr) { if (tryToFindPtrOrigin( Value, /*StopAtFirstRefCountedObj=*/false, + [&](const clang::CXXRecordDecl *Record) { + return isSafePtr(Record); + }, + [&](const clang::QualType Type) { return isSafePtrType(Type); }, [&](const clang::Expr *InitArgOrigin, bool IsSafe) { if (!InitArgOrigin || IsSafe) return true; @@ -288,6 +313,9 @@ class RawPtrRefLocalVarsChecker if (EFA.isACallToEnsureFn(InitArgOrigin)) return true; + if (isSafeExpr(InitArgOrigin)) + return true; + if (auto *Ref = llvm::dyn_cast(InitArgOrigin)) { if (auto *MaybeGuardian = dyn_cast_or_null(Ref->getFoundDecl())) { @@ -298,8 +326,7 @@ class RawPtrRefLocalVarsChecker MaybeGuardianArgType->getAsCXXRecordDecl(); if (MaybeGuardianArgCXXRecord) { if (MaybeGuardian->isLocalVarDecl() && - (isRefCounted(MaybeGuardianArgCXXRecord) || - isCheckedPtr(MaybeGuardianArgCXXRecord) || + (isSafePtr(MaybeGuardianArgCXXRecord) || isRefcountedStringsHack(MaybeGuardian)) && isGuardedScopeEmbeddedInGuardianScope( V, MaybeGuardian)) @@ -324,6 +351,8 @@ class RawPtrRefLocalVarsChecker bool shouldSkipVarDecl(const VarDecl *V) const { assert(V); + if (isa(V)) + return true; return BR->getSourceManager().isInSystemHeader(V->getLocation()); } @@ -371,6 +400,12 @@ class UncountedLocalVarsChecker final : public RawPtrRefLocalVarsChecker { std::optional isUnsafePtr(const QualType T) const final { return isUncountedPtr(T); } + bool isSafePtr(const CXXRecordDecl *Record) const final { + return isRefCounted(Record) || isCheckedPtr(Record); + } + bool isSafePtrType(const QualType type) const final { + return isRefOrCheckedPtrType(type); + } const char *ptrKind() const final { return "uncounted"; } }; @@ -382,9 +417,38 @@ class UncheckedLocalVarsChecker final : public RawPtrRefLocalVarsChecker { std::optional isUnsafePtr(const QualType T) const final { return isUncheckedPtr(T); } + bool isSafePtr(const CXXRecordDecl *Record) const final { + return isRefCounted(Record) || isCheckedPtr(Record); + } + bool isSafePtrType(const QualType type) const final { + return isRefOrCheckedPtrType(type); + } const char *ptrKind() const final { return "unchecked"; } }; +class UnretainedLocalVarsChecker final : public RawPtrRefLocalVarsChecker { +public: + UnretainedLocalVarsChecker() + : RawPtrRefLocalVarsChecker("Unretained raw pointer or reference not " + "provably backed by a RetainPtr") { + RTC = RetainTypeChecker(); + } + std::optional isUnsafePtr(const QualType T) const final { + return RTC->isUnretained(T); + } + bool isSafePtr(const CXXRecordDecl *Record) const final { + return isRetainPtr(Record); + } + bool isSafePtrType(const QualType type) const final { + return isRetainPtrType(type); + } + bool isSafeExpr(const Expr *E) const final { + return ento::cocoa::isCocoaObjectRef(E->getType()) && + isa(E); + } + const char *ptrKind() const final { return "unretained"; } +}; + } // namespace void ento::registerUncountedLocalVarsChecker(CheckerManager &Mgr) { @@ -402,3 +466,11 @@ void ento::registerUncheckedLocalVarsChecker(CheckerManager &Mgr) { bool ento::shouldRegisterUncheckedLocalVarsChecker(const CheckerManager &) { return true; } + +void ento::registerUnretainedLocalVarsChecker(CheckerManager &Mgr) { + Mgr.registerChecker(); +} + +bool ento::shouldRegisterUnretainedLocalVarsChecker(const CheckerManager &) { + return true; +} diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp index 8357f0599aa4f..245710f1aee71 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp @@ -31,13 +31,16 @@ class RawPtrRefMemberChecker BugType Bug; mutable BugReporter *BR; +protected: + mutable std::optional RTC; + public: RawPtrRefMemberChecker(const char *description) : Bug(this, description, "WebKit coding guidelines") {} virtual std::optional - isPtrCompatible(const clang::CXXRecordDecl *) const = 0; - virtual bool isPtrCls(const clang::CXXRecordDecl *) const = 0; + isPtrCompatible(const clang::QualType, + const clang::CXXRecordDecl *R) const = 0; virtual const char *typeName() const = 0; virtual const char *invariant() const = 0; @@ -58,6 +61,12 @@ class RawPtrRefMemberChecker bool shouldVisitTemplateInstantiations() const { return true; } bool shouldVisitImplicitCode() const { return false; } + bool VisitTypedefDecl(const TypedefDecl *TD) { + if (Checker->RTC) + Checker->RTC->visitTypedef(TD); + return true; + } + bool VisitRecordDecl(const RecordDecl *RD) { Checker->visitRecordDecl(RD); return true; @@ -70,6 +79,8 @@ class RawPtrRefMemberChecker }; LocalVisitor visitor(this); + if (RTC) + RTC->visitTranslationUnitDecl(TUD); visitor.TraverseDecl(const_cast(TUD)); } @@ -78,22 +89,36 @@ class RawPtrRefMemberChecker return; for (auto *Member : RD->fields()) { - const Type *MemberType = Member->getType().getTypePtrOrNull(); + auto QT = Member->getType(); + const Type *MemberType = QT.getTypePtrOrNull(); if (!MemberType) continue; if (auto *MemberCXXRD = MemberType->getPointeeCXXRecordDecl()) { - // If we don't see the definition we just don't know. - if (MemberCXXRD->hasDefinition()) { - std::optional isRCAble = isPtrCompatible(MemberCXXRD); - if (isRCAble && *isRCAble) - reportBug(Member, MemberType, MemberCXXRD, RD); + std::optional IsCompatible = isPtrCompatible(QT, MemberCXXRD); + if (IsCompatible && *IsCompatible) + reportBug(Member, MemberType, MemberCXXRD, RD); + } else { + std::optional IsCompatible = isPtrCompatible(QT, nullptr); + auto *PointeeType = MemberType->getPointeeType().getTypePtrOrNull(); + if (IsCompatible && *IsCompatible) { + auto *Desugared = PointeeType->getUnqualifiedDesugaredType(); + if (auto *ObjCType = dyn_cast_or_null(Desugared)) + reportBug(Member, MemberType, ObjCType->getDecl(), RD); } } } } void visitObjCDecl(const ObjCContainerDecl *CD) const { + if (BR->getSourceManager().isInSystemHeader(CD->getLocation())) + return; + + ObjCContainerDecl::PropertyMap map; + CD->collectPropertiesToImplement(map); + for (auto it : map) + visitObjCPropertyDecl(CD, it.second); + if (auto *ID = dyn_cast(CD)) { for (auto *Ivar : ID->ivars()) visitIvarDecl(CD, Ivar); @@ -108,13 +133,47 @@ class RawPtrRefMemberChecker void visitIvarDecl(const ObjCContainerDecl *CD, const ObjCIvarDecl *Ivar) const { - const Type *IvarType = Ivar->getType().getTypePtrOrNull(); + if (BR->getSourceManager().isInSystemHeader(Ivar->getLocation())) + return; + auto QT = Ivar->getType(); + const Type *IvarType = QT.getTypePtrOrNull(); if (!IvarType) return; if (auto *IvarCXXRD = IvarType->getPointeeCXXRecordDecl()) { - std::optional IsCompatible = isPtrCompatible(IvarCXXRD); + std::optional IsCompatible = isPtrCompatible(QT, IvarCXXRD); if (IsCompatible && *IsCompatible) reportBug(Ivar, IvarType, IvarCXXRD, CD); + } else { + std::optional IsCompatible = isPtrCompatible(QT, nullptr); + auto *PointeeType = IvarType->getPointeeType().getTypePtrOrNull(); + if (IsCompatible && *IsCompatible) { + auto *Desugared = PointeeType->getUnqualifiedDesugaredType(); + if (auto *ObjCType = dyn_cast_or_null(Desugared)) + reportBug(Ivar, IvarType, ObjCType->getDecl(), CD); + } + } + } + + void visitObjCPropertyDecl(const ObjCContainerDecl *CD, + const ObjCPropertyDecl *PD) const { + if (BR->getSourceManager().isInSystemHeader(PD->getLocation())) + return; + auto QT = PD->getType(); + const Type *PropType = QT.getTypePtrOrNull(); + if (!PropType) + return; + if (auto *PropCXXRD = PropType->getPointeeCXXRecordDecl()) { + std::optional IsCompatible = isPtrCompatible(QT, PropCXXRD); + if (IsCompatible && *IsCompatible) + reportBug(PD, PropType, PropCXXRD, CD); + } else { + std::optional IsCompatible = isPtrCompatible(QT, nullptr); + auto *PointeeType = PropType->getPointeeType().getTypePtrOrNull(); + if (IsCompatible && *IsCompatible) { + auto *Desugared = PointeeType->getUnqualifiedDesugaredType(); + if (auto *ObjCType = dyn_cast_or_null(Desugared)) + reportBug(PD, PropType, ObjCType->getDecl(), CD); + } } } @@ -146,34 +205,40 @@ class RawPtrRefMemberChecker // Ref-counted smartpointers actually have raw-pointer to uncounted type as // a member but we trust them to handle it correctly. auto CXXRD = llvm::dyn_cast_or_null(RD); - if (CXXRD) - return isPtrCls(CXXRD); + if (CXXRD && isSmartPtr(CXXRD)) + return true; return false; } - template + template void reportBug(const DeclType *Member, const Type *MemberType, - const CXXRecordDecl *MemberCXXRD, + const PointeeType *Pointee, const ParentDeclType *ClassCXXRD) const { assert(Member); assert(MemberType); - assert(MemberCXXRD); + assert(Pointee); SmallString<100> Buf; llvm::raw_svector_ostream Os(Buf); - if (isa(ClassCXXRD)) - Os << "Instance variable "; - else + if (isa(ClassCXXRD)) { + if (isa(Member)) + Os << "Property "; + else + Os << "Instance variable "; + } else Os << "Member variable "; printQuotedName(Os, Member); Os << " in "; printQuotedQualifiedName(Os, ClassCXXRD); - Os << " is a " - << (isa(MemberType) ? "raw pointer" : "reference") << " to " - << typeName() << " "; - printQuotedQualifiedName(Os, MemberCXXRD); + Os << " is a "; + if (printPointer(Os, MemberType) == PrintDeclKind::Pointer) { + auto Typedef = MemberType->getAs(); + assert(Typedef); + printQuotedQualifiedName(Os, Typedef->getDecl()); + } else + printQuotedQualifiedName(Os, Pointee); Os << "; " << invariant() << "."; PathDiagnosticLocation BSLoc(Member->getSourceRange().getBegin(), @@ -182,6 +247,15 @@ class RawPtrRefMemberChecker Report->addRange(Member->getSourceRange()); BR->emitReport(std::move(Report)); } + + enum class PrintDeclKind { Pointee, Pointer }; + virtual PrintDeclKind printPointer(llvm::raw_svector_ostream &Os, + const Type *T) const { + T = T->getUnqualifiedDesugaredType(); + bool IsPtr = isa(T) || isa(T); + Os << (IsPtr ? "raw pointer" : "reference") << " to " << typeName() << " "; + return PrintDeclKind::Pointee; + } }; class NoUncountedMemberChecker final : public RawPtrRefMemberChecker { @@ -191,12 +265,9 @@ class NoUncountedMemberChecker final : public RawPtrRefMemberChecker { "reference-countable type") {} std::optional - isPtrCompatible(const clang::CXXRecordDecl *R) const final { - return isRefCountable(R); - } - - bool isPtrCls(const clang::CXXRecordDecl *R) const final { - return isRefCounted(R); + isPtrCompatible(const clang::QualType, + const clang::CXXRecordDecl *R) const final { + return R ? isRefCountable(R) : std::nullopt; } const char *typeName() const final { return "ref-countable type"; } @@ -213,12 +284,9 @@ class NoUncheckedPtrMemberChecker final : public RawPtrRefMemberChecker { "checked-pointer capable type") {} std::optional - isPtrCompatible(const clang::CXXRecordDecl *R) const final { - return isCheckedPtrCapable(R); - } - - bool isPtrCls(const clang::CXXRecordDecl *R) const final { - return isCheckedPtr(R); + isPtrCompatible(const clang::QualType, + const clang::CXXRecordDecl *R) const final { + return R ? isCheckedPtrCapable(R) : std::nullopt; } const char *typeName() const final { return "CheckedPtr capable type"; } @@ -229,6 +297,36 @@ class NoUncheckedPtrMemberChecker final : public RawPtrRefMemberChecker { } }; +class NoUnretainedMemberChecker final : public RawPtrRefMemberChecker { +public: + NoUnretainedMemberChecker() + : RawPtrRefMemberChecker("Member variable is a raw-pointer/reference to " + "retainable type") { + RTC = RetainTypeChecker(); + } + + std::optional + isPtrCompatible(const clang::QualType QT, + const clang::CXXRecordDecl *) const final { + return RTC->isUnretained(QT); + } + + const char *typeName() const final { return "retainable type"; } + + const char *invariant() const final { + return "member variables must be a RetainPtr"; + } + + PrintDeclKind printPointer(llvm::raw_svector_ostream &Os, + const Type *T) const final { + if (!isa(T) && T->getAs()) { + Os << typeName() << " "; + return PrintDeclKind::Pointer; + } + return RawPtrRefMemberChecker::printPointer(Os, T); + } +}; + } // namespace void ento::registerNoUncountedMemberChecker(CheckerManager &Mgr) { @@ -247,3 +345,11 @@ bool ento::shouldRegisterNoUncheckedPtrMemberChecker( const CheckerManager &Mgr) { return true; } + +void ento::registerNoUnretainedMemberChecker(CheckerManager &Mgr) { + Mgr.registerChecker(); +} + +bool ento::shouldRegisterNoUnretainedMemberChecker(const CheckerManager &Mgr) { + return true; +} diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp index e80246f49a310..961e54f83ad9a 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp @@ -203,6 +203,13 @@ class RefCntblBaseVirtualDtorChecker if (!C) continue; + bool isExempt = T.getAsString() == "NoVirtualDestructorBase" && + safeGetName(C->getParent()) == "WTF"; + if (isExempt || ExemptDecls.contains(C)) { + ExemptDecls.insert(RD); + continue; + } + if (auto *CTSD = dyn_cast(C)) { for (auto &Arg : CTSD->getTemplateArgs().asArray()) { if (Arg.getKind() != TemplateArgument::Type) @@ -224,12 +231,13 @@ class RefCntblBaseVirtualDtorChecker llvm::SetVector Decls; llvm::DenseSet CRTPs; + llvm::DenseSet ExemptDecls; }; LocalVisitor visitor(this); visitor.TraverseDecl(const_cast(TUD)); for (auto *RD : visitor.Decls) { - if (visitor.CRTPs.contains(RD)) + if (visitor.CRTPs.contains(RD) || visitor.ExemptDecls.contains(RD)) continue; visitCXXRecordDecl(RD); } diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp new file mode 100644 index 0000000000000..4c6cbc7f15f69 --- /dev/null +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RetainPtrCtorAdoptChecker.cpp @@ -0,0 +1,653 @@ +//=======- RetainPtrCtorAdoptChecker.cpp -------------------------*- C++ -*-==// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "ASTUtils.h" +#include "DiagOutputUtils.h" +#include "PtrTypesSemantics.h" +#include "clang/AST/CXXInheritance.h" +#include "clang/AST/RecursiveASTVisitor.h" +#include "clang/AST/StmtVisitor.h" +#include "clang/Analysis/DomainSpecific/CocoaConventions.h" +#include "clang/Analysis/RetainSummaryManager.h" +#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" +#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h" +#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" +#include "clang/StaticAnalyzer/Core/Checker.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" +#include "llvm/ADT/DenseSet.h" +#include "llvm/ADT/SetVector.h" +#include + +using namespace clang; +using namespace ento; + +namespace { + +class RetainPtrCtorAdoptChecker + : public Checker> { +private: + BugType Bug; + mutable BugReporter *BR; + mutable std::unique_ptr Summaries; + mutable llvm::DenseSet CreateOrCopyOutArguments; + mutable llvm::DenseSet CreateOrCopyFnCall; + mutable RetainTypeChecker RTC; + +public: + RetainPtrCtorAdoptChecker() + : Bug(this, "Correct use of RetainPtr, adoptNS, and adoptCF", + "WebKit coding guidelines") {} + + void checkASTDecl(const TranslationUnitDecl *TUD, AnalysisManager &MGR, + BugReporter &BRArg) const { + BR = &BRArg; + + // The calls to checkAST* from AnalysisConsumer don't + // visit template instantiations or lambda classes. We + // want to visit those, so we make our own RecursiveASTVisitor. + struct LocalVisitor : public RecursiveASTVisitor { + const RetainPtrCtorAdoptChecker *Checker; + Decl *DeclWithIssue{nullptr}; + + using Base = RecursiveASTVisitor; + + explicit LocalVisitor(const RetainPtrCtorAdoptChecker *Checker) + : Checker(Checker) { + assert(Checker); + } + + bool shouldVisitTemplateInstantiations() const { return true; } + bool shouldVisitImplicitCode() const { return false; } + + bool TraverseDecl(Decl *D) { + llvm::SaveAndRestore SavedDecl(DeclWithIssue); + if (D && (isa(D) || isa(D))) + DeclWithIssue = D; + return Base::TraverseDecl(D); + } + + bool TraverseClassTemplateDecl(ClassTemplateDecl *CTD) { + if (isRetainPtr(safeGetName(CTD))) + return true; // Skip the contents of RetainPtr. + return Base::TraverseClassTemplateDecl(CTD); + } + + bool VisitTypedefDecl(TypedefDecl *TD) { + Checker->RTC.visitTypedef(TD); + return true; + } + + bool VisitCallExpr(const CallExpr *CE) { + Checker->visitCallExpr(CE, DeclWithIssue); + return true; + } + + bool VisitCXXConstructExpr(const CXXConstructExpr *CE) { + Checker->visitConstructExpr(CE, DeclWithIssue); + return true; + } + + bool VisitObjCMessageExpr(const ObjCMessageExpr *ObjCMsgExpr) { + Checker->visitObjCMessageExpr(ObjCMsgExpr, DeclWithIssue); + return true; + } + + bool VisitReturnStmt(const ReturnStmt *RS) { + Checker->visitReturnStmt(RS, DeclWithIssue); + return true; + } + + bool VisitVarDecl(const VarDecl *VD) { + Checker->visitVarDecl(VD); + return true; + } + + bool VisitBinaryOperator(const BinaryOperator *BO) { + Checker->visitBinaryOperator(BO); + return true; + } + }; + + LocalVisitor visitor(this); + Summaries = std::make_unique( + TUD->getASTContext(), true /* trackObjCAndCFObjects */, + false /* trackOSObjects */); + RTC.visitTranslationUnitDecl(TUD); + visitor.TraverseDecl(const_cast(TUD)); + } + + bool isAdoptFn(const Decl *FnDecl) const { + return isAdoptFnName(safeGetName(FnDecl)); + } + + bool isAdoptFnName(const std::string &Name) const { + return isAdoptNS(Name) || Name == "adoptCF" || Name == "adoptCFArc"; + } + + bool isAdoptNS(const std::string &Name) const { + return Name == "adoptNS" || Name == "adoptNSArc"; + } + + void visitCallExpr(const CallExpr *CE, const Decl *DeclWithIssue) const { + if (BR->getSourceManager().isInSystemHeader(CE->getExprLoc())) + return; + + std::string FnName; + if (auto *F = CE->getDirectCallee()) { + FnName = safeGetName(F); + if (isAdoptFnName(FnName)) + checkAdoptCall(CE, FnName, DeclWithIssue); + else { + checkCreateOrCopyFunction(CE, DeclWithIssue); + checkBridgingRelease(CE, F, DeclWithIssue); + } + return; + } + + auto *CalleeExpr = CE->getCallee(); + if (!CalleeExpr) + return; + CalleeExpr = CalleeExpr->IgnoreParenCasts(); + if (auto *UnresolvedExpr = dyn_cast(CalleeExpr)) { + auto Name = UnresolvedExpr->getName(); + if (!Name.isIdentifier()) + return; + FnName = Name.getAsString(); + if (isAdoptFnName(FnName)) + checkAdoptCall(CE, FnName, DeclWithIssue); + } + checkCreateOrCopyFunction(CE, DeclWithIssue); + } + + void checkAdoptCall(const CallExpr *CE, const std::string &FnName, + const Decl *DeclWithIssue) const { + if (!CE->getNumArgs()) + return; + + auto *Arg = CE->getArg(0)->IgnoreParenCasts(); + auto Result = isOwned(Arg); + if (Result == IsOwnedResult::Unknown) + Result = IsOwnedResult::NotOwned; + + const Expr *Inner = nullptr; + if (isAllocInit(Arg, &Inner) || isCreateOrCopy(Arg)) { + if (Inner) + CreateOrCopyFnCall.insert(Inner); + CreateOrCopyFnCall.insert(Arg); // Avoid double reporting. + return; + } + if (Result == IsOwnedResult::Owned || Result == IsOwnedResult::Skip) { + CreateOrCopyFnCall.insert(Arg); + return; + } + + if (auto *DRE = dyn_cast(Arg)) { + if (CreateOrCopyOutArguments.contains(DRE->getDecl())) + return; + } + if (RTC.isARCEnabled() && isAdoptFnName(FnName)) + reportUseAfterFree(FnName, CE, DeclWithIssue, "when ARC is disabled"); + else + reportUseAfterFree(FnName, CE, DeclWithIssue); + } + + void visitObjCMessageExpr(const ObjCMessageExpr *ObjCMsgExpr, + const Decl *DeclWithIssue) const { + if (BR->getSourceManager().isInSystemHeader(ObjCMsgExpr->getExprLoc())) + return; + + auto Selector = ObjCMsgExpr->getSelector(); + if (Selector.getAsString() == "autorelease") { + auto *Receiver = ObjCMsgExpr->getInstanceReceiver()->IgnoreParenCasts(); + if (!Receiver) + return; + ObjCMsgExpr = dyn_cast(Receiver); + if (!ObjCMsgExpr) + return; + const Expr *Inner = nullptr; + if (!isAllocInit(ObjCMsgExpr, &Inner)) + return; + CreateOrCopyFnCall.insert(ObjCMsgExpr); + if (Inner) + CreateOrCopyFnCall.insert(Inner); + return; + } + + const Expr *Inner = nullptr; + if (!isAllocInit(ObjCMsgExpr, &Inner)) + return; + if (RTC.isARCEnabled()) + return; // ARC never leaks. + if (CreateOrCopyFnCall.contains(ObjCMsgExpr)) + return; + if (Inner) + CreateOrCopyFnCall.insert(Inner); // Avoid double reporting. + reportLeak(ObjCMsgExpr, DeclWithIssue); + } + + void checkCreateOrCopyFunction(const CallExpr *CE, + const Decl *DeclWithIssue) const { + unsigned ArgCount = CE->getNumArgs(); + auto *CalleeDecl = CE->getCalleeDecl(); + auto *FnDecl = CalleeDecl ? CalleeDecl->getAsFunction() : nullptr; + for (unsigned ArgIndex = 0; ArgIndex < ArgCount; ++ArgIndex) { + auto *Arg = CE->getArg(ArgIndex)->IgnoreParenCasts(); + auto *Unary = dyn_cast(Arg); + if (!Unary) + continue; + if (Unary->getOpcode() != UO_AddrOf) + continue; + auto *SubExpr = Unary->getSubExpr(); + if (!SubExpr) + continue; + auto *DRE = dyn_cast(SubExpr->IgnoreParenCasts()); + if (!DRE) + continue; + auto *Decl = DRE->getDecl(); + if (!Decl) + continue; + if (FnDecl && ArgIndex < FnDecl->getNumParams()) { + // Manually check attributes on argumenet since RetainSummaryManager + // basically ignores CF_RETRUNS_RETAINED on out arguments. + auto *ParamDecl = FnDecl->getParamDecl(ArgIndex); + if (ParamDecl->hasAttr()) + CreateOrCopyOutArguments.insert(Decl); + } else { + // No callee or a variadic argument. + // Conservatively assume it's an out argument. + if (RTC.isUnretained(Decl->getType())) + CreateOrCopyOutArguments.insert(Decl); + } + } + auto Summary = Summaries->getSummary(AnyCall(CE)); + switch (Summary->getRetEffect().getKind()) { + case RetEffect::OwnedSymbol: + case RetEffect::OwnedWhenTrackedReceiver: + if (!CreateOrCopyFnCall.contains(CE)) + reportLeak(CE, DeclWithIssue); + break; + default: + break; + } + } + + void checkBridgingRelease(const CallExpr *CE, const FunctionDecl *Callee, + const Decl *DeclWithIssue) const { + if (safeGetName(Callee) != "CFBridgingRelease" || CE->getNumArgs() != 1) + return; + + auto *Arg = CE->getArg(0)->IgnoreParenCasts(); + auto *InnerCE = dyn_cast(Arg); + if (!InnerCE) + return; + + auto *InnerF = InnerCE->getDirectCallee(); + if (!InnerF || !isCreateOrCopyFunction(InnerF)) + return; + + CreateOrCopyFnCall.insert(InnerCE); + } + + void visitConstructExpr(const CXXConstructExpr *CE, + const Decl *DeclWithIssue) const { + if (BR->getSourceManager().isInSystemHeader(CE->getExprLoc())) + return; + + auto *Ctor = CE->getConstructor(); + if (!Ctor) + return; + + auto *Cls = Ctor->getParent(); + if (!Cls) + return; + + if (!isRetainPtr(safeGetName(Cls)) || !CE->getNumArgs()) + return; + + // Ignore RetainPtr construction inside adoptNS, adoptCF, and retainPtr. + if (isAdoptFn(DeclWithIssue) || safeGetName(DeclWithIssue) == "retainPtr") + return; + + std::string Name = "RetainPtr constructor"; + auto *Arg = CE->getArg(0)->IgnoreParenCasts(); + auto Result = isOwned(Arg); + + if (isCreateOrCopy(Arg)) + CreateOrCopyFnCall.insert(Arg); // Avoid double reporting. + + const Expr *Inner = nullptr; + if (isAllocInit(Arg, &Inner)) { + CreateOrCopyFnCall.insert(Arg); + if (Inner) + CreateOrCopyFnCall.insert(Inner); + } + + if (Result == IsOwnedResult::Skip) + return; + + if (Result == IsOwnedResult::Unknown) + Result = IsOwnedResult::NotOwned; + if (Result == IsOwnedResult::Owned) + reportLeak(Name, CE, DeclWithIssue); + else if (RTC.isARCEnabled() && isAllocInit(Arg)) + reportLeak(Name, CE, DeclWithIssue, "when ARC is disabled"); + else if (isCreateOrCopy(Arg)) + reportLeak(Name, CE, DeclWithIssue); + } + + void visitVarDecl(const VarDecl *VD) const { + auto *Init = VD->getInit(); + if (!Init || !RTC.isARCEnabled()) + return; + Init = Init->IgnoreParenCasts(); + const Expr *Inner = nullptr; + if (isAllocInit(Init, &Inner)) { + CreateOrCopyFnCall.insert(Init); + if (Inner) + CreateOrCopyFnCall.insert(Inner); + } + } + + void visitBinaryOperator(const BinaryOperator *BO) const { + if (!BO->isAssignmentOp()) + return; + if (!isa(BO->getLHS())) + return; + auto *RHS = BO->getRHS()->IgnoreParenCasts(); + const Expr *Inner = nullptr; + if (isAllocInit(RHS, &Inner)) { + CreateOrCopyFnCall.insert(RHS); + if (Inner) + CreateOrCopyFnCall.insert(Inner); + } + } + + void visitReturnStmt(const ReturnStmt *RS, const Decl *DeclWithIssue) const { + if (!DeclWithIssue) + return; + auto *RetValue = RS->getRetValue(); + if (!RetValue) + return; + RetValue = RetValue->IgnoreParenCasts(); + std::optional retainsRet; + if (auto *FnDecl = dyn_cast(DeclWithIssue)) + retainsRet = retainsReturnValue(FnDecl); + else if (auto *MethodDecl = dyn_cast(DeclWithIssue)) + retainsRet = retainsReturnValue(MethodDecl); + else + return; + if (!retainsRet || !*retainsRet) { + // Under ARC, returning [[X alloc] init] doesn't leak X. + if (RTC.isUnretained(RetValue->getType())) + return; + } + if (auto *CE = dyn_cast(RetValue)) { + auto *Callee = CE->getDirectCallee(); + if (!Callee || !isCreateOrCopyFunction(Callee)) + return; + CreateOrCopyFnCall.insert(CE); + return; + } + const Expr *Inner = nullptr; + if (isAllocInit(RetValue, &Inner)) { + CreateOrCopyFnCall.insert(RetValue); + if (Inner) + CreateOrCopyFnCall.insert(Inner); + } + } + + template + std::optional retainsReturnValue(const CallableType *FnDecl) const { + auto Summary = Summaries->getSummary(AnyCall(FnDecl)); + auto RetEffect = Summary->getRetEffect(); + switch (RetEffect.getKind()) { + case RetEffect::NoRet: + return std::nullopt; + case RetEffect::OwnedSymbol: + return true; + case RetEffect::NotOwnedSymbol: + return false; + case RetEffect::OwnedWhenTrackedReceiver: + return std::nullopt; + case RetEffect::NoRetHard: + return std::nullopt; + } + 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) + return false; + auto *Callee = CE->getDirectCallee(); + if (!Callee) + return false; + return isCreateOrCopyFunction(Callee); + } + + bool isCreateOrCopyFunction(const FunctionDecl *FnDecl) const { + auto CalleeName = safeGetName(FnDecl); + return CalleeName.find("Create") != std::string::npos || + CalleeName.find("Copy") != std::string::npos; + } + + enum class IsOwnedResult { Unknown, Skip, Owned, NotOwned }; + IsOwnedResult isOwned(const Expr *E) const { + while (1) { + if (auto *POE = dyn_cast(E)) { + if (unsigned SemanticExprCount = POE->getNumSemanticExprs()) { + E = POE->getSemanticExpr(SemanticExprCount - 1); + continue; + } + } + if (isa(E)) + return IsOwnedResult::NotOwned; + if (auto *DRE = dyn_cast(E)) { + auto QT = DRE->getType(); + if (isRetainPtrType(QT)) + return IsOwnedResult::NotOwned; + QT = QT.getCanonicalType(); + if (RTC.isUnretained(QT, true /* ignoreARC */)) + return IsOwnedResult::NotOwned; + auto *PointeeType = QT->getPointeeType().getTypePtrOrNull(); + if (PointeeType && PointeeType->isVoidType()) + return IsOwnedResult::NotOwned; // Assume reading void* as +0. + } + if (auto *TE = dyn_cast(E)) { + E = TE->getSubExpr(); + continue; + } + if (auto *ObjCMsgExpr = dyn_cast(E)) { + auto Summary = Summaries->getSummary(AnyCall(ObjCMsgExpr)); + auto RetEffect = Summary->getRetEffect(); + switch (RetEffect.getKind()) { + case RetEffect::NoRet: + return IsOwnedResult::Unknown; + case RetEffect::OwnedSymbol: + return IsOwnedResult::Owned; + case RetEffect::NotOwnedSymbol: + return IsOwnedResult::NotOwned; + case RetEffect::OwnedWhenTrackedReceiver: + if (auto *Receiver = ObjCMsgExpr->getInstanceReceiver()) { + E = Receiver->IgnoreParenCasts(); + continue; + } + return IsOwnedResult::Unknown; + case RetEffect::NoRetHard: + return IsOwnedResult::Unknown; + } + } + if (auto *CXXCE = dyn_cast(E)) { + if (auto *MD = CXXCE->getMethodDecl()) { + auto *Cls = MD->getParent(); + if (auto *CD = dyn_cast(MD)) { + auto QT = CD->getConversionType().getCanonicalType(); + auto *ResultType = QT.getTypePtrOrNull(); + if (isRetainPtr(safeGetName(Cls)) && ResultType && + (ResultType->isPointerType() || ResultType->isReferenceType() || + ResultType->isObjCObjectPointerType())) + return IsOwnedResult::NotOwned; + } + if (safeGetName(MD) == "leakRef" && isRetainPtr(safeGetName(Cls))) + return IsOwnedResult::Owned; + } + } + if (auto *CE = dyn_cast(E)) { + if (auto *Callee = CE->getDirectCallee()) { + if (isAdoptFn(Callee)) + return IsOwnedResult::NotOwned; + auto Name = safeGetName(Callee); + if (Name == "__builtin___CFStringMakeConstantString") + return IsOwnedResult::NotOwned; + if ((Name == "checked_cf_cast" || Name == "dynamic_cf_cast" || + Name == "checked_objc_cast" || Name == "dynamic_objc_cast") && + CE->getNumArgs() == 1) { + E = CE->getArg(0)->IgnoreParenCasts(); + continue; + } + auto RetType = Callee->getReturnType(); + if (isRetainPtrType(RetType)) + return IsOwnedResult::NotOwned; + if (isCreateOrCopyFunction(Callee)) { + CreateOrCopyFnCall.insert(CE); + return IsOwnedResult::Owned; + } + } else if (auto *CalleeExpr = CE->getCallee()) { + if (isa(CalleeExpr)) + return IsOwnedResult::Skip; // Wait for instantiation. + if (isa(CalleeExpr)) + return IsOwnedResult::Skip; // Wait for instantiation. + } + auto Summary = Summaries->getSummary(AnyCall(CE)); + auto RetEffect = Summary->getRetEffect(); + switch (RetEffect.getKind()) { + case RetEffect::NoRet: + return IsOwnedResult::Unknown; + case RetEffect::OwnedSymbol: + return IsOwnedResult::Owned; + case RetEffect::NotOwnedSymbol: + return IsOwnedResult::NotOwned; + case RetEffect::OwnedWhenTrackedReceiver: + return IsOwnedResult::Unknown; + case RetEffect::NoRetHard: + return IsOwnedResult::Unknown; + } + } + break; + } + return IsOwnedResult::Unknown; + } + + void reportUseAfterFree(const std::string &Name, const CallExpr *CE, + const Decl *DeclWithIssue, + const char *condition = nullptr) const { + SmallString<100> Buf; + llvm::raw_svector_ostream Os(Buf); + + Os << "Incorrect use of " << Name + << ". The argument is +0 and results in an use-after-free"; + if (condition) + Os << " " << condition; + Os << "."; + + PathDiagnosticLocation BSLoc(CE->getSourceRange().getBegin(), + BR->getSourceManager()); + auto Report = std::make_unique(Bug, Os.str(), BSLoc); + Report->addRange(CE->getSourceRange()); + Report->setDeclWithIssue(DeclWithIssue); + BR->emitReport(std::move(Report)); + } + + void reportLeak(std::string &Name, const CXXConstructExpr *CE, + const Decl *DeclWithIssue, + const char *condition = nullptr) const { + SmallString<100> Buf; + llvm::raw_svector_ostream Os(Buf); + + Os << "Incorrect use of " << Name + << ". The argument is +1 and results in a memory leak"; + if (condition) + Os << " " << condition; + Os << "."; + + PathDiagnosticLocation BSLoc(CE->getSourceRange().getBegin(), + BR->getSourceManager()); + auto Report = std::make_unique(Bug, Os.str(), BSLoc); + Report->addRange(CE->getSourceRange()); + Report->setDeclWithIssue(DeclWithIssue); + BR->emitReport(std::move(Report)); + } + + template + void reportLeak(const ExprType *E, const Decl *DeclWithIssue) const { + SmallString<100> Buf; + llvm::raw_svector_ostream Os(Buf); + + Os << "The return value is +1 and results in a memory leak."; + + PathDiagnosticLocation BSLoc(E->getSourceRange().getBegin(), + BR->getSourceManager()); + auto Report = std::make_unique(Bug, Os.str(), BSLoc); + Report->addRange(E->getSourceRange()); + Report->setDeclWithIssue(DeclWithIssue); + BR->emitReport(std::move(Report)); + } +}; +} // namespace + +void ento::registerRetainPtrCtorAdoptChecker(CheckerManager &Mgr) { + Mgr.registerChecker(); +} + +bool ento::shouldRegisterRetainPtrCtorAdoptChecker(const CheckerManager &mgr) { + return true; +} diff --git a/clang/test/Analysis/Checkers/WebKit/binding-to-refptr.cpp b/clang/test/Analysis/Checkers/WebKit/binding-to-refptr.cpp new file mode 100644 index 0000000000000..012868fcb329a --- /dev/null +++ b/clang/test/Analysis/Checkers/WebKit/binding-to-refptr.cpp @@ -0,0 +1,36 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UncountedCallArgsChecker -verify %s -std=c++2c +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UncheckedLocalVarsChecker -verify %s -std=c++2c + +// expected-no-diagnostics + +#include "mock-types.h" + +class Node { +public: + Node* nextSibling() const; + + void ref() const; + void deref() const; +}; + +template struct pair { + A a; + B b; + template requires ( I == 0 ) A& get(); + template requires ( I == 1 ) B& get(); +}; + +namespace std { + template struct tuple_size; + template struct tuple_element; + template struct tuple_size<::pair> { static constexpr int value = 2; }; + template struct tuple_element<0, ::pair> { using type = A; }; + template struct tuple_element<1, ::pair> { using type = B; }; +} + +pair, RefPtr> &getPair(); + +static void testUnpackedAssignment() { + auto [a, b] = getPair(); + a->nextSibling(); +} diff --git a/clang/test/Analysis/Checkers/WebKit/call-args-checked.cpp b/clang/test/Analysis/Checkers/WebKit/call-args-checked.cpp index 49b6bfcd7cadf..e24b04dcd3cf9 100644 --- a/clang/test/Analysis/Checkers/WebKit/call-args-checked.cpp +++ b/clang/test/Analysis/Checkers/WebKit/call-args-checked.cpp @@ -1,4 +1,4 @@ -// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UncountedCallArgsChecker -verify %s +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UncheckedCallArgsChecker -verify %s #include "mock-types.h" @@ -10,10 +10,10 @@ namespace call_args_unchecked_uncounted { static void foo() { someFunction(makeObj()); - // expected-warning@-1{{Call argument is uncounted and unsafe [alpha.webkit.UncountedCallArgsChecker]}} + // expected-warning@-1{{Call argument is unchecked and unsafe [alpha.webkit.UncheckedCallArgsChecker]}} } -} // namespace call_args_checked +} // namespace call_args_unchecked_uncounted namespace call_args_checked { @@ -35,7 +35,7 @@ static void baz() { namespace call_args_default { void someFunction(RefCountableAndCheckable* = makeObj()); -// expected-warning@-1{{Call argument is uncounted and unsafe [alpha.webkit.UncountedCallArgsChecker]}} +// expected-warning@-1{{Call argument is unchecked and unsafe [alpha.webkit.UncheckedCallArgsChecker]}} void otherFunction(RefCountableAndCheckable* = makeObjChecked().ptr()); void foo() { @@ -44,3 +44,13 @@ void foo() { } } + +namespace call_args_checked_assignment { + +CheckedObj* provide(); +void foo() { + CheckedPtr ptr; + ptr = provide(); +} + +} diff --git a/clang/test/Analysis/Checkers/WebKit/call-args.cpp b/clang/test/Analysis/Checkers/WebKit/call-args.cpp index b4613d5090f29..d95ae9216edcf 100644 --- a/clang/test/Analysis/Checkers/WebKit/call-args.cpp +++ b/clang/test/Analysis/Checkers/WebKit/call-args.cpp @@ -359,6 +359,41 @@ namespace call_with_ptr_on_ref { } } +namespace call_with_explicit_construct_from_auto { + + struct Impl { + void ref() const; + void deref() const; + + static Ref create(); + }; + + template + struct ArgObj { + T* t; + }; + + struct Object { + Object(); + Object(Ref&&); + + Impl* impl() const { return m_impl.get(); } + + static Object create(ArgObj&) { return Impl::create(); } + static void bar(Impl&); + + private: + RefPtr m_impl; + }; + + template void foo() + { + auto result = Object::create(ArgObj { }); + Object::bar(Ref { *result.impl() }); + } + +} + namespace call_with_explicit_temporary_obj { void foo() { Ref { *provide() }->method(); @@ -372,6 +407,17 @@ namespace call_with_explicit_temporary_obj { void baz() { bar(); } + + class Foo { + Ref ensure(); + void foo() { + Ref { ensure() }->method(); + } + }; + + void baz(Ref&& arg) { + Ref { arg }->method(); + } } namespace call_with_explicit_construct { diff --git a/clang/test/Analysis/Checkers/WebKit/forward-decl-checker.mm b/clang/test/Analysis/Checkers/WebKit/forward-decl-checker.mm new file mode 100644 index 0000000000000..084b47322d7f9 --- /dev/null +++ b/clang/test/Analysis/Checkers/WebKit/forward-decl-checker.mm @@ -0,0 +1,157 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.ForwardDeclChecker -verify %s + +#include "mock-types.h" +#include "objc-mock-types.h" +#include "mock-system-header.h" + +namespace std { + +template struct remove_reference { + typedef T type; +}; + +template struct remove_reference { + typedef T type; +}; + +template typename remove_reference::type&& move(T&& t); + +} // namespace std + +typedef struct OpaqueJSString * JSStringRef; + +class Obj; +@class ObjCObj; + +Obj* provide_obj_ptr(); +void receive_obj_ptr(Obj* p = nullptr); +sqlite3* open_db(); +void close_db(sqlite3*); + +Obj* ptr(Obj* arg) { + receive_obj_ptr(provide_obj_ptr()); + // expected-warning@-1{{Call argument for parameter 'p' uses a forward declared type 'Obj *'}} + auto *obj = provide_obj_ptr(); + // expected-warning@-1{{Local variable 'obj' uses a forward declared type 'Obj *'}} + receive_obj_ptr(arg); + receive_obj_ptr(nullptr); + receive_obj_ptr(); + auto* db = open_db(); + close_db(db); + return obj; +} + +Obj& provide_obj_ref(); +void receive_obj_ref(Obj& p); + +Obj& ref() { + receive_obj_ref(provide_obj_ref()); + // expected-warning@-1{{Call argument for parameter 'p' uses a forward declared type 'Obj &'}} + auto &obj = provide_obj_ref(); + // expected-warning@-1{{Local variable 'obj' uses a forward declared type 'Obj &'}} + return obj; +} + +Obj&& provide_obj_rval(); +void receive_obj_rval(Obj&& p); + +void rval(Obj&& arg) { + receive_obj_rval(provide_obj_rval()); + // expected-warning@-1{{Call argument for parameter 'p' uses a forward declared type 'Obj &&'}} + auto &&obj = provide_obj_rval(); + // expected-warning@-1{{Local variable 'obj' uses a forward declared type 'Obj &&'}} + receive_obj_rval(std::move(arg)); +} + +ObjCObj *provide_objcobj(); +void receive_objcobj(ObjCObj *p); +ObjCObj *objc_ptr() { + receive_objcobj(provide_objcobj()); + auto *objcobj = provide_objcobj(); + return objcobj; +} + +struct WrapperObj { + Obj* ptr { nullptr }; + // expected-warning@-1{{Member variable 'ptr' uses a forward declared type 'Obj *'}} + + WrapperObj(Obj* obj); + WrapperObj(Obj& obj); + WrapperObj(Obj&& obj); +}; + +void construct_ptr(Obj&& arg) { + WrapperObj wrapper1(provide_obj_ptr()); + // expected-warning@-1{{Call argument for parameter 'obj' uses a forward declared type 'Obj *'}} + WrapperObj wrapper2(provide_obj_ref()); + // expected-warning@-1{{Call argument for parameter 'obj' uses a forward declared type 'Obj &'}} + WrapperObj wrapper3(std::move(arg)); +} + +JSStringRef provide_opaque_ptr(); +void receive_opaque_ptr(JSStringRef); +NSZone *provide_zone(); + +JSStringRef opaque_ptr() { + receive_opaque_ptr(provide_opaque_ptr()); + auto ref = provide_opaque_ptr(); + return ref; +} + +@interface AnotherObj : NSObject +- (Obj *)ptr; +- (Obj &)ref; +- (void)objc; +- (void)doMoreWork:(ObjCObj *)obj; +@end + +@implementation AnotherObj +- (Obj *)ptr { + receive_obj_ptr(provide_obj_ptr()); + // expected-warning@-1{{Call argument for parameter 'p' uses a forward declared type 'Obj *'}} + auto *obj = provide_obj_ptr(); + // expected-warning@-1{{Local variable 'obj' uses a forward declared type 'Obj *'}} + return obj; +} + +- (Obj &)ref { + receive_obj_ref(provide_obj_ref()); + // expected-warning@-1{{Call argument for parameter 'p' uses a forward declared type 'Obj &'}} + auto &obj = provide_obj_ref(); + // expected-warning@-1{{Local variable 'obj' uses a forward declared type 'Obj &'}} + return obj; +} + +- (void)objc { + auto *obj = provide_objcobj(); + [obj doWork]; + [self doMoreWork:provide_objcobj()]; + [self doMoreWork:nil]; +} + +- (void)doMoreWork:(ObjCObj *)obj { + auto array = CFArrayCreateMutable(kCFAllocatorDefault, 10); + CFArrayAppendValue(array, nullptr); + auto log = os_log_create("Foo", "Bar"); + os_log_msg(log, OS_LOG_TYPE_DEFAULT, "Some Log"); + auto *zone = provide_zone(); +} + +@end + +namespace template_forward_declare { + +template class HashSet; + +template +using SingleThreadHashSet = HashSet; + +template class HashSet { }; + +struct Font { }; + +struct ComplexTextController { + SingleThreadHashSet* fallbackFonts { nullptr }; +}; + +} diff --git a/clang/test/Analysis/Checkers/WebKit/mock-system-header.h b/clang/test/Analysis/Checkers/WebKit/mock-system-header.h index a1d30957b19cb..1e44de8eb62ad 100644 --- a/clang/test/Analysis/Checkers/WebKit/mock-system-header.h +++ b/clang/test/Analysis/Checkers/WebKit/mock-system-header.h @@ -15,3 +15,30 @@ template struct MemberVariable { T* obj { nullptr }; }; + +typedef struct sqlite3 sqlite3; + +typedef unsigned char uint8_t; + +enum os_log_type_t : uint8_t { + OS_LOG_TYPE_DEFAULT = 0x00, + OS_LOG_TYPE_INFO = 0x01, + OS_LOG_TYPE_DEBUG = 0x02, + OS_LOG_TYPE_ERROR = 0x10, + OS_LOG_TYPE_FAULT = 0x11, +}; + +typedef struct os_log_s *os_log_t; +os_log_t os_log_create(const char *subsystem, const char *category); +void os_log_msg(os_log_t oslog, os_log_type_t type, const char *msg, ...); + +typedef const struct __attribute__((objc_bridge(NSString))) __CFString * CFStringRef; + +#ifdef __OBJC__ +@class NSString; +@interface SystemObject { + NSString *ns_string; + CFStringRef cf_string; +} +@end +#endif diff --git a/clang/test/Analysis/Checkers/WebKit/mock-types.h b/clang/test/Analysis/Checkers/WebKit/mock-types.h index a1f0cc8b046b9..a03d31870ee0d 100644 --- a/clang/test/Analysis/Checkers/WebKit/mock-types.h +++ b/clang/test/Analysis/Checkers/WebKit/mock-types.h @@ -249,7 +249,7 @@ template struct CheckedPtr { T *get() const { return t; } T *operator->() const { return t; } T &operator*() const { return *t; } - CheckedPtr &operator=(T *) { return *this; } + CheckedPtr &operator=(T *); operator bool() const { return t; } }; diff --git a/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h b/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h new file mode 100644 index 0000000000000..93e7dfd77b9e9 --- /dev/null +++ b/clang/test/Analysis/Checkers/WebKit/objc-mock-types.h @@ -0,0 +1,463 @@ +@class NSString; +@class NSArray; +@class NSMutableArray; +@class NSDictionary; +@class NSMutableDictionary; +#define nil ((id)0) +#define CF_BRIDGED_TYPE(T) __attribute__((objc_bridge(T))) +#define CF_BRIDGED_MUTABLE_TYPE(T) __attribute__((objc_bridge_mutable(T))) +typedef CF_BRIDGED_TYPE(id) void * CFTypeRef; +typedef unsigned long long CFTypeID; +typedef signed char BOOL; +typedef unsigned char Boolean; +typedef signed long CFIndex; +typedef unsigned long NSUInteger; +typedef const struct CF_BRIDGED_TYPE(id) __CFAllocator * CFAllocatorRef; +typedef const struct CF_BRIDGED_TYPE(NSString) __CFString * CFStringRef; +typedef const struct CF_BRIDGED_TYPE(NSArray) __CFArray * CFArrayRef; +typedef struct CF_BRIDGED_MUTABLE_TYPE(NSMutableArray) __CFArray * CFMutableArrayRef; +typedef struct CF_BRIDGED_MUTABLE_TYPE(CFRunLoopRef) __CFRunLoop * CFRunLoopRef; +typedef struct CF_BRIDGED_TYPE(id) CGImage *CGImageRef; + +#define NS_RETURNS_RETAINED __attribute__((ns_returns_retained)) +#define CF_CONSUMED __attribute__((cf_consumed)) +#define CF_RETURNS_RETAINED __attribute__((cf_returns_retained)) + +extern const CFAllocatorRef kCFAllocatorDefault; +typedef struct _NSZone NSZone; + +CFTypeID CFGetTypeID(CFTypeRef cf); + +CFTypeID CFGetTypeID(CFTypeRef cf); +CFTypeID CFArrayGetTypeID(); +CFMutableArrayRef CFArrayCreateMutable(CFAllocatorRef allocator, CFIndex capacity); +extern void CFArrayAppendValue(CFMutableArrayRef theArray, const void *value); +CFArrayRef CFArrayCreate(CFAllocatorRef allocator, const void **values, CFIndex numValues); +CFIndex CFArrayGetCount(CFArrayRef theArray); + +typedef const struct CF_BRIDGED_TYPE(NSDictionary) __CFDictionary * CFDictionaryRef; +typedef struct CF_BRIDGED_MUTABLE_TYPE(NSMutableDictionary) __CFDictionary * CFMutableDictionaryRef; + +CFTypeID CFDictionaryGetTypeID(); +CFDictionaryRef CFDictionaryCreate(CFAllocatorRef allocator, const void **keys, const void **values, CFIndex numValues); +CFDictionaryRef CFDictionaryCreateCopy(CFAllocatorRef allocator, CFDictionaryRef theDict); +CFDictionaryRef CFDictionaryCreateMutableCopy(CFAllocatorRef allocator, CFIndex capacity, CFDictionaryRef theDict); +CFIndex CFDictionaryGetCount(CFDictionaryRef theDict); +Boolean CFDictionaryContainsKey(CFDictionaryRef theDict, const void *key); +Boolean CFDictionaryContainsValue(CFDictionaryRef theDict, const void *value); +const void *CFDictionaryGetValue(CFDictionaryRef theDict, const void *key); +void CFDictionaryAddValue(CFMutableDictionaryRef theDict, const void *key, const void *value); +void CFDictionarySetValue(CFMutableDictionaryRef theDict, const void *key, const void *value); +void CFDictionaryReplaceValue(CFMutableDictionaryRef theDict, const void *key, const void *value); +void CFDictionaryRemoveValue(CFMutableDictionaryRef theDict, const void *key); + +typedef struct CF_BRIDGED_TYPE(id) __SecTask * SecTaskRef; +SecTaskRef SecTaskCreateFromSelf(CFAllocatorRef allocator); + +typedef struct CF_BRIDGED_TYPE(id) CF_BRIDGED_MUTABLE_TYPE(IOSurface) __IOSurface * IOSurfaceRef; +IOSurfaceRef IOSurfaceCreate(CFDictionaryRef properties); + +typedef struct CF_BRIDGED_TYPE(id) __CVBuffer *CVBufferRef; +typedef CVBufferRef CVImageBufferRef; +typedef CVImageBufferRef CVPixelBufferRef; +typedef signed int CVReturn; +CVReturn CVPixelBufferCreateWithIOSurface(CFAllocatorRef allocator, IOSurfaceRef surface, CFDictionaryRef pixelBufferAttributes, CF_RETURNS_RETAINED CVPixelBufferRef * pixelBufferOut); + +CFRunLoopRef CFRunLoopGetCurrent(void); +CFRunLoopRef CFRunLoopGetMain(void); +extern CFTypeRef CFRetain(CFTypeRef cf); +extern void CFRelease(CFTypeRef cf); +#define CFSTR(cStr) ((CFStringRef) __builtin___CFStringMakeConstantString ("" cStr "")) +extern Class NSClassFromString(NSString *aClassName); + +#if __has_feature(objc_arc) +id CFBridgingRelease(CFTypeRef X) { + return (__bridge_transfer id)X; +} +#endif + +__attribute__((objc_root_class)) +@interface NSObject ++ (instancetype) alloc; ++ (Class) class; ++ (Class) superclass; +- (instancetype) init; +- (instancetype)retain; +- (instancetype)autorelease; +- (void)release; +- (BOOL)isKindOfClass:(Class)aClass; +@end + +@protocol NSCopying +- (id)copyWithZone:(NSZone *)zone; +@end + +@protocol NSFastEnumeration +- (int)countByEnumeratingWithState:(void *)state objects:(id *)objects count:(unsigned)count; +- (void)protocolMethod; +@end + +@interface NSEnumerator +@end + +@interface NSDictionary : NSObject +- (NSUInteger)count; +- (id)objectForKey:(id)aKey; +- (id)objectForKeyedSubscript:(id)aKey; +- (NSEnumerator *)keyEnumerator; ++ (id)dictionary; ++ (id)dictionaryWithObject:(id)object forKey:(id )key; +- (NSMutableDictionary *)mutableCopy; ++ (instancetype)dictionaryWithObjects:(const id [])objects forKeys:(const id [])keys count:(NSUInteger)cnt; +@end + +@interface NSArray : NSObject +- (NSUInteger)count; +- (NSEnumerator *)objectEnumerator; +- (NSMutableArray *)mutableCopy; ++ (NSArray *)arrayWithObjects:(const id [])objects count:(NSUInteger)count; +@end + +@interface NSString : NSObject +- (NSUInteger)length; +- (NSString *)stringByAppendingString:(NSString *)aString; +- ( const char *)UTF8String; +- (id)initWithUTF8String:(const char *)nullTerminatedCString; +- (NSString *)copy; ++ (id)stringWithUTF8String:(const char *)nullTerminatedCString; +@end + +@interface NSMutableString : NSString +@end + +@interface NSValue : NSObject +- (void)getValue:(void *)value; +@end + +@interface NSNumber : NSValue +- (char)charValue; +- (int)intValue; +- (id)initWithInt:(int)value; ++ (NSNumber *)numberWithInt:(int)value; +@end + +@interface SomeObj : NSObject +- (instancetype)_init; +- (SomeObj *)mutableCopy; +- (SomeObj *)copyWithValue:(int)value; +- (void)doWork; +- (SomeObj *)other; +- (SomeObj *)next; +- (int)value; +@end + +@interface OtherObj : SomeObj +- (void)doMoreWork:(OtherObj *)other; +@end + +namespace WTF { + +void WTFCrash(void); + +#if __has_feature(objc_arc) +#define RetainPtr RetainPtrArc +#endif + +template class RetainPtr; +template RetainPtr adoptNS(T*); +template RetainPtr adoptCF(T); + +template T *downcast(S *t) { return static_cast(t); } + +template struct RemovePointer { + typedef T Type; +}; + +template struct RemovePointer { + typedef T Type; +}; + +template struct RetainPtr { + using ValueType = typename RemovePointer::Type; + using PtrType = ValueType*; + PtrType t; + + RetainPtr() : t(nullptr) { } + RetainPtr(PtrType t) + : t(t) { + if (t) + CFRetain(toCFTypeRef(t)); + } + RetainPtr(RetainPtr&&); + RetainPtr(const RetainPtr&); + template + RetainPtr(const RetainPtr& o) + : RetainPtr(o.t) + {} + RetainPtr operator=(const RetainPtr& o) + { + if (t) + CFRelease(toCFTypeRef(t)); + t = o.t; + if (t) + CFRetain(toCFTypeRef(t)); + return *this; + } + template + RetainPtr operator=(const RetainPtr& o) + { + if (t) + CFRelease(toCFTypeRef(t)); + t = o.t; + if (t) + CFRetain(toCFTypeRef(t)); + return *this; + } + ~RetainPtr() { + clear(); + } + void clear() { + if (t) + CFRelease(toCFTypeRef(t)); + t = nullptr; + } + void swap(RetainPtr& o) { + PtrType tmp = t; + t = o.t; + o.t = tmp; + } + PtrType get() const { return t; } + PtrType operator->() const { return t; } + T &operator*() const { return *t; } + RetainPtr &operator=(PtrType t); + PtrType leakRef() + { + PtrType s = t; + t = nullptr; + return s; + } + operator PtrType() const { return t; } + operator bool() const { return t; } + +#if !__has_feature(objc_arc) + PtrType autorelease() { [[clang::suppress]] return [t autorelease]; } +#endif + +private: + CFTypeRef toCFTypeRef(id ptr) { return (__bridge CFTypeRef)ptr; } + CFTypeRef toCFTypeRef(const void* ptr) { return (CFTypeRef)ptr; } + + template friend RetainPtr adoptNS(U*); + template friend RetainPtr adoptCF(U); + + enum AdoptTag { Adopt }; + RetainPtr(PtrType t, AdoptTag) : t(t) { } +}; + +template +RetainPtr::RetainPtr(RetainPtr&& o) + : RetainPtr(o.t) +{ + o.t = nullptr; +} + +template +RetainPtr::RetainPtr(const RetainPtr& o) + : RetainPtr(o.t) +{ +} + +template +RetainPtr adoptNS(T* t) { +#if __has_feature(objc_arc) + return t; +#else + return RetainPtr(t, RetainPtr::Adopt); +#endif +} + +template +RetainPtr adoptCF(T t) { + return RetainPtr(t, RetainPtr::Adopt); +} + +template inline RetainPtr retainPtr(T ptr) +{ + return ptr; +} + +template inline RetainPtr retainPtr(T* ptr) +{ + return ptr; +} + +inline NSObject *bridge_cast(CFTypeRef object) +{ + return (__bridge NSObject *)object; +} + +inline CFTypeRef bridge_cast(NSObject *object) +{ + return (__bridge CFTypeRef)object; +} + +inline id bridge_id_cast(CFTypeRef object) +{ + return (__bridge id)object; +} + +inline RetainPtr bridge_id_cast(RetainPtr&& object) +{ +#if __has_feature(objc_arc) + return adoptNS((__bridge_transfer id)object.leakRef()); +#else + return adoptNS((__bridge id)object.leakRef()); +#endif +} + +template +struct ObjCTypeCastTraits { +public: + static bool isType(id object) { return [object isKindOfClass:[ExpectedType class]]; } + + template + static bool isType(const ArgType *object) { return [object isKindOfClass:[ExpectedType class]]; } +}; + +template +inline bool is_objc(ArgType * source) +{ + return source && ObjCTypeCastTraits::isType(source); +} + +template inline T *checked_objc_cast(id object) +{ + if (!object) + return nullptr; + + if (!is_objc(object)) + WTFCrash(); + + return reinterpret_cast(object); +} + +template inline T *checked_objc_cast(U *object) +{ + if (!object) + return nullptr; + + if (!is_objc(object)) + WTFCrash(); + + return static_cast(object); +} + +template RetainPtr dynamic_objc_cast(RetainPtr&& object) +{ + if (!is_objc(object.get())) + return nullptr; + return adoptNS(static_cast(object.leakRef())); +} + +template RetainPtr dynamic_objc_cast(RetainPtr&& object) +{ + if (!is_objc(object.get())) + return nullptr; + return adoptNS(reinterpret_cast(object.leakRef())); +} + +template RetainPtr dynamic_objc_cast(const RetainPtr& object) +{ + if (!is_objc(object.get())) + return nullptr; + return static_cast(object.get()); +} + +template RetainPtr dynamic_objc_cast(const RetainPtr& object) +{ + if (!is_objc(object.get())) + return nullptr; + return reinterpret_cast(object.get()); +} + +template T *dynamic_objc_cast(NSObject *object) +{ + if (!is_objc(object)) + return nullptr; + return static_cast(object); +} + +template T *dynamic_objc_cast(id object) +{ + if (!is_objc(object)) + return nullptr; + return reinterpret_cast(object); +} + +template struct CFTypeTrait; + +template T dynamic_cf_cast(CFTypeRef object) +{ + if (!object) + return nullptr; + + if (CFGetTypeID(object) != CFTypeTrait::typeID()) + return nullptr; + + return static_cast(const_cast(object)); +} + +template T checked_cf_cast(CFTypeRef object) +{ + if (!object) + return nullptr; + + if (CFGetTypeID(object) != CFTypeTrait::typeID()) + WTFCrash(); + + return static_cast(const_cast(object)); +} + +template RetainPtr dynamic_cf_cast(RetainPtr&& object) +{ + if (!object) + return nullptr; + + if (CFGetTypeID(object.get()) != CFTypeTrait::typeID()) + return nullptr; + + return adoptCF(static_cast(const_cast(object.leakRef()))); +} + +} // namespace WTF + +#define WTF_DECLARE_CF_TYPE_TRAIT(ClassName) \ +template <> \ +struct WTF::CFTypeTrait { \ + static inline CFTypeID typeID(void) { return ClassName##GetTypeID(); } \ +}; + +WTF_DECLARE_CF_TYPE_TRAIT(CFArray); +WTF_DECLARE_CF_TYPE_TRAIT(CFDictionary); + +#define WTF_DECLARE_CF_MUTABLE_TYPE_TRAIT(ClassName, MutableClassName) \ +template <> \ +struct WTF::CFTypeTrait { \ + static inline CFTypeID typeID(void) { return ClassName##GetTypeID(); } \ +}; + +WTF_DECLARE_CF_MUTABLE_TYPE_TRAIT(CFArray, CFMutableArray); +WTF_DECLARE_CF_MUTABLE_TYPE_TRAIT(CFDictionary, CFMutableDictionary); + +using WTF::RetainPtr; +using WTF::adoptNS; +using WTF::adoptCF; +using WTF::retainPtr; +using WTF::downcast; +using WTF::bridge_cast; +using WTF::bridge_id_cast; +using WTF::is_objc; +using WTF::checked_objc_cast; +using WTF::dynamic_objc_cast; +using WTF::checked_cf_cast; +using WTF::dynamic_cf_cast; diff --git a/clang/test/Analysis/Checkers/WebKit/ref-cntbl-base-virtual-dtor.cpp b/clang/test/Analysis/Checkers/WebKit/ref-cntbl-base-virtual-dtor.cpp index 5cf7e7614d06e..fd4144d572e01 100644 --- a/clang/test/Analysis/Checkers/WebKit/ref-cntbl-base-virtual-dtor.cpp +++ b/clang/test/Analysis/Checkers/WebKit/ref-cntbl-base-virtual-dtor.cpp @@ -1,5 +1,13 @@ // RUN: %clang_analyze_cc1 -analyzer-checker=webkit.RefCntblBaseVirtualDtor -verify %s +namespace WTF { + +class NoVirtualDestructorBase { }; + +}; + +using WTF::NoVirtualDestructorBase; + struct RefCntblBase { void ref() {} void deref() {} @@ -19,6 +27,15 @@ struct [[clang::suppress]] SuppressedDerivedWithVirtualDtor : RefCntblBase { virtual ~SuppressedDerivedWithVirtualDtor() {} }; +class ClassWithoutVirtualDestructor : public NoVirtualDestructorBase { +public: + void ref() const; + void deref() const; +}; + +class DerivedClassWithoutVirtualDestructor : public ClassWithoutVirtualDestructor { +}; + // FIXME: Support attributes on base specifiers? Currently clang // doesn't support such attributes at all, even though it knows // how to parse them. diff --git a/clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use-arc.mm b/clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use-arc.mm new file mode 100644 index 0000000000000..43d155d5e7f95 --- /dev/null +++ b/clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use-arc.mm @@ -0,0 +1,310 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.RetainPtrCtorAdoptChecker -fobjc-arc -verify %s + +#include "objc-mock-types.h" + +CFTypeRef CFCopyArray(CFArrayRef); + +void basic_correct() { + auto ns1 = adoptNS([SomeObj alloc]); + auto ns2 = adoptNS([[SomeObj alloc] init]); + RetainPtr ns3 = [ns1.get() next]; + auto ns4 = adoptNS([ns3 mutableCopy]); + auto ns5 = adoptNS([ns3 copyWithValue:3]); + auto ns6 = retainPtr([ns3 next]); + CFMutableArrayRef cf1 = adoptCF(CFArrayCreateMutable(kCFAllocatorDefault, 10)); + auto cf2 = adoptCF(SecTaskCreateFromSelf(kCFAllocatorDefault)); + auto cf3 = adoptCF(checked_cf_cast(CFCopyArray(cf1))); +} + +CFMutableArrayRef provide_cf(); + +void basic_wrong() { + RetainPtr ns1 = [[SomeObj alloc] init]; + // expected-warning@-1{{Incorrect use of RetainPtr constructor. The argument is +1 and results in a memory leak when ARC is disabled [alpha.webkit.RetainPtrCtorAdoptChecker]}} + auto ns2 = adoptNS([ns1.get() next]); + // expected-warning@-1{{Incorrect use of adoptNS. The argument is +0 and results in an use-after-free when ARC is disabled [alpha.webkit.RetainPtrCtorAdoptChecker]}} + RetainPtr cf1 = CFArrayCreateMutable(kCFAllocatorDefault, 10); + // expected-warning@-1{{Incorrect use of RetainPtr constructor. The argument is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}} + RetainPtr cf2 = adoptCF(provide_cf()); + // expected-warning@-1{{Incorrect use of adoptCF. The argument is +0 and results in an use-after-free when ARC is disabled [alpha.webkit.RetainPtrCtorAdoptChecker]}} + RetainPtr cf3 = SecTaskCreateFromSelf(kCFAllocatorDefault); + // expected-warning@-1{{Incorrect use of RetainPtr constructor. The argument is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}} + CFCopyArray(cf1); + // expected-warning@-1{{The return value is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}} +} + +void basic_correct_arc() { + auto *obj = [[SomeObj alloc] init]; + [obj doWork]; +} + +@implementation SomeObj { + NSNumber *_number; + SomeObj *_next; + SomeObj *_other; +} + +- (instancetype)_init { + self = [super init]; + _number = nil; + _next = nil; + _other = nil; + return self; +} + +- (SomeObj *)mutableCopy { + auto *copy = [[SomeObj alloc] init]; + [copy setValue:_number]; + [copy setNext:_next]; + [copy setOther:_other]; + return copy; +} + +- (SomeObj *)copyWithValue:(int)value { + auto *copy = [[SomeObj alloc] init]; + [copy setValue:_number]; + [copy setNext:_next]; + [copy setOther:_other]; + return copy; +} + +- (void)doWork { + _number = [[NSNumber alloc] initWithInt:5]; +} + +- (SomeObj *)other { + return _other; +} + +- (void)setOther:(SomeObj *)obj { + _other = obj; +} + +- (SomeObj *)next { + return _next; +} + +- (void)setNext:(SomeObj *)obj { + _next = obj; +} + +- (int)value { + return [_number intValue]; +} + +- (void)setValue:(NSNumber *)value { + _number = value; +} + +@end; + +RetainPtr cf_out_argument() { + auto surface = adoptCF(IOSurfaceCreate(nullptr)); + CVPixelBufferRef rawBuffer = nullptr; + auto status = CVPixelBufferCreateWithIOSurface(kCFAllocatorDefault, surface.get(), nullptr, &rawBuffer); + return adoptCF(rawBuffer); +} + +RetainPtr return_nullptr() { + return nullptr; +} + +RetainPtr return_retainptr() { + RetainPtr foo; + return foo; +} + +CFTypeRef CopyValueForSomething(); + +void cast_retainptr() { + RetainPtr foo; + RetainPtr bar = static_cast(foo); + + auto baz = adoptCF(CopyValueForSomething()).get(); + RetainPtr v = static_cast(baz); +} + +CFTypeRef CopyWrapper() { + return CopyValueForSomething(); +} + +CFTypeRef LeakWrapper() { + return CopyValueForSomething(); + // expected-warning@-1{{The return value is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}} +} + +NSArray *makeArray() NS_RETURNS_RETAINED { + return CFBridgingRelease(CFArrayCreateMutable(kCFAllocatorDefault, 10)); +} + +extern Class (*getNSArrayClass)(); +NSArray *allocArrayInstance() NS_RETURNS_RETAINED { + return [[getNSArrayClass() alloc] init]; +} + +extern int (*GetObj)(CF_RETURNS_RETAINED CFTypeRef* objOut); +RetainPtr getObject() { + CFTypeRef obj = nullptr; + if (GetObj(&obj)) + return nullptr; + return adoptCF(obj); +} + +CFArrayRef CreateSingleArray(CFStringRef); +CFArrayRef CreateSingleArray(CFDictionaryRef); +CFArrayRef CreateSingleArray(CFArrayRef); +template +static RetainPtr makeArrayWithSingleEntry(ElementType arg) { + return adoptCF(CreateSingleArray(arg)); +} + +void callMakeArayWithSingleEntry() { + auto dictionary = adoptCF(CFDictionaryCreate(kCFAllocatorDefault, nullptr, nullptr, 0)); + makeArrayWithSingleEntry(dictionary.get()); +} + +SomeObj* allocSomeObj() CF_RETURNS_RETAINED; + +void adopt_retainptr() { + RetainPtr foo = adoptNS([[SomeObj alloc] init]); + auto bar = adoptNS([allocSomeObj() init]); +} + +RetainPtr return_arg(CFArrayRef arg) { + return arg; +} + +class MemberInit { +public: + MemberInit(RetainPtr&& array, NSString *str, CFRunLoopRef runLoop) + : m_array(array) + , m_str(str) + , m_runLoop(runLoop) + { } + +private: + RetainPtr m_array; + RetainPtr m_str; + RetainPtr m_runLoop; +}; +void create_member_init() { + MemberInit init { adoptCF(CFArrayCreateMutable(kCFAllocatorDefault, 10)), @"hello", CFRunLoopGetCurrent() }; +} + +RetainPtr cfstr() { + return CFSTR(""); +} + +template +static RetainPtr bridge_cast(RetainPtr&& ptr) +{ + return adoptNS((__bridge NSArray *)(ptr.leakRef())); +} + +RetainPtr create_cf_array(); +RetainPtr return_bridge_cast() { + return bridge_cast(create_cf_array()); +} + +void mutable_copy_dictionary() { + RetainPtr mutableDictionary = adoptNS(@{ + @"Content-Type": @"text/html", + }.mutableCopy); +} + +void mutable_copy_array() { + RetainPtr mutableArray = adoptNS(@[ + @"foo", + ].mutableCopy); +} + +void string_copy(NSString *str) { + RetainPtr copy = adoptNS(str.copy); +} + +void alloc_init_spi() { + auto ptr = adoptNS([[SomeObj alloc] _init]); +} + +void alloc_init_c_function() { + RetainPtr ptr = adoptNS([allocSomeObj() init]); +} + +CFArrayRef make_array() CF_RETURNS_RETAINED; + +RetainPtr adopt_make_array() { + return adoptCF(make_array()); +} + +@interface SomeObject : NSObject +-(void)basic_correct; +-(void)basic_wrong; +-(NSString *)leak_string; +-(NSString *)make_string NS_RETURNS_RETAINED; +@property (nonatomic, readonly) SomeObj *obj; +@end + +@implementation SomeObject +-(void)basic_correct { + auto ns1 = adoptNS([SomeObj alloc]); + auto ns2 = adoptNS([[SomeObj alloc] init]); + RetainPtr ns3 = [ns1.get() next]; + auto ns4 = adoptNS([ns3 mutableCopy]); + auto ns5 = adoptNS([ns3 copyWithValue:3]); + auto ns6 = retainPtr([ns3 next]); + CFMutableArrayRef cf1 = adoptCF(CFArrayCreateMutable(kCFAllocatorDefault, 10)); + auto cf2 = adoptCF(SecTaskCreateFromSelf(kCFAllocatorDefault)); + auto cf3 = adoptCF(checked_cf_cast(CFCopyArray(cf1))); +} + +-(void)basic_wrong { + RetainPtr ns1 = [[SomeObj alloc] init]; + // expected-warning@-1{{Incorrect use of RetainPtr constructor. The argument is +1 and results in a memory leak when ARC is disabled [alpha.webkit.RetainPtrCtorAdoptChecker]}} + auto ns2 = adoptNS([ns1.get() next]); + // expected-warning@-1{{Incorrect use of adoptNS. The argument is +0 and results in an use-after-free when ARC is disabled [alpha.webkit.RetainPtrCtorAdoptChecker]}} + RetainPtr cf1 = CFArrayCreateMutable(kCFAllocatorDefault, 10); + // expected-warning@-1{{Incorrect use of RetainPtr constructor. The argument is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}} + RetainPtr cf2 = adoptCF(provide_cf()); + // expected-warning@-1{{Incorrect use of adoptCF. The argument is +0 and results in an use-after-free when ARC is disabled [alpha.webkit.RetainPtrCtorAdoptChecker]}} + RetainPtr cf3 = SecTaskCreateFromSelf(kCFAllocatorDefault); + // expected-warning@-1{{Incorrect use of RetainPtr constructor. The argument is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}} + CFCopyArray(cf1); + // expected-warning@-1{{The return value is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}} +} + +-(NSString *)leak_string { + return [[NSString alloc] initWithUTF8String:"hello"]; +} + +-(NSString *)make_string { + return [[NSString alloc] initWithUTF8String:"hello"]; +} + +-(void)local_leak_string { + if ([[NSString alloc] initWithUTF8String:"hello"]) { + } +} + +-(void)make_some_obj { + auto some_obj = adoptNS([allocSomeObj() init]); +} + +-(void)alloc_init_bad_order { + auto *obj = [NSObject alloc]; + auto ptr = adoptNS([obj init]); + // expected-warning@-1{{Incorrect use of adoptNS. The argument is +0 and results in an use-after-free when ARC is disabled [alpha.webkit.RetainPtrCtorAdoptChecker]}} +} + +-(void)alloc_init_good_order { + auto obj = adoptNS([NSObject alloc]); + (void)[obj init]; +} + +-(void)copy_assign_ivar { + _obj = [allocSomeObj() init]; +} + +-(void)do_more_work:(OtherObj *)otherObj { + [otherObj doMoreWork:[[OtherObj alloc] init]]; +} +@end 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 new file mode 100644 index 0000000000000..dc61136c39d57 --- /dev/null +++ b/clang/test/Analysis/Checkers/WebKit/retain-ptr-ctor-adopt-use.mm @@ -0,0 +1,323 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.RetainPtrCtorAdoptChecker -verify %s + +#include "objc-mock-types.h" + +CFTypeRef CFCopyArray(CFArrayRef); +void* CreateCopy(); + +void basic_correct() { + auto ns1 = adoptNS([SomeObj alloc]); + auto ns2 = adoptNS([[SomeObj alloc] init]); + RetainPtr ns3 = [ns1.get() next]; + auto ns4 = adoptNS([ns3 mutableCopy]); + auto ns5 = adoptNS([ns3 copyWithValue:3]); + auto ns6 = retainPtr([ns3 next]); + CFMutableArrayRef cf1 = adoptCF(CFArrayCreateMutable(kCFAllocatorDefault, 10)); + auto cf2 = adoptCF(SecTaskCreateFromSelf(kCFAllocatorDefault)); + auto cf3 = adoptCF(checked_cf_cast(CFCopyArray(cf1))); + CreateCopy(); +} + +CFMutableArrayRef provide_cf(); + +void basic_wrong() { + RetainPtr ns1 = [[SomeObj alloc] init]; + // expected-warning@-1{{Incorrect use of RetainPtr constructor. The argument is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}} + auto ns2 = adoptNS([ns1.get() next]); + // expected-warning@-1{{Incorrect use of adoptNS. The argument is +0 and results in an use-after-free [alpha.webkit.RetainPtrCtorAdoptChecker]}} + RetainPtr cf1 = CFArrayCreateMutable(kCFAllocatorDefault, 10); + // expected-warning@-1{{Incorrect use of RetainPtr constructor. The argument is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}} + RetainPtr cf2 = adoptCF(provide_cf()); + // expected-warning@-1{{Incorrect use of adoptCF. The argument is +0 and results in an use-after-free [alpha.webkit.RetainPtrCtorAdoptChecker]}} + RetainPtr cf3 = SecTaskCreateFromSelf(kCFAllocatorDefault); + // expected-warning@-1{{Incorrect use of RetainPtr constructor. The argument is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}} + CFCopyArray(cf1); + // expected-warning@-1{{The return value is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}} +} + +void basic_correct_arc() { + auto *obj = [[SomeObj alloc] init]; + // expected-warning@-1{{The return value is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}} + [obj doWork]; +} + +@implementation SomeObj { + NSNumber *_number; + SomeObj *_next; + SomeObj *_other; +} + +- (instancetype)_init { + self = [super init]; + _number = nil; + _next = nil; + _other = nil; + return self; +} + +- (SomeObj *)mutableCopy { + auto *copy = [[SomeObj alloc] init]; + // expected-warning@-1{{The return value is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}} + [copy setValue:_number]; + [copy setNext:_next]; + [copy setOther:_other]; + return copy; +} + +- (SomeObj *)copyWithValue:(int)value { + auto *copy = [[SomeObj alloc] init]; + // expected-warning@-1{{The return value is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}} + [copy setValue:_number]; + [copy setNext:_next]; + [copy setOther:_other]; + return copy; +} + +- (void)doWork { + _number = [[NSNumber alloc] initWithInt:5]; +} + +- (SomeObj *)other { + return _other; +} + +- (void)setOther:(SomeObj *)obj { + _other = obj; +} + +- (SomeObj *)next { + return _next; +} + +- (void)setNext:(SomeObj *)obj { + _next = obj; +} + +- (int)value { + return [_number intValue]; +} + +- (void)setValue:value { + _number = value; +} + +@end; + +RetainPtr cf_out_argument() { + auto surface = adoptCF(IOSurfaceCreate(nullptr)); + CVPixelBufferRef rawBuffer = nullptr; + auto status = CVPixelBufferCreateWithIOSurface(kCFAllocatorDefault, surface.get(), nullptr, &rawBuffer); + return adoptCF(rawBuffer); +} + +RetainPtr return_nullptr() { + return nullptr; +} + +RetainPtr return_retainptr() { + RetainPtr foo; + return foo; +} + +CFTypeRef CopyValueForSomething(); + +void cast_retainptr() { + RetainPtr foo; + RetainPtr bar = static_cast(foo); + + auto baz = adoptCF(CopyValueForSomething()).get(); + RetainPtr v = static_cast(baz); +} + +CFTypeRef CopyWrapper() { + return CopyValueForSomething(); +} + +CFTypeRef LeakWrapper() { + return CopyValueForSomething(); + // expected-warning@-1{{The return value is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}} +} + +NSArray *makeArray() NS_RETURNS_RETAINED { + return (__bridge NSArray *)CFArrayCreateMutable(kCFAllocatorDefault, 10); +} + +extern Class (*getNSArrayClass)(); +NSArray *allocArrayInstance() NS_RETURNS_RETAINED { + return [[getNSArrayClass() alloc] init]; +} + +extern int (*GetObj)(CF_RETURNS_RETAINED CFTypeRef* objOut); +RetainPtr getObject() { + CFTypeRef obj = nullptr; + if (GetObj(&obj)) + return nullptr; + return adoptCF(obj); +} + +CFArrayRef CreateSingleArray(CFStringRef); +CFArrayRef CreateSingleArray(CFDictionaryRef); +CFArrayRef CreateSingleArray(CFArrayRef); +template +static RetainPtr makeArrayWithSingleEntry(ElementType arg) { + return adoptCF(CreateSingleArray(arg)); +} + +void callMakeArayWithSingleEntry() { + auto dictionary = adoptCF(CFDictionaryCreate(kCFAllocatorDefault, nullptr, nullptr, 0)); + makeArrayWithSingleEntry(dictionary.get()); +} + +SomeObj* allocSomeObj() CF_RETURNS_RETAINED; + +void adopt_retainptr() { + RetainPtr foo = adoptNS([[SomeObj alloc] init]); + auto bar = adoptNS([allocSomeObj() init]); +} + +RetainPtr return_arg(CFArrayRef arg) { + return arg; +} + +class MemberInit { +public: + MemberInit(RetainPtr&& array, NSString *str, CFRunLoopRef runLoop) + : m_array(array) + , m_str(str) + , m_runLoop(runLoop) + { } + +private: + RetainPtr m_array; + RetainPtr m_str; + RetainPtr m_runLoop; +}; +void create_member_init() { + MemberInit init { adoptCF(CFArrayCreateMutable(kCFAllocatorDefault, 10)), @"hello", CFRunLoopGetCurrent() }; +} + +RetainPtr cfstr() { + return CFSTR(""); +} + +template +static RetainPtr bridge_cast(RetainPtr&& ptr) +{ + return adoptNS((__bridge NSArray *)(ptr.leakRef())); +} + +RetainPtr create_cf_array(); +RetainPtr return_bridge_cast() { + return bridge_cast(create_cf_array()); +} + +void mutable_copy_dictionary() { + RetainPtr mutableDictionary = adoptNS(@{ + @"Content-Type": @"text/html", + }.mutableCopy); +} + +void mutable_copy_array() { + RetainPtr mutableArray = adoptNS(@[ + @"foo", + ].mutableCopy); +} + +void string_copy(NSString *str) { + RetainPtr copy = adoptNS(str.copy); +} + +void alloc_init_spi() { + auto ptr = adoptNS([[SomeObj alloc] _init]); +} + +void alloc_init_c_function() { + RetainPtr ptr = adoptNS([allocSomeObj() init]); +} + +void alloc_init_autorelease() { + [[[SomeObj alloc] init] autorelease]; +} + +CFArrayRef make_array() CF_RETURNS_RETAINED; + +RetainPtr adopt_make_array() { + return adoptCF(make_array()); +} + +@interface SomeObject : NSObject +-(void)basic_correct; +-(void)basic_wrong; +-(NSString *)leak_string; +-(NSString *)make_string NS_RETURNS_RETAINED; +@property (nonatomic, readonly) SomeObj *obj; +@end + +@implementation SomeObject +-(void)basic_correct { + auto ns1 = adoptNS([SomeObj alloc]); + auto ns2 = adoptNS([[SomeObj alloc] init]); + RetainPtr ns3 = [ns1.get() next]; + auto ns4 = adoptNS([ns3 mutableCopy]); + auto ns5 = adoptNS([ns3 copyWithValue:3]); + auto ns6 = retainPtr([ns3 next]); + CFMutableArrayRef cf1 = adoptCF(CFArrayCreateMutable(kCFAllocatorDefault, 10)); + auto cf2 = adoptCF(SecTaskCreateFromSelf(kCFAllocatorDefault)); + auto cf3 = adoptCF(checked_cf_cast(CFCopyArray(cf1))); +} + +-(void)basic_wrong { + RetainPtr ns1 = [[SomeObj alloc] init]; + // expected-warning@-1{{Incorrect use of RetainPtr constructor. The argument is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}} + auto ns2 = adoptNS([ns1.get() next]); + // expected-warning@-1{{Incorrect use of adoptNS. The argument is +0 and results in an use-after-free [alpha.webkit.RetainPtrCtorAdoptChecker]}} + RetainPtr cf1 = CFArrayCreateMutable(kCFAllocatorDefault, 10); + // expected-warning@-1{{Incorrect use of RetainPtr constructor. The argument is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}} + RetainPtr cf2 = adoptCF(provide_cf()); + // expected-warning@-1{{Incorrect use of adoptCF. The argument is +0 and results in an use-after-free [alpha.webkit.RetainPtrCtorAdoptChecker]}} + RetainPtr cf3 = SecTaskCreateFromSelf(kCFAllocatorDefault); + // expected-warning@-1{{Incorrect use of RetainPtr constructor. The argument is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}} + CFCopyArray(cf1); + // expected-warning@-1{{The return value is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}} +} + +-(NSString *)leak_string { + return [[NSString alloc] initWithUTF8String:"hello"]; + // expected-warning@-1{{The return value is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}} +} + +-(NSString *)make_string { + return [[NSString alloc] initWithUTF8String:"hello"]; +} + +-(void)local_leak_string { + if ([[NSString alloc] initWithUTF8String:"hello"]) { + // expected-warning@-1{{The return value is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}} + } +} + +-(void)make_some_obj { + auto some_obj = adoptNS([allocSomeObj() init]); +} + +-(void)alloc_init_bad_order { + auto *obj = [NSObject alloc]; + // expected-warning@-1{{The return value is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}} + auto ptr = adoptNS([obj init]); + // expected-warning@-1{{Incorrect use of adoptNS. The argument is +0 and results in an use-after-free [alpha.webkit.RetainPtrCtorAdoptChecker]}} +} + +-(void)alloc_init_good_order { + auto obj = adoptNS([NSObject alloc]); + (void)[obj init]; +} + +-(void)copy_assign_ivar { + _obj = [allocSomeObj() init]; +} + +-(void)do_more_work:(OtherObj *)otherObj { + [otherObj doMoreWork:[[OtherObj alloc] init]]; + // expected-warning@-1{{The return value is +1 and results in a memory leak [alpha.webkit.RetainPtrCtorAdoptChecker]}} +} +@end diff --git a/clang/test/Analysis/Checkers/WebKit/unchecked-call-arg.cpp b/clang/test/Analysis/Checkers/WebKit/unchecked-call-arg.cpp new file mode 100644 index 0000000000000..8685978ebf1ac --- /dev/null +++ b/clang/test/Analysis/Checkers/WebKit/unchecked-call-arg.cpp @@ -0,0 +1,34 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UncheckedCallArgsChecker -verify %s + +void WTFCrash(void); + +enum class Tag : bool { Value }; + +template class CanMakeCheckedPtrBase { +public: + void incrementCheckedPtrCount() const { ++m_checkedPtrCount; } + inline void decrementCheckedPtrCount() const + { + if (!m_checkedPtrCount) + WTFCrash(); + --m_checkedPtrCount; + } + +private: + mutable StorageType m_checkedPtrCount { 0 }; +}; + +template +class CanMakeCheckedPtr : public CanMakeCheckedPtrBase { +}; + +class CheckedObject : public CanMakeCheckedPtr { +public: + void doWork(); +}; + +CheckedObject* provide(); +void foo() { + provide()->doWork(); + // expected-warning@-1{{Call argument for 'this' parameter is unchecked and unsafe}} +} diff --git a/clang/test/Analysis/Checkers/WebKit/unchecked-members.cpp b/clang/test/Analysis/Checkers/WebKit/unchecked-members.cpp index 0189b0cd50fcc..048ffbffcdefb 100644 --- a/clang/test/Analysis/Checkers/WebKit/unchecked-members.cpp +++ b/clang/test/Analysis/Checkers/WebKit/unchecked-members.cpp @@ -50,3 +50,12 @@ namespace ignore_unions { void forceTmplToInstantiate(FooTmpl) { } } // namespace ignore_unions + +namespace checked_ptr_ref_ptr_capable { + + RefCountableAndCheckable* provide(); + void foo() { + RefPtr foo = provide(); + } + +} // checked_ptr_ref_ptr_capable diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures-decl-protects-this-crash.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures-decl-protects-this-crash.cpp index 840433db5133a..0c10c69a97eda 100644 --- a/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures-decl-protects-this-crash.cpp +++ b/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures-decl-protects-this-crash.cpp @@ -28,9 +28,9 @@ struct Obj { void foo(Foo foo) { bar([this](auto baz) { - // expected-warning@-1{{Captured raw-pointer 'this' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}} + // expected-warning@-1{{Captured raw-pointer 'this' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}} bar([this, foo = *baz, foo2 = !baz](auto&&) { - // expected-warning@-1{{Captured raw-pointer 'this' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}} + // expected-warning@-1{{Captured raw-pointer 'this' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}} someFunction(); }); }); diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp index 82058bf13d137..daa15d55aee5a 100644 --- a/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp +++ b/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp @@ -2,6 +2,15 @@ #include "mock-types.h" +namespace std { + +template +T&& move(T& t) { + return static_cast(t); +} + +} + namespace WTF { namespace Detail { @@ -94,22 +103,22 @@ void callAsync(const WTF::Function&); void raw_ptr() { RefCountable* ref_countable = make_obj(); auto foo1 = [ref_countable](){ - // expected-warning@-1{{Captured raw-pointer 'ref_countable' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}} + // expected-warning@-1{{Captured raw-pointer 'ref_countable' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}} ref_countable->method(); }; auto foo2 = [&ref_countable](){ - // expected-warning@-1{{Captured raw-pointer 'ref_countable' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}} + // expected-warning@-1{{Captured raw-pointer 'ref_countable' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}} ref_countable->method(); }; auto foo3 = [&](){ ref_countable->method(); - // expected-warning@-1{{Implicitly captured raw-pointer 'ref_countable' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}} + // expected-warning@-1{{Implicitly captured raw-pointer 'ref_countable' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}} ref_countable = nullptr; }; auto foo4 = [=](){ ref_countable->method(); - // expected-warning@-1{{Implicitly captured raw-pointer 'ref_countable' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}} + // expected-warning@-1{{Implicitly captured raw-pointer 'ref_countable' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}} }; call(foo1); @@ -128,13 +137,13 @@ void references() { RefCountable automatic; RefCountable& ref_countable_ref = automatic; auto foo1 = [ref_countable_ref](){ ref_countable_ref.constMethod(); }; - // expected-warning@-1{{Captured reference 'ref_countable_ref' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}} + // expected-warning@-1{{Captured reference 'ref_countable_ref' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}} auto foo2 = [&ref_countable_ref](){ ref_countable_ref.method(); }; - // expected-warning@-1{{Captured reference 'ref_countable_ref' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}} + // expected-warning@-1{{Captured reference 'ref_countable_ref' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}} auto foo3 = [&](){ ref_countable_ref.method(); }; - // expected-warning@-1{{Implicitly captured reference 'ref_countable_ref' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}} + // expected-warning@-1{{Implicitly captured reference 'ref_countable_ref' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}} auto foo4 = [=](){ ref_countable_ref.constMethod(); }; - // expected-warning@-1{{Implicitly captured reference 'ref_countable_ref' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}} + // expected-warning@-1{{Implicitly captured reference 'ref_countable_ref' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}} call(foo1); call(foo2); @@ -187,7 +196,7 @@ void noescape_lambda() { otherObj->method(); }, [&](RefCountable& obj) { otherObj->method(); - // expected-warning@-1{{Implicitly captured raw-pointer 'otherObj' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}} + // expected-warning@-1{{Implicitly captured raw-pointer 'otherObj' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}} }); ([&] { someObj->method(); @@ -217,7 +226,7 @@ struct RefCountableWithLambdaCapturingThis { void method_captures_this_unsafe() { auto lambda = [&]() { nonTrivial(); - // expected-warning@-1{{Implicitly captured raw-pointer 'this' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}} + // expected-warning@-1{{Implicitly captured raw-pointer 'this' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}} }; call(lambda); } @@ -225,7 +234,7 @@ struct RefCountableWithLambdaCapturingThis { void method_captures_this_unsafe_capture_local_var_explicitly() { RefCountable* x = make_obj(); call([this, protectedThis = RefPtr { this }, x]() { - // expected-warning@-1{{Captured raw-pointer 'x' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}} + // expected-warning@-1{{Captured raw-pointer 'x' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}} nonTrivial(); x->method(); }); @@ -234,7 +243,7 @@ struct RefCountableWithLambdaCapturingThis { void method_captures_this_with_other_protected_var() { RefCountable* x = make_obj(); call([this, protectedX = RefPtr { x }]() { - // expected-warning@-1{{Captured raw-pointer 'this' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}} + // expected-warning@-1{{Captured raw-pointer 'this' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}} nonTrivial(); protectedX->method(); }); @@ -243,7 +252,7 @@ struct RefCountableWithLambdaCapturingThis { void method_captures_this_unsafe_capture_local_var_explicitly_with_deref() { RefCountable* x = make_obj(); call([this, protectedThis = Ref { *this }, x]() { - // expected-warning@-1{{Captured raw-pointer 'x' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}} + // expected-warning@-1{{Captured raw-pointer 'x' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}} nonTrivial(); x->method(); }); @@ -252,7 +261,7 @@ struct RefCountableWithLambdaCapturingThis { void method_captures_this_unsafe_local_var_via_vardecl() { RefCountable* x = make_obj(); auto lambda = [this, protectedThis = Ref { *this }, x]() { - // expected-warning@-1{{Captured raw-pointer 'x' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}} + // expected-warning@-1{{Captured raw-pointer 'x' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}} nonTrivial(); x->method(); }; @@ -321,7 +330,7 @@ struct RefCountableWithLambdaCapturingThis { void method_nested_lambda2() { callAsync([this, protectedThis = RefPtr { this }] { - callAsync([this, protectedThis = static_cast&&>(*protectedThis)] { + callAsync([this, protectedThis = std::move(*protectedThis)] { nonTrivial(); }); }); @@ -330,7 +339,7 @@ struct RefCountableWithLambdaCapturingThis { void method_nested_lambda3() { callAsync([this, protectedThis = RefPtr { this }] { callAsync([this] { - // expected-warning@-1{{Captured raw-pointer 'this' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}} + // expected-warning@-1{{Captured raw-pointer 'this' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}} nonTrivial(); }); }); @@ -363,6 +372,17 @@ void trivial_lambda() { trivial_lambda(); } +bool call_lambda_var_decl() { + RefCountable* ref_countable = make_obj(); + auto lambda1 = [&]() -> bool { + return ref_countable->next(); + }; + auto lambda2 = [=]() -> bool { + return ref_countable->next(); + }; + return lambda1() && lambda2(); +} + void lambda_with_args(RefCountable* obj) { auto trivial_lambda = [&](int v) { obj->method(); @@ -380,10 +400,10 @@ void lambda_converted_to_function(RefCountable* obj) { callFunction([&]() { obj->method(); - // expected-warning@-1{{Implicitly captured raw-pointer 'obj' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}} + // expected-warning@-1{{Implicitly captured raw-pointer 'obj' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}} }); callFunctionOpaque([&]() { obj->method(); - // expected-warning@-1{{Implicitly captured raw-pointer 'obj' to ref-counted type or CheckedPtr-capable type is unsafe [webkit.UncountedLambdaCapturesChecker]}} + // expected-warning@-1{{Implicitly captured raw-pointer 'obj' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}} }); } diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp index 52854cd10f68c..07b6de21df80f 100644 --- a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp +++ b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp @@ -464,4 +464,17 @@ namespace local_var_for_singleton { RefCountable* bar = singleton(); RefCountable* baz = otherSingleton(); } +} + +namespace virtual_function { + struct SomeObject { + virtual RefCountable* provide() { return nullptr; } + virtual RefCountable* operator&() { return nullptr; } + }; + void foo(SomeObject* obj) { + auto* bar = obj->provide(); + // expected-warning@-1{{Local variable 'bar' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}} + auto* baz = &*obj; + // expected-warning@-1{{Local variable 'baz' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}} + } } \ No newline at end of file diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-members.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-members.cpp index bca7b3bad3a15..130777a9a5fee 100644 --- a/clang/test/Analysis/Checkers/WebKit/uncounted-members.cpp +++ b/clang/test/Analysis/Checkers/WebKit/uncounted-members.cpp @@ -34,8 +34,7 @@ namespace members { private: RefCountable* a = nullptr; }; -} - +} // members namespace ignore_unions { union Foo { @@ -50,7 +49,7 @@ namespace ignore_unions { }; void forceTmplToInstantiate(RefPtr) {} -} +} // ignore_unions namespace ignore_system_header { @@ -60,3 +59,21 @@ void foo(RefCountable* t) { } } // ignore_system_header + +namespace ignore_non_ref_countable { + struct Foo { + }; + + struct Bar { + Foo* foo; + }; +} // ignore_non_ref_countable + +namespace checked_ptr_ref_ptr_capable { + + RefCountableAndCheckable* provide(); + void foo() { + CheckedPtr foo = provide(); + } + +} // checked_ptr_ref_ptr_capable diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp index fe7ce158eb8ba..69842264af56b 100644 --- a/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp +++ b/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp @@ -196,6 +196,10 @@ class ComplexNumber { ComplexNumber& operator+(); const Number& real() const { return realPart; } + const Number& complex() const; + + void ref() const; + void deref() const; private: Number realPart; @@ -240,6 +244,11 @@ class SomeType : public BaseType { using BaseType::BaseType; }; +struct OtherObj { + unsigned v { 0 }; + OtherObj* children[4] { nullptr }; +}; + void __libcpp_verbose_abort(const char *__format, ...); class RefCounted { @@ -375,7 +384,7 @@ class RefCounted { double y; }; void trivial68() { point pt = { 1.0 }; } - unsigned trivial69() { return offsetof(RefCounted, children); } + unsigned trivial69() { return offsetof(OtherObj, children); } DerivedNumber* trivial70() { [[clang::suppress]] return static_cast(number); } static RefCounted& singleton() { @@ -467,6 +476,8 @@ class RefCounted { unsigned nonTrivial22() { return ComplexNumber(123, "456").real().value(); } unsigned nonTrivial23() { return DerivedNumber("123").value(); } SomeType nonTrivial24() { return SomeType("123"); } + virtual void nonTrivial25() { } + virtual ComplexNumber* operator->() { return nullptr; } static unsigned s_v; unsigned v { 0 }; @@ -642,6 +653,10 @@ class UnrelatedClass { // expected-warning@-1{{Call argument for 'this' parameter is uncounted and unsafe}} getFieldTrivial().nonTrivial24(); // expected-warning@-1{{Call argument for 'this' parameter is uncounted and unsafe}} + getFieldTrivial().nonTrivial25(); + // expected-warning@-1{{Call argument for 'this' parameter is uncounted and unsafe}} + getFieldTrivial()->complex(); + // expected-warning@-1{{Call argument for 'this' parameter is uncounted and unsafe}} } void setField(RefCounted*); @@ -680,9 +695,13 @@ RefPtr object(); void someFunction(const RefCounted&); void test2() { - someFunction(*object()); + someFunction(*object()); } void system_header() { callMethod(object); } + +void log(RefCountable* obj) { + os_log_msg(os_log_create("WebKit", "DOM"), OS_LOG_TYPE_INFO, "obj: %p next: %p", obj, obj->next()); +} \ No newline at end of file diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.mm b/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.mm index 08319016023e3..b78a67610df3c 100644 --- a/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.mm +++ b/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.mm @@ -50,3 +50,7 @@ @interface WrapperObj : NSObject static void foo(WrapperObj *configuration) { configuration._protectedWebExtensionControllerConfiguration->copy(); } + +void log(RefCountable* obj) { + os_log_msg(os_log_create("WebKit", "DOM"), OS_LOG_TYPE_INFO, "obj: %p next: %p", obj, obj->next()); +} diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-call-args-arc.mm b/clang/test/Analysis/Checkers/WebKit/unretained-call-args-arc.mm new file mode 100644 index 0000000000000..fa866258a2f6d --- /dev/null +++ b/clang/test/Analysis/Checkers/WebKit/unretained-call-args-arc.mm @@ -0,0 +1,68 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UnretainedCallArgsChecker -fobjc-arc -verify %s + +#import "objc-mock-types.h" + +SomeObj *provide(); +CFMutableArrayRef provide_cf(); +void someFunction(); +CGImageRef provideImage(); +NSString *stringForImage(CGImageRef); + +namespace raw_ptr { + +void foo() { + [provide() doWork]; + CFArrayAppendValue(provide_cf(), nullptr); + // expected-warning@-1{{Call argument for parameter 'theArray' is unretained and unsafe [alpha.webkit.UnretainedCallArgsChecker]}} +} + +} // namespace raw_ptr + +namespace const_global { + +extern NSString * const SomeConstant; +extern CFDictionaryRef const SomeDictionary; +void doWork(NSString *str, CFDictionaryRef dict); +void use_const_global() { + doWork(SomeConstant, SomeDictionary); +} + +NSString *provide_str(); +CFDictionaryRef provide_dict(); +void use_const_local() { + doWork(provide_str(), provide_dict()); + // expected-warning@-1{{Call argument for parameter 'dict' is unretained and unsafe}} +} + +} // namespace const_global + +@interface AnotherObj : NSObject +- (void)foo:(SomeObj *)obj; +- (SomeObj *)getSomeObj; +@end + +@implementation AnotherObj +- (void)foo:(SomeObj*)obj { + [obj doWork]; + [provide() doWork]; + CFArrayAppendValue(provide_cf(), nullptr); + // expected-warning@-1{{Call argument for parameter 'theArray' is unretained and unsafe [alpha.webkit.UnretainedCallArgsChecker]}} +} + +- (SomeObj *)getSomeObj { + return provide(); +} + +- (void)doWorkOnSomeObj { + [[self getSomeObj] doWork]; +} + +- (CGImageRef)createImage { + return provideImage(); +} + +- (NSString *)convertImage { + RetainPtr image = [self createImage]; + return stringForImage(image.get()); +} +@end diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm b/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm new file mode 100644 index 0000000000000..ed4783f70f678 --- /dev/null +++ b/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm @@ -0,0 +1,472 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UnretainedCallArgsChecker -verify %s + +#include "objc-mock-types.h" + +SomeObj *provide(); +void consume_obj(SomeObj*); + +CFMutableArrayRef provide_cf(); +void consume_cf(CFMutableArrayRef); + +CGImageRef provideImage(); +NSString *stringForImage(CGImageRef); + +void some_function(); + +namespace simple { + void foo() { + consume_obj(provide()); + // expected-warning@-1{{Call argument is unretained and unsafe}} + consume_cf(provide_cf()); + // expected-warning@-1{{Call argument is unretained and unsafe}} + } + + // Test that the checker works with [[clang::suppress]]. + void foo_suppressed() { + [[clang::suppress]] consume_obj(provide()); // no-warning + [[clang::suppress]] consume_cf(provide_cf()); // no-warning + } + +} + +namespace multi_arg { + void consume_retainable(int, SomeObj* foo, CFMutableArrayRef bar, bool); + void foo() { + consume_retainable(42, provide(), provide_cf(), true); + // expected-warning@-1{{Call argument for parameter 'foo' is unretained and unsafe}} + // expected-warning@-2{{Call argument for parameter 'bar' is unretained and unsafe}} + } + + void consume_retainable(SomeObj* foo, ...); + void bar() { + consume_retainable(provide(), 1, provide_cf(), RetainPtr { provide_cf() }.get()); + // expected-warning@-1{{Call argument for parameter 'foo' is unretained and unsafe}} + // expected-warning@-2{{Call argument is unretained and unsafe}} + consume_retainable(RetainPtr { provide() }.get(), 1, RetainPtr { provide_cf() }.get()); + } +} + +namespace retained { + RetainPtr provide_obj() { return RetainPtr{}; } + void consume_obj(RetainPtr) {} + + RetainPtr provide_cf() { return CFMutableArrayRef{}; } + void consume_cf(RetainPtr) {} + + void foo() { + consume_obj(provide_obj().get()); // no warning + consume_cf(provide_cf().get()); // no warning + } +} + +namespace methods { + struct Consumer { + void consume_obj(SomeObj* ptr); + void consume_cf(CFMutableArrayRef ref); + }; + + void foo() { + Consumer c; + + c.consume_obj(provide()); + // expected-warning@-1{{Call argument for parameter 'ptr' is unretained and unsafe}} + c.consume_cf(provide_cf()); + // expected-warning@-1{{Call argument for parameter 'ref' is unretained and unsafe}} + } + + void foo2() { + struct Consumer { + void consume(SomeObj*) { some_function(); } + void whatever() { + consume(provide()); + // expected-warning@-1{{Call argument is unretained and unsafe}} + } + + void consume_cf(CFMutableArrayRef) { some_function(); } + void something() { + consume_cf(provide_cf()); + // expected-warning@-1{{Call argument is unretained and unsafe}} + } + }; + } + + void foo3() { + struct Consumer { + void consume(SomeObj*) { some_function(); } + void whatever() { + this->consume(provide()); + // expected-warning@-1{{Call argument is unretained and unsafe}} + } + + void consume_cf(CFMutableArrayRef) { some_function(); } + void something() { + this->consume_cf(provide_cf()); + // expected-warning@-1{{Call argument is unretained and unsafe}} + } + }; + } + +} + +namespace casts { + void foo() { + consume_obj(provide()); + // expected-warning@-1{{Call argument is unretained and unsafe}} + + consume_obj(static_cast(provide())); + // expected-warning@-1{{Call argument is unretained and unsafe}} + + consume_obj(reinterpret_cast(provide())); + // expected-warning@-1{{Call argument is unretained and unsafe}} + + consume_obj(downcast(provide())); + // expected-warning@-1{{Call argument is unretained and unsafe}} + } +} + +namespace null_ptr { + void foo_ref() { + consume_obj(nullptr); + consume_obj(0); + consume_cf(nullptr); + consume_cf(0); + } +} + +namespace retain_ptr_lookalike { + struct Decoy { + SomeObj* get(); + }; + + void foo() { + Decoy D; + + consume_obj(D.get()); + // expected-warning@-1{{Call argument is unretained and unsafe}} + } + + struct Decoy2 { + CFMutableArrayRef get(); + }; + + void bar() { + Decoy2 D; + + consume_cf(D.get()); + // expected-warning@-1{{Call argument is unretained and unsafe}} + } +} + +namespace param_formarding_function { + void consume_more_obj(OtherObj*); + void consume_more_cf(CFMutableArrayRef); + + namespace objc { + void foo(SomeObj* param) { + consume_more_obj(downcast(param)); + } + } + + namespace cf { + void foo(CFMutableArrayRef param) { + consume_more_cf(param); + } + } +} + +namespace param_formarding_lambda { + auto consume_more_obj = [](OtherObj*) { some_function(); }; + auto consume_more_cf = [](CFMutableArrayRef) { some_function(); }; + + namespace objc { + void foo(SomeObj* param) { + consume_more_obj(downcast(param)); + } + } + + namespace cf { + void foo(CFMutableArrayRef param) { + consume_more_cf(param); + } + } +} + +namespace param_forwarding_method { + struct Consumer { + void consume_obj(SomeObj*); + static void consume_obj_s(SomeObj*); + void consume_cf(CFMutableArrayRef); + static void consume_cf_s(CFMutableArrayRef); + }; + + void bar(Consumer* consumer, SomeObj* param) { + consumer->consume_obj(param); + } + + void foo(SomeObj* param) { + Consumer::consume_obj_s(param); + } + + void baz(Consumer* consumer, CFMutableArrayRef param) { + consumer->consume_cf(param); + Consumer::consume_cf_s(param); + } +} + + +namespace default_arg { + SomeObj* global; + CFMutableArrayRef global_cf; + + void function_with_default_arg1(SomeObj* param = global); + // expected-warning@-1{{Call argument for parameter 'param' is unretained and unsafe}} + + void function_with_default_arg2(CFMutableArrayRef param = global_cf); + // expected-warning@-1{{Call argument for parameter 'param' is unretained and unsafe}} + + void foo() { + function_with_default_arg1(); + function_with_default_arg2(); + } +} + +namespace cxx_member_func { + RetainPtr protectedProvide(); + RetainPtr protectedProvideCF(); + + void foo() { + [provide() doWork]; + // expected-warning@-1{{Reciever is unretained and unsafe}} + [protectedProvide().get() doWork]; + + CFArrayAppendValue(provide_cf(), nullptr); + // expected-warning@-1{{Call argument for parameter 'theArray' is unretained and unsafe}} + CFArrayAppendValue(protectedProvideCF(), nullptr); + }; + + void bar() { + [downcast(protectedProvide().get()) doMoreWork:downcast(provide())]; + // expected-warning@-1{{Call argument for parameter 'other' is unretained and unsafe}} + [protectedProvide().get() doWork]; + }; + +} + +namespace cxx_member_operator_call { + // The hidden this-pointer argument without a corresponding parameter caused couple bugs in parameter <-> argument attribution. + struct Foo { + Foo& operator+(SomeObj* bad); + friend Foo& operator-(Foo& lhs, SomeObj* bad); + void operator()(SomeObj* bad); + }; + + SomeObj* global; + + void foo() { + Foo f; + f + global; + // expected-warning@-1{{Call argument for parameter 'bad' is unretained and unsafe}} + f - global; + // expected-warning@-1{{Call argument for parameter 'bad' is unretained and unsafe}} + f(global); + // expected-warning@-1{{Call argument for parameter 'bad' is unretained and unsafe}} + } +} + +namespace cxx_assignment_op { + + SomeObj* provide(); + void foo() { + RetainPtr ptr; + ptr = provide(); + } + +} + +namespace call_with_ptr_on_ref { + RetainPtr provideProtected(); + RetainPtr provideProtectedCF(); + void bar(SomeObj* bad); + void bar_cf(CFMutableArrayRef bad); + bool baz(); + void foo(bool v) { + bar(v ? nullptr : provideProtected().get()); + bar(baz() ? provideProtected().get() : nullptr); + bar(v ? provide() : provideProtected().get()); + // expected-warning@-1{{Call argument for parameter 'bad' is unretained and unsafe}} + bar(v ? provideProtected().get() : provide()); + // expected-warning@-1{{Call argument for parameter 'bad' is unretained and unsafe}} + + bar_cf(v ? nullptr : provideProtectedCF().get()); + bar_cf(baz() ? provideProtectedCF().get() : nullptr); + bar_cf(v ? provide_cf() : provideProtectedCF().get()); + // expected-warning@-1{{Call argument for parameter 'bad' is unretained and unsafe}} + bar_cf(v ? provideProtectedCF().get() : provide_cf()); + // expected-warning@-1{{Call argument for parameter 'bad' is unretained and unsafe}} + } +} + +namespace call_with_explicit_temporary_obj { + void foo() { + [RetainPtr(provide()).get() doWork]; + CFArrayAppendValue(RetainPtr { provide_cf() }.get(), nullptr); + } + template + void bar() { + [RetainPtr(provide()).get() doWork]; + CFArrayAppendValue(RetainPtr { provide_cf() }.get(), nullptr); + } + void baz() { + bar(); + } +} + +namespace call_with_adopt_ref { + void foo() { + [adoptNS(provide()).get() doWork]; + CFArrayAppendValue(adoptCF(provide_cf()).get(), nullptr); + } +} + +namespace call_with_cf_constant { + void bar(const NSArray *); + void baz(const NSDictionary *); + void foo() { + CFArrayCreateMutable(kCFAllocatorDefault, 10); + bar(@[@"hello"]); + baz(@{@"hello": @3}); + } +} + +namespace call_with_cf_string { + void bar(CFStringRef); + void foo() { + bar(CFSTR("hello")); + } +} + +namespace call_with_ns_string { + void bar(NSString *); + void foo() { + bar(@"world"); + } +} + +namespace bridge_cast_arg { + void bar(NSString *); + void baz(NSString *); + extern const CFStringRef kCFBundleNameKey; + + NSObject *foo(CFStringRef arg) { + bar((NSString *)bridge_cast((CFTypeRef)arg)); + auto dict = @{ + @"hello": @1, + }; + return dict[(__bridge NSString *)kCFBundleNameKey]; + } +} + +namespace alloc_init_pair { + void foo() { + auto obj = adoptNS([[SomeObj alloc] init]); + [obj doWork]; + } +} + +namespace alloc_class { + bool foo(NSObject *obj) { + return [obj isKindOfClass:SomeObj.class] && [obj isKindOfClass:NSClassFromString(@"SomeObj")]; + } + + bool bar(NSObject *obj) { + return [obj isKindOfClass:[SomeObj class]]; + } + + bool baz(NSObject *obj) { + return [obj isKindOfClass:[SomeObj superclass]]; + } +} + +namespace ptr_conversion { + +SomeObj *provide_obj(); + +void dobjc(SomeObj* obj) { + [dynamic_objc_cast(obj) doMoreWork:nil]; +} + +void cobjc(SomeObj* obj) { + [checked_objc_cast(obj) doMoreWork:nil]; +} + +unsigned dcf(CFTypeRef obj) { + return CFArrayGetCount(dynamic_cf_cast(obj)); +} + +unsigned ccf(CFTypeRef obj) { + return CFArrayGetCount(checked_cf_cast(obj)); +} + +void some_function(id); +void idcf(CFTypeRef obj) { + some_function(bridge_id_cast(obj)); +} + +} // ptr_conversion + +namespace const_global { + +extern NSString * const SomeConstant; +extern CFDictionaryRef const SomeDictionary; +void doWork(NSString *str, CFDictionaryRef dict); +void use_const_global() { + doWork(SomeConstant, SomeDictionary); +} + +NSString *provide_str(); +CFDictionaryRef provide_dict(); +void use_const_local() { + doWork(provide_str(), provide_dict()); + // expected-warning@-1{{Call argument for parameter 'str' is unretained and unsafe}} + // expected-warning@-2{{Call argument for parameter 'dict' is unretained and unsafe}} +} + +} // namespace const_global + +@interface TestObject : NSObject +- (void)doWork:(NSString *)msg, ...; +- (void)doWorkOnSelf; +- (SomeObj *)getSomeObj; +@end + +@implementation TestObject + +- (void)doWork:(NSString *)msg, ... { + some_function(); +} + +- (void)doWorkOnSelf { + [self doWork:nil]; + [self doWork:@"hello", provide(), provide_cf()]; + // expected-warning@-1{{Call argument is unretained and unsafe}} + // expected-warning@-2{{Call argument is unretained and unsafe}} + [self doWork:@"hello", RetainPtr { provide() }.get(), RetainPtr { provide_cf() }.get()]; +} + +- (SomeObj *)getSomeObj { + return RetainPtr(provide()).autorelease(); +} + +- (void)doWorkOnSomeObj { + [[self getSomeObj] doWork]; +} + +- (CGImageRef)createImage { + return provideImage(); +} + +- (NSString *)convertImage { + RetainPtr image = [self createImage]; + return stringForImage(image.get()); +} +@end diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-lambda-captures-arc.mm b/clang/test/Analysis/Checkers/WebKit/unretained-lambda-captures-arc.mm new file mode 100644 index 0000000000000..63526a5047157 --- /dev/null +++ b/clang/test/Analysis/Checkers/WebKit/unretained-lambda-captures-arc.mm @@ -0,0 +1,293 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UnretainedLambdaCapturesChecker -fobjc-arc -verify %s + +#include "objc-mock-types.h" + +namespace std { + +template +class unique_ptr { +private: + T *t; + +public: + unique_ptr() : t(nullptr) { } + unique_ptr(T *t) : t(t) { } + ~unique_ptr() { + if (t) + delete t; + } + template unique_ptr(unique_ptr&& u) + : t(u.t) + { + u.t = nullptr; + } + T *get() const { return t; } + T *operator->() const { return t; } + T &operator*() const { return *t; } + unique_ptr &operator=(T *) { return *this; } + explicit operator bool() const { return !!t; } +}; + +}; + +namespace WTF { + +namespace Detail { + +template +class CallableWrapperBase { +public: + virtual ~CallableWrapperBase() { } + virtual Out call(In...) = 0; +}; + +template class CallableWrapper; + +template +class CallableWrapper : public CallableWrapperBase { +public: + explicit CallableWrapper(CallableType& callable) + : m_callable(callable) { } + Out call(In... in) final { return m_callable(in...); } + +private: + CallableType m_callable; +}; + +} // namespace Detail + +template class Function; + +template Function adopt(Detail::CallableWrapperBase*); + +template +class Function { +public: + using Impl = Detail::CallableWrapperBase; + + Function() = default; + + template + Function(FunctionType f) + : m_callableWrapper(new Detail::CallableWrapper(f)) { } + + Out operator()(In... in) const { return m_callableWrapper->call(in...); } + explicit operator bool() const { return !!m_callableWrapper; } + +private: + enum AdoptTag { Adopt }; + Function(Impl* impl, AdoptTag) + : m_callableWrapper(impl) + { + } + + friend Function adopt(Impl*); + + std::unique_ptr m_callableWrapper; +}; + +template Function adopt(Detail::CallableWrapperBase* impl) +{ + return Function(impl, Function::Adopt); +} + +template +class HashMap { +public: + HashMap(); + HashMap([[clang::noescape]] const Function&); + void ensure(const KeyType&, [[clang::noescape]] const Function&); + bool operator+([[clang::noescape]] const Function&) const; + static void ifAny(HashMap, [[clang::noescape]] const Function&); + +private: + ValueType* m_table { nullptr }; +}; + +} // namespace WTF + +struct A { + static void b(); +}; + +SomeObj* make_obj(); +CFMutableArrayRef make_cf(); + +void someFunction(); +template void call(Callback callback) { + someFunction(); + callback(); +} +void callAsync(const WTF::Function&); + +void raw_ptr() { + SomeObj* obj = make_obj(); + auto foo1 = [obj](){ + [obj doWork]; + }; + call(foo1); + + auto foo2 = [&obj](){ + [obj doWork]; + }; + auto foo3 = [&](){ + [obj doWork]; + obj = nullptr; + }; + auto foo4 = [=](){ + [obj doWork]; + }; + + auto cf = make_cf(); + auto bar1 = [cf](){ + // expected-warning@-1{{Captured reference 'cf' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} + CFArrayAppendValue(cf, nullptr); + }; + auto bar2 = [&cf](){ + // expected-warning@-1{{Captured reference 'cf' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} + CFArrayAppendValue(cf, nullptr); + }; + auto bar3 = [&](){ + CFArrayAppendValue(cf, nullptr); + // expected-warning@-1{{Implicitly captured reference 'cf' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} + cf = nullptr; + }; + auto bar4 = [=](){ + CFArrayAppendValue(cf, nullptr); + // expected-warning@-1{{Implicitly captured reference 'cf' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} + }; + + call(foo1); + call(foo2); + call(foo3); + call(foo4); + + call(bar1); + call(bar2); + call(bar3); + call(bar4); +} + +void quiet() { +// This code is not expected to trigger any warnings. + SomeObj *obj; + + auto foo3 = [&]() {}; + auto foo4 = [=]() {}; + + call(foo3); + call(foo4); + + obj = nullptr; +} + +template +void map(SomeObj* start, [[clang::noescape]] Callback&& callback) +{ + while (start) { + callback(start); + start = [start next]; + } +} + +template +void doubleMap(SomeObj* start, [[clang::noescape]] Callback1&& callback1, Callback2&& callback2) +{ + while (start) { + callback1(start); + callback2(start); + start = [start next]; + } +} + +template +void get_count_cf(CFArrayRef array, [[clang::noescape]] Callback1&& callback1, Callback2&& callback2) +{ + auto count = CFArrayGetCount(array); + callback1(count); + callback2(count); +} + +void noescape_lambda() { + SomeObj* someObj = make_obj(); + SomeObj* otherObj = make_obj(); + map(make_obj(), [&](SomeObj *obj) { + [otherObj doWork]; + }); + doubleMap(make_obj(), [&](SomeObj *obj) { + [otherObj doWork]; + }, [&](SomeObj *obj) { + [otherObj doWork]; + }); + ([&] { + [someObj doWork]; + })(); + + CFMutableArrayRef someCF = make_cf(); + get_count_cf(make_cf(), [&](CFIndex count) { + CFArrayAppendValue(someCF, nullptr); + }, [&](CFIndex count) { + CFArrayAppendValue(someCF, nullptr); + // expected-warning@-1{{Implicitly captured reference 'someCF' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} + }); +} + +void callFunctionOpaque(WTF::Function&&); +void callFunction(WTF::Function&& function) { + someFunction(); + function(); +} + +void lambda_converted_to_function(SomeObj* obj, CFMutableArrayRef cf) +{ + callFunction([&]() { + [obj doWork]; + CFArrayAppendValue(cf, nullptr); + // expected-warning@-1{{Implicitly captured reference 'cf' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} + }); + callFunctionOpaque([&]() { + [obj doWork]; + CFArrayAppendValue(cf, nullptr); + // expected-warning@-1{{Implicitly captured reference 'cf' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} + }); +} + +@interface ObjWithSelf : NSObject { + RetainPtr delegate; +} +-(void)doWork; +-(void)doMoreWork; +-(void)run; +@end + +@implementation ObjWithSelf +-(void)doWork { + auto doWork = [&] { + someFunction(); + [delegate doWork]; + }; + auto doMoreWork = [=] { + someFunction(); + [delegate doWork]; + }; + auto doExtraWork = [&, protectedSelf = retainPtr(self)] { + someFunction(); + [delegate doWork]; + }; + callFunctionOpaque(doWork); + callFunctionOpaque(doMoreWork); + callFunctionOpaque(doExtraWork); +} + +-(void)doMoreWork { + auto doWork = [self, protectedSelf = retainPtr(self)] { + someFunction(); + [self doWork]; + }; + callFunctionOpaque(doWork); +} + +-(void)run { + someFunction(); +} +@end diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-lambda-captures.mm b/clang/test/Analysis/Checkers/WebKit/unretained-lambda-captures.mm new file mode 100644 index 0000000000000..dcc6672261d8c --- /dev/null +++ b/clang/test/Analysis/Checkers/WebKit/unretained-lambda-captures.mm @@ -0,0 +1,318 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UnretainedLambdaCapturesChecker -verify %s + +#include "objc-mock-types.h" + +namespace std { + +template +class unique_ptr { +private: + T *t; + +public: + unique_ptr() : t(nullptr) { } + unique_ptr(T *t) : t(t) { } + ~unique_ptr() { + if (t) + delete t; + } + template unique_ptr(unique_ptr&& u) + : t(u.t) + { + u.t = nullptr; + } + T *get() const { return t; } + T *operator->() const { return t; } + T &operator*() const { return *t; } + unique_ptr &operator=(T *) { return *this; } + explicit operator bool() const { return !!t; } +}; + +}; + +namespace WTF { + +namespace Detail { + +template +class CallableWrapperBase { +public: + virtual ~CallableWrapperBase() { } + virtual Out call(In...) = 0; +}; + +template class CallableWrapper; + +template +class CallableWrapper : public CallableWrapperBase { +public: + explicit CallableWrapper(CallableType& callable) + : m_callable(callable) { } + Out call(In... in) final { return m_callable(in...); } + +private: + CallableType m_callable; +}; + +} // namespace Detail + +template class Function; + +template Function adopt(Detail::CallableWrapperBase*); + +template +class Function { +public: + using Impl = Detail::CallableWrapperBase; + + Function() = default; + + template + Function(FunctionType f) + : m_callableWrapper(new Detail::CallableWrapper(f)) { } + + Out operator()(In... in) const { return m_callableWrapper->call(in...); } + explicit operator bool() const { return !!m_callableWrapper; } + +private: + enum AdoptTag { Adopt }; + Function(Impl* impl, AdoptTag) + : m_callableWrapper(impl) + { + } + + friend Function adopt(Impl*); + + std::unique_ptr m_callableWrapper; +}; + +template Function adopt(Detail::CallableWrapperBase* impl) +{ + return Function(impl, Function::Adopt); +} + +template +class HashMap { +public: + HashMap(); + HashMap([[clang::noescape]] const Function&); + void ensure(const KeyType&, [[clang::noescape]] const Function&); + bool operator+([[clang::noescape]] const Function&) const; + static void ifAny(HashMap, [[clang::noescape]] const Function&); + +private: + ValueType* m_table { nullptr }; +}; + +} // namespace WTF + +struct A { + static void b(); +}; + +SomeObj* make_obj(); +CFMutableArrayRef make_cf(); + +void someFunction(); +template void call(Callback callback) { + someFunction(); + callback(); +} +void callAsync(const WTF::Function&); + +void raw_ptr() { + SomeObj* obj = make_obj(); + auto foo1 = [obj](){ + // expected-warning@-1{{Captured raw-pointer 'obj' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} + [obj doWork]; + }; + call(foo1); + + auto foo2 = [&obj](){ + // expected-warning@-1{{Captured raw-pointer 'obj' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} + [obj doWork]; + }; + auto foo3 = [&](){ + [obj doWork]; + // expected-warning@-1{{Implicitly captured raw-pointer 'obj' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} + obj = nullptr; + }; + auto foo4 = [=](){ + [obj doWork]; + // expected-warning@-1{{Implicitly captured raw-pointer 'obj' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} + }; + + auto cf = make_cf(); + auto bar1 = [cf](){ + // expected-warning@-1{{Captured reference 'cf' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} + CFArrayAppendValue(cf, nullptr); + }; + auto bar2 = [&cf](){ + // expected-warning@-1{{Captured reference 'cf' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} + CFArrayAppendValue(cf, nullptr); + }; + auto bar3 = [&](){ + CFArrayAppendValue(cf, nullptr); + // expected-warning@-1{{Implicitly captured reference 'cf' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} + cf = nullptr; + }; + auto bar4 = [=](){ + CFArrayAppendValue(cf, nullptr); + // expected-warning@-1{{Implicitly captured reference 'cf' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} + }; + + call(foo1); + call(foo2); + call(foo3); + call(foo4); + + call(bar1); + call(bar2); + call(bar3); + call(bar4); + + // Confirm that the checker respects [[clang::suppress]]. + SomeObj* suppressed_obj = nullptr; + [[clang::suppress]] auto foo5 = [suppressed_obj](){ + [suppressed_obj doWork]; + }; + // no warning. + call(foo5); + + // Confirm that the checker respects [[clang::suppress]]. + CFMutableArrayRef suppressed_cf = nullptr; + [[clang::suppress]] auto bar5 = [suppressed_cf](){ + CFArrayAppendValue(suppressed_cf, nullptr); + }; + // no warning. + call(bar5); +} + +void quiet() { +// This code is not expected to trigger any warnings. + SomeObj *obj; + + auto foo3 = [&]() {}; + auto foo4 = [=]() {}; + + call(foo3); + call(foo4); + + obj = nullptr; +} + +template +void map(SomeObj* start, [[clang::noescape]] Callback&& callback) +{ + while (start) { + callback(start); + start = [start next]; + } +} + +template +void doubleMap(SomeObj* start, [[clang::noescape]] Callback1&& callback1, Callback2&& callback2) +{ + while (start) { + callback1(start); + callback2(start); + start = [start next]; + } +} + +template +void get_count_cf(CFArrayRef array, [[clang::noescape]] Callback1&& callback1, Callback2&& callback2) +{ + auto count = CFArrayGetCount(array); + callback1(count); + callback2(count); +} + +void noescape_lambda() { + SomeObj* someObj = make_obj(); + SomeObj* otherObj = make_obj(); + map(make_obj(), [&](SomeObj *obj) { + [otherObj doWork]; + }); + doubleMap(make_obj(), [&](SomeObj *obj) { + [otherObj doWork]; + }, [&](SomeObj *obj) { + [otherObj doWork]; + // expected-warning@-1{{Implicitly captured raw-pointer 'otherObj' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} + }); + ([&] { + [someObj doWork]; + })(); + + CFMutableArrayRef someCF = make_cf(); + get_count_cf(make_cf(), [&](CFIndex count) { + CFArrayAppendValue(someCF, nullptr); + }, [&](CFIndex count) { + CFArrayAppendValue(someCF, nullptr); + // expected-warning@-1{{Implicitly captured reference 'someCF' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} + }); +} + +void callFunctionOpaque(WTF::Function&&); +void callFunction(WTF::Function&& function) { + someFunction(); + function(); +} + +void lambda_converted_to_function(SomeObj* obj, CFMutableArrayRef cf) +{ + callFunction([&]() { + [obj doWork]; + // expected-warning@-1{{Implicitly captured raw-pointer 'obj' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} + CFArrayAppendValue(cf, nullptr); + // expected-warning@-1{{Implicitly captured reference 'cf' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} + }); + callFunctionOpaque([&]() { + [obj doWork]; + // expected-warning@-1{{Implicitly captured raw-pointer 'obj' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} + CFArrayAppendValue(cf, nullptr); + // expected-warning@-1{{Implicitly captured reference 'cf' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} + }); +} + +@interface ObjWithSelf : NSObject { + RetainPtr delegate; +} +-(void)doWork; +-(void)doMoreWork; +-(void)run; +@end + +@implementation ObjWithSelf +-(void)doWork { + auto doWork = [&] { + // expected-warning@-1{{Implicitly captured raw-pointer 'self' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} + someFunction(); + [delegate doWork]; + }; + auto doMoreWork = [=] { + // expected-warning@-1{{Implicitly captured raw-pointer 'self' to unretained type is unsafe [alpha.webkit.UnretainedLambdaCapturesChecker]}} + someFunction(); + [delegate doWork]; + }; + auto doExtraWork = [&, protectedSelf = retainPtr(self)] { + someFunction(); + [delegate doWork]; + }; + callFunctionOpaque(doWork); + callFunctionOpaque(doMoreWork); + callFunctionOpaque(doExtraWork); +} + +-(void)doMoreWork { + auto doWork = [self, protectedSelf = retainPtr(self)] { + someFunction(); + [self doWork]; + }; + callFunctionOpaque(doWork); +} + +-(void)run { + someFunction(); +} +@end diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-local-vars-arc.mm b/clang/test/Analysis/Checkers/WebKit/unretained-local-vars-arc.mm new file mode 100644 index 0000000000000..a84bee8529645 --- /dev/null +++ b/clang/test/Analysis/Checkers/WebKit/unretained-local-vars-arc.mm @@ -0,0 +1,66 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UnretainedLocalVarsChecker -fobjc-arc -verify %s + +#import "objc-mock-types.h" + +SomeObj *provide(); +void someFunction(); + +namespace raw_ptr { + +void foo() { + SomeObj *bar = [[SomeObj alloc] init]; + [bar doWork]; +} + +void foo2() { + SomeObj *bar = provide(); + [bar doWork]; +} + +void bar() { + CFMutableArrayRef array = CFArrayCreateMutable(kCFAllocatorDefault, 10); + // expected-warning@-1{{Local variable 'array' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}} + CFArrayAppendValue(array, nullptr); +} + +} // namespace raw_ptr + +namespace const_global { + +extern NSString * const SomeConstant; +extern CFDictionaryRef const SomeDictionary; +void doWork(NSString *, CFDictionaryRef); +void use_const_global() { + doWork(SomeConstant, SomeDictionary); +} + +NSString *provide_str(); +CFDictionaryRef provide_dict(); +void use_const_local() { + NSString * const str = provide_str(); + CFDictionaryRef dict = provide_dict(); + // expected-warning@-1{{Local variable 'dict' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}} + doWork(str, dict); +} + +} // namespace const_global + +@interface AnotherObj : NSObject +- (void)foo:(SomeObj *)obj; +@end + +@implementation AnotherObj +- (void)foo:(SomeObj*)obj { + SomeObj* obj2 = obj; + SomeObj* obj3 = provide(); + obj = nullptr; + [obj2 doWork]; + [obj3 doWork]; +} + +- (void)bar { + CFMutableArrayRef array = CFArrayCreateMutable(kCFAllocatorDefault, 10); + // expected-warning@-1{{Local variable 'array' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}} + CFArrayAppendValue(array, nullptr); +} +@end diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-local-vars.mm b/clang/test/Analysis/Checkers/WebKit/unretained-local-vars.mm new file mode 100644 index 0000000000000..10f7c9acb7a3c --- /dev/null +++ b/clang/test/Analysis/Checkers/WebKit/unretained-local-vars.mm @@ -0,0 +1,432 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UnretainedLocalVarsChecker -verify %s + +#import "objc-mock-types.h" + +void someFunction(); + +namespace raw_ptr { +void foo() { + SomeObj *bar; + // FIXME: later on we might warn on uninitialized vars too +} + +void bar(SomeObj *) {} +} // namespace raw_ptr + +namespace pointer { +SomeObj *provide(); +void foo_ref() { + SomeObj *bar = provide(); + // expected-warning@-1{{Local variable 'bar' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}} + [bar doWork]; +} + +bool bar_ref(SomeObj *obj) { + return !!obj; +} + +void cf_ptr() { + CFMutableArrayRef array = CFArrayCreateMutable(kCFAllocatorDefault, 10); + // expected-warning@-1{{Local variable 'array' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}} + CFArrayAppendValue(array, nullptr); +} +} // namespace pointer + +namespace guardian_scopes { +void foo1() { + RetainPtr foo; + { + SomeObj *bar = foo.get(); + } +} + +void foo2() { + RetainPtr foo; + // missing embedded scope here + SomeObj *bar = foo.get(); + // expected-warning@-1{{Local variable 'bar' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}} + [bar doWork]; +} + +void foo3() { + RetainPtr foo; + { + { SomeObj *bar = foo.get(); } + } +} + +void foo4() { + { + RetainPtr foo; + { SomeObj *bar = foo.get(); } + } +} + +struct SelfReferencingStruct { + SelfReferencingStruct* ptr; + SomeObj* obj { nullptr }; +}; + +void foo7(SomeObj* obj) { + SelfReferencingStruct bar = { &bar, obj }; + [bar.obj doWork]; +} + +void foo8(SomeObj* obj) { + RetainPtr foo; + + { + SomeObj *bar = foo.get(); + // expected-warning@-1{{Local variable 'bar' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}} + foo = nullptr; + [bar doWork]; + } + RetainPtr baz; + { + SomeObj *bar = baz.get(); + // expected-warning@-1{{Local variable 'bar' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}} + baz = obj; + [bar doWork]; + } + + foo = nullptr; + { + SomeObj *bar = foo.get(); + // No warning. It's okay to mutate RefPtr in an outer scope. + [bar doWork]; + } + foo = obj; + { + SomeObj *bar = foo.get(); + // expected-warning@-1{{Local variable 'bar' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}} + foo.clear(); + [bar doWork]; + } + { + SomeObj *bar = foo.get(); + // expected-warning@-1{{Local variable 'bar' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}} + foo = obj ? obj : nullptr; + [bar doWork]; + } + { + SomeObj *bar = [foo.get() other] ? foo.get() : nullptr; + // expected-warning@-1{{Local variable 'bar' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}} + foo = nullptr; + [bar doWork]; + } +} + +void foo9(SomeObj* o) { + RetainPtr guardian(o); + { + SomeObj *bar = guardian.get(); + // expected-warning@-1{{Local variable 'bar' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}} + guardian = o; // We don't detect that we're setting it to the same value. + [bar doWork]; + } + { + SomeObj *bar = guardian.get(); + // expected-warning@-1{{Local variable 'bar' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}} + RetainPtr other(bar); // We don't detect other has the same value as guardian. + guardian.swap(other); + [bar doWork]; + } + { + SomeObj *bar = guardian.get(); + // expected-warning@-1{{Local variable 'bar' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}} + RetainPtr other(static_cast&&>(guardian)); + [bar doWork]; + } + { + SomeObj *bar = guardian.get(); + // expected-warning@-1{{Local variable 'bar' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}} + guardian.clear(); + [bar doWork]; + } + { + SomeObj *bar = guardian.get(); + // expected-warning@-1{{Local variable 'bar' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}} + guardian = [o other] ? o : bar; + [bar doWork]; + } +} + +bool trivialFunction(CFMutableArrayRef array) { return !!array; } +void foo10() { + RetainPtr array = adoptCF(CFArrayCreateMutable(kCFAllocatorDefault, 10)); + { + CFMutableArrayRef arrayRef = array.get(); + CFArrayAppendValue(arrayRef, nullptr); + } + { + CFMutableArrayRef arrayRef = array.get(); + // expected-warning@-1{{Local variable 'arrayRef' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}} + array = nullptr; + CFArrayAppendValue(arrayRef, nullptr); + } + { + CFMutableArrayRef arrayRef = array.get(); + if (trivialFunction(arrayRef)) + arrayRef = nullptr; + } +} + +} // namespace guardian_scopes + +namespace auto_keyword { +class Foo { + SomeObj *provide_obj(); + CFMutableArrayRef provide_cf_array(); + void doWork(CFMutableArrayRef); + + void evil_func() { + SomeObj *bar = provide_obj(); + // expected-warning@-1{{Local variable 'bar' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}} + auto *baz = provide_obj(); + // expected-warning@-1{{Local variable 'baz' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}} + auto *baz2 = this->provide_obj(); + // expected-warning@-1{{Local variable 'baz2' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}} + [[clang::suppress]] auto *baz_suppressed = provide_obj(); // no-warning + } + + void func() { + SomeObj *bar = provide_obj(); + // expected-warning@-1{{Local variable 'bar' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}} + if (bar) + [bar doWork]; + } + + void bar() { + auto bar = provide_cf_array(); + // expected-warning@-1{{Local variable 'bar' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}} + doWork(bar); + [[clang::suppress]] auto baz = provide_cf_array(); // no-warning + doWork(baz); + } + +}; +} // namespace auto_keyword + +namespace guardian_casts { +void foo1() { + RetainPtr foo; + { + SomeObj *bar = downcast(foo.get()); + [bar doWork]; + } +} + +void foo2() { + RetainPtr foo; + { + SomeObj *bar = static_cast(downcast(foo.get())); + someFunction(); + } +} +} // namespace guardian_casts + +namespace conditional_op { +SomeObj *provide_obj(); +bool bar(); + +void foo() { + SomeObj *a = bar() ? nullptr : provide_obj(); + // expected-warning@-1{{Local variable 'a' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}} + RetainPtr b = provide_obj(); + { + SomeObj* c = bar() ? nullptr : b.get(); + [c doWork]; + SomeObj* d = bar() ? b.get() : nullptr; + [d doWork]; + } +} + +} // namespace conditional_op + +namespace local_assignment_basic { + +SomeObj *provide_obj(); + +void foo(SomeObj* a) { + SomeObj* b = a; + // expected-warning@-1{{Local variable 'b' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}} + if ([b other]) + b = provide_obj(); +} + +void bar(SomeObj* a) { + SomeObj* b; + // expected-warning@-1{{Local variable 'b' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}} + b = provide_obj(); +} + +void baz() { + RetainPtr a = provide_obj(); + { + SomeObj* b = a.get(); + // expected-warning@-1{{Local variable 'b' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}} + b = provide_obj(); + } +} + +} // namespace local_assignment_basic + +namespace local_assignment_to_parameter { + +SomeObj *provide_obj(); +void someFunction(); + +void foo(SomeObj* a) { + a = provide_obj(); + // expected-warning@-1{{Assignment to an unretained parameter 'a' is unsafe [alpha.webkit.UnretainedLocalVarsChecker]}} + someFunction(); + [a doWork]; +} + +CFMutableArrayRef provide_cf_array(); +void doWork(CFMutableArrayRef); + +void bar(CFMutableArrayRef a) { + a = provide_cf_array(); + // expected-warning@-1{{Assignment to an unretained parameter 'a' is unsafe [alpha.webkit.UnretainedLocalVarsChecker]}} + doWork(a); +} + +} // namespace local_assignment_to_parameter + +namespace local_assignment_to_static_local { + +SomeObj *provide_obj(); +void someFunction(); + +void foo() { + static SomeObj* a = nullptr; + // expected-warning@-1{{Static local variable 'a' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}} + a = provide_obj(); + someFunction(); + [a doWork]; +} + +CFMutableArrayRef provide_cf_array(); +void doWork(CFMutableArrayRef); + +void bar() { + static CFMutableArrayRef a = nullptr; + // expected-warning@-1{{Static local variable 'a' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}} + a = provide_cf_array(); + doWork(a); +} + +} // namespace local_assignment_to_static_local + +namespace local_assignment_to_global { + +SomeObj *provide_obj(); +void someFunction(); + +SomeObj* g_a = nullptr; +// expected-warning@-1{{Global variable 'local_assignment_to_global::g_a' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}} + +void foo() { + g_a = provide_obj(); + someFunction(); + [g_a doWork]; +} + +CFMutableArrayRef provide_cf_array(); +void doWork(CFMutableArrayRef); + +CFMutableArrayRef g_b = nullptr; +// expected-warning@-1{{Global variable 'local_assignment_to_global::g_b' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}} + +void bar() { + g_b = provide_cf_array(); + doWork(g_b); +} + +} // namespace local_assignment_to_global + +namespace local_var_for_singleton { + SomeObj *singleton(); + SomeObj *otherSingleton(); + void foo() { + SomeObj* bar = singleton(); + SomeObj* baz = otherSingleton(); + } + + CFMutableArrayRef cfSingleton(); + void bar() { + CFMutableArrayRef cf = cfSingleton(); + } +} + +namespace ptr_conversion { + +SomeObj *provide_obj(); + +void dobjc(SomeObj* obj) { + if (auto *otherObj = dynamic_objc_cast(obj)) + [otherObj doMoreWork:nil]; +} + +void cobjc(SomeObj* obj) { + auto *otherObj = checked_objc_cast(obj); + [otherObj doMoreWork:nil]; +} + +unsigned dcf(CFTypeRef obj) { + if (CFArrayRef array = dynamic_cf_cast(obj)) + return CFArrayGetCount(array); + return 0; +} + +unsigned ccf(CFTypeRef obj) { + CFArrayRef array = checked_cf_cast(obj); + return CFArrayGetCount(array); +} + +} // ptr_conversion + +namespace const_global { + +extern NSString * const SomeConstant; +extern CFDictionaryRef const SomeDictionary; +void doWork(NSString *, CFDictionaryRef); +void use_const_global() { + doWork(SomeConstant, SomeDictionary); +} + +NSString *provide_str(); +CFDictionaryRef provide_dict(); +void use_const_local() { + NSString * const str = provide_str(); + // expected-warning@-1{{Local variable 'str' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}} + CFDictionaryRef dict = provide_dict(); + // expected-warning@-1{{Local variable 'dict' is unretained and unsafe [alpha.webkit.UnretainedLocalVarsChecker]}} + doWork(str, dict); +} + +} // namespace const_global + +bool doMoreWorkOpaque(OtherObj*); +SomeObj* provide(); + +@implementation OtherObj +- (instancetype)init { + self = [super init]; + return self; +} + +- (void)doMoreWork:(OtherObj *)other { + doMoreWorkOpaque(other); +} + +- (SomeObj*)getSomeObj { + return RetainPtr(provide()).autorelease(); +} + +- (void)storeSomeObj { + auto *obj = [self getSomeObj]; + [obj doWork]; +} +@end diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-members-arc.mm b/clang/test/Analysis/Checkers/WebKit/unretained-members-arc.mm new file mode 100644 index 0000000000000..9820c875b87c0 --- /dev/null +++ b/clang/test/Analysis/Checkers/WebKit/unretained-members-arc.mm @@ -0,0 +1,39 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.NoUnretainedMemberChecker -fobjc-arc -verify %s + +#include "objc-mock-types.h" + +namespace members { + + struct Foo { + private: + SomeObj* a = nullptr; + + [[clang::suppress]] + SomeObj* a_suppressed = nullptr; + + protected: + RetainPtr b; + + public: + SomeObj* c = nullptr; + RetainPtr d; + + CFMutableArrayRef e = nullptr; +// expected-warning@-1{{Member variable 'e' in 'members::Foo' is a retainable type 'CFMutableArrayRef'}} + }; + + template + struct FooTmpl { + T* x; + S y; +// expected-warning@-1{{Member variable 'y' in 'members::FooTmpl' is a raw pointer to retainable type}} + }; + + void forceTmplToInstantiate(FooTmpl) {} + + struct [[clang::suppress]] FooSuppressed { + private: + SomeObj* a = nullptr; + }; + +} diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-members.mm b/clang/test/Analysis/Checkers/WebKit/unretained-members.mm new file mode 100644 index 0000000000000..fff1f8ede091b --- /dev/null +++ b/clang/test/Analysis/Checkers/WebKit/unretained-members.mm @@ -0,0 +1,73 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.NoUnretainedMemberChecker -verify %s + +@class SystemObject; + +#include "objc-mock-types.h" +#include "mock-system-header.h" + +namespace members { + + struct Foo { + private: + SomeObj* a = nullptr; +// expected-warning@-1{{Member variable 'a' in 'members::Foo' is a raw pointer to retainable type}} + + [[clang::suppress]] + SomeObj* a_suppressed = nullptr; +// No warning. + + protected: + RetainPtr b; +// No warning. + + public: + SomeObj* c = nullptr; +// expected-warning@-1{{Member variable 'c' in 'members::Foo' is a raw pointer to retainable type}} + RetainPtr d; + + CFMutableArrayRef e = nullptr; +// expected-warning@-1{{Member variable 'e' in 'members::Foo' is a retainable type 'CFMutableArrayRef'}} + }; + + template + struct FooTmpl { + T* a; +// expected-warning@-1{{Member variable 'a' in 'members::FooTmpl' is a raw pointer to retainable type}} + S b; +// expected-warning@-1{{Member variable 'b' in 'members::FooTmpl' is a raw pointer to retainable type}} + }; + + void forceTmplToInstantiate(FooTmpl) {} + + struct [[clang::suppress]] FooSuppressed { + private: + SomeObj* a = nullptr; +// No warning. + }; + +} + +namespace ignore_unions { + union Foo { + SomeObj* a; + RetainPtr b; + CFMutableArrayRef c; + }; + + template + union RefPtr { + T* a; + }; + + void forceTmplToInstantiate(RefPtr) {} +} + +@interface AnotherObject : NSObject { + NSString *ns_string; + // expected-warning@-1{{Instance variable 'ns_string' in 'AnotherObject' is a raw pointer to retainable type 'NSString'; member variables must be a RetainPtr}} + CFStringRef cf_string; + // expected-warning@-1{{Instance variable 'cf_string' in 'AnotherObject' is a retainable type 'CFStringRef'; member variables must be a RetainPtr}} +} +@property(nonatomic, strong) NSString *prop_string; +// expected-warning@-1{{Property 'prop_string' in 'AnotherObject' is a raw pointer to retainable type 'NSString'; member variables must be a RetainPtr}} +@end diff --git a/llvm/utils/gn/secondary/clang/lib/StaticAnalyzer/Checkers/BUILD.gn b/llvm/utils/gn/secondary/clang/lib/StaticAnalyzer/Checkers/BUILD.gn index 35886bcc7561a..d371fea8382cd 100644 --- a/llvm/utils/gn/secondary/clang/lib/StaticAnalyzer/Checkers/BUILD.gn +++ b/llvm/utils/gn/secondary/clang/lib/StaticAnalyzer/Checkers/BUILD.gn @@ -146,7 +146,7 @@ static_library("Checkers") { "WebKit/PtrTypesSemantics.cpp", "WebKit/RefCntblBaseVirtualDtorChecker.cpp", "WebKit/RawPtrRefCallArgsChecker.cpp", - "WebKit/UncountedLambdaCapturesChecker.cpp", + "WebKit/RawPtrRefLambdaCapturesChecker.cpp", "WebKit/RawPtrRefLocalVarsChecker.cpp", "cert/InvalidPtrChecker.cpp", ]