[clang] [analyzer] Support determining the orign in a conditional operator in WebKit checkers. (PR #91143)

via cfe-commits cfe-commits at lists.llvm.org
Sun May 5 13:50:52 PDT 2024


github-actions[bot] wrote:

<!--LLVM CODE FORMAT COMMENT: {clang-format}-->


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

<details>
<summary>
You can test this locally with the following command:
</summary>

``````````bash
git-clang-format --diff a312dd68c0ce368313164eb92cbdd95192afa3f8 ae2d8006c7f9237ed4e79cb3b729c4ce55a24b35 -- clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp clang/test/Analysis/Checkers/WebKit/call-args.cpp clang/test/Analysis/Checkers/WebKit/uncounted-local-vars.cpp
``````````

</details>

<details>
<summary>
View the diff from clang-format here.
</summary>

``````````diff
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
index f2e531837a..3e4bb276bb 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.cpp
@@ -16,8 +16,9 @@
 
 namespace clang {
 
-bool tryToFindPtrOrigin(const Expr *E, bool StopAtFirstRefCountedObj,
-  std::function<bool(const clang::Expr *, bool)> callback) {
+bool tryToFindPtrOrigin(
+    const Expr *E, bool StopAtFirstRefCountedObj,
+    std::function<bool(const clang::Expr *, bool)> callback) {
   while (E) {
     if (auto *tempExpr = dyn_cast<MaterializeTemporaryExpr>(E)) {
       E = tempExpr->getSubExpr();
@@ -27,10 +28,11 @@ bool tryToFindPtrOrigin(const Expr *E, bool StopAtFirstRefCountedObj,
       E = tempExpr->getSubExpr();
       continue;
     }
-    if (auto* Expr = dyn_cast<ConditionalOperator>(E)) {
+    if (auto *Expr = dyn_cast<ConditionalOperator>(E)) {
       return tryToFindPtrOrigin(Expr->getTrueExpr(), StopAtFirstRefCountedObj,
-        callback) && tryToFindPtrOrigin(Expr->getFalseExpr(),
-        StopAtFirstRefCountedObj, callback);
+                                callback) &&
+             tryToFindPtrOrigin(Expr->getFalseExpr(), StopAtFirstRefCountedObj,
+                                callback);
     }
     if (auto *cast = dyn_cast<CastExpr>(E)) {
       if (StopAtFirstRefCountedObj) {
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h
index cfcc046dc9..e972924e0c 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h
@@ -52,8 +52,9 @@ class Expr;
 /// Calls \p callback with the subexpression that we traversed to and if \p
 /// StopAtFirstRefCountedObj is true we also specify whether we stopped early.
 /// Returns false if any of calls to callbacks returned false. Otherwise true.
-bool tryToFindPtrOrigin(const clang::Expr *E, bool StopAtFirstRefCountedObj,
-  std::function<bool(const clang::Expr *, bool)> callback);
+bool tryToFindPtrOrigin(
+    const clang::Expr *E, bool StopAtFirstRefCountedObj,
+    std::function<bool(const clang::Expr *, bool)> callback);
 
 /// For \p E referring to a ref-countable/-counted pointer/reference we return
 /// whether it's a safe call argument. Examples: function parameter or
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
index cf98e2d213..658a23e255 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
@@ -127,22 +127,22 @@ public:
 
   bool isPtrOriginSafe(const Expr *Arg) const {
     return tryToFindPtrOrigin(Arg, /*StopAtFirstRefCountedObj=*/true,
-        [](const clang::Expr* ArgOrigin, bool IsSafe) {
-      if (IsSafe)
-        return true;
-      if (isa<CXXNullPtrLiteralExpr>(ArgOrigin)) {
-        // foo(nullptr)
-        return true;
-      }
-      if (isa<IntegerLiteral>(ArgOrigin)) {
-        // FIXME: Check the value.
-        // foo(NULL)
-        return true;
-      }
-      if (isASafeCallArg(ArgOrigin))
-        return true;
-      return false;
-    });
+                              [](const clang::Expr *ArgOrigin, bool IsSafe) {
+                                if (IsSafe)
+                                  return true;
+                                if (isa<CXXNullPtrLiteralExpr>(ArgOrigin)) {
+                                  // foo(nullptr)
+                                  return true;
+                                }
+                                if (isa<IntegerLiteral>(ArgOrigin)) {
+                                  // FIXME: Check the value.
+                                  // foo(NULL)
+                                  return true;
+                                }
+                                if (isASafeCallArg(ArgOrigin))
+                                  return true;
+                                return false;
+                              });
   }
 
   bool shouldSkipCall(const CallExpr *CE) const {
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp
index 1d1dfa12e9..a1a070d6a6 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLocalVarsChecker.cpp
@@ -188,46 +188,48 @@ public:
       if (!InitExpr)
         return; // FIXME: later on we might warn on uninitialized vars too
 
-      if (tryToFindPtrOrigin(InitExpr, /*StopAtFirstRefCountedObj=*/false,
-        [&](const clang::Expr* InitArgOrigin, bool IsSafe) {
-          if (!InitArgOrigin)
-            return true;
-
-          if (isa<CXXThisExpr>(InitArgOrigin))
-            return true;
-
-          if (isa<CXXNullPtrLiteralExpr>(InitArgOrigin))
-            return true;
-
-          if (isa<IntegerLiteral>(InitArgOrigin))
-            return true;
-
-          if (auto *Ref = llvm::dyn_cast<DeclRefExpr>(InitArgOrigin)) {
-            if (auto *MaybeGuardian =
-                    dyn_cast_or_null<VarDecl>(Ref->getFoundDecl())) {
-              const auto *MaybeGuardianArgType =
-                  MaybeGuardian->getType().getTypePtr();
-              if (MaybeGuardianArgType) {
-                const CXXRecordDecl *const MaybeGuardianArgCXXRecord =
-                    MaybeGuardianArgType->getAsCXXRecordDecl();
-                if (MaybeGuardianArgCXXRecord) {
-                  if (MaybeGuardian->isLocalVarDecl() &&
-                      (isRefCounted(MaybeGuardianArgCXXRecord) ||
-                       isRefcountedStringsHack(MaybeGuardian)) &&
-                      isGuardedScopeEmbeddedInGuardianScope(V, MaybeGuardian))
-                    return true;
+      if (tryToFindPtrOrigin(
+              InitExpr, /*StopAtFirstRefCountedObj=*/false,
+              [&](const clang::Expr *InitArgOrigin, bool IsSafe) {
+                if (!InitArgOrigin)
+                  return true;
+
+                if (isa<CXXThisExpr>(InitArgOrigin))
+                  return true;
+
+                if (isa<CXXNullPtrLiteralExpr>(InitArgOrigin))
+                  return true;
+
+                if (isa<IntegerLiteral>(InitArgOrigin))
+                  return true;
+
+                if (auto *Ref = llvm::dyn_cast<DeclRefExpr>(InitArgOrigin)) {
+                  if (auto *MaybeGuardian =
+                          dyn_cast_or_null<VarDecl>(Ref->getFoundDecl())) {
+                    const auto *MaybeGuardianArgType =
+                        MaybeGuardian->getType().getTypePtr();
+                    if (MaybeGuardianArgType) {
+                      const CXXRecordDecl *const MaybeGuardianArgCXXRecord =
+                          MaybeGuardianArgType->getAsCXXRecordDecl();
+                      if (MaybeGuardianArgCXXRecord) {
+                        if (MaybeGuardian->isLocalVarDecl() &&
+                            (isRefCounted(MaybeGuardianArgCXXRecord) ||
+                             isRefcountedStringsHack(MaybeGuardian)) &&
+                            isGuardedScopeEmbeddedInGuardianScope(
+                                V, MaybeGuardian))
+                          return true;
+                      }
+                    }
+
+                    // Parameters are guaranteed to be safe for the duration of
+                    // the call by another checker.
+                    if (isa<ParmVarDecl>(MaybeGuardian))
+                      return true;
+                  }
                 }
-              }
 
-              // Parameters are guaranteed to be safe for the duration of the
-              // call by another checker.
-              if (isa<ParmVarDecl>(MaybeGuardian))
-                return true;
-            }
-          }
-
-          return false;
-      }))
+                return false;
+              }))
         return;
 
       reportBug(V);

``````````

</details>


https://github.com/llvm/llvm-project/pull/91143


More information about the cfe-commits mailing list