[clang-tools-extra] [clang-tidy] Fix crash in bugprone-not-null-terminated-result check (PR #160727)

[Endre Fülöp via cfe-commits]

https://github.com/gamesh411 updated https://github.com/llvm/llvm-project/pull/160727

>From e5240070556aec55218c7b699f15fcb61b455ccd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= <endre.fulop at sigmatechnology.com>
Date: Fri, 19 Sep 2025 11:51:02 +0200
Subject: [PATCH 1/3] [clang-tidy] Fix crash in
 bugprone-not-null-terminated-result check

The check was crashing when trying to evaluate value-dependent expressions using
EvaluateAsInt() in cases where the src parameter of memcpy is value-dependent,
but the length is not. Added isValueDependent() check before EvaluateAsInt()
call to prevent the crash.
---
 .../bugprone/NotNullTerminatedResultCheck.cpp |  8 ++++---
 ...erminated-result-value-dependent-crash.cpp | 23 +++++++++++++++++++
 2 files changed, 28 insertions(+), 3 deletions(-)
 create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/not-null-terminated-result-value-dependent-crash.cpp

diff --git a/clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
index d4676842a97ff..463677d2d3af6 100644
--- a/clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
@@ -64,15 +64,17 @@ static unsigned getLength(const Expr *E,
   if (!E)
     return 0;
 
-  Expr::EvalResult Length;
   E = E->IgnoreImpCasts();
 
   if (const auto *LengthDRE = dyn_cast<DeclRefExpr>(E))
     if (const auto *LengthVD = dyn_cast<VarDecl>(LengthDRE->getDecl()))
       if (!isa<ParmVarDecl>(LengthVD))
         if (const Expr *LengthInit = LengthVD->getInit())
-          if (LengthInit->EvaluateAsInt(Length, *Result.Context))
-            return Length.Val.getInt().getZExtValue();
+          if (!LengthInit->isValueDependent()) {
+            Expr::EvalResult Length;
+            if (LengthInit->EvaluateAsInt(Length, *Result.Context))
+              return Length.Val.getInt().getZExtValue();
+          }
 
   if (const auto *LengthIL = dyn_cast<IntegerLiteral>(E))
     return LengthIL->getValue().getZExtValue();
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/not-null-terminated-result-value-dependent-crash.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/not-null-terminated-result-value-dependent-crash.cpp
new file mode 100644
index 0000000000000..5f361c35e448c
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/not-null-terminated-result-value-dependent-crash.cpp
@@ -0,0 +1,23 @@
+// RUN: %check_clang_tidy %s bugprone-not-null-terminated-result %t -- \
+// RUN: -- -std=c++17 -I %S/Inputs/not-null-terminated-result
+
+// This test case reproduces the crash when the check tries to evaluate
+// a value-dependent expression using EvaluateAsInt() in
+// bugprone-not-null-terminated-result, where the src parameter of memcpy is
+// value-dependent, but the length is not.
+
+// expected-no-diagnostics
+
+#include "not-null-terminated-result-cxx.h"
+
+template<size_t N>
+class ValueDependentClass {
+public:
+  void copyData(char* Dst) {
+    const char* Src = reinterpret_cast<const char*>(this);
+    // The length parameter is arbitrary, but the crash is not reproduced if it is N.
+    memcpy(Dst, Src, 32);
+  }
+};
+
+template class ValueDependentClass<42>; // The template parameter value is arbitrary.

>From d9361654e0f723f37d835fbb47160fa8554f6653 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= <endre.fulop at sigmatechnology.com>
Date: Wed, 15 Oct 2025 16:22:37 +0200
Subject: [PATCH 2/3] [squash-this] add release notes

---
 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 33cc401bcb78f..a94dd9737468c 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -274,6 +274,10 @@ Changes in existing checks
   <clang-tidy/checks/bugprone/narrowing-conversions>` check by fixing
   false positive from analysis of a conditional expression in C.
 
+- Improved :doc:`bugprone-not-null-terminated-result
+  <clang-tidy/checks/bugprone/not-null-terminated-result>` check by fixing
+  a crash caused by certain value-dependent expressions.
+
 - Improved :doc:`bugprone-reserved-identifier
   <clang-tidy/checks/bugprone/reserved-identifier>` check by ignoring
   declarations and macros in system headers.

>From 5aeb7f241ba3a6b22b2f1a19fbda2a6db8e93c5a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= <endre.fulop at sigmatechnology.com>
Date: Wed, 15 Oct 2025 16:25:02 +0200
Subject: [PATCH 3/3] [squash-this] merge if statements

---
 .../bugprone/NotNullTerminatedResultCheck.cpp        | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
index 463677d2d3af6..3dd0a50b70c8a 100644
--- a/clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
@@ -69,12 +69,12 @@ static unsigned getLength(const Expr *E,
   if (const auto *LengthDRE = dyn_cast<DeclRefExpr>(E))
     if (const auto *LengthVD = dyn_cast<VarDecl>(LengthDRE->getDecl()))
       if (!isa<ParmVarDecl>(LengthVD))
-        if (const Expr *LengthInit = LengthVD->getInit())
-          if (!LengthInit->isValueDependent()) {
-            Expr::EvalResult Length;
-            if (LengthInit->EvaluateAsInt(Length, *Result.Context))
-              return Length.Val.getInt().getZExtValue();
-          }
+        if (const Expr *LengthInit = LengthVD->getInit();
+            LengthInit && !LengthInit->isValueDependent()) {
+          Expr::EvalResult Length;
+          if (LengthInit->EvaluateAsInt(Length, *Result.Context))
+            return Length.Val.getInt().getZExtValue();
+        }
 
   if (const auto *LengthIL = dyn_cast<IntegerLiteral>(E))
     return LengthIL->getValue().getZExtValue();