Re: [C++ PATCH] Fix up -fstrong-eval-order handling of call exprs (PR c++/91974)

[Jason Merrill]

On 10/3/19 12:16 PM, Jakub Jelinek wrote:
C++17 and later say
"The postfix-expression is sequenced before each expression in the 
expression-list
and any default argument."
As the following testcase shows, we don't honor that.  Either
cp_gimplify_expr gimplifies some arguments (if CALL_EXPR_REVERSE_ARGS
or CALL_EXPR_ORDERED_ARGS or for !CALL_EXPR_OPERATOR_SYNTAX) and then
the postfix-expression's side-effects are evaluated after those arguments
that are pregimplified by cp_gimplify_expr, or, even if cp_gimplify_expr
doesn't do anything, gimplify_call_expr will:
   /* There is a sequence point before the call, so any side effects in
      the calling expression must occur before the actual call.  Force
      gimplify_expr to use an internal post queue.  */
   ret = gimplify_expr (&CALL_EXPR_FN (*expr_p), pre_p, NULL,
                        is_gimple_call_addr, fb_rvalue);
before gimplifying args, which is fine if there are side-effects, but
doesn't help the actual testcase, because say a user VAR_DECL or PARM_DECL
or RESULT_DECL with pointer to function type satisfies is_gimple_call_addr
predicate, so it will actually be evaluated only later after the arguments
are evaluated, which can actually overwrite the VAR_DECL etc.
So, the following patch gimplifies the postfix expression (CALL_EXPR_FN)
early, and if it isn't invariant or TYPE_OBJ_REF or SSA_NAME (in that case
we don't have to worry about something overwriting it), forces it into a
temporary.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2019-10-03  Jakub Jelinek  <ja...@redhat.com>

        PR c++/91974
        * cp-gimplify.c (cp_gimplify_expr) <case CALL_EXPR>: For
        -fstrong-eval-order ensure CALL_EXPR_FN side-effects are evaluated
        before any arguments.  Additionally, ensure CALL_EXPR_FN that isn't
        invariant nor OBJ_TYPE_REF nor SSA_NAME is forced into a temporary.

        * g++.dg/cpp1z/eval-order5.C: New test.

--- gcc/cp/cp-gimplify.c.jj     2019-10-03 00:26:22.184804539 +0200
+++ gcc/cp/cp-gimplify.c        2019-10-03 10:01:50.769221516 +0200
@@ -818,6 +818,21 @@ cp_gimplify_expr (tree *expr_p, gimple_s
  
      case CALL_EXPR:
        ret = GS_OK;
+      if (flag_strong_eval_order == 2
+         && CALL_EXPR_FN (*expr_p)
+         && cp_get_callee_fndecl_nofold (*expr_p) == NULL_TREE)
+       {
+         enum gimplify_status t
+           = gimplify_expr (&CALL_EXPR_FN (*expr_p), pre_p, NULL,
+                            is_gimple_call_addr, fb_rvalue);
+         if (t == GS_ERROR)
+           ret = GS_ERROR;
+         else if (is_gimple_variable (CALL_EXPR_FN (*expr_p))
+                  && TREE_CODE (CALL_EXPR_FN (*expr_p)) != SSA_NAME)
+           CALL_EXPR_FN (*expr_p)
+             = get_initialized_tmp_var (CALL_EXPR_FN (*expr_p), pre_p,
+                                        NULL);
+       }

Hmm, I'm surprised we don't already have an rvalue predicate stricter 
than is_gimple_val, that would be false for a variable other than a 
gimple temporary.  Such a thing (and a version of is_gimple_call_addr 
that uses it) would be useful here.  But this approach is also OK.

Jason