On 1/21/25 9:54 AM, Jason Merrill wrote:
On 1/20/25 5:58 PM, Marek Polacek wrote:
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?

OK, thanks!

-- >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;

Actually, I guess we also want to give an error if !allow_non_constant.

Jason

Reply via email to