[clang] [clang][ExprConst] Add diagnostics for invalid binary arithmetic (PR #118475)

[Timm Baeder via cfe-commits]

Timm =?utf-8?q?Bäder?= <tbaeder at redhat.com>
Message-ID:
In-Reply-To: <llvm.org/llvm/llvm-project/pull/118475 at github.com>


https://github.com/tbaederr updated https://github.com/llvm/llvm-project/pull/118475

>From ce52d3d0c04fc53b814debdcd2c5019f488eddc0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbaeder at redhat.com>
Date: Tue, 3 Dec 2024 11:54:07 +0100
Subject: [PATCH 1/2] [clang][ExprConst] Add diagnostics for invalid binary
 arithmetic

... between unrelated declarations or literals.
---
 clang/include/clang/Basic/DiagnosticASTKinds.td |  4 ++++
 clang/lib/AST/ExprConstant.cpp                  | 17 +++++++++++++++--
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/clang/include/clang/Basic/DiagnosticASTKinds.td b/clang/include/clang/Basic/DiagnosticASTKinds.td
index f630698757c5fb..4d078fc9ca6bb7 100644
--- a/clang/include/clang/Basic/DiagnosticASTKinds.td
+++ b/clang/include/clang/Basic/DiagnosticASTKinds.td
@@ -91,11 +91,15 @@ def note_constexpr_pointer_subtraction_zero_size : Note<
   "subtraction of pointers to type %0 of zero size">;
 def note_constexpr_pointer_comparison_unspecified : Note<
   "comparison between '%0' and '%1' has unspecified value">;
+def note_constexpr_pointer_arith_unspecified : Note<
+  "arithmetic involving '%0' and '%1' has unspecified value">;
 def note_constexpr_pointer_constant_comparison : Note<
   "comparison of numeric address '%0' with pointer '%1' can only be performed "
   "at runtime">;
 def note_constexpr_literal_comparison : Note<
   "comparison of addresses of literals has unspecified value">;
+def note_constexpr_literal_arith : Note<
+  "arithmetic on addresses of literals has unspecified value">;
 def note_constexpr_opaque_call_comparison : Note<
   "comparison against opaque constant address '%0' can only be performed at "
   "runtime">;
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 6b5b95aee35522..2aefcd870ebaf8 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -14548,8 +14548,21 @@ bool IntExprEvaluator::VisitBinaryOperator(const BinaryOperator *E) {
         return Error(E);
       const Expr *LHSExpr = LHSValue.Base.dyn_cast<const Expr *>();
       const Expr *RHSExpr = RHSValue.Base.dyn_cast<const Expr *>();
-      if (!LHSExpr || !RHSExpr)
-        return Error(E);
+      if (!LHSExpr || !RHSExpr) {
+        std::string LHS = LHSValue.toString(Info.Ctx, E->getLHS()->getType());
+        std::string RHS = RHSValue.toString(Info.Ctx, E->getRHS()->getType());
+        Info.FFDiag(E, diag::note_constexpr_pointer_arith_unspecified)
+            << LHS << RHS;
+        return false;
+      }
+
+      if (ArePotentiallyOverlappingStringLiterals(Info, LHSValue, RHSValue)) {
+        std::string LHS = LHSValue.toString(Info.Ctx, E->getLHS()->getType());
+        std::string RHS = RHSValue.toString(Info.Ctx, E->getRHS()->getType());
+        Info.FFDiag(E, diag::note_constexpr_literal_arith) << LHS << RHS;
+        return false;
+      }
+
       const AddrLabelExpr *LHSAddrExpr = dyn_cast<AddrLabelExpr>(LHSExpr);
       const AddrLabelExpr *RHSAddrExpr = dyn_cast<AddrLabelExpr>(RHSExpr);
       if (!LHSAddrExpr || !RHSAddrExpr)

>From e59b74fe979a17898096e0039dd4572241124bc9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbaeder at redhat.com>
Date: Wed, 4 Dec 2024 16:04:25 +0100
Subject: [PATCH 2/2] Address review comments

---
 .../include/clang/Basic/DiagnosticASTKinds.td | 10 ++++---
 clang/lib/AST/ExprConstant.cpp                | 27 ++++++++++---------
 2 files changed, 21 insertions(+), 16 deletions(-)

diff --git a/clang/include/clang/Basic/DiagnosticASTKinds.td b/clang/include/clang/Basic/DiagnosticASTKinds.td
index 4d078fc9ca6bb7..ab432606edbc4c 100644
--- a/clang/include/clang/Basic/DiagnosticASTKinds.td
+++ b/clang/include/clang/Basic/DiagnosticASTKinds.td
@@ -90,16 +90,18 @@ def note_constexpr_pointer_subtraction_not_same_array : Note<
 def note_constexpr_pointer_subtraction_zero_size : Note<
   "subtraction of pointers to type %0 of zero size">;
 def note_constexpr_pointer_comparison_unspecified : Note<
-  "comparison between '%0' and '%1' has unspecified value">;
+  "comparison between pointers to unrelated objects '%0' and '%1' has unspecified value">;
 def note_constexpr_pointer_arith_unspecified : Note<
-  "arithmetic involving '%0' and '%1' has unspecified value">;
+  "arithmetic involving unrelated objects '%0' and '%1' has unspecified value">;
 def note_constexpr_pointer_constant_comparison : Note<
   "comparison of numeric address '%0' with pointer '%1' can only be performed "
   "at runtime">;
 def note_constexpr_literal_comparison : Note<
-  "comparison of addresses of literals has unspecified value">;
+  "comparison of addresses of potentially overlapping literals has unspecified value">;
 def note_constexpr_literal_arith : Note<
-  "arithmetic on addresses of literals has unspecified value">;
+  "arithmetic on addresses of potentially overlapping literals has unspecified value">;
+def note_constexpr_repeated_literal_eval : Note<
+  "repeated evaluation of the same literal expression can produce different objects">;
 def note_constexpr_opaque_call_comparison : Note<
   "comparison against opaque constant address '%0' can only be performed at "
   "runtime">;
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 2aefcd870ebaf8..eed33a45e12c8a 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -14548,20 +14548,23 @@ bool IntExprEvaluator::VisitBinaryOperator(const BinaryOperator *E) {
         return Error(E);
       const Expr *LHSExpr = LHSValue.Base.dyn_cast<const Expr *>();
       const Expr *RHSExpr = RHSValue.Base.dyn_cast<const Expr *>();
-      if (!LHSExpr || !RHSExpr) {
-        std::string LHS = LHSValue.toString(Info.Ctx, E->getLHS()->getType());
-        std::string RHS = RHSValue.toString(Info.Ctx, E->getRHS()->getType());
-        Info.FFDiag(E, diag::note_constexpr_pointer_arith_unspecified)
-            << LHS << RHS;
-        return false;
-      }
 
-      if (ArePotentiallyOverlappingStringLiterals(Info, LHSValue, RHSValue)) {
-        std::string LHS = LHSValue.toString(Info.Ctx, E->getLHS()->getType());
-        std::string RHS = RHSValue.toString(Info.Ctx, E->getRHS()->getType());
-        Info.FFDiag(E, diag::note_constexpr_literal_arith) << LHS << RHS;
+      auto DiagArith = [&](unsigned DiagID) {
+        std::string LHS = LHSValue.toString(Info.Ctx, LHSExpr->getType());
+        std::string RHS = RHSValue.toString(Info.Ctx, RHSExpr->getType());
+        Info.FFDiag(E, DiagID) << LHS << RHS;
+        if (LHSExpr && LHSExpr == RHSExpr)
+          Info.Note(LHSExpr->getExprLoc(),
+                    diag::note_constexpr_repeated_literal_eval)
+              << LHSExpr->getSourceRange();
         return false;
-      }
+      };
+
+      if (!LHSExpr || !RHSExpr)
+        return DiagArith(diag::note_constexpr_pointer_arith_unspecified);
+
+      if (ArePotentiallyOverlappingStringLiterals(Info, LHSValue, RHSValue))
+        return DiagArith(diag::note_constexpr_literal_arith);
 
       const AddrLabelExpr *LHSAddrExpr = dyn_cast<AddrLabelExpr>(LHSExpr);
       const AddrLabelExpr *RHSAddrExpr = dyn_cast<AddrLabelExpr>(RHSExpr);