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  <jakub@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 --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
index 304f6b0c30e5527c334cd98e372b3978bba09c0a..89ff595ec5132700f960a63385ca61bd06ebca9e 100644 (file)
--- a/gcc/cp/typeck.cc
+++ b/gcc/cp/typeck.cc
@@ -4033,8 +4033,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)
@@ -4042,7 +4042,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 (file)
index 0000000..87c9be1
--- /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 ();
+}