Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 12 additions & 8 deletions clang/docs/analyzer/checkers.rst
Original file line number Diff line number Diff line change
Expand Up @@ -3633,26 +3633,28 @@ See `WebKit Guidelines for Safer C++ Programming <https://github.com/WebKit/WebK

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.
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 or OSObjectPtr is allowed for NS types when ARC is disabled.

.. code-block:: cpp

struct Foo {
NSObject *ptr; // warn
dispatch_queue_t queue; // warn
// ...
};

See `WebKit Guidelines for Safer C++ Programming <https://github.com/WebKit/WebKit/wiki/Safer-CPP-Guidelines>`_ 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.
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 or OSObjectPtr is allowed for NS types when ARC is disabled.

.. code-block:: cpp

void foo(NSObject *a, NSObject *b) {
void foo(NSObject *a, NSObject *b, dispatch_queue_t c) {
[&, a](){ // warn about 'a'
do_something(b); // warn about 'b'
dispatch_queue_get_specific(c, "some"); // warn about 'c'
};
};

Expand Down Expand Up @@ -3755,7 +3757,7 @@ 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.
The rules of when to use and not to use RetainPtr or OSObjectPtr are same as alpha.webkit.UncountedCallArgsChecker for ref-counted objects.

alpha.webkit.UncountedLocalVarsChecker
""""""""""""""""""""""""""""""""""""""
Expand Down Expand Up @@ -3845,9 +3847,9 @@ Here are some examples of situations that we warn about as they *might* be poten

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 goal of this rule is to make sure that any NS or CF local variable is backed by a RetainPtr or OSObjectPtr 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 or OSObjectPtr 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.
The rules of when to use and not to use RetainPtr or OSObjectPtr are same as alpha.webkit.UncountedCallArgsChecker for ref-counted objects.

These are examples of cases that we consider safe:

Expand Down Expand Up @@ -3890,8 +3892,8 @@ Here are some examples of situations that we warn about as they *might* be poten

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.
The goal of this rule is to make sure the constructors of RetainPtr and OSObjectPtr as well as adoptNS, adoptCF, and adoptOSObject are used correctly.
When creating a RetainPtr or OSObjectPtr with +1 semantics, adoptNS, adoptCF, or adoptOSObject should be used, and in +0 semantics, RetainPtr or OSObjectPtr constructor should be used.
Warn otherwise.

These are examples of cases that we consider correct:
Expand All @@ -3900,13 +3902,15 @@ These are examples of cases that we consider correct:

RetainPtr ptr = adoptNS([[NSObject alloc] init]); // ok
RetainPtr ptr = CGImageGetColorSpace(image); // ok
OSObjectPtr ptr = adoptOSObject(dispatch_queue_create("some queue", nullptr)); // 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
OSObjectPtr ptr = dispatch_queue_create("some queue", nullptr); // warn

Debug Checkers
---------------
Expand Down
2 changes: 1 addition & 1 deletion clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
Original file line number Diff line number Diff line change
Expand Up @@ -1726,7 +1726,7 @@ def UnretainedLocalVarsChecker : Checker<"UnretainedLocalVarsChecker">,
Documentation<HasDocumentation>;

def RetainPtrCtorAdoptChecker : Checker<"RetainPtrCtorAdoptChecker">,
HelpText<"Check for correct use of RetainPtr constructor, adoptNS, and adoptCF">,
HelpText<"Check for correct use of RetainPtr/OSObjectPtr constructor, adoptNS, adoptCF, and adoptOSObject">,
Documentation<HasDocumentation>;

} // end alpha.webkit
2 changes: 1 addition & 1 deletion clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ bool tryToFindPtrOrigin(
if (auto *Callee = operatorCall->getDirectCallee()) {
auto ClsName = safeGetName(Callee->getParent());
if (isRefType(ClsName) || isCheckedPtr(ClsName) ||
isRetainPtr(ClsName) || ClsName == "unique_ptr" ||
isRetainPtrOrOSPtr(ClsName) || ClsName == "unique_ptr" ||
ClsName == "UniqueRef" || ClsName == "WeakPtr" ||
ClsName == "WeakRef") {
if (operatorCall->getNumArgs() == 1) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ class ForwardDeclChecker : public Checker<check::ASTDecl<TranslationUnitDecl>> {
// 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<CXXRecordDecl>(RD);
if (!R || isRefCounted(R) || isCheckedPtr(R) || isRetainPtr(R))
if (!R || isRefCounted(R) || isCheckedPtr(R) || isRetainPtrOrOSPtr(R))
return;

for (auto *Member : RD->fields()) {
Expand Down
29 changes: 16 additions & 13 deletions clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -129,16 +129,17 @@ bool isRefType(const std::string &Name) {
Name == "RefPtr" || Name == "RefPtrAllowingPartiallyDestroyed";
}

bool isRetainPtr(const std::string &Name) {
return Name == "RetainPtr" || Name == "RetainPtrArc";
bool isRetainPtrOrOSPtr(const std::string &Name) {
return Name == "RetainPtr" || Name == "RetainPtrArc" ||
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Maybe rename isRetainPtr to indicate its new functionality.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed to isRetainPtrOrOSPtr.

Name == "OSObjectPtr" || Name == "OSObjectPtrArc";
}

bool isCheckedPtr(const std::string &Name) {
return Name == "CheckedPtr" || Name == "CheckedRef";
}

bool isSmartPtrClass(const std::string &Name) {
return isRefType(Name) || isCheckedPtr(Name) || isRetainPtr(Name) ||
return isRefType(Name) || isCheckedPtr(Name) || isRetainPtrOrOSPtr(Name) ||
Name == "WeakPtr" || Name == "WeakPtrFactory" ||
Name == "WeakPtrFactoryWithBitField" || Name == "WeakPtrImplBase" ||
Name == "WeakPtrImplBaseSingleThread" || Name == "ThreadSafeWeakPtr" ||
Expand Down Expand Up @@ -166,15 +167,17 @@ bool isCtorOfCheckedPtr(const clang::FunctionDecl *F) {
return isCheckedPtr(safeGetName(F));
}

bool isCtorOfRetainPtr(const clang::FunctionDecl *F) {
bool isCtorOfRetainPtrOrOSPtr(const clang::FunctionDecl *F) {
const std::string &FunctionName = safeGetName(F);
return FunctionName == "RetainPtr" || FunctionName == "adoptNS" ||
FunctionName == "adoptCF" || FunctionName == "retainPtr" ||
FunctionName == "RetainPtrArc" || FunctionName == "adoptNSArc";
FunctionName == "RetainPtrArc" || FunctionName == "adoptNSArc" ||
FunctionName == "adoptOSObject" || FunctionName == "adoptOSObjectArc";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Same as above - rename function to reflect the new functionality.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed to isCtorOfRetainPtrOrOSPtr

}

bool isCtorOfSafePtr(const clang::FunctionDecl *F) {
return isCtorOfRefCounted(F) || isCtorOfCheckedPtr(F) || isCtorOfRetainPtr(F);
return isCtorOfRefCounted(F) || isCtorOfCheckedPtr(F) ||
isCtorOfRetainPtrOrOSPtr(F);
}

template <typename Predicate>
Expand All @@ -198,8 +201,8 @@ bool isRefOrCheckedPtrType(const clang::QualType T) {
T, [](auto Name) { return isRefType(Name) || isCheckedPtr(Name); });
}

bool isRetainPtrType(const clang::QualType T) {
return isPtrOfType(T, [](auto Name) { return isRetainPtr(Name); });
bool isRetainPtrOrOSPtrType(const clang::QualType T) {
return isPtrOfType(T, [](auto Name) { return isRetainPtrOrOSPtr(Name); });
}

bool isOwnerPtrType(const clang::QualType T) {
Expand Down Expand Up @@ -273,7 +276,7 @@ bool RetainTypeChecker::isUnretained(const QualType QT, bool ignoreARC) {
std::optional<bool> isUnretained(const QualType T, bool IsARCEnabled) {
if (auto *Subst = dyn_cast<SubstTemplateTypeParmType>(T)) {
if (auto *Decl = Subst->getAssociatedDecl()) {
if (isRetainPtr(safeGetName(Decl)))
if (isRetainPtrOrOSPtr(safeGetName(Decl)))
return false;
}
}
Expand Down Expand Up @@ -373,7 +376,7 @@ std::optional<bool> isGetterOfSafePtr(const CXXMethodDecl *M) {
method == "impl"))
return true;

if (isRetainPtr(className) && method == "get")
if (isRetainPtrOrOSPtr(className) && method == "get")
return true;

// Ref<T> -> T conversion
Expand All @@ -394,7 +397,7 @@ std::optional<bool> isGetterOfSafePtr(const CXXMethodDecl *M) {
}
}

if (isRetainPtr(className)) {
if (isRetainPtrOrOSPtr(className)) {
if (auto *maybeRefToRawOperator = dyn_cast<CXXConversionDecl>(M)) {
auto QT = maybeRefToRawOperator->getConversionType();
auto *T = QT.getTypePtrOrNull();
Expand Down Expand Up @@ -425,10 +428,10 @@ bool isCheckedPtr(const CXXRecordDecl *R) {
return false;
}

bool isRetainPtr(const CXXRecordDecl *R) {
bool isRetainPtrOrOSPtr(const CXXRecordDecl *R) {
assert(R);
if (auto *TmplR = R->getTemplateInstantiationPattern())
return isRetainPtr(safeGetName(TmplR));
return isRetainPtrOrOSPtr(safeGetName(TmplR));
return false;
}

Expand Down
6 changes: 3 additions & 3 deletions clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ bool isRefCounted(const clang::CXXRecordDecl *Class);
bool isCheckedPtr(const clang::CXXRecordDecl *Class);

/// \returns true if \p Class is a RetainPtr, false if not.
bool isRetainPtr(const clang::CXXRecordDecl *Class);
bool isRetainPtrOrOSPtr(const clang::CXXRecordDecl *Class);

/// \returns true if \p Class is a smart pointer (RefPtr, WeakPtr, etc...),
/// false if not.
Expand Down Expand Up @@ -116,7 +116,7 @@ std::optional<bool> isUnsafePtr(const QualType T, bool IsArcEnabled);
bool isRefOrCheckedPtrType(const clang::QualType T);

/// \returns true if \p T is a RetainPtr, false if not.
bool isRetainPtrType(const clang::QualType T);
bool isRetainPtrOrOSPtrType(const clang::QualType T);

/// \returns true if \p T is a RefPtr, Ref, CheckedPtr, CheckedRef, or
/// unique_ptr, false if not.
Expand All @@ -141,7 +141,7 @@ bool isRefType(const std::string &Name);
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);
bool isRetainPtrOrOSPtr(const std::string &Name);

/// \returns true if \p Name is a smart pointer type name, false if not.
bool isSmartPtrClass(const std::string &Name);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -465,11 +465,11 @@ class UnretainedCallArgsChecker final : public RawPtrRefCallArgsChecker {
}

bool isSafePtr(const CXXRecordDecl *Record) const final {
return isRetainPtr(Record);
return isRetainPtrOrOSPtr(Record);
}

bool isSafePtrType(const QualType type) const final {
return isRetainPtrType(type);
return isRetainPtrOrOSPtrType(type);
}

bool isSafeExpr(const Expr *E) const final {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -490,7 +490,7 @@ class UnretainedLambdaCapturesChecker : public RawPtrRefLambdaCapturesChecker {
}

virtual bool isPtrType(const std::string &Name) const final {
return isRetainPtr(Name);
return isRetainPtrOrOSPtr(Name);
}

const char *ptrKind(QualType QT) const final { return "unretained"; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -434,10 +434,10 @@ class UnretainedLocalVarsChecker final : public RawPtrRefLocalVarsChecker {
return RTC->isUnretained(T);
}
bool isSafePtr(const CXXRecordDecl *Record) const final {
return isRetainPtr(Record);
return isRetainPtrOrOSPtr(Record);
}
bool isSafePtrType(const QualType type) const final {
return isRetainPtrType(type);
return isRetainPtrOrOSPtrType(type);
}
bool isSafeExpr(const Expr *E) const final {
return ento::cocoa::isCocoaObjectRef(E->getType()) &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,10 +119,11 @@ class RawPtrRefMemberChecker
auto *Desugared = PointeeType->getUnqualifiedDesugaredType();
if (!Desugared)
return nullptr;
auto *ObjCType = dyn_cast<ObjCInterfaceType>(Desugared);
if (!ObjCType)
return nullptr;
return ObjCType->getDecl();
if (auto *ObjCType = dyn_cast<ObjCInterfaceType>(Desugared))
return ObjCType->getDecl();
if (auto *ObjCType = dyn_cast<ObjCObjectType>(Desugared))
return ObjCType->getInterface();
return nullptr;
}

void visitObjCDecl(const ObjCContainerDecl *CD) const {
Expand Down Expand Up @@ -369,7 +370,7 @@ class NoUnretainedMemberChecker final : public RawPtrRefMemberChecker {
const char *typeName() const final { return "retainable type"; }

const char *invariant() const final {
return "member variables must be a RetainPtr";
return "member variables must be a RetainPtr or OSObjectPtr";
}

PrintDeclKind printPointer(llvm::raw_svector_ostream &Os,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ class RetainPtrCtorAdoptChecker
}

bool TraverseClassTemplateDecl(ClassTemplateDecl *CTD) {
if (isRetainPtr(safeGetName(CTD)))
if (isRetainPtrOrOSPtr(safeGetName(CTD)))
return true; // Skip the contents of RetainPtr.
return Base::TraverseClassTemplateDecl(CTD);
}
Expand Down Expand Up @@ -121,7 +121,8 @@ class RetainPtrCtorAdoptChecker
}

bool isAdoptFnName(const std::string &Name) const {
return isAdoptNS(Name) || Name == "adoptCF" || Name == "adoptCFArc";
return isAdoptNS(Name) || Name == "adoptCF" || Name == "adoptCFArc" ||
Name == "adoptOSObject" || Name == "adoptOSObjectArc";
}

bool isAdoptNS(const std::string &Name) const {
Expand Down Expand Up @@ -304,7 +305,7 @@ class RetainPtrCtorAdoptChecker
if (!Cls)
return;

if (!isRetainPtr(safeGetName(Cls)) || !CE->getNumArgs())
if (!isRetainPtrOrOSPtr(safeGetName(Cls)) || !CE->getNumArgs())
return;

// Ignore RetainPtr construction inside adoptNS, adoptCF, and retainPtr.
Expand Down Expand Up @@ -491,7 +492,7 @@ class RetainPtrCtorAdoptChecker
return IsOwnedResult::NotOwned;
if (auto *DRE = dyn_cast<DeclRefExpr>(E)) {
auto QT = DRE->getType();
if (isRetainPtrType(QT))
if (isRetainPtrOrOSPtrType(QT))
return IsOwnedResult::NotOwned;
QT = QT.getCanonicalType();
if (RTC.isUnretained(QT, true /* ignoreARC */))
Expand Down Expand Up @@ -530,12 +531,13 @@ class RetainPtrCtorAdoptChecker
if (auto *CD = dyn_cast<CXXConversionDecl>(MD)) {
auto QT = CD->getConversionType().getCanonicalType();
auto *ResultType = QT.getTypePtrOrNull();
if (isRetainPtr(safeGetName(Cls)) && ResultType &&
if (isRetainPtrOrOSPtr(safeGetName(Cls)) && ResultType &&
(ResultType->isPointerType() || ResultType->isReferenceType() ||
ResultType->isObjCObjectPointerType()))
return IsOwnedResult::NotOwned;
}
if (safeGetName(MD) == "leakRef" && isRetainPtr(safeGetName(Cls)))
if (safeGetName(MD) == "leakRef" &&
isRetainPtrOrOSPtr(safeGetName(Cls)))
return IsOwnedResult::Owned;
}
}
Expand All @@ -553,7 +555,7 @@ class RetainPtrCtorAdoptChecker
continue;
}
auto RetType = Callee->getReturnType();
if (isRetainPtrType(RetType))
if (isRetainPtrOrOSPtrType(RetType))
return IsOwnedResult::NotOwned;
if (isCreateOrCopyFunction(Callee)) {
CreateOrCopyFnCall.insert(CE);
Expand Down
14 changes: 0 additions & 14 deletions clang/test/Analysis/Checkers/WebKit/call-args-checked.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,6 @@

#include "mock-types.h"

namespace std {

template <typename T> struct remove_reference {
typedef T type;
};

template <typename T> struct remove_reference<T&> {
typedef T type;
};

template<typename T> typename remove_reference<T>::type&& move(T&& t);

} // namespace std

RefCountableAndCheckable* makeObj();
CheckedRef<RefCountableAndCheckable> makeObjChecked();
void someFunction(RefCountableAndCheckable*);
Expand Down
14 changes: 0 additions & 14 deletions clang/test/Analysis/Checkers/WebKit/forward-decl-checker.mm
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,6 @@
#include "objc-mock-types.h"
#include "mock-system-header.h"

namespace std {

template <typename T> struct remove_reference {
typedef T type;
};

template <typename T> struct remove_reference<T&> {
typedef T type;
};

template<typename T> typename remove_reference<T>::type&& move(T&& t);

} // namespace std

typedef struct OpaqueJSString * JSStringRef;

class Obj;
Expand Down
Loading