https://gcc.gnu.org/g:0e4f03c6701f6ef9493c78cf3bbf4aa8e41cf04b
commit r14-11234-g0e4f03c6701f6ef9493c78cf3bbf4aa8e41cf04b
Author: Jakub Jelinek <ja...@redhat.com>
Date: Wed Jan 22 00:18:24 2025 +0100
c++: Wrap force_target_expr in get_member_function_from_ptrfunc with
save_expr [PR118509]
My October PR117259 fix to get_member_function_from_ptrfunc to use a
TARGET_EXPR rather than SAVE_EXPR unfortunately caused some regressions as
well as the following testcase shows.
What happens is that
get_member_function_from_ptrfunc -> build_base_path calls save_expr,
so since the PR117259 change in mnay cases it will call save_expr on
a TARGET_EXPR. And, for some strange reason a TARGET_EXPR is not considered
an invariant, so we get a SAVE_EXPR wrapped around the TARGET_EXPR.
That SAVE_EXPR <TARGET_EXPR <...>> gets initially added only to the second
operand of ?:, so at that point it would still work fine during expansion.
But unfortunately an expression with that subexpression is handed to the
caller also through *instance_ptrptr = instance_ptr; and gets evaluated
once again when computing the first argument to the method.
So, essentially, we end up with
(TARGET_EXPR <D.2907, ...>, (... ? ... SAVE_EXPR <TARGET_EXPR <D.2907, ...>
... : ...)) (... SAVE_EXPR <TARGET_EXPR <D.2907, ...> ..., ...);
and while D.2907 is initialized during gimplification in the code dominating
everything that uses it, the extra temporary created for the SAVE_EXPR
is initialized only conditionally (if the ?: condition is true) but then
used unconditionally, so we get
pmf-4.C: In function ‘void foo(C, B*)’:
pmf-4.C:12:11: warning: ‘<anonymous>’ may be used uninitialized
[-Wmaybe-uninitialized]
12 | (y->*x) ();
| ~~~~~~~~^~
pmf-4.C:12:11: note: ‘<anonymous>’ was declared here
12 | (y->*x) ();
| ~~~~~~~~^~
diagnostic and wrong-code issue too.
As the trunk fix to just treat TARGET_EXPR as invariant seems a little bit
risky
and I'd like to get it tested on the trunk for a while, for 14.2.1 this
patch
instead wraps those TARGET_EXPRs into SAVE_EXPRs. Eventually that can be
reverted
and the trunk fix backported.
2025-01-21 Jakub Jelinek <ja...@redhat.com>
PR c++/118509
* typeck.cc (get_member_function_from_ptrfunc): Wrap
force_target_expr
with save_expr.
* g++.dg/expr/pmf-4.C: New test.
Diff:
---
gcc/cp/typeck.cc | 7 ++++---
gcc/testsuite/g++.dg/expr/pmf-4.C | 22 ++++++++++++++++++++++
2 files changed, 26 insertions(+), 3 deletions(-)
diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
index ebb4c8b7bb9b..e32e4a7b7acd 100644
--- a/gcc/cp/typeck.cc
+++ b/gcc/cp/typeck.cc
@@ -4187,8 +4187,8 @@ get_member_function_from_ptrfunc (tree *instance_ptrptr,
tree function,
&& !DECL_P (instance_ptr)
&& !TREE_CONSTANT (instance_ptr)))
instance_ptr = instance_save_expr
- = force_target_expr (TREE_TYPE (instance_ptr), instance_ptr,
- complain);
+ = save_expr (force_target_expr (TREE_TYPE (instance_ptr),
+ instance_ptr, complain));
/* See above comment. */
if (TREE_SIDE_EFFECTS (function)
@@ -4196,7 +4196,8 @@ get_member_function_from_ptrfunc (tree *instance_ptrptr,
tree function,
&& !DECL_P (function)
&& !TREE_CONSTANT (function)))
function
- = force_target_expr (TREE_TYPE (function), function, complain);
+ = save_expr (force_target_expr (TREE_TYPE (function), function,
+ complain));
/* Start by extracting all the information from the PMF itself. */
e3 = pfn_from_ptrmemfunc (function);
diff --git a/gcc/testsuite/g++.dg/expr/pmf-4.C
b/gcc/testsuite/g++.dg/expr/pmf-4.C
new file mode 100644
index 000000000000..87c9be18f70d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/expr/pmf-4.C
@@ -0,0 +1,22 @@
+// PR c++/118509
+// { dg-do run }
+// { dg-options "-Wall -O2" }
+
+struct A { void foo () { a = 1; } int a; A () : a (0) {} };
+struct B : virtual A {};
+typedef void (A::*C) ();
+
+__attribute__((noipa)) void
+foo (C x, B *y)
+{
+ (y->*x) ();
+}
+
+int
+main ()
+{
+ B b;
+ foo (&A::foo, &b);
+ if (b.a != 1)
+ __builtin_abort ();
+}
