[clang-tools-extra] [clang-tidy] fix crash in altera-id-dependent-backward-branch (PR #113833)

Julian Schmidt via cfe-commits cfe-commits at lists.llvm.org
Sun Oct 27 12:57:25 PDT 2024


https://github.com/5chmidti updated https://github.com/llvm/llvm-project/pull/113833

>From 99090f44fdfe049d7e01e469787ebb0039b965c4 Mon Sep 17 00:00:00 2001
From: Julian Schmidt <git.julian.schmidt at gmail.com>
Date: Sun, 27 Oct 2024 20:47:21 +0100
Subject: [PATCH 1/2] [clang-tidy] fix crash in
 altera-id-dependent-backward-branch

Add some checks for `nullptr` and change some `dyn_cast` to
`dyn_cast_if_present` to avoid crashes.

Fixes #55408
---
 .../altera/IdDependentBackwardBranchCheck.cpp | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.cpp b/clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.cpp
index 09f7d8c5c2f952..94db0a793cf53d 100644
--- a/clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.cpp
+++ b/clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.cpp
@@ -78,16 +78,22 @@ void IdDependentBackwardBranchCheck::registerMatchers(MatchFinder *Finder) {
 
 IdDependentBackwardBranchCheck::IdDependencyRecord *
 IdDependentBackwardBranchCheck::hasIdDepVar(const Expr *Expression) {
+  if (!Expression)
+    return nullptr;
+
   if (const auto *Declaration = dyn_cast<DeclRefExpr>(Expression)) {
     // It is a DeclRefExpr, so check if it's an ID-dependent variable.
-    const auto *CheckVariable = dyn_cast<VarDecl>(Declaration->getDecl());
+    const auto *CheckVariable =
+        dyn_cast_if_present<VarDecl>(Declaration->getDecl());
+    if (!CheckVariable)
+      return nullptr;
     auto FoundVariable = IdDepVarsMap.find(CheckVariable);
     if (FoundVariable == IdDepVarsMap.end())
       return nullptr;
     return &(FoundVariable->second);
   }
   for (const auto *Child : Expression->children())
-    if (const auto *ChildExpression = dyn_cast<Expr>(Child))
+    if (const auto *ChildExpression = dyn_cast_if_present<Expr>(Child))
       if (IdDependencyRecord *Result = hasIdDepVar(ChildExpression))
         return Result;
   return nullptr;
@@ -95,16 +101,21 @@ IdDependentBackwardBranchCheck::hasIdDepVar(const Expr *Expression) {
 
 IdDependentBackwardBranchCheck::IdDependencyRecord *
 IdDependentBackwardBranchCheck::hasIdDepField(const Expr *Expression) {
+  if (!Expression)
+    return nullptr;
+
   if (const auto *MemberExpression = dyn_cast<MemberExpr>(Expression)) {
     const auto *CheckField =
-        dyn_cast<FieldDecl>(MemberExpression->getMemberDecl());
+        dyn_cast_if_present<FieldDecl>(MemberExpression->getMemberDecl());
+    if (!CheckField)
+      return nullptr;
     auto FoundField = IdDepFieldsMap.find(CheckField);
     if (FoundField == IdDepFieldsMap.end())
       return nullptr;
     return &(FoundField->second);
   }
   for (const auto *Child : Expression->children())
-    if (const auto *ChildExpression = dyn_cast<Expr>(Child))
+    if (const auto *ChildExpression = dyn_cast_if_present<Expr>(Child))
       if (IdDependencyRecord *Result = hasIdDepField(ChildExpression))
         return Result;
   return nullptr;

>From 0db4a08356dbe732be01d2da7564984aa9a5d433 Mon Sep 17 00:00:00 2001
From: Julian Schmidt <git.julian.schmidt at gmail.com>
Date: Sun, 27 Oct 2024 20:57:06 +0100
Subject: [PATCH 2/2] add release note

---
 clang-tools-extra/docs/ReleaseNotes.rst | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 876689c40fcdb2..ae75b632a44e5e 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -141,6 +141,10 @@ New check aliases
 Changes in existing checks
 ^^^^^^^^^^^^^^^^^^^^^^^^^^
 
+- Improved :doc:`altera-id-dependent-backward-branch
+  <clang-tidy/checks/altera/id-dependent-backward-branch>` check by fixing
+  crashes from invalid code.
+
 - Improved :doc:`bugprone-casting-through-void
   <clang-tidy/checks/bugprone/casting-through-void>` check to suggest replacing
   the offending code with ``reinterpret_cast``, to more clearly express intent.



More information about the cfe-commits mailing list