https://github.com/rniwa created https://github.com/llvm/llvm-project/pull/188162
This PR fixes a hole in WebKit's static analysis that we weren't checking the soundness of argumnets to a function call via a (member) function pointer. >From 7b9d0f818f85acef0a16bd3038cbf3806028294d Mon Sep 17 00:00:00 2001 From: Ryosuke Niwa <[email protected]> Date: Tue, 24 Mar 2026 10:42:55 +0800 Subject: [PATCH] [alpha.webkit.UncountedCallArgsChecker] Check arguments of function pointers This PR fixes a hole in WebKit's static analysis that we weren't checking the soundness of argumnets to a function call via a (member) function pointer. --- .../WebKit/RawPtrRefCallArgsChecker.cpp | 106 ++++++++++-------- .../Analysis/Checkers/WebKit/call-args.cpp | 23 ++++ 2 files changed, 81 insertions(+), 48 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp index b2d87bc267700..ccbec68f9be92 100644 --- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefCallArgsChecker.cpp @@ -113,62 +113,35 @@ class RawPtrRefCallArgsChecker unsigned ArgIdx = isa<CXXOperatorCallExpr>(CE) && isa_and_nonnull<CXXMethodDecl>(F); - if (auto *MemberCallExpr = dyn_cast<CXXMemberCallExpr>(CE)) { - if (auto *MD = MemberCallExpr->getMethodDecl()) { - auto name = safeGetName(MD); - if (name == "ref" || name == "deref") - return; - if (name == "incrementCheckedPtrCount" || - name == "decrementCheckedPtrCount") - return; - } - auto *E = MemberCallExpr->getImplicitObjectArgument(); - QualType ArgType = MemberCallExpr->getObjectType().getCanonicalType(); - std::optional<bool> IsUnsafe = isUnsafeType(ArgType); - if (IsUnsafe && *IsUnsafe && !isPtrOriginSafe(E)) - reportBugOnThis(E, D); - } + if (auto *MemberCallExpr = dyn_cast<CXXMemberCallExpr>(CE)) + checkThisArg(MemberCallExpr, D); for (auto P = F->param_begin(); - // FIXME: Also check variadic function parameters. - // FIXME: Also check default function arguments. Probably a different - // checker. In case there are default arguments the call can have - // fewer arguments than the callee has parameters. P < F->param_end() && ArgIdx < CE->getNumArgs(); ++P, ++ArgIdx) { // TODO: attributes. // if ((*P)->hasAttr<SafeRefCntblRawPtrAttr>()) // continue; - - QualType ArgType = (*P)->getType(); - // FIXME: more complex types (arrays, references to raw pointers, etc) - std::optional<bool> IsUncounted = isUnsafePtr(ArgType); - if (!IsUncounted || !(*IsUncounted)) - continue; - - const auto *Arg = CE->getArg(ArgIdx); - - if (auto *defaultArg = dyn_cast<CXXDefaultArgExpr>(Arg)) - Arg = defaultArg->getExpr(); - - if (isPtrOriginSafe(Arg)) - continue; - - reportBug(Arg, *P, D); + checkArg(CE->getArg(ArgIdx), (*P)->getType(), *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); + auto *Arg = CE->getArg(ArgIdx); + checkArg(Arg, Arg->getType(), nullptr, D); + } + } else if (auto *Decl = CE->getCalleeDecl()) { + if (auto *FnType = Decl->getFunctionType()) { + if (auto *ProtoType = dyn_cast<FunctionProtoType>(FnType)) { + if (auto *MemberCallExpr = dyn_cast<CXXMemberCallExpr>(CE)) + checkThisArg(MemberCallExpr, D); + unsigned ArgIdx = 0; + for (auto PT = ProtoType->param_type_begin(); + PT < ProtoType->param_type_end() && ArgIdx < CE->getNumArgs(); + ++PT, ++ArgIdx) + checkArg(CE->getArg(ArgIdx), *PT, nullptr, D); + for (; ArgIdx < CE->getNumArgs(); ++ArgIdx) { + auto *Arg = CE->getArg(ArgIdx); + checkArg(Arg, Arg->getType(), nullptr, D); + } + } } } } @@ -205,6 +178,43 @@ class RawPtrRefCallArgsChecker } } + void checkThisArg(const CXXMemberCallExpr* MemberCallExpr, + const Decl* DeclWithIssue) const { + if (auto *MD = MemberCallExpr->getMethodDecl()) { + auto name = safeGetName(MD); + if (name == "ref" || name == "deref") + return; + if (name == "incrementCheckedPtrCount" || + name == "decrementCheckedPtrCount") + return; + } + auto *ThisExpr = MemberCallExpr->getImplicitObjectArgument(); + QualType ArgType = MemberCallExpr->getObjectType().getCanonicalType(); + std::optional<bool> IsUnsafe = isUnsafeType(ArgType); + if (!IsUnsafe || !*IsUnsafe) + return; + + if (isPtrOriginSafe(ThisExpr)) + return; + + reportBugOnThis(MemberCallExpr, DeclWithIssue); + } + + void checkArg(const Expr* Arg, QualType ParamType, const ParmVarDecl *Param, + const Decl* DeclWithIssue) const { + std::optional<bool> IsUncounted = isUnsafePtr(ParamType); + if (!IsUncounted || !(*IsUncounted)) + return; + + if (auto *DefaultArg = dyn_cast<CXXDefaultArgExpr>(Arg)) + Arg = DefaultArg->getExpr(); + + if (isPtrOriginSafe(Arg)) + return; + + reportBug(Arg, Param, DeclWithIssue); + } + bool isPtrOriginSafe(const Expr *Arg) const { return tryToFindPtrOrigin( Arg, /*StopAtFirstRefCountedObj=*/true, diff --git a/clang/test/Analysis/Checkers/WebKit/call-args.cpp b/clang/test/Analysis/Checkers/WebKit/call-args.cpp index 1a8bde29080ac..7f6554283b5fb 100644 --- a/clang/test/Analysis/Checkers/WebKit/call-args.cpp +++ b/clang/test/Analysis/Checkers/WebKit/call-args.cpp @@ -345,6 +345,29 @@ namespace cxx_member_operator_call { } } +namespace call_function_ptr { + class RefCountableWithWeakPtr : public RefCountable, public CanMakeWeakPtr<RefCountableWithWeakPtr> { + public: + void method(); + }; + + RefCountableWithWeakPtr* provide(); + + void foo(void (*consume)(void*, RefCountableWithWeakPtr*), void (*consumeVar)(RefCountableWithWeakPtr*, ...), void (RefCountableWithWeakPtr::*method)()) { + consume(nullptr, provide()); + // expected-warning@-1{{Call argument is uncounted and unsafe}} + consumeVar(nullptr, provide()); + // expected-warning@-1{{Call argument is uncounted and unsafe}} + (provide()->*method)(); + // expected-warning@-1{{Call argument for 'this' parameter is uncounted and unsafe}} + } + + template <typename T, typename U, typename... Arg> + void bar(T* obj, int (U::* function)(void*, Arg... args), Arg... args) { + (obj->*function)(args...); + } +} + namespace call_with_ptr_on_ref { Ref<RefCountable> provideProtected(); void bar(RefCountable* bad); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
