[clang] [alpha.webkit.UncountedCallArgsChecker] Check the safety of the object argument in a member function call. (PR #81400)
Ryosuke Niwa via cfe-commits
cfe-commits at lists.llvm.org
Tue Feb 13 20:14:33 PST 2024
https://github.com/rniwa updated https://github.com/llvm/llvm-project/pull/81400
>From 6f94c583f5201fbd73156969fa410669d6e1be93 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Tue, 13 Feb 2024 20:05:18 -0800
Subject: [PATCH 1/3] [alpha.webkit.UncountedCallArgsChecker] Check the safety
of the object argument in a member function call.
This PR makes alpha.webkit.UncountedCallArgsChecker eplicitly check the safety of the object argument in
a member function call.
---
.../WebKit/UncountedCallArgsChecker.cpp | 67 +++++++++++++------
.../Checkers/WebKit/uncounted-obj-arg.cpp | 18 +++++
2 files changed, 66 insertions(+), 19 deletions(-)
create mode 100644 clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
index f4e6191cf05a3c..d4bf3e2c2e75db 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
@@ -70,6 +70,15 @@ class UncountedCallArgsChecker
// or std::function call operator).
unsigned ArgIdx = isa<CXXOperatorCallExpr>(CE) && isa_and_nonnull<CXXMethodDecl>(F);
+ if (auto *MemberCallExpr = dyn_cast<CXXMemberCallExpr>(CE)) {
+ auto *E = MemberCallExpr->getImplicitObjectArgument();
+ auto *ArgType = MemberCallExpr->getObjectType().getTypePtrOrNull();
+ std::optional<bool> IsUncounted =
+ isUncounted(ArgType->getAsCXXRecordDecl());
+ if (IsUncounted && *IsUncounted && !isPtrOriginSafe(E))
+ reportBugOnThis(E);
+ }
+
for (auto P = F->param_begin();
// FIXME: Also check variadic function parameters.
// FIXME: Also check default function arguments. Probably a different
@@ -94,25 +103,7 @@ class UncountedCallArgsChecker
if (auto *defaultArg = dyn_cast<CXXDefaultArgExpr>(Arg))
Arg = defaultArg->getExpr();
- std::pair<const clang::Expr *, bool> ArgOrigin =
- tryToFindPtrOrigin(Arg, true);
-
- // Temporary ref-counted object created as part of the call argument
- // would outlive the call.
- if (ArgOrigin.second)
- continue;
-
- if (isa<CXXNullPtrLiteralExpr>(ArgOrigin.first)) {
- // foo(nullptr)
- continue;
- }
- if (isa<IntegerLiteral>(ArgOrigin.first)) {
- // FIXME: Check the value.
- // foo(NULL)
- continue;
- }
-
- if (isASafeCallArg(ArgOrigin.first))
+ if (isPtrOriginSafe(Arg))
continue;
reportBug(Arg, *P);
@@ -120,6 +111,28 @@ class UncountedCallArgsChecker
}
}
+ bool isPtrOriginSafe(const Expr *Arg) const {
+ std::pair<const clang::Expr *, bool> ArgOrigin =
+ tryToFindPtrOrigin(Arg, true);
+
+ // Temporary ref-counted object created as part of the call argument
+ // would outlive the call.
+ if (ArgOrigin.second)
+ return true;
+
+ if (isa<CXXNullPtrLiteralExpr>(ArgOrigin.first)) {
+ // foo(nullptr)
+ return true;
+ }
+ if (isa<IntegerLiteral>(ArgOrigin.first)) {
+ // FIXME: Check the value.
+ // foo(NULL)
+ return true;
+ }
+
+ return isASafeCallArg(ArgOrigin.first);
+ }
+
bool shouldSkipCall(const CallExpr *CE) const {
if (CE->getNumArgs() == 0)
return false;
@@ -196,6 +209,22 @@ class UncountedCallArgsChecker
Report->addRange(CallArg->getSourceRange());
BR->emitReport(std::move(Report));
}
+
+ void reportBugOnThis(const Expr *CallArg) const {
+ assert(CallArg);
+
+ SmallString<100> Buf;
+ llvm::raw_svector_ostream Os(Buf);
+
+ Os << "Call argument for 'this' parameter is uncounted and unsafe.";
+
+ const SourceLocation SrcLocToReport = CallArg->getSourceRange().getBegin();
+
+ PathDiagnosticLocation BSLoc(SrcLocToReport, BR->getSourceManager());
+ auto Report = std::make_unique<BasicBugReport>(Bug, Os.str(), BSLoc);
+ Report->addRange(CallArg->getSourceRange());
+ BR->emitReport(std::move(Report));
+ }
};
} // namespace
diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp
new file mode 100644
index 00000000000000..e5e39e3faac714
--- /dev/null
+++ b/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UncountedCallArgsChecker -verify %s
+
+#include "mock-types.h"
+
+class RefCounted {
+public:
+ void ref() const;
+ void deref() const;
+ void someFunction();
+};
+
+RefCounted* refCountedObj();
+
+void test()
+{
+ refCountedObj()->someFunction();
+ // expected-warning at -1{{Call argument for 'this' parameter is uncounted and unsafe}}
+}
>From c15967f8f09198c01aec8b0161cc08484c78f66a Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Tue, 13 Feb 2024 19:36:09 -0800
Subject: [PATCH 2/3] Address the review comment. Namely avoid calling
getTypePtrOrNull and don't use raw_svector_ostream.
---
.../Checkers/WebKit/UncountedCallArgsChecker.cpp | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
index d4bf3e2c2e75db..3554f7c4677958 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
@@ -72,7 +72,7 @@ class UncountedCallArgsChecker
if (auto *MemberCallExpr = dyn_cast<CXXMemberCallExpr>(CE)) {
auto *E = MemberCallExpr->getImplicitObjectArgument();
- auto *ArgType = MemberCallExpr->getObjectType().getTypePtrOrNull();
+ QualType ArgType = MemberCallExpr->getObjectType();
std::optional<bool> IsUncounted =
isUncounted(ArgType->getAsCXXRecordDecl());
if (IsUncounted && *IsUncounted && !isPtrOriginSafe(E))
@@ -213,15 +213,10 @@ class UncountedCallArgsChecker
void reportBugOnThis(const Expr *CallArg) const {
assert(CallArg);
- SmallString<100> Buf;
- llvm::raw_svector_ostream Os(Buf);
-
- Os << "Call argument for 'this' parameter is uncounted and unsafe.";
-
const SourceLocation SrcLocToReport = CallArg->getSourceRange().getBegin();
PathDiagnosticLocation BSLoc(SrcLocToReport, BR->getSourceManager());
- auto Report = std::make_unique<BasicBugReport>(Bug, Os.str(), BSLoc);
+ auto Report = std::make_unique<BasicBugReport>(Bug, "Call argument for 'this' parameter is uncounted and unsafe.", BSLoc);
Report->addRange(CallArg->getSourceRange());
BR->emitReport(std::move(Report));
}
>From 423cbb19c4887adb4ba6d9babc092a12cf8940c3 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Tue, 13 Feb 2024 20:13:54 -0800
Subject: [PATCH 3/3] Fix the formatting.
---
.../Checkers/WebKit/UncountedCallArgsChecker.cpp | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
index 3554f7c4677958..c84e1f9c244a88 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
@@ -216,7 +216,9 @@ class UncountedCallArgsChecker
const SourceLocation SrcLocToReport = CallArg->getSourceRange().getBegin();
PathDiagnosticLocation BSLoc(SrcLocToReport, BR->getSourceManager());
- auto Report = std::make_unique<BasicBugReport>(Bug, "Call argument for 'this' parameter is uncounted and unsafe.", BSLoc);
+ auto Report = std::make_unique<BasicBugReport>(
+ Bug, "Call argument for 'this' parameter is uncounted and unsafe.",
+ BSLoc);
Report->addRange(CallArg->getSourceRange());
BR->emitReport(std::move(Report));
}
More information about the cfe-commits
mailing list