[Bug tree-optimization/98499] [11 Regression] Possibly bad std::string initialization in constructors

["slyfox at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98499

--- Comment #8 from Sergei Trofimovich <slyfox at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #7)
> (In reply to Sergei Trofimovich from comment #6)
> > (In reply to Richard Biener from comment #5)
> > > Possibly in discovering pure/constness.  See uses of
> > > gimple_call_return_slot_opt_p in tree-ssa-structalias.c
> > 
> > Aha, that's useful!
> > 
> > Trying to understand the problem better myself: `-fdump-tree-all` has
> > seemingly relevant `036t.ealias` that precedes breaking `037t.fre1`.
> > 
> > I assume `036t.ealias` analyses individual functions locally and does not
> > take into account other details, thus main() analysis should be enough:
> 
> Well - it does take into account fnspecs derived by modref analysis for
> Importer::Importer - specifically ...

Oh, thank you! Only after many printf() attempts it sunk in that `036t.ealias`
is using data from seemingly later `043t.modref1` pass. It is so confusing!

> > ```
> > ...
> > Points-to sets
> > 
> > ANYTHING = { ANYTHING }
> > ESCAPED = { ESCAPED NONLOCAL }
> > NONLOCAL = { ESCAPED NONLOCAL }
> > STOREDANYTHING = { }
> > INTEGER = { ANYTHING }
> > _ZN8ImporterC1Ev = { }
> > imp.0+64 = { ESCAPED NONLOCAL } same as _6
> > imp.64+8 = { ESCAPED NONLOCAL }
> > __builtin_trap = { }
> > main = { }
> > CALLUSED(9) = { ESCAPED NONLOCAL imp.0+64 imp.64+8 } same as callarg(11)
> > CALLCLOBBERED(10) = { ESCAPED NONLOCAL imp.0+64 imp.64+8 } same as
> > callarg(11)
> > callarg(11) = { ESCAPED NONLOCAL imp.0+64 imp.64+8 }
> 
> the above shows we do not consider 'imp' to escape, and thus
> 
> > _6 = { ESCAPED NONLOCAL }
> 
> _6 does not point to 'imp'.
> 
> Relevant parts of handle_rhs_call are probably
> 
>       /* As we compute ESCAPED context-insensitive we do not gain
>          any precision with just EAF_NOCLOBBER but not EAF_NOESCAPE
>          set.  The argument would still get clobbered through the
>          escape solution.  */
>       if ((flags & EAF_NOCLOBBER)
>            && (flags & (EAF_NOESCAPE | EAF_NODIRECTESCAPE)))
>         {
> ...
> 
> specifically lines
> 
>           if (!(flags & (EAF_NOESCAPE | EAF_DIRECT)))
>             make_indirect_escape_constraint (tem);
> 
> probably do not trigger because of the invalid modref analysis.  I suggest
> to look at the early modref pass dump (it's after FRE but still applies
> to callers)

Yeah, that makes sense.

Minor correction: we get into second branch of handle_rhs_call():

        else if (flags & (EAF_NOESCAPE | EAF_NODIRECTESCAPE))

I traced it through around and I agree it looks like an ipa-modref bug.

Mechanically ipa-modref does not handle `gimple_call_return_slot_opt_p()` and
assumes 'foo = bar() [return slot optimization]' never escape 'foo'.

As a workaround I attempted to pessimize modref and it fixes the test case:

```diff
--- a/gcc/ipa-modref.c
+++ b/gcc/ipa-modref.c
@@ -1614,23 +1614,26 @@ analyze_ssa_name_flags (tree name, vec<modref_lattice>
&lattice, int depth,
        {
          if (gimple_return_retval (ret) == name)
            lattice[index].merge (~EAF_UNUSED);
          else if (memory_access_to (gimple_return_retval (ret), name))
            lattice[index].merge_direct_load ();
        }
       /* Account for LHS store, arg loads and flags from callee function.  */
       else if (gcall *call = dyn_cast <gcall *> (use_stmt))
        {
          tree callee = gimple_call_fndecl (call);
-
+          if (gimple_call_return_slot_opt_p (call)
+              && gimple_call_lhs (call) != NULL_TREE
+              && TREE_ADDRESSABLE (TREE_TYPE (gimple_call_lhs (call))))
+           lattice[index].merge (0);
          /* Recursion would require bit of propagation; give up for now.  */
-         if (callee && !ipa && recursive_call_p (current_function_decl,
+         else if (callee && !ipa && recursive_call_p (current_function_decl,
                                                  callee))
            lattice[index].merge (0);
          else
            {
              int ecf_flags = gimple_call_flags (call);
              bool ignore_stores = ignore_stores_p (current_function_decl,
                                                    ecf_flags);
              bool ignore_retval = ignore_retval_p (current_function_decl,
                                                    ecf_flags);
```

Mechanically ESCAPE bit was lost in Importer::Importer at:

1. in "this->base_path = dir_name (); [r s o]" ipa-modref derived 'DIRECT
NODIRECTESCAPE NOESCAPE' flags as it assumed it's just a memory store without
'this' escape.
2. main() optimised Inporter::Importer(&imp) as a noescape using

  handle_rhs_call()
    -> gimple_call_arg_flags(arg_index = 0)
       -> - fnspec was empty
          - modref's get_modref_function_summary() adds 'DIRECT NODIRECTESCAPE
NOESCAPE'

Is it a reasonable fix? Or it's too conservative and we could easily do better?

I copied predicate from handle_rhs_call(), but did not pick constrain copying:

  /* And if we applied NRV the address of the return slot escapes as well.  */
  if (gimple_call_return_slot_opt_p (stmt)
      && gimple_call_lhs (stmt) != NULL_TREE
      && TREE_ADDRESSABLE (TREE_TYPE (gimple_call_lhs (stmt))))
    {
      auto_vec<ce_s> tmpc;
      struct constraint_expr lhsc, *c;
      get_constraint_for_address_of (gimple_call_lhs (stmt), &tmpc);
      lhsc.var = escaped_id;
      lhsc.offset = 0;
      lhsc.type = SCALAR;
      FOR_EACH_VEC_ELT (tmpc, i, c)
        process_constraint (new_constraint (lhsc, *c));
    }

Would constraint copy transfer to ipa-modref framework roughly the same?