Skip to content

Commit

Permalink
[analyzer] Check the safety of the object argument in a member functi…
Browse files Browse the repository at this point in the history
…on 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.
  • Loading branch information
rniwa committed Feb 14, 2024
1 parent 271e073 commit 3a49dfb
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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();
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
Expand All @@ -94,32 +103,36 @@ 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);
}
}
}

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;
Expand Down Expand Up @@ -196,6 +209,19 @@ class UncountedCallArgsChecker
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

Expand Down
18 changes: 18 additions & 0 deletions clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp
Original file line number Diff line number Diff line change
@@ -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}}
}

0 comments on commit 3a49dfb

Please sign in to comment.