Revert "[alpha.webkit.UnretainedCallArgsChecker] Add a checker for NS or CF type call arguments." #130828
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… or CF t…" This reverts commit 9e64fc6.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Ryosuke Niwa (rniwa) ChangesReverts llvm/llvm-project#130729 Patch is 27.10 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/130828.diff 7 Files Affected:
diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index e9df204480643..c947f26204159 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -3642,12 +3642,6 @@ 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.
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 0903e65297f05..7ee2ec1aaf09f 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -1785,10 +1785,6 @@ def UncheckedCallArgsChecker : Checker<"UncheckedCallArgsChecker">,
HelpText<"Check unchecked call arguments.">,
Documentation<HasDocumentation>;
-def UnretainedCallArgsChecker : Checker<"UnretainedCallArgsChecker">,
- HelpText<"Check unretained call arguments.">,
- Documentation<HasDocumentation>;
-
def UncountedLocalVarsChecker : Checker<"UncountedLocalVarsChecker">,
HelpText<"Check uncounted local variables.">,
Documentation<HasDocumentation>;
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
index ae0f8ad09f26e..5e67cb29d08e4 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
@@ -8,7 +8,6 @@
#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"
@@ -29,15 +28,6 @@ bool tryToFindPtrOrigin(
std::function<bool(const clang::QualType)> isSafePtrType,
std::function<bool(const clang::Expr *, bool)> callback) {
while (E) {
- if (auto *DRE = dyn_cast<DeclRefExpr>(E)) {
- auto *ValDecl = DRE->getDecl();
- auto QT = ValDecl->getType();
- auto ValName = ValDecl->getName();
- if (ValDecl && (ValName.starts_with('k') || ValName.starts_with("_k")) &&
- QT.isConstQualified()) { // Treat constants such as kCF* as safe.
- return callback(E, true);
- }
- }
if (auto *tempExpr = dyn_cast<MaterializeTemporaryExpr>(E)) {
E = tempExpr->getSubExpr();
continue;
@@ -67,10 +57,6 @@ bool tryToFindPtrOrigin(
E = tempExpr->getSubExpr();
continue;
}
- if (auto *OpaqueValue = dyn_cast<OpaqueValueExpr>(E)) {
- E = OpaqueValue->getSourceExpr();
- continue;
- }
if (auto *Expr = dyn_cast<ConditionalOperator>(E)) {
return tryToFindPtrOrigin(Expr->getTrueExpr(), StopAtFirstRefCountedObj,
isSafePtr, isSafePtrType, callback) &&
@@ -143,11 +129,6 @@ 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<ObjCMessageExpr>(E)) {
@@ -155,18 +136,7 @@ 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<ObjCDictionaryLiteral>(E))
- return callback(ObjCDict, true);
- if (auto *ObjCArray = dyn_cast<ObjCArrayLiteral>(E))
- return callback(ObjCArray, true);
- if (auto *ObjCStr = dyn_cast<ObjCStringLiteral>(E))
- return callback(ObjCStr, true);
if (auto *unaryOp = dyn_cast<UnaryOperator>(E)) {
// FIXME: Currently accepts ANY unary operator. Is it OK?
E = unaryOp->getSubExpr();
@@ -186,14 +156,6 @@ bool isASafeCallArg(const Expr *E) {
if (auto *D = dyn_cast_or_null<VarDecl>(FoundDecl)) {
if (isa<ParmVarDecl>(D) || D->isLocalVarDecl())
return true;
- if (auto *ImplicitP = dyn_cast<ImplicitParamDecl>(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<BindingDecl>(FoundDecl)) {
VarDecl *VD = BD->getHoldingVar();
if (VD && (isa<ParmVarDecl>(VD) || VD->isLocalVarDecl()))
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index e780f610eb389..419d9c2325412 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -372,8 +372,7 @@ std::optional<bool> isGetterOfSafePtr(const CXXMethodDecl *M) {
if (auto *maybeRefToRawOperator = dyn_cast<CXXConversionDecl>(M)) {
auto QT = maybeRefToRawOperator->getConversionType();
auto *T = QT.getTypePtrOrNull();
- return T && (T->isPointerType() || T->isReferenceType() ||
- T->isObjCObjectPointerType());
+ return T && (T->isPointerType() || T->isReferenceType());
}
}
}
@@ -416,8 +415,7 @@ bool isPtrConversion(const FunctionDecl *F) {
if (FunctionName == "getPtr" || FunctionName == "WeakPtr" ||
FunctionName == "dynamicDowncast" || FunctionName == "downcast" ||
FunctionName == "checkedDowncast" ||
- FunctionName == "uncheckedDowncast" || FunctionName == "bitwise_cast" ||
- FunctionName == "bridge_cast")
+ FunctionName == "uncheckedDowncast" || FunctionName == "bitwise_cast")
return true;
return false;
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp
index 2dc17f048f922..9d07c65da88af 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp
@@ -13,7 +13,6 @@
#include "clang/AST/Decl.h"
#include "clang/AST/DeclCXX.h"
#include "clang/AST/DynamicRecursiveASTVisitor.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"
@@ -36,9 +35,6 @@ class RawPtrRefCallArgsChecker
TrivialFunctionAnalysis TFA;
EnsureFunctionAnalysis EFA;
-protected:
- mutable std::optional<RetainTypeChecker> RTC;
-
public:
RawPtrRefCallArgsChecker(const char *description)
: Bug(this, description, "WebKit coding guidelines") {}
@@ -84,22 +80,9 @@ class RawPtrRefCallArgsChecker
Checker->visitCallExpr(CE, DeclWithIssue);
return true;
}
-
- bool VisitTypedefDecl(TypedefDecl *TD) override {
- if (Checker->RTC)
- Checker->RTC->visitTypedef(TD);
- return true;
- }
-
- bool VisitObjCMessageExpr(ObjCMessageExpr *ObjCMsgExpr) override {
- Checker->visitObjCMessageExpr(ObjCMsgExpr, DeclWithIssue);
- return true;
- }
};
LocalVisitor visitor(this);
- if (RTC)
- RTC->visitTranslationUnitDecl(TUD);
visitor.TraverseDecl(const_cast<TranslationUnitDecl *>(TUD));
}
@@ -139,7 +122,7 @@ class RawPtrRefCallArgsChecker
// if ((*P)->hasAttr<SafeRefCntblRawPtrAttr>())
// continue;
- QualType ArgType = (*P)->getType();
+ QualType ArgType = (*P)->getType().getCanonicalType();
// FIXME: more complex types (arrays, references to raw pointers, etc)
std::optional<bool> IsUncounted = isUnsafePtr(ArgType);
if (!IsUncounted || !(*IsUncounted))
@@ -155,58 +138,6 @@ class RawPtrRefCallArgsChecker
reportBug(Arg, *P, D);
}
- for (; ArgIdx < CE->getNumArgs(); ++ArgIdx) {
- const auto *Arg = CE->getArg(ArgIdx);
- auto ArgType = Arg->getType();
- std::optional<bool> IsUncounted = isUnsafePtr(ArgType);
- if (!IsUncounted || !(*IsUncounted))
- continue;
-
- if (auto *defaultArg = dyn_cast<CXXDefaultArgExpr>(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()->IgnoreParenCasts()) {
- std::optional<bool> IsUnsafe = isUnsafePtr(E->getReceiverType());
- if (IsUnsafe && *IsUnsafe && !isPtrOriginSafe(Receiver)) {
- if (auto *InnerMsg = dyn_cast<ObjCMessageExpr>(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<bool> IsUnsafe = isUnsafePtr(ArgType);
- if (!IsUnsafe || !(*IsUnsafe))
- continue;
- if (isPtrOriginSafe(Arg))
- continue;
- reportBug(Arg, Param, D);
}
}
@@ -227,8 +158,6 @@ class RawPtrRefCallArgsChecker
// foo(NULL)
return true;
}
- if (isa<ObjCStringLiteral>(ArgOrigin))
- return true;
if (isASafeCallArg(ArgOrigin))
return true;
if (EFA.isACallToEnsureFn(ArgOrigin))
@@ -283,7 +212,7 @@ class RawPtrRefCallArgsChecker
overloadedOperatorType == OO_PipePipe)
return true;
- if (isCtorOfSafePtr(Callee))
+ if (isCtorOfRefCounted(Callee))
return true;
auto name = safeGetName(Callee);
@@ -348,10 +277,9 @@ class RawPtrRefCallArgsChecker
}
Os << " is " << ptrKind() << " and unsafe.";
- bool usesDefaultArgValue = isa<CXXDefaultArgExpr>(CallArg) && Param;
const SourceLocation SrcLocToReport =
- usesDefaultArgValue ? Param->getDefaultArg()->getExprLoc()
- : CallArg->getSourceRange().getBegin();
+ isa<CXXDefaultArgExpr>(CallArg) ? Param->getDefaultArg()->getExprLoc()
+ : CallArg->getSourceRange().getBegin();
PathDiagnosticLocation BSLoc(SrcLocToReport, BR->getSourceManager());
auto Report = std::make_unique<BasicBugReport>(Bug, Os.str(), BSLoc);
@@ -376,23 +304,6 @@ 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<BasicBugReport>(Bug, Os.str(), BSLoc);
- Report->addRange(CallArg->getSourceRange());
- Report->setDeclWithIssue(DeclWithIssue);
- BR->emitReport(std::move(Report));
- }
};
class UncountedCallArgsChecker final : public RawPtrRefCallArgsChecker {
@@ -406,7 +317,7 @@ class UncountedCallArgsChecker final : public RawPtrRefCallArgsChecker {
}
std::optional<bool> isUnsafePtr(QualType QT) const final {
- return isUncountedPtr(QT.getCanonicalType());
+ return isUncountedPtr(QT);
}
bool isSafePtr(const CXXRecordDecl *Record) const final {
@@ -431,7 +342,7 @@ class UncheckedCallArgsChecker final : public RawPtrRefCallArgsChecker {
}
std::optional<bool> isUnsafePtr(QualType QT) const final {
- return isUncheckedPtr(QT.getCanonicalType());
+ return isUncheckedPtr(QT);
}
bool isSafePtr(const CXXRecordDecl *Record) const final {
@@ -445,33 +356,6 @@ class UncheckedCallArgsChecker final : public RawPtrRefCallArgsChecker {
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<bool> isUnsafeType(QualType QT) const final {
- return RTC->isUnretained(QT);
- }
-
- std::optional<bool> 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);
- }
-
- const char *ptrKind() const final { return "unretained"; }
-};
-
} // namespace
void ento::registerUncountedCallArgsChecker(CheckerManager &Mgr) {
@@ -489,11 +373,3 @@ void ento::registerUncheckedCallArgsChecker(CheckerManager &Mgr) {
bool ento::shouldRegisterUncheckedCallArgsChecker(const CheckerManager &) {
return true;
}
-
-void ento::registerUnretainedCallArgsChecker(CheckerManager &Mgr) {
- Mgr.registerChecker<UnretainedCallArgsChecker>();
-}
-
-bool ento::shouldRegisterUnretainedCallArgsChecker(const CheckerManager &) {
- return true;
-}
diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-call-args-arc.mm b/clang/test/Analysis/Checkers/WebKit/unretained-call-args-arc.mm
deleted file mode 100644
index eb4735da60a05..0000000000000
--- a/clang/test/Analysis/Checkers/WebKit/unretained-call-args-arc.mm
+++ /dev/null
@@ -1,30 +0,0 @@
-// 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();
-
-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
-
-@interface AnotherObj : NSObject
-- (void)foo:(SomeObj *)obj;
-@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]}}
-}
-@end
diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm b/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm
deleted file mode 100644
index 3411cbdf5aa9b..0000000000000
--- a/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm
+++ /dev/null
@@ -1,396 +0,0 @@
-// 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);
-
-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<CFMutableArrayRef> { 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<SomeObj> { provide() }.get(), 1, RetainPtr<CFMutableArrayRef> { provide_cf() }.get());
- }
-}
-
-namespace retained {
- RetainPtr<SomeObj> provide_obj() { return RetainPtr<SomeObj>{}; }
- void consume_obj(RetainPtr<SomeObj>) {}
-
- RetainPtr<CFMutableArrayRef> provide_cf() { return CFMutableArrayRef{}; }
- void consume_cf(RetainPtr<CFMutableArrayRef>) {}
-
- 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<OtherObj*>(provide()));
- // expected-warning@-1{{Call argument is unretained and unsafe}}
-
- consume_obj(reinterpret_cast<OtherObj*>(provide()));
- // expected-warning@-1{{Call argument is unretained and unsafe}}
-
- consume_obj(downcast<OtherObj>(provide()));
- // expect...
[truncated]
|
|
@llvm/pr-subscribers-clang-static-analyzer-1 Author: Ryosuke Niwa (rniwa) ChangesReverts llvm/llvm-project#130729 Patch is 27.10 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/130828.diff 7 Files Affected:
diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index e9df204480643..c947f26204159 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -3642,12 +3642,6 @@ 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.
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 0903e65297f05..7ee2ec1aaf09f 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -1785,10 +1785,6 @@ def UncheckedCallArgsChecker : Checker<"UncheckedCallArgsChecker">,
HelpText<"Check unchecked call arguments.">,
Documentation<HasDocumentation>;
-def UnretainedCallArgsChecker : Checker<"UnretainedCallArgsChecker">,
- HelpText<"Check unretained call arguments.">,
- Documentation<HasDocumentation>;
-
def UncountedLocalVarsChecker : Checker<"UncountedLocalVarsChecker">,
HelpText<"Check uncounted local variables.">,
Documentation<HasDocumentation>;
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
index ae0f8ad09f26e..5e67cb29d08e4 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
@@ -8,7 +8,6 @@
#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"
@@ -29,15 +28,6 @@ bool tryToFindPtrOrigin(
std::function<bool(const clang::QualType)> isSafePtrType,
std::function<bool(const clang::Expr *, bool)> callback) {
while (E) {
- if (auto *DRE = dyn_cast<DeclRefExpr>(E)) {
- auto *ValDecl = DRE->getDecl();
- auto QT = ValDecl->getType();
- auto ValName = ValDecl->getName();
- if (ValDecl && (ValName.starts_with('k') || ValName.starts_with("_k")) &&
- QT.isConstQualified()) { // Treat constants such as kCF* as safe.
- return callback(E, true);
- }
- }
if (auto *tempExpr = dyn_cast<MaterializeTemporaryExpr>(E)) {
E = tempExpr->getSubExpr();
continue;
@@ -67,10 +57,6 @@ bool tryToFindPtrOrigin(
E = tempExpr->getSubExpr();
continue;
}
- if (auto *OpaqueValue = dyn_cast<OpaqueValueExpr>(E)) {
- E = OpaqueValue->getSourceExpr();
- continue;
- }
if (auto *Expr = dyn_cast<ConditionalOperator>(E)) {
return tryToFindPtrOrigin(Expr->getTrueExpr(), StopAtFirstRefCountedObj,
isSafePtr, isSafePtrType, callback) &&
@@ -143,11 +129,6 @@ 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<ObjCMessageExpr>(E)) {
@@ -155,18 +136,7 @@ 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<ObjCDictionaryLiteral>(E))
- return callback(ObjCDict, true);
- if (auto *ObjCArray = dyn_cast<ObjCArrayLiteral>(E))
- return callback(ObjCArray, true);
- if (auto *ObjCStr = dyn_cast<ObjCStringLiteral>(E))
- return callback(ObjCStr, true);
if (auto *unaryOp = dyn_cast<UnaryOperator>(E)) {
// FIXME: Currently accepts ANY unary operator. Is it OK?
E = unaryOp->getSubExpr();
@@ -186,14 +156,6 @@ bool isASafeCallArg(const Expr *E) {
if (auto *D = dyn_cast_or_null<VarDecl>(FoundDecl)) {
if (isa<ParmVarDecl>(D) || D->isLocalVarDecl())
return true;
- if (auto *ImplicitP = dyn_cast<ImplicitParamDecl>(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<BindingDecl>(FoundDecl)) {
VarDecl *VD = BD->getHoldingVar();
if (VD && (isa<ParmVarDecl>(VD) || VD->isLocalVarDecl()))
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index e780f610eb389..419d9c2325412 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -372,8 +372,7 @@ std::optional<bool> isGetterOfSafePtr(const CXXMethodDecl *M) {
if (auto *maybeRefToRawOperator = dyn_cast<CXXConversionDecl>(M)) {
auto QT = maybeRefToRawOperator->getConversionType();
auto *T = QT.getTypePtrOrNull();
- return T && (T->isPointerType() || T->isReferenceType() ||
- T->isObjCObjectPointerType());
+ return T && (T->isPointerType() || T->isReferenceType());
}
}
}
@@ -416,8 +415,7 @@ bool isPtrConversion(const FunctionDecl *F) {
if (FunctionName == "getPtr" || FunctionName == "WeakPtr" ||
FunctionName == "dynamicDowncast" || FunctionName == "downcast" ||
FunctionName == "checkedDowncast" ||
- FunctionName == "uncheckedDowncast" || FunctionName == "bitwise_cast" ||
- FunctionName == "bridge_cast")
+ FunctionName == "uncheckedDowncast" || FunctionName == "bitwise_cast")
return true;
return false;
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp
index 2dc17f048f922..9d07c65da88af 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp
@@ -13,7 +13,6 @@
#include "clang/AST/Decl.h"
#include "clang/AST/DeclCXX.h"
#include "clang/AST/DynamicRecursiveASTVisitor.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"
@@ -36,9 +35,6 @@ class RawPtrRefCallArgsChecker
TrivialFunctionAnalysis TFA;
EnsureFunctionAnalysis EFA;
-protected:
- mutable std::optional<RetainTypeChecker> RTC;
-
public:
RawPtrRefCallArgsChecker(const char *description)
: Bug(this, description, "WebKit coding guidelines") {}
@@ -84,22 +80,9 @@ class RawPtrRefCallArgsChecker
Checker->visitCallExpr(CE, DeclWithIssue);
return true;
}
-
- bool VisitTypedefDecl(TypedefDecl *TD) override {
- if (Checker->RTC)
- Checker->RTC->visitTypedef(TD);
- return true;
- }
-
- bool VisitObjCMessageExpr(ObjCMessageExpr *ObjCMsgExpr) override {
- Checker->visitObjCMessageExpr(ObjCMsgExpr, DeclWithIssue);
- return true;
- }
};
LocalVisitor visitor(this);
- if (RTC)
- RTC->visitTranslationUnitDecl(TUD);
visitor.TraverseDecl(const_cast<TranslationUnitDecl *>(TUD));
}
@@ -139,7 +122,7 @@ class RawPtrRefCallArgsChecker
// if ((*P)->hasAttr<SafeRefCntblRawPtrAttr>())
// continue;
- QualType ArgType = (*P)->getType();
+ QualType ArgType = (*P)->getType().getCanonicalType();
// FIXME: more complex types (arrays, references to raw pointers, etc)
std::optional<bool> IsUncounted = isUnsafePtr(ArgType);
if (!IsUncounted || !(*IsUncounted))
@@ -155,58 +138,6 @@ class RawPtrRefCallArgsChecker
reportBug(Arg, *P, D);
}
- for (; ArgIdx < CE->getNumArgs(); ++ArgIdx) {
- const auto *Arg = CE->getArg(ArgIdx);
- auto ArgType = Arg->getType();
- std::optional<bool> IsUncounted = isUnsafePtr(ArgType);
- if (!IsUncounted || !(*IsUncounted))
- continue;
-
- if (auto *defaultArg = dyn_cast<CXXDefaultArgExpr>(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()->IgnoreParenCasts()) {
- std::optional<bool> IsUnsafe = isUnsafePtr(E->getReceiverType());
- if (IsUnsafe && *IsUnsafe && !isPtrOriginSafe(Receiver)) {
- if (auto *InnerMsg = dyn_cast<ObjCMessageExpr>(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<bool> IsUnsafe = isUnsafePtr(ArgType);
- if (!IsUnsafe || !(*IsUnsafe))
- continue;
- if (isPtrOriginSafe(Arg))
- continue;
- reportBug(Arg, Param, D);
}
}
@@ -227,8 +158,6 @@ class RawPtrRefCallArgsChecker
// foo(NULL)
return true;
}
- if (isa<ObjCStringLiteral>(ArgOrigin))
- return true;
if (isASafeCallArg(ArgOrigin))
return true;
if (EFA.isACallToEnsureFn(ArgOrigin))
@@ -283,7 +212,7 @@ class RawPtrRefCallArgsChecker
overloadedOperatorType == OO_PipePipe)
return true;
- if (isCtorOfSafePtr(Callee))
+ if (isCtorOfRefCounted(Callee))
return true;
auto name = safeGetName(Callee);
@@ -348,10 +277,9 @@ class RawPtrRefCallArgsChecker
}
Os << " is " << ptrKind() << " and unsafe.";
- bool usesDefaultArgValue = isa<CXXDefaultArgExpr>(CallArg) && Param;
const SourceLocation SrcLocToReport =
- usesDefaultArgValue ? Param->getDefaultArg()->getExprLoc()
- : CallArg->getSourceRange().getBegin();
+ isa<CXXDefaultArgExpr>(CallArg) ? Param->getDefaultArg()->getExprLoc()
+ : CallArg->getSourceRange().getBegin();
PathDiagnosticLocation BSLoc(SrcLocToReport, BR->getSourceManager());
auto Report = std::make_unique<BasicBugReport>(Bug, Os.str(), BSLoc);
@@ -376,23 +304,6 @@ 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<BasicBugReport>(Bug, Os.str(), BSLoc);
- Report->addRange(CallArg->getSourceRange());
- Report->setDeclWithIssue(DeclWithIssue);
- BR->emitReport(std::move(Report));
- }
};
class UncountedCallArgsChecker final : public RawPtrRefCallArgsChecker {
@@ -406,7 +317,7 @@ class UncountedCallArgsChecker final : public RawPtrRefCallArgsChecker {
}
std::optional<bool> isUnsafePtr(QualType QT) const final {
- return isUncountedPtr(QT.getCanonicalType());
+ return isUncountedPtr(QT);
}
bool isSafePtr(const CXXRecordDecl *Record) const final {
@@ -431,7 +342,7 @@ class UncheckedCallArgsChecker final : public RawPtrRefCallArgsChecker {
}
std::optional<bool> isUnsafePtr(QualType QT) const final {
- return isUncheckedPtr(QT.getCanonicalType());
+ return isUncheckedPtr(QT);
}
bool isSafePtr(const CXXRecordDecl *Record) const final {
@@ -445,33 +356,6 @@ class UncheckedCallArgsChecker final : public RawPtrRefCallArgsChecker {
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<bool> isUnsafeType(QualType QT) const final {
- return RTC->isUnretained(QT);
- }
-
- std::optional<bool> 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);
- }
-
- const char *ptrKind() const final { return "unretained"; }
-};
-
} // namespace
void ento::registerUncountedCallArgsChecker(CheckerManager &Mgr) {
@@ -489,11 +373,3 @@ void ento::registerUncheckedCallArgsChecker(CheckerManager &Mgr) {
bool ento::shouldRegisterUncheckedCallArgsChecker(const CheckerManager &) {
return true;
}
-
-void ento::registerUnretainedCallArgsChecker(CheckerManager &Mgr) {
- Mgr.registerChecker<UnretainedCallArgsChecker>();
-}
-
-bool ento::shouldRegisterUnretainedCallArgsChecker(const CheckerManager &) {
- return true;
-}
diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-call-args-arc.mm b/clang/test/Analysis/Checkers/WebKit/unretained-call-args-arc.mm
deleted file mode 100644
index eb4735da60a05..0000000000000
--- a/clang/test/Analysis/Checkers/WebKit/unretained-call-args-arc.mm
+++ /dev/null
@@ -1,30 +0,0 @@
-// 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();
-
-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
-
-@interface AnotherObj : NSObject
-- (void)foo:(SomeObj *)obj;
-@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]}}
-}
-@end
diff --git a/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm b/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm
deleted file mode 100644
index 3411cbdf5aa9b..0000000000000
--- a/clang/test/Analysis/Checkers/WebKit/unretained-call-args.mm
+++ /dev/null
@@ -1,396 +0,0 @@
-// 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);
-
-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<CFMutableArrayRef> { 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<SomeObj> { provide() }.get(), 1, RetainPtr<CFMutableArrayRef> { provide_cf() }.get());
- }
-}
-
-namespace retained {
- RetainPtr<SomeObj> provide_obj() { return RetainPtr<SomeObj>{}; }
- void consume_obj(RetainPtr<SomeObj>) {}
-
- RetainPtr<CFMutableArrayRef> provide_cf() { return CFMutableArrayRef{}; }
- void consume_cf(RetainPtr<CFMutableArrayRef>) {}
-
- 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<OtherObj*>(provide()));
- // expected-warning@-1{{Call argument is unretained and unsafe}}
-
- consume_obj(reinterpret_cast<OtherObj*>(provide()));
- // expected-warning@-1{{Call argument is unretained and unsafe}}
-
- consume_obj(downcast<OtherObj>(provide()));
- // expect...
[truncated]
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reverts #130729