diff options
author | Ryosuke Niwa <rniwa@webkit.org> | 2024-02-14 14:44:51 -0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-02-14 14:44:51 -0800 |
commit | 3a49dfb28fed8f784484ce2ce6d687550f27ad59 (patch) | |
tree | 0f01e4baa1a0a994b08a56aa7c2790426089d8ff | |
parent | 271e07321bd0673f5472055012b4cfd1671be9ec (diff) |
[analyzer] Check the safety of the object argument in a member function call. (#81400)
This PR makes alpha.webkit.UncountedCallArgsChecker eplicitly check the
safety of the object argument in a member function call. It also removes
the exemption of local variables from this checker so that each local
variable's safety is checked if it's used in a function call instead of
relying on the local variable checker to find those since local variable
checker currently has exemption for "for" and "if" statements.
-rw-r--r-- | clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp | 64 | ||||
-rw-r--r-- | clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp | 18 |
2 files changed, 63 insertions, 19 deletions
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp index f4e6191cf05a..c84e1f9c244a 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp @@ -70,6 +70,15 @@ public: // 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(); + QualType ArgType = MemberCallExpr->getObjectType(); + 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 @@ public: 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 @@ public: } } + 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,19 @@ public: Report->addRange(CallArg->getSourceRange()); BR->emitReport(std::move(Report)); } + + void reportBugOnThis(const Expr *CallArg) const { + assert(CallArg); + + 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); + 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 000000000000..e5e39e3faac7 --- /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@-1{{Call argument for 'this' parameter is uncounted and unsafe}} +} |