[clang] [alpha.webkit.UncountedCallArgsChecker] Detect & ignore trivial function calls. (PR #81808)
Ryosuke Niwa via cfe-commits
cfe-commits at lists.llvm.org
Wed Feb 14 18:25:53 PST 2024
https://github.com/rniwa updated https://github.com/llvm/llvm-project/pull/81808
>From 857decc27550e2b15938a7846a03561f9ad73f48 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Wed, 14 Feb 2024 16:21:33 -0800
Subject: [PATCH 1/5] [alpha.webkit.UncountedCallArgsChecker] Detect & ignore
trivial function calls.
This PR introduces the concept of a "trivial function" which applies to a function
that only calls other trivial functions and contain literals and expressions that
don't result in heap mutations (specifically it does not call deref). This is
implemented using ConstStmtVisitor and checking each statement and expression's
trivialness.
This PR also introduces the concept of a "ingleton function", which is a static
member function or a free standing function which ends with the suffix "singleton".
Such a function's return value is understood to be safe to call any function with.
---
.../Checkers/WebKit/ASTUtils.cpp | 4 +
.../Checkers/WebKit/PtrTypesSemantics.cpp | 207 ++++++++++++++++
.../Checkers/WebKit/PtrTypesSemantics.h | 21 ++
.../WebKit/UncountedCallArgsChecker.cpp | 8 +-
.../Analysis/Checkers/WebKit/call-args.cpp | 33 +--
.../Checkers/WebKit/uncounted-obj-arg.cpp | 231 ++++++++++++++++++
6 files changed, 487 insertions(+), 17 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
index b76c0551c77bb0..94eaa81af51772 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
@@ -66,9 +66,13 @@ tryToFindPtrOrigin(const Expr *E, bool StopAtFirstRefCountedObj) {
E = call->getArg(0);
continue;
}
+
if (isReturnValueRefCounted(callee))
return {E, true};
+ if (isSingleton(callee))
+ return {E, true};
+
if (isPtrConversion(callee)) {
E = call->getArg(0);
continue;
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index 907244013d0871..17f59c2d304ddc 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -12,6 +12,7 @@
#include "clang/AST/Decl.h"
#include "clang/AST/DeclCXX.h"
#include "clang/AST/ExprCXX.h"
+#include "clang/AST/StmtVisitor.h"
#include <optional>
using namespace clang;
@@ -222,4 +223,210 @@ bool isPtrConversion(const FunctionDecl *F) {
return false;
}
+bool isSingleton(const FunctionDecl *F) {
+ assert(F);
+ // FIXME: check # of params == 1
+ if (auto *MethodDecl = dyn_cast<CXXMethodDecl>(F)) {
+ if (!MethodDecl->isStatic())
+ return false;
+ }
+ const auto &Name = safeGetName(F);
+ std::string SingletonStr = "singleton";
+ auto index = Name.find(SingletonStr);
+ return index != std::string::npos &&
+ index == Name.size() - SingletonStr.size();
+}
+
+// We only care about statements so let's use the simple
+// (non-recursive) visitor.
+class TrivialFunctionAnalysisVisitor
+ : public ConstStmtVisitor<TrivialFunctionAnalysisVisitor, bool> {
+
+ // Returns false if at least one child is non-trivial.
+ bool VisitChildren(const Stmt *S) {
+ for (const Stmt *Child : S->children()) {
+ if (Child && !Visit(Child))
+ return false;
+ }
+
+ return true;
+ }
+
+public:
+ using CacheTy = TrivialFunctionAnalysis::CacheTy;
+
+ TrivialFunctionAnalysisVisitor(CacheTy &Cache) : Cache(Cache) {}
+
+ bool VisitStmt(const Stmt *S) {
+ // All statements are non-trivial unless overriden later.
+ // Don't even recurse into children by default.
+ return false;
+ }
+
+ bool VisitCompoundStmt(const CompoundStmt *CS) {
+ // A compound statement is allowed as long each individual sub-statement
+ // is trivial.
+ return VisitChildren(CS);
+ }
+
+ bool VisitReturnStmt(const ReturnStmt *RS) {
+ // A return statement is allowed as long as the return value is trivial.
+ return Visit(RS->getRetValue());
+ }
+
+ bool VisitDeclStmt(const DeclStmt *DS) { return VisitChildren(DS); }
+ bool VisitDoStmt(const DoStmt *DS) { return VisitChildren(DS); }
+ bool VisitIfStmt(const IfStmt *IS) { return VisitChildren(IS); }
+ bool VisitSwitchStmt(const SwitchStmt *SS) { return VisitChildren(SS); }
+ bool VisitCaseStmt(const CaseStmt *CS) { return VisitChildren(CS); }
+ bool VisitDefaultStmt(const DefaultStmt *DS) { return VisitChildren(DS); }
+
+ bool VisitUnaryOperator(const UnaryOperator *UO) {
+ // Operator '*' and '!' are allowed as long as the operand is trivial.
+ if (UO->getOpcode() == UO_Deref || UO->getOpcode() == UO_LNot)
+ return Visit(UO->getSubExpr());
+
+ // Other operators are non-trivial.
+ return false;
+ }
+
+ bool VisitBinaryOperator(const BinaryOperator *BO) {
+ // Binary operators are trivial if their operands are trivial.
+ return Visit(BO->getLHS()) && Visit(BO->getRHS());
+ }
+
+ bool VisitConditionalOperator(const ConditionalOperator *CO) {
+ // Ternary operators are trivial if their conditions & values are trivial.
+ return VisitChildren(CO);
+ }
+
+ bool VisitDeclRefExpr(const DeclRefExpr *DRE) {
+ if (auto *decl = DRE->getDecl()) {
+ if (isa<ParmVarDecl>(decl))
+ return true;
+ }
+ return false;
+ }
+
+ bool VisitStaticAssertDecl(const StaticAssertDecl *SAD) {
+ // Any static_assert is considered trivial.
+ return true;
+ }
+
+ bool VisitCallExpr(const CallExpr *CE) {
+ if (auto *MCE = dyn_cast<CXXMemberCallExpr>(CE))
+ return VisitCXXMemberCallExpr(MCE);
+
+ for (const Expr *Arg : CE->arguments()) {
+ if (Arg && !Visit(Arg))
+ return false;
+ }
+
+ auto *Callee = CE->getDirectCallee();
+ if (!Callee)
+ return false;
+ const auto &Name = safeGetName(Callee);
+
+ if (Name == "WTFCrashWithInfo" || Name == "WTFBreakpointTrap" ||
+ Name == "compilerFenceForCrash" || Name == "__builtin_unreachable")
+ return true;
+
+ return TrivialFunctionAnalysis::isTrivialImpl(Callee, Cache);
+ }
+
+ bool VisitCXXMemberCallExpr(const CXXMemberCallExpr *MCE) {
+ for (const Expr *Arg : MCE->arguments()) {
+ if (Arg && !Visit(Arg))
+ return false;
+ }
+
+ bool TrivialThis = Visit(MCE->getImplicitObjectArgument());
+ if (!TrivialThis)
+ return false;
+
+ std::optional<bool> IsGetterOfRefCounted =
+ isGetterOfRefCounted(MCE->getMethodDecl());
+ if (IsGetterOfRefCounted && *IsGetterOfRefCounted)
+ return true;
+
+ // Recursively descend into the callee to confirm that it's trivial as well.
+ return TrivialFunctionAnalysis::isTrivialImpl(MCE->getDirectCallee(),
+ Cache);
+ }
+
+ bool VisitCXXConstructExpr(const CXXConstructExpr *CE) {
+ for (const Expr *Arg : CE->arguments())
+ if (Arg && !Visit(Arg))
+ return false;
+
+ // Recursively descend into the callee to confirm that it's trivial.
+ return TrivialFunctionAnalysis::isTrivialImpl(CE->getConstructor(), Cache);
+ }
+
+ bool VisitImplicitCastExpr(const ImplicitCastExpr *ICE) {
+ return Visit(ICE->getSubExpr());
+ }
+
+ bool VisitExplicitCastExpr(const ExplicitCastExpr *ECE) {
+ return Visit(ECE->getSubExpr());
+ }
+
+ bool VisitParenExpr(const ParenExpr *PE) { return Visit(PE->getSubExpr()); }
+
+ bool VisitInitListExpr(const InitListExpr *ILE) {
+ for (const Expr *Child : ILE->inits()) {
+ if (Child && !Visit(Child))
+ return false;
+ }
+ return true;
+ }
+
+ bool VisitMemberExpr(const MemberExpr *ME) {
+ // Field access is allowed but the base pointer may itself be non-trivial.
+ return Visit(ME->getBase());
+ }
+
+ bool VisitCXXThisExpr(const CXXThisExpr *CTE) {
+ // The expression 'this' is always trivial, be it explicit or implicit.
+ return true;
+ }
+
+ // Constant literal expressions are always trivial
+ bool VisitIntegerLiteral(const IntegerLiteral *E) { return true; }
+ bool VisitFloatingLiteral(const FloatingLiteral *E) { return true; }
+ bool VisitFixedPointLiteral(const FixedPointLiteral *E) { return true; }
+ bool VisitCharacterLiteral(const CharacterLiteral *E) { return true; }
+ bool VisitStringLiteral(const StringLiteral *E) { return true; }
+
+ bool VisitConstantExpr(const ConstantExpr *CE) {
+ // Constant expressions are trivial.
+ return true;
+ }
+
+private:
+ CacheTy Cache;
+};
+
+bool TrivialFunctionAnalysis::isTrivialImpl(
+ const Decl *D, TrivialFunctionAnalysis::CacheTy &Cache) {
+ // If the function isn't in the cache, conservatively assume that
+ // it's not trivial until analysis completes. This makes every recursive
+ // function non-trivial. This also guarantees that each function
+ // will be scanned at most once.
+ auto [It, IsNew] = Cache.insert(std::make_pair(D, false));
+ if (!IsNew)
+ return It->second;
+
+ const Stmt *Body = D->getBody();
+ if (!Body)
+ return false;
+
+ TrivialFunctionAnalysisVisitor V(Cache);
+ bool Result = V.Visit(Body);
+ if (Result)
+ Cache[D] = true;
+
+ return Result;
+}
+
} // namespace clang
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
index c2c5b74442ba43..e07cd31395747d 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
@@ -10,12 +10,14 @@
#define LLVM_CLANG_ANALYZER_WEBKIT_PTRTYPESEMANTICS_H
#include "llvm/ADT/APInt.h"
+#include "llvm/ADT/DenseMap.h"
#include <optional>
namespace clang {
class CXXBaseSpecifier;
class CXXMethodDecl;
class CXXRecordDecl;
+class Decl;
class FunctionDecl;
class Type;
@@ -60,6 +62,25 @@ std::optional<bool> isGetterOfRefCounted(const clang::CXXMethodDecl* Method);
/// pointer types.
bool isPtrConversion(const FunctionDecl *F);
+/// \returns true if \p F is a static singleton function.
+bool isSingleton(const FunctionDecl *F);
+
+/// An inter-procedural analysis facility that detects functions with "trivial"
+/// behavior with respect to reference counting, such as simple field getters.
+class TrivialFunctionAnalysis {
+public:
+ /// \returns true if \p D is a "trivial" function.
+ bool isTrivial(const Decl *D) const { return isTrivialImpl(D, TheCache); }
+
+private:
+ friend class TrivialFunctionAnalysisVisitor;
+
+ using CacheTy = llvm::DenseMap<const Decl *, bool>;
+ mutable CacheTy TheCache{};
+
+ static bool isTrivialImpl(const Decl *D, CacheTy &Cache);
+};
+
} // namespace clang
#endif
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
index e2e1add31c9b17..17a64e1b1b8e04 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
@@ -32,6 +32,8 @@ class UncountedCallArgsChecker
"WebKit coding guidelines"};
mutable BugReporter *BR;
+ TrivialFunctionAnalysis TFA;
+
public:
void checkASTDecl(const TranslationUnitDecl *TUD, AnalysisManager &MGR,
@@ -134,6 +136,11 @@ class UncountedCallArgsChecker
}
bool shouldSkipCall(const CallExpr *CE) const {
+ const auto *Callee = CE->getDirectCallee();
+
+ if (Callee && TFA.isTrivial(Callee))
+ return true;
+
if (CE->getNumArgs() == 0)
return false;
@@ -155,7 +162,6 @@ class UncountedCallArgsChecker
return false;
}
- const auto *Callee = CE->getDirectCallee();
if (!Callee)
return false;
diff --git a/clang/test/Analysis/Checkers/WebKit/call-args.cpp b/clang/test/Analysis/Checkers/WebKit/call-args.cpp
index e5c49881070420..f2e1f9bc5a2464 100644
--- a/clang/test/Analysis/Checkers/WebKit/call-args.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/call-args.cpp
@@ -2,8 +2,9 @@
#include "mock-types.h"
-RefCountable* provide() { return nullptr; }
-void consume_refcntbl(RefCountable*) {}
+RefCountable* provide();
+void consume_refcntbl(RefCountable*);
+void some_function();
namespace simple {
void foo() {
@@ -19,7 +20,7 @@ namespace simple {
}
namespace multi_arg {
- void consume_refcntbl(int, RefCountable* foo, bool) {}
+ void consume_refcntbl(int, RefCountable* foo, bool);
void foo() {
consume_refcntbl(42, provide(), true);
// expected-warning at -1{{Call argument for parameter 'foo' is uncounted and unsafe}}
@@ -38,8 +39,8 @@ namespace ref_counted {
namespace methods {
struct Consumer {
- void consume_ptr(RefCountable* ptr) {}
- void consume_ref(const RefCountable& ref) {}
+ void consume_ptr(RefCountable* ptr);
+ void consume_ref(const RefCountable& ref);
};
void foo() {
@@ -53,7 +54,7 @@ namespace methods {
void foo2() {
struct Consumer {
- void consume(RefCountable*) { }
+ void consume(RefCountable*) { some_function(); }
void whatever() {
consume(provide());
// expected-warning at -1{{Call argument is uncounted and unsafe}}
@@ -63,7 +64,7 @@ namespace methods {
void foo3() {
struct Consumer {
- void consume(RefCountable*) { }
+ void consume(RefCountable*) { some_function(); }
void whatever() {
this->consume(provide());
// expected-warning at -1{{Call argument is uncounted and unsafe}}
@@ -73,7 +74,7 @@ namespace methods {
}
namespace casts {
- RefCountable* downcast(RefCountable*) { return nullptr; }
+ RefCountable* downcast(RefCountable*);
void foo() {
consume_refcntbl(provide());
@@ -145,8 +146,8 @@ namespace Ref_to_reference_conversion_operator {
}
namespace param_formarding_function {
- void consume_ref_countable_ref(RefCountable&) {}
- void consume_ref_countable_ptr(RefCountable*) {}
+ void consume_ref_countable_ref(RefCountable&);
+ void consume_ref_countable_ptr(RefCountable*);
namespace ptr {
void foo(RefCountable* param) {
@@ -185,8 +186,8 @@ namespace param_formarding_function {
}
namespace param_formarding_lambda {
- auto consume_ref_countable_ref = [](RefCountable&) {};
- auto consume_ref_countable_ptr = [](RefCountable*) {};
+ auto consume_ref_countable_ref = [](RefCountable&) { some_function(); };
+ auto consume_ref_countable_ptr = [](RefCountable*) { some_function(); };
namespace ptr {
void foo(RefCountable* param) {
@@ -304,7 +305,7 @@ namespace string_impl {
namespace default_arg {
RefCountable* global;
- void function_with_default_arg(RefCountable* param = global) {}
+ void function_with_default_arg(RefCountable* param = global);
// expected-warning at -1{{Call argument for parameter 'param' is uncounted and unsafe}}
void foo() {
@@ -315,9 +316,9 @@ namespace default_arg {
namespace cxx_member_operator_call {
// The hidden this-pointer argument without a corresponding parameter caused couple bugs in parameter <-> argument attribution.
struct Foo {
- Foo& operator+(RefCountable* bad) { return *this; }
- friend Foo& operator-(Foo& lhs, RefCountable* bad) { return lhs; }
- void operator()(RefCountable* bad) { }
+ Foo& operator+(RefCountable* bad);
+ friend Foo& operator-(Foo& lhs, RefCountable* bad);
+ void operator()(RefCountable* bad);
};
RefCountable* global;
diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp
index e5e39e3faac714..9ec7f4884377cf 100644
--- a/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp
@@ -1,12 +1,174 @@
// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.webkit.UncountedCallArgsChecker -verify %s
#include "mock-types.h"
+//#include <type_traits>
+
+void WTFBreakpointTrap();
+void WTFCrashWithInfo(int, const char*, const char*, int);
+
+inline void compilerFenceForCrash()
+{
+ asm volatile("" ::: "memory");
+}
+
+inline void isIntegralOrPointerType() { }
+
+template<typename T, typename... Types>
+void isIntegralOrPointerType(T, Types... types)
+{
+ static_assert(sizeof(char) < sizeof(short), "All types need to be bitwise_cast-able to integral type for logging");
+ isIntegralOrPointerType(types...);
+}
+
+#define CRASH_WITH_INFO(...) do { \
+ isIntegralOrPointerType(__VA_ARGS__); \
+ compilerFenceForCrash(); \
+ WTFBreakpointTrap(); \
+ __builtin_unreachable(); \
+} while (0)
+
+#define RELEASE_ASSERT(assertion, ...) do { \
+ if (!(assertion)) \
+ CRASH_WITH_INFO(__VA_ARGS__); \
+} while (0)
+
+#if !defined(NOT_TAIL_CALLED)
+#if __has_attribute(not_tail_called)
+#define NOT_TAIL_CALLED __attribute__((not_tail_called))
+#else
+#define NOT_TAIL_CALLED
+#endif
+#endif
+#define NO_RETURN_DUE_TO_CRASH
+
+#if !defined(ALWAYS_INLINE)
+#define ALWAYS_INLINE inline
+#endif
+
+NO_RETURN_DUE_TO_CRASH NOT_TAIL_CALLED void WTFCrashWithInfoImpl(int line, const char* file, const char* function, int counter, unsigned long reason);
+NO_RETURN_DUE_TO_CRASH NOT_TAIL_CALLED void WTFCrashWithInfo(int line, const char* file, const char* function, int counter);
+
+template<typename T>
+ALWAYS_INLINE unsigned long wtfCrashArg(T* arg) { return reinterpret_cast<unsigned long>(arg); }
+
+template<typename T>
+ALWAYS_INLINE unsigned long wtfCrashArg(T arg) { return arg; }
+
+template<typename T>
+NO_RETURN_DUE_TO_CRASH ALWAYS_INLINE void WTFCrashWithInfo(int line, const char* file, const char* function, int counter, T reason)
+{
+ WTFCrashWithInfoImpl(line, file, function, counter, wtfCrashArg(reason));
+}
+
+class Number {
+public:
+ Number(int v) : v(v) { }
+ Number(double);
+ Number operator+(const Number&);
+private:
+ int v;
+};
class RefCounted {
public:
void ref() const;
void deref() const;
+
void someFunction();
+ int otherFunction();
+
+ int trivial1() { return 123; }
+ float trivial2() { return 0.3; }
+ float trivial3() { return (float)0.4; }
+ float trivial4() { return 0.5f; }
+ char trivial5() { return 'a'; }
+ const char *trivial6() { return "abc"; }
+ int trivial7() { return (1); }
+ Number trivial8() { return Number { 5 }; }
+ int trivial9() { return 3 + 4; }
+ int trivial10() { return 0x1010 | 0x1; }
+ int trivial11(int v) { return v + 1; }
+ const char *trivial12(char *p) { return p ? "str" : "null"; }
+ int trivial13(int v) {
+ if (v)
+ return 123;
+ else
+ return 0;
+ }
+ int trivial14(int v) {
+ switch (v) {
+ case 1:
+ return 100;
+ case 2:
+ return 200;
+ default:
+ return 300;
+ }
+ return 0;
+ }
+ void *trivial15() { return static_cast<void*>(this); }
+ unsigned long trivial16() { return reinterpret_cast<unsigned long>(this); }
+ RefCounted& trivial17() const { return const_cast<RefCounted&>(*this); }
+ RefCounted& trivial18() const { RELEASE_ASSERT(this, "this must be not null"); return const_cast<RefCounted&>(*this); }
+
+ static RefCounted& singleton() {
+ static RefCounted s_RefCounted;
+ s_RefCounted.ref();
+ return s_RefCounted;
+ }
+
+ Number nonTrivial1() { return Number(3) + Number(4); }
+ Number nonTrivial2() { return Number { 0.3 }; }
+ int nonTrivial3() { return v ? otherFunction() : 0; }
+ int nonTrivial4() {
+ if (v)
+ return 8;
+ else
+ return otherFunction();
+ }
+
+ int nonTrivial5() {
+ if (v)
+ return otherFunction();
+ else
+ return 9;
+ }
+
+ int nonTrivial6() {
+ if (otherFunction())
+ return 1;
+ else
+ return 0;
+ }
+
+ int nonTrivial7() {
+ switch (v) {
+ case 1:
+ return otherFunction();
+ default:
+ return 7;
+ }
+ }
+
+ int nonTrivial8() {
+ switch (v) {
+ case 1:
+ return 9;
+ default:
+ return otherFunction();
+ }
+ }
+
+ int nonTrivial9() {
+ switch (otherFunction()) {
+ case 0:
+ return -1;
+ default:
+ return 12;
+ }
+ }
+
+ unsigned v { 0 };
};
RefCounted* refCountedObj();
@@ -16,3 +178,72 @@ void test()
refCountedObj()->someFunction();
// expected-warning at -1{{Call argument for 'this' parameter is uncounted and unsafe}}
}
+
+class UnrelatedClass {
+ RefPtr<RefCounted> Field;
+ bool value;
+
+public:
+ RefCounted &getFieldTrivial() { return *Field.get(); }
+ RefCounted *getFieldTernary() { return value ? Field.get() : nullptr; }
+
+ void test() {
+ getFieldTrivial().trivial1(); // no-warning
+ getFieldTrivial().trivial2(); // no-warning
+ getFieldTrivial().trivial3(); // no-warning
+ getFieldTrivial().trivial4(); // no-warning
+ getFieldTrivial().trivial5(); // no-warning
+ getFieldTrivial().trivial6(); // no-warning
+ getFieldTrivial().trivial7(); // no-warning
+ getFieldTrivial().trivial8(); // no-warning
+ getFieldTrivial().trivial9(); // no-warning
+ getFieldTrivial().trivial10(); // no-warning
+ getFieldTrivial().trivial11(1); // no-warning
+ getFieldTrivial().trivial12(nullptr); // no-warning
+ getFieldTrivial().trivial13(0); // no-warning
+ getFieldTrivial().trivial14(3); // no-warning
+ getFieldTrivial().trivial15(); // no-warning
+ getFieldTrivial().trivial16(); // no-warning
+ getFieldTrivial().trivial17(); // no-warning
+ getFieldTrivial().trivial18(); // no-warning
+ RefCounted::singleton().trivial18(); // no-warning
+ RefCounted::singleton().someFunction(); // no-warning
+
+ getFieldTrivial().someFunction();
+ // expected-warning at -1{{Call argument for 'this' parameter is uncounted and unsafe}}
+ getFieldTrivial().nonTrivial1();
+ // expected-warning at -1{{Call argument for 'this' parameter is uncounted and unsafe}}
+ getFieldTrivial().nonTrivial2();
+ // expected-warning at -1{{Call argument for 'this' parameter is uncounted and unsafe}}
+ getFieldTrivial().nonTrivial3();
+ // expected-warning at -1{{Call argument for 'this' parameter is uncounted and unsafe}}
+ getFieldTrivial().nonTrivial4();
+ // expected-warning at -1{{Call argument for 'this' parameter is uncounted and unsafe}}
+ getFieldTrivial().nonTrivial5();
+ // expected-warning at -1{{Call argument for 'this' parameter is uncounted and unsafe}}
+ getFieldTrivial().nonTrivial6();
+ // expected-warning at -1{{Call argument for 'this' parameter is uncounted and unsafe}}
+ getFieldTrivial().nonTrivial7();
+ // expected-warning at -1{{Call argument for 'this' parameter is uncounted and unsafe}}
+ getFieldTrivial().nonTrivial8();
+ // expected-warning at -1{{Call argument for 'this' parameter is uncounted and unsafe}}
+ getFieldTrivial().nonTrivial9();
+ // expected-warning at -1{{Call argument for 'this' parameter is uncounted and unsafe}}
+ }
+};
+
+class UnrelatedClass2 {
+ RefPtr<UnrelatedClass> Field;
+
+public:
+ UnrelatedClass &getFieldTrivial() { return *Field.get(); }
+ RefCounted &getFieldTrivialRecursively() { return getFieldTrivial().getFieldTrivial(); }
+ RefCounted *getFieldTrivialTernary() { return Field ? Field->getFieldTernary() : nullptr; }
+
+ void test() {
+ getFieldTrivialRecursively().trivial1(); // no-warning
+ getFieldTrivialTernary()->trivial2(); // no-warning
+ getFieldTrivialRecursively().someFunction();
+ // expected-warning at -1{{Call argument for 'this' parameter is uncounted and unsafe}}
+ }
+};
>From b041a768b647cee55947db3d61852c9a16c7c979 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Wed, 14 Feb 2024 17:57:00 -0800
Subject: [PATCH 2/5] Fix a crash when evaluating a function with void return
type.
---
.../Checkers/WebKit/PtrTypesSemantics.cpp | 27 ++++++++++---------
.../Checkers/WebKit/uncounted-obj-arg.cpp | 2 ++
2 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index 17f59c2d304ddc..9449f670c4d1bf 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -271,7 +271,9 @@ class TrivialFunctionAnalysisVisitor
bool VisitReturnStmt(const ReturnStmt *RS) {
// A return statement is allowed as long as the return value is trivial.
- return Visit(RS->getRetValue());
+ if (auto *RV = RS->getRetValue())
+ return Visit(RV);
+ return true;
}
bool VisitDeclStmt(const DeclStmt *DS) { return VisitChildren(DS); }
@@ -314,13 +316,8 @@ class TrivialFunctionAnalysisVisitor
}
bool VisitCallExpr(const CallExpr *CE) {
- if (auto *MCE = dyn_cast<CXXMemberCallExpr>(CE))
- return VisitCXXMemberCallExpr(MCE);
-
- for (const Expr *Arg : CE->arguments()) {
- if (Arg && !Visit(Arg))
- return false;
- }
+ if (!checkArguments(CE))
+ return false;
auto *Callee = CE->getDirectCallee();
if (!Callee)
@@ -335,10 +332,8 @@ class TrivialFunctionAnalysisVisitor
}
bool VisitCXXMemberCallExpr(const CXXMemberCallExpr *MCE) {
- for (const Expr *Arg : MCE->arguments()) {
- if (Arg && !Visit(Arg))
- return false;
- }
+ if (!checkArguments(MCE))
+ return false;
bool TrivialThis = Visit(MCE->getImplicitObjectArgument());
if (!TrivialThis)
@@ -353,6 +348,14 @@ class TrivialFunctionAnalysisVisitor
return TrivialFunctionAnalysis::isTrivialImpl(MCE->getDirectCallee(),
Cache);
}
+
+ bool checkArguments(const CallExpr *CE) {
+ for (const Expr *Arg : CE->arguments()) {
+ if (Arg && !Visit(Arg))
+ return false;
+ }
+ return true;
+ }
bool VisitCXXConstructExpr(const CXXConstructExpr *CE) {
for (const Expr *Arg : CE->arguments())
diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp
index 9ec7f4884377cf..156a2480901bf0 100644
--- a/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp
@@ -110,6 +110,7 @@ class RefCounted {
unsigned long trivial16() { return reinterpret_cast<unsigned long>(this); }
RefCounted& trivial17() const { return const_cast<RefCounted&>(*this); }
RefCounted& trivial18() const { RELEASE_ASSERT(this, "this must be not null"); return const_cast<RefCounted&>(*this); }
+ void trivial19() const { return; }
static RefCounted& singleton() {
static RefCounted s_RefCounted;
@@ -206,6 +207,7 @@ class UnrelatedClass {
getFieldTrivial().trivial16(); // no-warning
getFieldTrivial().trivial17(); // no-warning
getFieldTrivial().trivial18(); // no-warning
+ getFieldTrivial().trivial19(); // no-warning
RefCounted::singleton().trivial18(); // no-warning
RefCounted::singleton().someFunction(); // no-warning
>From 18bc98140e6176ed478cbead19e41e55f5a4bcae Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Wed, 14 Feb 2024 18:07:02 -0800
Subject: [PATCH 3/5] Fix formatting.
---
clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index 9449f670c4d1bf..a27d83dbd60faf 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -348,7 +348,7 @@ class TrivialFunctionAnalysisVisitor
return TrivialFunctionAnalysis::isTrivialImpl(MCE->getDirectCallee(),
Cache);
}
-
+
bool checkArguments(const CallExpr *CE) {
for (const Expr *Arg : CE->arguments()) {
if (Arg && !Visit(Arg))
>From 37caff626ff08f0752651b5dd81f66e13f942555 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Wed, 14 Feb 2024 18:19:29 -0800
Subject: [PATCH 4/5] Fix a crash in VisitCXXMemberCallExpr.
---
.../Checkers/WebKit/PtrTypesSemantics.cpp | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index a27d83dbd60faf..84af578f254d14 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -339,14 +339,16 @@ class TrivialFunctionAnalysisVisitor
if (!TrivialThis)
return false;
- std::optional<bool> IsGetterOfRefCounted =
- isGetterOfRefCounted(MCE->getMethodDecl());
+ auto* Callee = MCE->getMethodDecl();
+ if (!Callee)
+ return false;
+
+ std::optional<bool> IsGetterOfRefCounted = isGetterOfRefCounted(Callee);
if (IsGetterOfRefCounted && *IsGetterOfRefCounted)
return true;
// Recursively descend into the callee to confirm that it's trivial as well.
- return TrivialFunctionAnalysis::isTrivialImpl(MCE->getDirectCallee(),
- Cache);
+ return TrivialFunctionAnalysis::isTrivialImpl(Callee, Cache);
}
bool checkArguments(const CallExpr *CE) {
@@ -358,9 +360,10 @@ class TrivialFunctionAnalysisVisitor
}
bool VisitCXXConstructExpr(const CXXConstructExpr *CE) {
- for (const Expr *Arg : CE->arguments())
+ for (const Expr *Arg : CE->arguments()) {
if (Arg && !Visit(Arg))
return false;
+ }
// Recursively descend into the callee to confirm that it's trivial.
return TrivialFunctionAnalysis::isTrivialImpl(CE->getConstructor(), Cache);
>From af66c777fd07e261c2e6b68595bb6a96d45801e6 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa <rniwa at webkit.org>
Date: Wed, 14 Feb 2024 18:25:19 -0800
Subject: [PATCH 5/5] Fix formatting (2).
---
clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
index 84af578f254d14..bf6f9a64877c64 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
@@ -339,7 +339,7 @@ class TrivialFunctionAnalysisVisitor
if (!TrivialThis)
return false;
- auto* Callee = MCE->getMethodDecl();
+ auto *Callee = MCE->getMethodDecl();
if (!Callee)
return false;
More information about the cfe-commits
mailing list