[clang] Add flag to suppress overflow errors in C++ constant expressions. (PR #102390)

Eli Friedman via cfe-commits cfe-commits at lists.llvm.org
Wed Aug 7 15:00:09 PDT 2024


https://github.com/efriedma-quic created https://github.com/llvm/llvm-project/pull/102390

Recent Android NDK headers are broken on 32-bit because they contain an invalid shift, and older versions of clang didn't catch this. Demote the error to a default-error with a flag so it can be suppressed.  See discussion on #70307.

Not sure if this is really worth doing if it only affects the Android NDK; Android trunk has been fixed.  And presumably most people will use the compiler from the NDK itself, so they'll update their headers before they ever run into this.  But posting so we can discuss...

CC @glandium

>From 73ce4b1a5349fc2114a66cb096c665f2e93abbb9 Mon Sep 17 00:00:00 2001
From: Eli Friedman <efriedma at quicinc.com>
Date: Wed, 7 Aug 2024 14:35:24 -0700
Subject: [PATCH] Add flag to suppress overflow errors in C++ constant
 expressions.

The latest Android NDK is broken on 32-bit because it contains an
invalid shift, and older versions of clang didn't catch this. Demote
the error to a default-error with a flag so it can be suppressed.

Not sure if this is really worth doing if it only affects the Android
NDK; presumably most people will use the compiler from the NDK itself,
so they'll update their headers before they ever run into this.
---
 clang/include/clang/Basic/DiagnosticSemaKinds.td |  8 ++++++++
 clang/include/clang/Sema/Sema.h                  |  1 +
 clang/lib/Sema/SemaExpr.cpp                      | 16 +++++++++++++---
 clang/test/C/drs/dr2xx.c                         |  2 +-
 clang/test/Sema/shift-count-overflow.c           |  3 +++
 5 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 581434d33c5c9a..e51f8828562214 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -74,6 +74,14 @@ def err_expr_not_ice : Error<
 def ext_expr_not_ice : Extension<
   "expression is not an %select{integer|integral}0 constant expression; "
   "folding it to a constant is a GNU extension">, InGroup<GNUFoldingConstant>;
+def ext_expr_ice_overflow : Extension<
+  "expression is not an integer constant expression "
+  "because of arithmetic overflow; folding it to a constant is a GNU "
+  "extension">, InGroup<GNUFoldingConstant>;
+def ext_expr_ice_overflow_cxx : Extension<
+  "expression is not an integral constant expression "
+  "because of arithmetic overflow">,
+  InGroup<DiagGroup<"constant-overflow">>, DefaultError, SFINAEFailure;
 def err_typecheck_converted_constant_expression : Error<
   "value of type %0 is not implicitly convertible to %1">;
 def err_typecheck_converted_constant_expression_disallowed : Error<
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 2ec6367eccea01..96af6427b38eac 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -7242,6 +7242,7 @@ class Sema final : public SemaBase {
     virtual SemaDiagnosticBuilder diagnoseNotICE(Sema &S,
                                                  SourceLocation Loc) = 0;
     virtual SemaDiagnosticBuilder diagnoseFold(Sema &S, SourceLocation Loc);
+    virtual SemaDiagnosticBuilder diagnoseOverflow(Sema &S, SourceLocation Loc);
     virtual ~VerifyICEDiagnoser() {}
   };
 
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 74c0e017059055..84bc1e37215cdc 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -16913,6 +16913,13 @@ Sema::VerifyICEDiagnoser::diagnoseFold(Sema &S, SourceLocation Loc) {
   return S.Diag(Loc, diag::ext_expr_not_ice) << S.LangOpts.CPlusPlus;
 }
 
+Sema::SemaDiagnosticBuilder
+Sema::VerifyICEDiagnoser::diagnoseOverflow(Sema &S, SourceLocation Loc) {
+  if (S.LangOpts.CPlusPlus)
+    return S.Diag(Loc, diag::ext_expr_ice_overflow_cxx);
+  return S.Diag(Loc, diag::ext_expr_ice_overflow);
+}
+
 ExprResult
 Sema::VerifyIntegerConstantExpression(Expr *E, llvm::APSInt *Result,
                                       VerifyICEDiagnoser &Diagnoser,
@@ -17031,6 +17038,7 @@ Sema::VerifyIntegerConstantExpression(Expr *E, llvm::APSInt *Result,
     }
 
     Diagnoser.diagnoseFold(*this, DiagLoc) << E->getSourceRange();
+
     for (const PartialDiagnosticAt &Note : Notes)
       Diag(Note.first, Note.second);
     
@@ -17045,8 +17053,7 @@ Sema::VerifyIntegerConstantExpression(Expr *E, llvm::APSInt *Result,
   // not a constant expression as a side-effect.
   bool Folded =
       E->EvaluateAsRValue(EvalResult, Context, /*isConstantContext*/ true) &&
-      EvalResult.Val.isInt() && !EvalResult.HasSideEffects &&
-      (!getLangOpts().CPlusPlus || !EvalResult.HasUndefinedBehavior);
+      EvalResult.Val.isInt() && !EvalResult.HasSideEffects;
 
   if (!isa<ConstantExpr>(E))
     E = ConstantExpr::Create(Context, E, EvalResult.Val);
@@ -17079,7 +17086,10 @@ Sema::VerifyIntegerConstantExpression(Expr *E, llvm::APSInt *Result,
     return ExprError();
   }
 
-  Diagnoser.diagnoseFold(*this, DiagLoc) << E->getSourceRange();
+  if (EvalResult.HasUndefinedBehavior)
+    Diagnoser.diagnoseOverflow(*this, DiagLoc) << E->getSourceRange();
+  else
+    Diagnoser.diagnoseFold(*this, DiagLoc) << E->getSourceRange();
   for (const PartialDiagnosticAt &Note : Notes)
     Diag(Note.first, Note.second);
 
diff --git a/clang/test/C/drs/dr2xx.c b/clang/test/C/drs/dr2xx.c
index ffdf5aac377d97..5df6f056e5c50c 100644
--- a/clang/test/C/drs/dr2xx.c
+++ b/clang/test/C/drs/dr2xx.c
@@ -287,7 +287,7 @@ void dr261(void) {
    * but we fold it as a constant expression anyway as a GNU extension.
    */
   enum e2 {
-    ex2 = __INT_MAX__ + (0, 1) /* expected-warning {{expression is not an integer constant expression; folding it to a constant is a GNU extension}}
+    ex2 = __INT_MAX__ + (0, 1) /* expected-warning {{expression is not an integer constant expression because of arithmetic overflow; folding it to a constant is a GNU extension}}
                                   expected-note {{value 2147483648 is outside the range of representable values of type 'int'}}
                                   expected-warning {{left operand of comma operator has no effect}}
                                 */
diff --git a/clang/test/Sema/shift-count-overflow.c b/clang/test/Sema/shift-count-overflow.c
index b5186586c2272f..69890d8d6cb283 100644
--- a/clang/test/Sema/shift-count-overflow.c
+++ b/clang/test/Sema/shift-count-overflow.c
@@ -1,6 +1,9 @@
 // RUN: %clang_cc1 -fsyntax-only -verify=expected,c -pedantic %s
 // RUN: %clang_cc1 -x c++ -std=c++98 -fsyntax-only -verify=expected,cpp %s
 // RUN: %clang_cc1 -x c++ -std=c++11 -fsyntax-only -verify=expected,cpp %s
+// RUN: %clang_cc1 -x c++ -std=c++11 -fsyntax-only -Wno-constant-overflow -verify=suppress %s
+
+// suppress-no-diagnostics
 
 enum shiftof {
     X = (1<<32) // c-warning {{expression is not an integer constant expression; folding it to a constant is a GNU extension}}



More information about the cfe-commits mailing list