On Mon, Jan 20, 2025 at 12:39:03PM -0500, Jason Merrill wrote:
> On 1/20/25 12:27 PM, Marek Polacek wrote:
> > On Mon, Jan 20, 2025 at 11:46:44AM -0500, Jason Merrill wrote:
> > > On 1/20/25 10:27 AM, Marek Polacek wrote:
> > > > On Fri, Jan 17, 2025 at 06:38:45PM -0500, Jason Merrill wrote:
> > > > > On 1/17/25 1:31 PM, Marek Polacek wrote:
> > > > > > On Fri, Jan 17, 2025 at 08:10:24AM -0500, Jason Merrill wrote:
> > > > > > > On 1/16/25 8:04 PM, Marek Polacek wrote:
> > > > > > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> > > > > > > >
> > > > > > > > -- >8 --
> > > > > > > > The recent r15-6369 unfortunately caused a bad wrong-code issue.
> > > > > > > > Here we have
> > > > > > > >
> > > > > > > > TARGET_EXPR <D.2996, (void) (D.2996 = {.status=0,
> > > > > > > > .data={._vptr.Foo=&_ZTV3Foo + 16}})>
> > > > > > > >
> > > > > > > > and call cp_fold_r -> maybe_constant_init with object=D.2996.
> > > > > > > > In
> > > > > > > > cxx_eval_outermost_constant_expr we now take the type of the
> > > > > > > > object
> > > > > > > > if present. An object can't have type 'void' and so we
> > > > > > > > continue to
> > > > > > > > evaluate the initializer. That evaluates into a VOID_CST,
> > > > > > > > meaning
> > > > > > > > we disregard the whole initializer, and terrible things ensue.
> > > > > > >
> > > > > > > In that case, I'd think we want to use the value of 'object'
> > > > > > > (which should
> > > > > > > be in ctx.ctor?) instead of the return value of
> > > > > > > cxx_eval_constant_expression.
> > > > > >
> > > > > > Ah, I'm sorry I didn't choose that approach. Maybe like this, then?
> > > > > >
> > > > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> > > > >
> > > > > OK. Maybe also add an assert that TREE_TYPE (r) is close enough to
> > > > > type?
> > > >
> > > > Thanks. dg.exp passed with this extra assert:
> > > >
> > > > @@ -8986,7 +8986,11 @@ cxx_eval_outermost_constant_expr (tree t, bool
> > > > allow_non_constant,
> > > > /* If we got a non-simple TARGET_EXPR, the initializer was a
> > > > sequence
> > > > of statements, and the result ought to be stored in ctx.ctor.
> > > > */
> > > > if (r == void_node && !constexpr_dtor && ctx.ctor)
> > > > - r = ctx.ctor;
> > > > + {
> > > > + r = ctx.ctor;
> > > > + gcc_checking_assert (same_type_ignoring_top_level_qualifiers_p
> > > > + (TREE_TYPE (r), type));
> > > > + }
> > >
> > > I was thinking to add that assert in general, not just in this case, to
> > > catch any other instances of trying to return the wrong type.
> >
> > Unfortunately this
> > + /* Check we are not trying to return the wrong type. */
> > + gcc_checking_assert (same_type_ignoring_top_level_qualifiers_p
> > + (initialized_type (r), type)
>
> Why not just TREE_TYPE (r)?
Adjusted to use TREE_TYPE now.
> > + || error_operand_p (type));
> > breaks too much, e.g. constexpr-prvalue2.C with struct A x struct B,
> > or pr82128.C
> > *(((struct C *) a)->D.2903._vptr.A + 8)
> > x
> > int (*) ()
> >
> > I've also tried can_convert, or similar_type_p but no luck. Any thoughts?
>
> Those both sound like the sort of bugs the assert is intended to catch. But
> I suppose we can't add it without fixing them first.
>
> In the latter case, probably by adding an explicit conversion from the vtbl
> slot type to the desired function pointer type.
>
> In the former case, I don't see a constant-expression, so we shouldn't be
> trying to check the type of a nonexistent constant result?
As discussed earlier, this patch just returns the original expression if
the types don't match:
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
-- >8 --
The recent r15-6369 unfortunately caused a bad wrong-code issue.
Here we have
TARGET_EXPR <D.2996, (void) (D.2996 = {.status=0, .data={._vptr.Foo=&_ZTV3Foo
+ 16}})>
and call cp_fold_r -> maybe_constant_init with object=D.2996. In
cxx_eval_outermost_constant_expr we now take the type of the object
if present. An object can't have type 'void' and so we continue to
evaluate the initializer. That evaluates into a VOID_CST, meaning
we disregard the whole initializer, and terrible things ensue.
For non-simple TARGET_EXPRs, we should return ctx.ctor rather than
the result of cxx_eval_constant_expression.
PR c++/118396
PR c++/118523
gcc/cp/ChangeLog:
* constexpr.cc (cxx_eval_outermost_constant_expr): For non-simple
TARGET_EXPRs, return ctx.ctor rather than the result of
cxx_eval_constant_expression. If TYPE and the type of R don't
match, return the original expression.
gcc/testsuite/ChangeLog:
* g++.dg/cpp0x/constexpr-prvalue4.C: New test.
* g++.dg/cpp1y/constexpr-prvalue3.C: New test.
Reviewed-by: Jason Merrill <ja...@redhat.com>
---
gcc/cp/constexpr.cc | 9 +++-
.../g++.dg/cpp0x/constexpr-prvalue4.C | 33 ++++++++++++++
.../g++.dg/cpp1y/constexpr-prvalue3.C | 45 +++++++++++++++++++
3 files changed, 86 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-prvalue4.C
create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-prvalue3.C
diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index 7ff38f8b5e5..9f950ffed74 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -8983,6 +8983,11 @@ cxx_eval_outermost_constant_expr (tree t, bool
allow_non_constant,
r = cxx_eval_constant_expression (&ctx, r, vc_prvalue,
&non_constant_p, &overflow_p);
+ /* If we got a non-simple TARGET_EXPR, the initializer was a sequence
+ of statements, and the result ought to be stored in ctx.ctor. */
+ if (r == void_node && !constexpr_dtor && ctx.ctor)
+ r = ctx.ctor;
+
if (!constexpr_dtor)
verify_constant (r, allow_non_constant, &non_constant_p, &overflow_p);
else
@@ -9087,7 +9092,9 @@ cxx_eval_outermost_constant_expr (tree t, bool
allow_non_constant,
return r;
else if (non_constant_p && TREE_CONSTANT (r))
r = mark_non_constant (r);
- else if (non_constant_p)
+ else if (non_constant_p
+ /* Check we are not trying to return the wrong type. */
+ || !same_type_ignoring_top_level_qualifiers_p (type, TREE_TYPE (r)))
return t;
if (should_unshare)
diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-prvalue4.C
b/gcc/testsuite/g++.dg/cpp0x/constexpr-prvalue4.C
new file mode 100644
index 00000000000..afcee65f880
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-prvalue4.C
@@ -0,0 +1,33 @@
+// PR c++/118396
+// { dg-do run { target c++11 } }
+// { dg-options "-O" }
+
+void *operator new(__SIZE_TYPE__, void *__p) { return __p; }
+
+struct Foo {
+ virtual ~Foo() = default;
+};
+struct Data {
+ int status;
+ Foo data{};
+};
+
+Data *P, *Q;
+
+struct vector {
+ vector (const Data &__value) {
+ P = static_cast<Data *>(__builtin_operator_new(0));
+ new (P) Data (__value);
+ Q = P + 1;
+ }
+ Data *begin() { return P; }
+ Data *end() { return Q; }
+};
+
+int
+main ()
+{
+ vector items_(Data{});
+ for (auto item : items_)
+ item.status == 0 ? void() : __builtin_abort ();
+}
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-prvalue3.C
b/gcc/testsuite/g++.dg/cpp1y/constexpr-prvalue3.C
new file mode 100644
index 00000000000..8ea86c60be5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-prvalue3.C
@@ -0,0 +1,45 @@
+// PR c++/118523
+// { dg-do compile { target c++14 } }
+// { dg-options "-O2 -Wall" }
+
+struct __new_allocator {
+ constexpr __new_allocator() {}
+ __new_allocator(__new_allocator &) {}
+};
+template <typename> using __allocator_base = __new_allocator;
+template <typename> struct allocator_traits;
+template <typename> struct allocator : __allocator_base<int> {};
+template <typename _Tp> struct allocator_traits<allocator<_Tp>> {
+ using pointer = _Tp *;
+ template <typename _Up> using rebind_alloc = allocator<_Up>;
+ static void deallocate(allocator<_Tp>, pointer, long);
+};
+struct __alloc_traits : allocator_traits<allocator<int>> {};
+struct _Vector_impl_data {
+ __alloc_traits::pointer _M_start;
+ __alloc_traits::pointer _M_end_of_storage;
+ constexpr _Vector_impl_data() : _M_start(), _M_end_of_storage() {}
+};
+struct _Vector_impl : __alloc_traits::rebind_alloc<int>, _Vector_impl_data {};
+struct _Vector_base {
+ ~_Vector_base() {
+ _M_deallocate(_M_impl._M_start,
+ _M_impl._M_end_of_storage - _M_impl._M_start);
+ }
+ _Vector_impl _M_impl;
+ void _M_deallocate(__alloc_traits::pointer __p, long __n) {
+ if (__p)
+ __alloc_traits::deallocate(_M_impl, __p, __n);
+ }
+};
+struct vector : protected _Vector_base {};
+struct S {
+ vector a{};
+};
+struct B2 {
+ B2(S);
+};
+struct E : B2 {
+ E(S opts = {}) : B2{opts} {}
+};
+void fun() { E{}; }
base-commit: 07f62ed9a7b09951f83855e19d41641b098190b1
--
2.48.1
