Skip to content

Conversation

Sirraide
Copy link
Member

This addresses two problems observed in #89407 wrt user-defined static_assert messages:

  1. In Expr::EvaluateCharRangeAsString, we were calling getExtValue() instead of getZExtValue(), which would assert if a negative or very large number was returned from size().
  2. If the value could not be converted to std::size_t, attempting to diagnose that would crash because ext_cce_narrowing was missing two %select cases.

This fixes #89407.

@Sirraide Sirraide added the clang:frontend Language frontend issues, e.g. anything involving "Sema" label Apr 19, 2024
@llvmbot llvmbot added the clang Clang issues not falling into any other category label Apr 19, 2024
@llvmbot
Copy link
Member

llvmbot commented Apr 19, 2024

@llvm/pr-subscribers-clang

Author: None (Sirraide)

Changes

This addresses two problems observed in #89407 wrt user-defined static_assert messages:

  1. In Expr::EvaluateCharRangeAsString, we were calling getExtValue() instead of getZExtValue(), which would assert if a negative or very large number was returned from size().
  2. If the value could not be converted to std::size_t, attempting to diagnose that would crash because ext_cce_narrowing was missing two %select cases.

This fixes #89407.


Full diff: https://github.com/llvm/llvm-project/pull/89420.diff

3 Files Affected:

  • (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+3-3)
  • (modified) clang/lib/AST/ExprConstant.cpp (+2-2)
  • (modified) clang/test/SemaCXX/static-assert-cxx26.cpp (+74)
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 5ec0218aedfe86..85c0cf93ee732f 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -87,9 +87,9 @@ def err_expr_not_cce : Error<
   "call to 'size()'|call to 'data()'}0 is not a constant expression">;
 def ext_cce_narrowing : ExtWarn<
   "%select{case value|enumerator value|non-type template argument|"
-  "array size|explicit specifier argument|noexcept specifier argument}0 "
-  "%select{cannot be narrowed from type %2 to %3|"
-  "evaluates to %2, which cannot be narrowed to type %3}1">,
+  "array size|explicit specifier argument|noexcept specifier argument|"
+  "call to 'size()'|call to 'data()'}0 %select{cannot be narrowed from "
+  "type %2 to %3|evaluates to %2, which cannot be narrowed to type %3}1">,
   InGroup<CXX11Narrowing>, DefaultError, SFINAEFailure;
 def err_ice_not_integral : Error<
   "%select{integer|integral}1 constant expression must have "
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 88c8eaf6ef9b6e..e1f060f77ccd9d 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -16853,13 +16853,13 @@ bool Expr::EvaluateCharRangeAsString(std::string &Result,
   if (!::EvaluateInteger(SizeExpression, SizeValue, Info))
     return false;
 
-  int64_t Size = SizeValue.getExtValue();
+  uint64_t Size = SizeValue.getZExtValue();
 
   if (!::EvaluatePointer(PtrExpression, String, Info))
     return false;
 
   QualType CharTy = PtrExpression->getType()->getPointeeType();
-  for (int64_t I = 0; I < Size; ++I) {
+  for (uint64_t I = 0; I < Size; ++I) {
     APValue Char;
     if (!handleLValueToRValueConversion(Info, PtrExpression, CharTy, String,
                                         Char))
diff --git a/clang/test/SemaCXX/static-assert-cxx26.cpp b/clang/test/SemaCXX/static-assert-cxx26.cpp
index f4ede74f9214a4..7d896d8b365b74 100644
--- a/clang/test/SemaCXX/static-assert-cxx26.cpp
+++ b/clang/test/SemaCXX/static-assert-cxx26.cpp
@@ -341,3 +341,77 @@ struct Callable {
     } data;
 };
 static_assert(false, Callable{}); // expected-error {{static assertion failed: hello}}
+
+namespace GH89407 {
+struct A {
+  constexpr __SIZE_TYPE__ size() const { return -1; }
+  constexpr const char* data() const { return ""; }
+};
+
+struct B {
+  constexpr long long size() const { return 18446744073709551615U; }
+  constexpr const char* data() const { return ""; }
+};
+
+struct C {
+  constexpr __int128 size() const { return -1; }
+  constexpr const char* data() const { return ""; }
+};
+
+struct D {
+  constexpr unsigned __int128 size() const { return -1; }
+  constexpr const char* data() const { return ""; }
+};
+
+struct E {
+  constexpr __SIZE_TYPE__ size() const { return 18446744073709551615U; }
+  constexpr const char* data() const { return ""; }
+};
+
+static_assert(true, A{}); // expected-error {{the message in this static assertion is not a constant expression}}
+                          // expected-note@-1 {{read of dereferenced one-past-the-end pointer is not allowed in a constant expression}}
+static_assert(true, B{}); // expected-error {{call to 'size()' evaluates to -1, which cannot be narrowed to type 'unsigned long'}}
+                          // expected-error@-1 {{the message in this static assertion is not a constant expression}}
+                          // expected-note@-2 {{read of dereferenced one-past-the-end pointer is not allowed in a constant expression}}
+static_assert(true, C{}); // expected-error {{call to 'size()' evaluates to -1, which cannot be narrowed to type 'unsigned long'}}
+                          // expected-error@-1 {{the message in this static assertion is not a constant expression}}
+                          // expected-note@-2 {{read of dereferenced one-past-the-end pointer is not allowed in a constant expression}}
+static_assert(true, D{}); // expected-error {{call to 'size()' evaluates to 340282366920938463463374607431768211455, which cannot be narrowed to type 'unsigned long'}}
+                          // expected-error@-1 {{the message in this static assertion is not a constant expression}}
+                          // expected-note@-2 {{read of dereferenced one-past-the-end pointer is not allowed in a constant expression}}
+static_assert(true, E{}); // expected-error {{the message in this static assertion is not a constant expression}}
+                          // expected-note@-1 {{read of dereferenced one-past-the-end pointer is not allowed in a constant expression}}
+
+static_assert(
+  false, // expected-error {{static assertion failed}}
+  A{} // expected-error {{the message in a static assertion must be produced by a constant expression}}
+      // expected-note@-1 {{read of dereferenced one-past-the-end pointer is not allowed in a constant expression}}
+);
+
+static_assert(
+  false, // expected-error {{static assertion failed}}
+  B{} // expected-error {{call to 'size()' evaluates to -1, which cannot be narrowed to type 'unsigned long'}}
+      // expected-error@-1 {{the message in a static assertion must be produced by a constant expression}}
+      // expected-note@-2 {{read of dereferenced one-past-the-end pointer is not allowed in a constant expression}}
+);
+
+static_assert(
+  false, // expected-error {{static assertion failed}}
+  C{} // expected-error {{call to 'size()' evaluates to -1, which cannot be narrowed to type 'unsigned long'}}
+      // expected-error@-1 {{the message in a static assertion must be produced by a constant expression}}
+      // expected-note@-2 {{read of dereferenced one-past-the-end pointer is not allowed in a constant expression}}
+);
+
+static_assert(
+  false, // expected-error {{static assertion failed}}
+  D{} // expected-error {{call to 'size()' evaluates to 340282366920938463463374607431768211455, which cannot be narrowed to type 'unsigned long'}}
+      // expected-error@-1 {{the message in a static assertion must be produced by a constant expression}}
+      // expected-note@-2 {{read of dereferenced one-past-the-end pointer is not allowed in a constant expression}}
+);
+
+static_assert(
+  false, // expected-error {{static assertion failed}}
+  E{} // expected-error {{the message in a static assertion must be produced by a constant expression}}
+      // expected-note@-1 {{read of dereferenced one-past-the-end pointer is not allowed in a constant expression}}
+);
+}

@cor3ntin
Copy link
Contributor

The change LGTM but it does need a release note
Thanks for fixing that!

@Sirraide
Copy link
Member Author

The change LGTM but it does need a release note.

Oh, yeah, I somehow forgot about that because I got distracted by the second bug; will do.

@Sirraide Sirraide merged commit b6628c2 into llvm:main Apr 22, 2024
@Sirraide Sirraide deleted the static-assert-negative branch April 22, 2024 16:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Clang] Crash on user-defined static_assert message if size() returns a negative value
3 participants