diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp index b7b2f8a16f07b3..9d34dfd3cea636 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp @@ -17,6 +17,10 @@ namespace clang { +bool isSafePtr(clang::CXXRecordDecl *Decl) { + return isRefCounted(Decl) || isCheckedPtr(Decl); +} + bool tryToFindPtrOrigin( const Expr *E, bool StopAtFirstRefCountedObj, std::function callback) { @@ -31,7 +35,7 @@ bool tryToFindPtrOrigin( } if (auto *tempExpr = dyn_cast(E)) { if (auto *C = tempExpr->getConstructor()) { - if (auto *Class = C->getParent(); Class && isRefCounted(Class)) + if (auto *Class = C->getParent(); Class && isSafePtr(Class)) return callback(E, true); break; } @@ -56,7 +60,7 @@ bool tryToFindPtrOrigin( if (StopAtFirstRefCountedObj) { if (auto *ConversionFunc = dyn_cast_or_null(cast->getConversionFunction())) { - if (isCtorOfRefCounted(ConversionFunc)) + if (isCtorOfSafePtr(ConversionFunc)) return callback(E, true); } } @@ -68,7 +72,7 @@ bool tryToFindPtrOrigin( if (auto *call = dyn_cast(E)) { if (auto *memberCall = dyn_cast(call)) { if (auto *decl = memberCall->getMethodDecl()) { - std::optional IsGetterOfRefCt = isGetterOfRefCounted(decl); + std::optional IsGetterOfRefCt = isGetterOfSafePtr(decl); if (IsGetterOfRefCt && *IsGetterOfRefCt) { E = memberCall->getImplicitObjectArgument(); if (StopAtFirstRefCountedObj) { @@ -87,7 +91,7 @@ bool tryToFindPtrOrigin( } if (auto *callee = call->getDirectCallee()) { - if (isCtorOfRefCounted(callee)) { + if (isCtorOfRefCounted(callee) || isCtorOfCheckedPtr(callee)) { if (StopAtFirstRefCountedObj) return callback(E, true); @@ -95,7 +99,7 @@ bool tryToFindPtrOrigin( continue; } - if (isRefType(callee->getReturnType())) + if (isSafePtrType(callee->getReturnType())) return callback(E, true); if (isSingleton(callee)) @@ -114,7 +118,7 @@ bool tryToFindPtrOrigin( } if (auto *ObjCMsgExpr = dyn_cast(E)) { if (auto *Method = ObjCMsgExpr->getMethodDecl()) { - if (isRefType(Method->getReturnType())) + if (isSafePtrType(Method->getReturnType())) return callback(E, true); } } diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp index 71440e6d08a1c9..2293dcf1d4bd64 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp @@ -135,7 +135,16 @@ bool isCtorOfRefCounted(const clang::FunctionDecl *F) { || FunctionName == "Identifier"; } -bool isRefType(const clang::QualType T) { +bool isCtorOfCheckedPtr(const clang::FunctionDecl *F) { + assert(F); + return isCheckedPtr(safeGetName(F)); +} + +bool isCtorOfSafePtr(const clang::FunctionDecl *F) { + return isCtorOfRefCounted(F) || isCtorOfCheckedPtr(F); +} + +bool isSafePtrType(const clang::QualType T) { QualType type = T; while (!type.isNull()) { if (auto *elaboratedT = type->getAs()) { @@ -145,7 +154,7 @@ bool isRefType(const clang::QualType T) { if (auto *specialT = type->getAs()) { if (auto *decl = specialT->getTemplateName().getAsTemplateDecl()) { auto name = decl->getNameAsString(); - return isRefType(name); + return isRefType(name) || isCheckedPtr(name); } return false; } @@ -177,6 +186,12 @@ std::optional isUncounted(const CXXRecordDecl* Class) return (*IsRefCountable); } +std::optional isUnchecked(const CXXRecordDecl *Class) { + if (isCheckedPtr(Class)) + return false; // Cheaper than below + return isCheckedPtrCapable(Class); +} + std::optional isUncountedPtr(const QualType T) { if (T->isPointerType() || T->isReferenceType()) { if (auto *CXXRD = T->getPointeeCXXRecordDecl()) @@ -185,8 +200,16 @@ std::optional isUncountedPtr(const QualType T) { return false; } -std::optional isGetterOfRefCounted(const CXXMethodDecl* M) -{ +std::optional isUnsafePtr(const QualType T) { + if (T->isPointerType() || T->isReferenceType()) { + if (auto *CXXRD = T->getPointeeCXXRecordDecl()) { + return isUncounted(CXXRD) || isUnchecked(CXXRD); + } + } + return false; +} + +std::optional isGetterOfSafePtr(const CXXMethodDecl *M) { assert(M); if (isa(M)) { @@ -194,6 +217,9 @@ std::optional isGetterOfRefCounted(const CXXMethodDecl* M) auto className = safeGetName(calleeMethodsClass); auto method = safeGetName(M); + if (isCheckedPtr(className) && (method == "get" || method == "ptr")) + return true; + if ((isRefType(className) && (method == "get" || method == "ptr")) || ((className == "String" || className == "AtomString" || className == "AtomStringImpl" || className == "UniqueString" || @@ -205,7 +231,12 @@ std::optional isGetterOfRefCounted(const CXXMethodDecl* M) // FIXME: Currently allowing any Ref -> whatever cast. if (isRefType(className)) { if (auto *maybeRefToRawOperator = dyn_cast(M)) - return isUncountedPtr(maybeRefToRawOperator->getConversionType()); + return isUnsafePtr(maybeRefToRawOperator->getConversionType()); + } + + if (isCheckedPtr(className)) { + if (auto *maybeRefToRawOperator = dyn_cast(M)) + return isUnsafePtr(maybeRefToRawOperator->getConversionType()); } } return false; @@ -448,7 +479,7 @@ class TrivialFunctionAnalysisVisitor if (!Callee) return false; - std::optional IsGetterOfRefCounted = isGetterOfRefCounted(Callee); + std::optional IsGetterOfRefCounted = isGetterOfSafePtr(Callee); if (IsGetterOfRefCounted && *IsGetterOfRefCounted) return true; diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h index 8e6aadf63b6d67..4b41ca96e1df1d 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h @@ -63,18 +63,30 @@ std::optional isUncounted(const clang::CXXRecordDecl* Class); /// class, false if not, std::nullopt if inconclusive. std::optional isUncountedPtr(const clang::QualType T); -/// \returns true if Name is a RefPtr, Ref, or its variant, false if not. -bool isRefType(const std::string &Name); +/// \returns true if \p T is a RefPtr, Ref, CheckedPtr, CheckedRef, or its +/// variant, false if not. +bool isSafePtrType(const clang::QualType T); /// \returns true if \p F creates ref-countable object from uncounted parameter, /// false if not. bool isCtorOfRefCounted(const clang::FunctionDecl *F); -/// \returns true if \p T is RefPtr, Ref, or its variant, false if not. -bool isRefType(const clang::QualType T); +/// \returns true if \p F creates checked ptr object from uncounted parameter, +/// false if not. +bool isCtorOfCheckedPtr(const clang::FunctionDecl *F); + +/// \returns true if \p F creates ref-countable or checked ptr object from +/// uncounted parameter, false if not. +bool isCtorOfSafePtr(const clang::FunctionDecl *F); + +/// \returns true if \p Name is RefPtr, Ref, or its variant, false if not. +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 M is getter of a ref-counted class, false if not. -std::optional isGetterOfRefCounted(const clang::CXXMethodDecl* Method); +std::optional isGetterOfSafePtr(const clang::CXXMethodDecl *Method); /// \returns true if \p F is a conversion between ref-countable or ref-counted /// pointer types. diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp index cea3503fa2c314..1a5a7309a54f16 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp @@ -96,6 +96,8 @@ class UncountedCallArgsChecker auto name = safeGetName(MD); if (name == "ref" || name == "deref") return; + if (name == "incrementPtrCount" || name == "decrementPtrCount") + return; } auto *E = MemberCallExpr->getImplicitObjectArgument(); QualType ArgType = MemberCallExpr->getObjectType().getCanonicalType(); diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp index 81d21100de878d..5cdf047738abcb 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp @@ -227,6 +227,7 @@ class UncountedLocalVarsChecker if (MaybeGuardianArgCXXRecord) { if (MaybeGuardian->isLocalVarDecl() && (isRefCounted(MaybeGuardianArgCXXRecord) || + isCheckedPtr(MaybeGuardianArgCXXRecord) || isRefcountedStringsHack(MaybeGuardian)) && isGuardedScopeEmbeddedInGuardianScope( V, MaybeGuardian)) diff --git a/clang/test/Analysis/Checkers/WebKit/call-args-checked.cpp b/clang/test/Analysis/Checkers/WebKit/call-args-checked.cpp new file mode 100644 index 00000000000000..49b6bfcd7cadfd --- /dev/null +++ b/clang/test/Analysis/Checkers/WebKit/call-args-checked.cpp @@ -0,0 +1,46 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UncountedCallArgsChecker -verify %s + +#include "mock-types.h" + +RefCountableAndCheckable* makeObj(); +CheckedRef makeObjChecked(); +void someFunction(RefCountableAndCheckable*); + +namespace call_args_unchecked_uncounted { + +static void foo() { + someFunction(makeObj()); + // expected-warning@-1{{Call argument is uncounted and unsafe [alpha.webkit.UncountedCallArgsChecker]}} +} + +} // namespace call_args_checked + +namespace call_args_checked { + +static void foo() { + CheckedPtr ptr = makeObj(); + someFunction(ptr.get()); +} + +static void bar() { + someFunction(CheckedPtr { makeObj() }.get()); +} + +static void baz() { + someFunction(makeObjChecked().ptr()); +} + +} // namespace call_args_checked + +namespace call_args_default { + +void someFunction(RefCountableAndCheckable* = makeObj()); +// expected-warning@-1{{Call argument is uncounted and unsafe [alpha.webkit.UncountedCallArgsChecker]}} +void otherFunction(RefCountableAndCheckable* = makeObjChecked().ptr()); + +void foo() { + someFunction(); + otherFunction(); +} + +} diff --git a/clang/test/Analysis/Checkers/WebKit/mock-types.h b/clang/test/Analysis/Checkers/WebKit/mock-types.h index 933b4c5e62a79c..8d8a90f0afae0e 100644 --- a/clang/test/Analysis/Checkers/WebKit/mock-types.h +++ b/clang/test/Analysis/Checkers/WebKit/mock-types.h @@ -114,8 +114,8 @@ template struct CheckedRef { public: CheckedRef() : t{} {}; - CheckedRef(T &t) : t(t) { t->incrementPtrCount(); } - CheckedRef(const CheckedRef& o) : t(o.t) { if (t) t->incrementPtrCount(); } + CheckedRef(T &t) : t(&t) { t.incrementPtrCount(); } + CheckedRef(const CheckedRef &o) : t(o.t) { if (t) t->incrementPtrCount(); } ~CheckedRef() { if (t) t->decrementPtrCount(); } T &get() { return *t; } T *ptr() { return t; } @@ -135,7 +135,7 @@ template struct CheckedPtr { if (t) t->incrementPtrCount(); } - CheckedPtr(Ref&& o) + CheckedPtr(Ref &&o) : t(o.leakRef()) { } ~CheckedPtr() { @@ -156,4 +156,14 @@ class CheckedObj { void decrementPtrCount(); }; +class RefCountableAndCheckable { +public: + void incrementPtrCount() const; + void decrementPtrCount() const; + void ref() const; + void deref() const; + void method(); + int trivial() { return 0; } +}; + #endif diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp index b5f6b8535bf418..1c0df42cdda663 100644 --- a/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp +++ b/clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp @@ -290,6 +290,57 @@ void foo() { } // namespace local_assignment_to_global +namespace local_refcountable_checkable_object { + +RefCountableAndCheckable* provide_obj(); + +void local_raw_ptr() { + RefCountableAndCheckable* a = nullptr; + // expected-warning@-1{{Local variable 'a' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}} + a = provide_obj(); + a->method(); +} + +void local_checked_ptr() { + CheckedPtr a = nullptr; + a = provide_obj(); + a->method(); +} + +void local_var_with_guardian_checked_ptr() { + CheckedPtr a = provide_obj(); + { + auto* b = a.get(); + b->method(); + } +} + +void local_var_with_guardian_checked_ptr_with_assignment() { + CheckedPtr a = provide_obj(); + { + RefCountableAndCheckable* b = a.get(); + // expected-warning@-1{{Local variable 'b' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}} + b = provide_obj(); + b->method(); + } +} + +void local_var_with_guardian_checked_ref() { + CheckedRef a = *provide_obj(); + { + RefCountableAndCheckable& b = a; + b.method(); + } +} + +void static_var() { + static RefCountableAndCheckable* a = nullptr; + // expected-warning@-1{{Static local variable 'a' is uncounted and unsafe [alpha.webkit.UncountedLocalVarsChecker]}} + a = provide_obj(); +} + +} // namespace local_refcountable_checkable_object + namespace local_var_in_recursive_function { struct TreeNode {