[clang] [alpha.webkit.UncountedCallArgsChecker] Check the safety of the object argument in a member function call. (PR #81400)

2024-02-14 Thread Artem Dergachev via cfe-commits 

https://github.com/haoNoQ closed https://github.com/llvm/llvm-project/pull/81400
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [alpha.webkit.UncountedCallArgsChecker] Check the safety of the object argument in a member function call. (PR #81400)

2024-02-13 Thread Ryosuke Niwa via cfe-commits 

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 
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(CE) && 
isa_and_nonnull(F);
 
+  if (auto *MemberCallExpr = dyn_cast(CE)) {
+auto *E = MemberCallExpr->getImplicitObjectArgument();
+auto *ArgType = MemberCallExpr->getObjectType().getTypePtrOrNull();
+std::optional 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(Arg))
   Arg = defaultArg->getExpr();
 
-std::pair 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(ArgOrigin.first)) {
-  // foo(nullptr)
-  continue;
-}
-if (isa(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 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(ArgOrigin.first)) {
+  // foo(nullptr)
+  return true;
+}
+if (isa(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(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 00..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@-1{{Call argument for 'this' parameter is uncounted and 
unsafe}}
+}

>From c15967f8f09198c01aec8b0161cc08484c78f66a Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa 
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 chang

[clang] [alpha.webkit.UncountedCallArgsChecker] Check the safety of the object argument in a member function call. (PR #81400)

2024-02-13 Thread Ryosuke Niwa via cfe-commits 

rniwa wrote:

Rebased.

https://github.com/llvm/llvm-project/pull/81400
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [alpha.webkit.UncountedCallArgsChecker] Check the safety of the object argument in a member function call. (PR #81400)

2024-02-13 Thread Ryosuke Niwa via cfe-commits 

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 
Date: Tue, 13 Feb 2024 20:05:18 -0800
Subject: [PATCH 1/2] [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(CE) && 
isa_and_nonnull(F);
 
+  if (auto *MemberCallExpr = dyn_cast(CE)) {
+auto *E = MemberCallExpr->getImplicitObjectArgument();
+auto *ArgType = MemberCallExpr->getObjectType().getTypePtrOrNull();
+std::optional 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(Arg))
   Arg = defaultArg->getExpr();
 
-std::pair 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(ArgOrigin.first)) {
-  // foo(nullptr)
-  continue;
-}
-if (isa(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 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(ArgOrigin.first)) {
+  // foo(nullptr)
+  return true;
+}
+if (isa(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(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 00..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@-1{{Call argument for 'this' parameter is uncounted and 
unsafe}}
+}

>From c15967f8f09198c01aec8b0161cc08484c78f66a Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa 
Date: Tue, 13 Feb 2024 19:36:09 -0800
Subject: [PATCH 2/2] Address the review comment. Namely avoid calling
 getTypePtrOrNull and don't use raw_svector_ostream.

---
 .../Checkers/WebKit/UncountedCallArgsChecker.cpp | 9 ++---
 1 file chang

[clang] [alpha.webkit.UncountedCallArgsChecker] Check the safety of the object argument in a member function call. (PR #81400)

2024-02-13 Thread Artem Dergachev via cfe-commits 

https://github.com/haoNoQ approved this pull request.

Great! Needs a rebase now though.

https://github.com/llvm/llvm-project/pull/81400
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [alpha.webkit.UncountedCallArgsChecker] Check the safety of the object argument in a member function call. (PR #81400)

2024-02-13 Thread Ryosuke Niwa via cfe-commits 


@@ -183,6 +196,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.";

rniwa wrote:

Good point. Fixed.

https://github.com/llvm/llvm-project/pull/81400
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [alpha.webkit.UncountedCallArgsChecker] Check the safety of the object argument in a member function call. (PR #81400)

2024-02-13 Thread Ryosuke Niwa via cfe-commits 

https://github.com/rniwa updated https://github.com/llvm/llvm-project/pull/81400

>From 04e18254efc4f671e0bbd9625c7e994fe47c1636 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa 
Date: Sun, 11 Feb 2024 00:07:30 -0800
Subject: [PATCH 1/2] [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 31ccae8b097b89..cda96b70ea8735 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(CE) && 
isa_and_nonnull(F);
 
+  if (auto *MemberCallExpr = dyn_cast(CE)) {
+auto *E = MemberCallExpr->getImplicitObjectArgument();
+auto *ArgType = MemberCallExpr->getObjectType().getTypePtrOrNull();
+std::optional 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
@@ -91,25 +100,7 @@ class UncountedCallArgsChecker
 
 const auto *Arg = CE->getArg(ArgIdx);
 
-std::pair 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(ArgOrigin.first)) {
-  // foo(nullptr)
-  continue;
-}
-if (isa(ArgOrigin.first)) {
-  // FIXME: Check the value.
-  // foo(NULL)
-  continue;
-}
-
-if (isASafeCallArg(ArgOrigin.first))
+if (isPtrOriginSafe(Arg))
   continue;
 
 reportBug(Arg, *P);
@@ -117,6 +108,28 @@ class UncountedCallArgsChecker
 }
   }
 
+  bool isPtrOriginSafe(const Expr *Arg) const {
+std::pair 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(ArgOrigin.first)) {
+  // foo(nullptr)
+  return true;
+}
+if (isa(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;
@@ -183,6 +196,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(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 00..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@-1{{Call argument for 'this' parameter is uncounted and 
unsafe}}
+}

>From 2d780389a46e5c053053c1432d2ec3af2a7653e9 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa 
Date: Tue, 13 Feb 2024 19:36:09 -0800
Subject: [PATCH 2/2] 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(-)

d

[clang] [alpha.webkit.UncountedCallArgsChecker] Check the safety of the object argument in a member function call. (PR #81400)

2024-02-13 Thread Ryosuke Niwa via cfe-commits 


@@ -70,6 +70,15 @@ class UncountedCallArgsChecker
   // or std::function call operator).
   unsigned ArgIdx = isa(CE) && 
isa_and_nonnull(F);
 
+  if (auto *MemberCallExpr = dyn_cast(CE)) {
+auto *E = MemberCallExpr->getImplicitObjectArgument();
+auto *ArgType = MemberCallExpr->getObjectType().getTypePtrOrNull();

rniwa wrote:

Ah, okay. Will do that.

https://github.com/llvm/llvm-project/pull/81400
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [alpha.webkit.UncountedCallArgsChecker] Check the safety of the object argument in a member function call. (PR #81400)

2024-02-13 Thread Artem Dergachev via cfe-commits 


@@ -183,6 +196,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.";

haoNoQ wrote:

This is a constant string, no need to stream it, just put it straight into the 
`BugReport` constructor.

https://github.com/llvm/llvm-project/pull/81400
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [alpha.webkit.UncountedCallArgsChecker] Check the safety of the object argument in a member function call. (PR #81400)

2024-02-13 Thread Artem Dergachev via cfe-commits 


@@ -70,6 +70,15 @@ class UncountedCallArgsChecker
   // or std::function call operator).
   unsigned ArgIdx = isa(CE) && 
isa_and_nonnull(F);
 
+  if (auto *MemberCallExpr = dyn_cast(CE)) {
+auto *E = MemberCallExpr->getImplicitObjectArgument();
+auto *ArgType = MemberCallExpr->getObjectType().getTypePtrOrNull();

haoNoQ wrote:

```suggestion
QualType ArgType = MemberCallExpr->getObjectType();
```

`getTypePtrOrNull()` is typically redundant; `QualType` provides an overloaded 
`operator ->()` that allows you to call methods on the underlying unqualified 
`Type` directly.

https://github.com/llvm/llvm-project/pull/81400
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [alpha.webkit.UncountedCallArgsChecker] Check the safety of the object argument in a member function call. (PR #81400)

2024-02-12 Thread Ryosuke Niwa via cfe-commits 

https://github.com/rniwa updated https://github.com/llvm/llvm-project/pull/81400

>From 04e18254efc4f671e0bbd9625c7e994fe47c1636 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa 
Date: Sun, 11 Feb 2024 00:07:30 -0800
Subject: [PATCH] [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 31ccae8b097b89..cda96b70ea8735 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(CE) && 
isa_and_nonnull(F);
 
+  if (auto *MemberCallExpr = dyn_cast(CE)) {
+auto *E = MemberCallExpr->getImplicitObjectArgument();
+auto *ArgType = MemberCallExpr->getObjectType().getTypePtrOrNull();
+std::optional 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
@@ -91,25 +100,7 @@ class UncountedCallArgsChecker
 
 const auto *Arg = CE->getArg(ArgIdx);
 
-std::pair 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(ArgOrigin.first)) {
-  // foo(nullptr)
-  continue;
-}
-if (isa(ArgOrigin.first)) {
-  // FIXME: Check the value.
-  // foo(NULL)
-  continue;
-}
-
-if (isASafeCallArg(ArgOrigin.first))
+if (isPtrOriginSafe(Arg))
   continue;
 
 reportBug(Arg, *P);
@@ -117,6 +108,28 @@ class UncountedCallArgsChecker
 }
   }
 
+  bool isPtrOriginSafe(const Expr *Arg) const {
+std::pair 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(ArgOrigin.first)) {
+  // foo(nullptr)
+  return true;
+}
+if (isa(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;
@@ -183,6 +196,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(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 00..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@-1{{Call argument for 'this' parameter is uncounted and 
unsafe}}
+}

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [alpha.webkit.UncountedCallArgsChecker] Check the safety of the object argument in a member function call. (PR #81400)

2024-02-11 Thread Ryosuke Niwa via cfe-commits 

https://github.com/rniwa updated https://github.com/llvm/llvm-project/pull/81400

>From c8ac9ad6ecd95a3dbd023458a572b08a4664de03 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa 
Date: Sun, 11 Feb 2024 00:07:30 -0800
Subject: [PATCH] [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. 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.
---
 .../Checkers/WebKit/ASTUtils.cpp  |  2 +-
 .../WebKit/UncountedCallArgsChecker.cpp   | 67 +--
 .../WebKit/call-args-inside-if-statement.cpp  | 20 ++
 3 files changed, 69 insertions(+), 20 deletions(-)
 create mode 100644 
clang/test/Analysis/Checkers/WebKit/call-args-inside-if-statement.cpp

diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp 
b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
index 4526fac64735bf..da0d52e361c946 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
@@ -85,7 +85,7 @@ bool isASafeCallArg(const Expr *E) {
   assert(E);
   if (auto *Ref = dyn_cast(E)) {
 if (auto *D = dyn_cast_or_null(Ref->getFoundDecl())) {
-  if (isa(D) || D->isLocalVarDecl())
+  if (isa(D))
 return true;
 }
   }
diff --git 
a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
index 31ccae8b097b89..cda96b70ea8735 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(CE) && 
isa_and_nonnull(F);
 
+  if (auto *MemberCallExpr = dyn_cast(CE)) {
+auto *E = MemberCallExpr->getImplicitObjectArgument();
+auto *ArgType = MemberCallExpr->getObjectType().getTypePtrOrNull();
+std::optional 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
@@ -91,25 +100,7 @@ class UncountedCallArgsChecker
 
 const auto *Arg = CE->getArg(ArgIdx);
 
-std::pair 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(ArgOrigin.first)) {
-  // foo(nullptr)
-  continue;
-}
-if (isa(ArgOrigin.first)) {
-  // FIXME: Check the value.
-  // foo(NULL)
-  continue;
-}
-
-if (isASafeCallArg(ArgOrigin.first))
+if (isPtrOriginSafe(Arg))
   continue;
 
 reportBug(Arg, *P);
@@ -117,6 +108,28 @@ class UncountedCallArgsChecker
 }
   }
 
+  bool isPtrOriginSafe(const Expr *Arg) const {
+std::pair 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(ArgOrigin.first)) {
+  // foo(nullptr)
+  return true;
+}
+if (isa(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;
@@ -183,6 +196,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(Bug, Os.str(), BSLoc);
+Report->addRange(CallArg->getSourceRange());
+BR->emitReport(std::move(Report));
+  }
 };
 } // namespace
 
diff --git 
a/clang/test/Analysis/Checkers/WebKit/call-args-inside-if-statement.cpp 
b/clang/test/Analysis/Checkers/WebKit/call-args-inside-if-statement.cpp
new fi

[clang] [alpha.webkit.UncountedCallArgsChecker] Check the safety of the object argument in a member function call. (PR #81400)

2024-02-11 Thread Ryosuke Niwa via cfe-commits 

https://github.com/rniwa updated https://github.com/llvm/llvm-project/pull/81400

>From 16d5c240639cdf25b824f5a1a60c256d33fe3565 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa 
Date: Sun, 11 Feb 2024 00:07:30 -0800
Subject: [PATCH] [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. 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.
---
 .../Checkers/WebKit/ASTUtils.cpp  |  2 +-
 .../WebKit/UncountedCallArgsChecker.cpp   | 66 +--
 .../WebKit/call-args-inside-if-statement.cpp  | 20 ++
 3 files changed, 68 insertions(+), 20 deletions(-)
 create mode 100644 
clang/test/Analysis/Checkers/WebKit/call-args-inside-if-statement.cpp

diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp 
b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
index 4526fac64735bf..da0d52e361c946 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
@@ -85,7 +85,7 @@ bool isASafeCallArg(const Expr *E) {
   assert(E);
   if (auto *Ref = dyn_cast(E)) {
 if (auto *D = dyn_cast_or_null(Ref->getFoundDecl())) {
-  if (isa(D) || D->isLocalVarDecl())
+  if (isa(D))
 return true;
 }
   }
diff --git 
a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
index 31ccae8b097b89..170ff7f531f9e9 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
@@ -70,6 +70,14 @@ class UncountedCallArgsChecker
   // or std::function call operator).
   unsigned ArgIdx = isa(CE) && 
isa_and_nonnull(F);
 
+  if (auto *MemberCallExpr = dyn_cast(CE)) {
+auto *E = MemberCallExpr->getImplicitObjectArgument();
+auto *ArgType = MemberCallExpr->getObjectType().getTypePtrOrNull();
+std::optional 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
@@ -91,25 +99,7 @@ class UncountedCallArgsChecker
 
 const auto *Arg = CE->getArg(ArgIdx);
 
-std::pair 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(ArgOrigin.first)) {
-  // foo(nullptr)
-  continue;
-}
-if (isa(ArgOrigin.first)) {
-  // FIXME: Check the value.
-  // foo(NULL)
-  continue;
-}
-
-if (isASafeCallArg(ArgOrigin.first))
+if (isPtrOriginSafe(Arg))
   continue;
 
 reportBug(Arg, *P);
@@ -117,6 +107,28 @@ class UncountedCallArgsChecker
 }
   }
 
+  bool isPtrOriginSafe(const Expr *Arg) const {
+std::pair 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(ArgOrigin.first)) {
+  // foo(nullptr)
+  return true;
+}
+if (isa(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;
@@ -183,6 +195,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(Bug, Os.str(), BSLoc);
+Report->addRange(CallArg->getSourceRange());
+BR->emitReport(std::move(Report));
+  }
 };
 } // namespace
 
diff --git 
a/clang/test/Analysis/Checkers/WebKit/call-args-inside-if-statement.cpp 
b/clang/test/Analysis/Checkers/WebKit/call-args-inside-if-statement.cpp
new file mode 10064

[clang] [alpha.webkit.UncountedCallArgsChecker] Check the safety of the object argument in a member function call. (PR #81400)

2024-02-11 Thread via cfe-commits 

github-actions[bot] wrote:




:warning: C/C++ code formatter, clang-format found issues in your code. 
:warning:



You can test this locally with the following command:


``bash
git-clang-format --diff 8ea7f1d20ad8ab8c381800eefda948d85c6860cc 
b7121ce4f2ef69b4a410f2399fbd9d9525156b93 -- 
clang/test/Analysis/Checkers/WebKit/call-args-inside-if-statement.cpp 
clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp 
clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
``





View the diff from clang-format here.


``diff
diff --git 
a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
index fa6aeb4741..cda96b70ea 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
@@ -70,10 +70,11 @@ public:
   // or std::function call operator).
   unsigned ArgIdx = isa(CE) && 
isa_and_nonnull(F);
 
-  if (auto* MemberCallExpr = dyn_cast(CE)) {
+  if (auto *MemberCallExpr = dyn_cast(CE)) {
 auto *E = MemberCallExpr->getImplicitObjectArgument();
 auto *ArgType = MemberCallExpr->getObjectType().getTypePtrOrNull();
-std::optional IsUncounted = 
isUncounted(ArgType->getAsCXXRecordDecl());
+std::optional IsUncounted =
+isUncounted(ArgType->getAsCXXRecordDecl());
 if (IsUncounted && *IsUncounted && !isPtrOriginSafe(E))
   reportBugOnThis(E);
   }
@@ -98,7 +99,7 @@ public:
   continue;
 
 const auto *Arg = CE->getArg(ArgIdx);
-
+
 if (isPtrOriginSafe(Arg))
   continue;
 
@@ -106,7 +107,7 @@ public:
   }
 }
   }
-  
+
   bool isPtrOriginSafe(const Expr *Arg) const {
 std::pair ArgOrigin =
 tryToFindPtrOrigin(Arg, true);

``




https://github.com/llvm/llvm-project/pull/81400
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [alpha.webkit.UncountedCallArgsChecker] Check the safety of the object argument in a member function call. (PR #81400)

2024-02-11 Thread via cfe-commits 

llvmbot wrote:



@llvm/pr-subscribers-clang-static-analyzer-1

@llvm/pr-subscribers-clang

Author: Ryosuke Niwa (rniwa)


Changes

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.

---
Full diff: https://github.com/llvm/llvm-project/pull/81400.diff


3 Files Affected:

- (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp (+1-1) 
- (modified) 
clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp (+48-20) 
- (added) clang/test/Analysis/Checkers/WebKit/call-args-inside-if-statement.cpp 
(+20) 


``diff
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp 
b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
index 4526fac64735bf..da0d52e361c946 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
@@ -85,7 +85,7 @@ bool isASafeCallArg(const Expr *E) {
   assert(E);
   if (auto *Ref = dyn_cast(E)) {
 if (auto *D = dyn_cast_or_null(Ref->getFoundDecl())) {
-  if (isa(D) || D->isLocalVarDecl())
+  if (isa(D))
 return true;
 }
   }
diff --git 
a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
index 31ccae8b097b89..fa6aeb4741d0b7 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
@@ -70,6 +70,14 @@ class UncountedCallArgsChecker
   // or std::function call operator).
   unsigned ArgIdx = isa(CE) && 
isa_and_nonnull(F);
 
+  if (auto* MemberCallExpr = dyn_cast(CE)) {
+auto *E = MemberCallExpr->getImplicitObjectArgument();
+auto *ArgType = MemberCallExpr->getObjectType().getTypePtrOrNull();
+std::optional 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
@@ -90,32 +98,36 @@ class UncountedCallArgsChecker
   continue;
 
 const auto *Arg = CE->getArg(ArgIdx);
-
-std::pair 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(ArgOrigin.first)) {
-  // foo(nullptr)
-  continue;
-}
-if (isa(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 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(ArgOrigin.first)) {
+  // foo(nullptr)
+  return true;
+}
+if (isa(ArgOrigin.first)) {
+  // FIXME: Check the value.
+  // foo(NULL)
+  return true;
+}
+
+return isASafeCallArg(ArgOrigin.first);
+  }
 
   bool shouldSkipCall(const CallExpr *CE) const {
 if (CE->getNumArgs() == 0)
@@ -183,6 +195,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(Bug, Os.str(), BSLoc);
+Report->addRange(CallArg->getSourceRange());
+BR->emitReport(std::move(Report));
+  }
 };
 } // namespace
 
diff --git 
a/clang/test/Analysis/Checkers/WebKit/call-args-inside-if-statement.cpp 
b/clang/test/Analysis/Checkers/WebKit/call-args-inside-if-statement.cpp
new file mode 100644
index 00..6f7c959b2fccca
--- /dev/null
+++ b/clang/test/Analysis/Checkers/WebKit/call-args-inside-if-statement.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_analyze_cc1 
-analyzer-checker=alpha.webkit

[clang] [alpha.webkit.UncountedCallArgsChecker] Check the safety of the object argument in a member function call. (PR #81400)

2024-02-11 Thread Ryosuke Niwa via cfe-commits 

https://github.com/rniwa created https://github.com/llvm/llvm-project/pull/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.

>From b7121ce4f2ef69b4a410f2399fbd9d9525156b93 Mon Sep 17 00:00:00 2001
From: Ryosuke Niwa 
Date: Sun, 11 Feb 2024 00:07:30 -0800
Subject: [PATCH] [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. 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.
---
 .../Checkers/WebKit/ASTUtils.cpp  |  2 +-
 .../WebKit/UncountedCallArgsChecker.cpp   | 68 +--
 .../WebKit/call-args-inside-if-statement.cpp  | 20 ++
 3 files changed, 69 insertions(+), 21 deletions(-)
 create mode 100644 
clang/test/Analysis/Checkers/WebKit/call-args-inside-if-statement.cpp

diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp 
b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
index 4526fac64735bf..da0d52e361c946 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
@@ -85,7 +85,7 @@ bool isASafeCallArg(const Expr *E) {
   assert(E);
   if (auto *Ref = dyn_cast(E)) {
 if (auto *D = dyn_cast_or_null(Ref->getFoundDecl())) {
-  if (isa(D) || D->isLocalVarDecl())
+  if (isa(D))
 return true;
 }
   }
diff --git 
a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
index 31ccae8b097b89..fa6aeb4741d0b7 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
@@ -70,6 +70,14 @@ class UncountedCallArgsChecker
   // or std::function call operator).
   unsigned ArgIdx = isa(CE) && 
isa_and_nonnull(F);
 
+  if (auto* MemberCallExpr = dyn_cast(CE)) {
+auto *E = MemberCallExpr->getImplicitObjectArgument();
+auto *ArgType = MemberCallExpr->getObjectType().getTypePtrOrNull();
+std::optional 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
@@ -90,32 +98,36 @@ class UncountedCallArgsChecker
   continue;
 
 const auto *Arg = CE->getArg(ArgIdx);
-
-std::pair 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(ArgOrigin.first)) {
-  // foo(nullptr)
-  continue;
-}
-if (isa(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 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(ArgOrigin.first)) {
+  // foo(nullptr)
+  return true;
+}
+if (isa(ArgOrigin.first)) {
+  // FIXME: Check the value.
+  // foo(NULL)
+  return true;
+}
+
+return isASafeCallArg(ArgOrigin.first);
+  }
 
   bool shouldSkipCall(const CallExpr *CE) const {
 if (CE->getNumArgs() == 0)
@@ -183,6 +195,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