diff mbox series

c++: ICE with reflect_constant and std::allocator::allocate [PR125649]

Message ID 20261002211900.1415338-1-polacek@redhat.com
State New
Headers
Series c++: ICE with reflect_constant and std::allocator::allocate [PR125649] |

Commit Message

Marek Polacek Oct. 2, 2026, 9:19 p.m. UTC 
  Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

-- >8 --
In this PR we have

  consteval{
    const int* m = std::allocator<int>().allocate(1);
    std::meta::reflect_constant(m);
  }

which is invalid: reflect_constant throws meta::exception unless the
template-id TCls<V> would be valid ([meta.reflection.result]/5).

[temp.arg.nontype]/1 wants the invented declaration T x = E; to satisfy
the semantic constraints of a constexpr variable with static storage duration.

[dcl.constexpr]/6: A constexpr variable shall be constant-initializable.

[expr.const.init]/4: A variable v is constant-initializable if [...]
-- the full-expression of its initialization is a constant expression
-- if x has static or thread storage duration, x is constexpr-representable [...]

[expr.const.init]/3: x is constexpr-representable at P if every object
that a constituent value of x points to is constexpr-referenceable
from P.

[expr.const.init]/2: An object o is constexpr-referenceable from a point P
if ... it has static storage duration, or automatic storage duration with
certain scope conditions.

But the array object allocate gave us has dynamic storage duration as per
[allocator.members]/5 -> it's not constexpr-referenceable -> this TCls<V>
is invalid so reflect_constant should throw.  But we trip on the assert
because decl is a heap variable.

There is no way AFAIK to check the failure sooner than invalid_tparm_referent_p
(here we are called from eval_reflect_constant) so this patch extends the
existing assert.

	PR c++/125649

gcc/cp/ChangeLog:

	* constexpr.cc (addr_of_non_const_var): Use heap_var_p.
	(non_const_var_error): Likewise.
	(cxx_eval_constant_expression): Likewise.
	(find_heap_var_refs): Likewise.
	(heap_var_p): New.
	* cp-tree.h (heap_var_p): Declare.
	* pt.cc (invalid_tparm_referent_p): Add heap_var_p to an
	assert.

gcc/testsuite/ChangeLog:

	* g++.dg/reflect/reflect_constant10.C: New test.
---
 gcc/cp/constexpr.cc                           | 35 +++++++++--------
 gcc/cp/cp-tree.h                              |  1 +
 gcc/cp/pt.cc                                  | 18 ++++++---
 .../g++.dg/reflect/reflect_constant10.C       | 38 +++++++++++++++++++
 4 files changed, 68 insertions(+), 24 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/reflect/reflect_constant10.C


base-commit: f31d4844cf3c35566ba7e06a2097dc51ea55369d

Comments

Jason Merrill Oct. 3, 2026, 7:32 a.m. UTC | #1 
On 10/2/26 11:19 PM, Marek Polacek wrote:
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> 
> -- >8 --
> In this PR we have
> 
>    consteval{
>      const int* m = std::allocator<int>().allocate(1);
>      std::meta::reflect_constant(m);
>    }
> 
> which is invalid: reflect_constant throws meta::exception unless the
> template-id TCls<V> would be valid ([meta.reflection.result]/5).
> 
> [temp.arg.nontype]/1 wants the invented declaration T x = E; to satisfy
> the semantic constraints of a constexpr variable with static storage duration.
> 
> [dcl.constexpr]/6: A constexpr variable shall be constant-initializable.
> 
> [expr.const.init]/4: A variable v is constant-initializable if [...]
> -- the full-expression of its initialization is a constant expression
> -- if x has static or thread storage duration, x is constexpr-representable [...]
> 
> [expr.const.init]/3: x is constexpr-representable at P if every object
> that a constituent value of x points to is constexpr-referenceable
> from P.
> 
> [expr.const.init]/2: An object o is constexpr-referenceable from a point P
> if ... it has static storage duration, or automatic storage duration with
> certain scope conditions.
> 
> But the array object allocate gave us has dynamic storage duration as per
> [allocator.members]/5 -> it's not constexpr-referenceable -> this TCls<V>
> is invalid so reflect_constant should throw.  But we trip on the assert
> because decl is a heap variable.
> 
> There is no way AFAIK to check the failure sooner than invalid_tparm_referent_p
> (here we are called from eval_reflect_constant) so this patch extends the
> existing assert.
> 
> 	PR c++/125649
> 
> gcc/cp/ChangeLog:
> 
> 	* constexpr.cc (addr_of_non_const_var): Use heap_var_p.
> 	(non_const_var_error): Likewise.
> 	(cxx_eval_constant_expression): Likewise.
> 	(find_heap_var_refs): Likewise.
> 	(heap_var_p): New.
> 	* cp-tree.h (heap_var_p): Declare.
> 	* pt.cc (invalid_tparm_referent_p): Add heap_var_p to an
> 	assert.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/reflect/reflect_constant10.C: New test.
> ---
>   gcc/cp/constexpr.cc                           | 35 +++++++++--------
>   gcc/cp/cp-tree.h                              |  1 +
>   gcc/cp/pt.cc                                  | 18 ++++++---
>   .../g++.dg/reflect/reflect_constant10.C       | 38 +++++++++++++++++++
>   4 files changed, 68 insertions(+), 24 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/reflect/reflect_constant10.C
> 
> diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
> index 31ef5bf1111..a99659f9685 100644
> --- a/gcc/cp/constexpr.cc
> +++ b/gcc/cp/constexpr.cc
> @@ -3276,10 +3276,7 @@ addr_of_non_const_var (tree *tp, int *walk_subtrees, void *data)
>       if (tree var = get_base_address (TREE_OPERAND (*tp, 0)))
>         if (VAR_P (var) && TREE_STATIC (var))
>   	{
> -	  if (DECL_NAME (var) == heap_uninit_identifier
> -	      || DECL_NAME (var) == heap_identifier
> -	      || DECL_NAME (var) == heap_vec_uninit_identifier
> -	      || DECL_NAME (var) == heap_vec_identifier)
> +	  if (heap_var_p (var))

Why don't we want to include _deleted in all cases?

Jason
Marek Polacek Oct. 5, 2026, 2:59 p.m. UTC | #2 
On Sat, Oct 03, 2026 at 09:32:38AM +0200, Jason Merrill wrote:
> On 10/2/26 11:19 PM, Marek Polacek wrote:
> > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> > 
> > -- >8 --
> > In this PR we have
> > 
> >    consteval{
> >      const int* m = std::allocator<int>().allocate(1);
> >      std::meta::reflect_constant(m);
> >    }
> > 
> > which is invalid: reflect_constant throws meta::exception unless the
> > template-id TCls<V> would be valid ([meta.reflection.result]/5).
> > 
> > [temp.arg.nontype]/1 wants the invented declaration T x = E; to satisfy
> > the semantic constraints of a constexpr variable with static storage duration.
> > 
> > [dcl.constexpr]/6: A constexpr variable shall be constant-initializable.
> > 
> > [expr.const.init]/4: A variable v is constant-initializable if [...]
> > -- the full-expression of its initialization is a constant expression
> > -- if x has static or thread storage duration, x is constexpr-representable [...]
> > 
> > [expr.const.init]/3: x is constexpr-representable at P if every object
> > that a constituent value of x points to is constexpr-referenceable
> > from P.
> > 
> > [expr.const.init]/2: An object o is constexpr-referenceable from a point P
> > if ... it has static storage duration, or automatic storage duration with
> > certain scope conditions.
> > 
> > But the array object allocate gave us has dynamic storage duration as per
> > [allocator.members]/5 -> it's not constexpr-referenceable -> this TCls<V>
> > is invalid so reflect_constant should throw.  But we trip on the assert
> > because decl is a heap variable.
> > 
> > There is no way AFAIK to check the failure sooner than invalid_tparm_referent_p
> > (here we are called from eval_reflect_constant) so this patch extends the
> > existing assert.
> > 
> > 	PR c++/125649
> > 
> > gcc/cp/ChangeLog:
> > 
> > 	* constexpr.cc (addr_of_non_const_var): Use heap_var_p.
> > 	(non_const_var_error): Likewise.
> > 	(cxx_eval_constant_expression): Likewise.
> > 	(find_heap_var_refs): Likewise.
> > 	(heap_var_p): New.
> > 	* cp-tree.h (heap_var_p): Declare.
> > 	* pt.cc (invalid_tparm_referent_p): Add heap_var_p to an
> > 	assert.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > 	* g++.dg/reflect/reflect_constant10.C: New test.
> > ---
> >   gcc/cp/constexpr.cc                           | 35 +++++++++--------
> >   gcc/cp/cp-tree.h                              |  1 +
> >   gcc/cp/pt.cc                                  | 18 ++++++---
> >   .../g++.dg/reflect/reflect_constant10.C       | 38 +++++++++++++++++++
> >   4 files changed, 68 insertions(+), 24 deletions(-)
> >   create mode 100644 gcc/testsuite/g++.dg/reflect/reflect_constant10.C
> > 
> > diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
> > index 31ef5bf1111..a99659f9685 100644
> > --- a/gcc/cp/constexpr.cc
> > +++ b/gcc/cp/constexpr.cc
> > @@ -3276,10 +3276,7 @@ addr_of_non_const_var (tree *tp, int *walk_subtrees, void *data)
> >       if (tree var = get_base_address (TREE_OPERAND (*tp, 0)))
> >         if (VAR_P (var) && TREE_STATIC (var))
> >   	{
> > -	  if (DECL_NAME (var) == heap_uninit_identifier
> > -	      || DECL_NAME (var) == heap_identifier
> > -	      || DECL_NAME (var) == heap_vec_uninit_identifier
> > -	      || DECL_NAME (var) == heap_vec_identifier)
> > +	  if (heap_var_p (var))
> 
> Why don't we want to include _deleted in all cases?

I didn't want to change behavior by introducing that helper.  E.g.,
in non_const_var_error the second branch would be dead if the first
one accepted _deleted, and cxx_eval_constant_expression would also
behave differently for the void* -> T* cast.

Marek
diff mbox series

Patch

diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index 31ef5bf1111..a99659f9685 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -3276,10 +3276,7 @@  addr_of_non_const_var (tree *tp, int *walk_subtrees, void *data)
     if (tree var = get_base_address (TREE_OPERAND (*tp, 0)))
       if (VAR_P (var) && TREE_STATIC (var))
 	{
-	  if (DECL_NAME (var) == heap_uninit_identifier
-	      || DECL_NAME (var) == heap_identifier
-	      || DECL_NAME (var) == heap_vec_uninit_identifier
-	      || DECL_NAME (var) == heap_vec_identifier)
+	  if (heap_var_p (var))
 	    return var;
 
 	  constexpr_global_ctx *global = (constexpr_global_ctx *) data;
@@ -8221,10 +8218,7 @@  non_const_var_error (location_t loc, tree r, bool fundef_p)
 {
   auto_diagnostic_group d;
   tree type = TREE_TYPE (r);
-  if (DECL_NAME (r) == heap_uninit_identifier
-      || DECL_NAME (r) == heap_identifier
-      || DECL_NAME (r) == heap_vec_uninit_identifier
-      || DECL_NAME (r) == heap_vec_identifier)
+  if (heap_var_p (r))
     {
       if (constexpr_error (loc, fundef_p, "the content of uninitialized "
 			   "storage is not usable in a constant expression"))
@@ -10789,10 +10783,7 @@  cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
 	    if (decl
 		&& VAR_P (decl)
 		&& DECL_ARTIFICIAL (decl)
-		&& (DECL_NAME (decl) == heap_identifier
-		    || DECL_NAME (decl) == heap_uninit_identifier
-		    || DECL_NAME (decl) == heap_vec_identifier
-		    || DECL_NAME (decl) == heap_vec_uninit_identifier))
+		&& heap_var_p (decl))
 	      /* OK */;
 	    /* P2738 (C++26): a conversion from a prvalue P of type "pointer to
 	       cv void" to a pointer-to-object type T unless P is a null
@@ -11417,12 +11408,7 @@  instantiate_constexpr_fns (tree t)
 static tree
 find_heap_var_refs (tree *tp, int *walk_subtrees, void */*data*/)
 {
-  if (VAR_P (*tp)
-      && (DECL_NAME (*tp) == heap_uninit_identifier
-	  || DECL_NAME (*tp) == heap_identifier
-	  || DECL_NAME (*tp) == heap_vec_uninit_identifier
-	  || DECL_NAME (*tp) == heap_vec_identifier
-	  || DECL_NAME (*tp) == heap_deleted_identifier))
+  if (VAR_P (*tp) && heap_var_p (*tp, /*include_deleted_p=*/true))
     return *tp;
 
   if (TYPE_P (*tp))
@@ -13918,4 +13904,17 @@  fini_constexpr (void)
   fundef_copies_table = NULL;
 }
 
+/* Return true if T is one of the heap vars.  Include "heap deleted" if
+   INCLUDE_DELETED_P.  */
+
+bool
+heap_var_p (tree t, bool include_deleted_p/*=false*/)
+{
+  return (DECL_NAME (t) == heap_uninit_identifier
+	  || DECL_NAME (t) == heap_identifier
+	  || DECL_NAME (t) == heap_vec_uninit_identifier
+	  || DECL_NAME (t) == heap_vec_identifier
+	  || (include_deleted_p && DECL_NAME (t) == heap_deleted_identifier));
+}
+
 #include "gt-cp-constexpr.h"
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 947bf62ff98..2d6445d6d4f 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -9470,6 +9470,7 @@  extern void diagnose_failing_condition		(tree, location_t, bool,
 extern bool replace_decl			(tree *, tree, tree);
 extern tree cxa_allocate_and_throw_exception	(location_t, const constexpr_ctx *,
 						 tree);
+extern bool heap_var_p				(tree, bool = false);
 
 /* Whether our evaluation wants a prvalue (e.g. CONSTRUCTOR or _CST),
    a glvalue (e.g. VAR_DECL or _REF), or nothing.  */
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 6c8d0e2de71..c7917f38c9b 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -7556,11 +7556,11 @@  invalid_tparm_referent_p (tree type, tree expr, tsubst_flags_t complain)
 			"because %qD has no linkage", expr, type, decl);
 	    return true;
 	  }
-	/* For a constant template parameter of reference or pointer type,
-	   or for each non-static data member of reference or pointer type
-	   in a constant template parameter of class type or subobject thereof,
-	   the reference or pointer value shall not refer or point to
-	   (respectively):
+	/* [temp.arg.nontype]/6: For a constant template parameter of
+	   reference or pointer type, or for each non-static data member of
+	   reference or pointer type in a constant template parameter of class
+	   type or subobject thereof, the reference or pointer value shall not
+	   refer or point to (respectively):
 	   -- a temporary object,
 	   -- a string literal object,
 	   -- the result of a typeid expression,
@@ -7569,9 +7569,15 @@  invalid_tparm_referent_p (tree type, tree expr, tsubst_flags_t complain)
 	else if (VAR_P (decl) && DECL_ARTIFICIAL (decl)
 		 && !DECL_NTTP_OBJECT_P (decl))
 	  {
+	    /* The heap_var_p check doesn't cover any of the bullets above,
+	       but we can get here with an invalid reflect_constant arg.
+	       Heap vars are not constexpr-referenceable, so they're not valid
+	       here either.  */
 	    gcc_checking_assert (DECL_TINFO_P (decl)
 				 || DECL_FNAME_P (decl)
-				 || DECL_IGNORED_P (decl));
+				 || DECL_IGNORED_P (decl)
+				 || heap_var_p (decl,
+						/*include_deleted_p=*/true));
 	    if (complain & tf_error)
 	      error ("the address of %qD is not a valid template argument",
 		     decl);
diff --git a/gcc/testsuite/g++.dg/reflect/reflect_constant10.C b/gcc/testsuite/g++.dg/reflect/reflect_constant10.C
new file mode 100644
index 00000000000..43deea408d8
--- /dev/null
+++ b/gcc/testsuite/g++.dg/reflect/reflect_constant10.C
@@ -0,0 +1,38 @@ 
+// PR c++/125649
+// { dg-do compile { target c++26 } }
+// { dg-additional-options "-freflection" }
+
+#include <memory>
+#include <meta>
+
+consteval {
+  const int *m = std::allocator<int>().allocate(1);
+  std::meta::reflect_constant(m);
+} // { dg-error "reflect_constant failed" }
+
+consteval {
+  const int *m = std::allocator<int>().allocate(1);
+  std::meta::reflect_constant(m + 1);
+} // { dg-error "reflect_constant failed" }
+
+// This is well-formed.
+consteval {
+  int *p = std::allocator<int>().allocate(1);
+  std::allocator<int>().deallocate(p, 1);
+  bool threw = false;
+  try { std::meta::reflect_constant(p); }
+  catch (std::meta::exception&) { threw = true; }
+  if (!threw)
+    throw;
+}
+
+// This is well-formed.
+consteval {
+  int *p = std::allocator<int>().allocate(1);
+  bool threw = false;
+  try { std::meta::reflect_constant(p); }
+  catch (std::meta::exception&) { threw = true; }
+  std::allocator<int>().deallocate(p, 1);
+  if (!threw)
+    throw;
+}