[Bug c++/98499] New: [11 Regression] Possibly bad std::string initialization in constructors

[Bug c++/98499] New: [11 Regression] Possibly bad std::string initialization in constructors

[slyfox at gcc dot gnu.org]

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

            Bug ID: 98499
           Summary: [11 Regression] Possibly bad std::string
                    initialization in constructors
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: slyfox at gcc dot gnu.org
  Target Milestone: ---

Initially bug is observed on a usage crash of libsass-3.6.4. Code snippet
around the crash:
https://github.com/sass/libsass/blob/3.6.4/src/context.cpp#L621

I think I extracted a small example that illustrates the problem:

```c++
// cat main.cc
#include <string>

__attribute__((noinline))
static std::string dir_name() { return "c"; }
__attribute__((noinline))
static std::string make_canonical_path (std::string path) { return path; }

class Importer {
  public:
    std::string imp_path;
    std::string ctx_path;
    std::string base_path;
  public:
    __attribute__((noinline)) Importer(std::string imp_path, std::string
ctx_path)
    : imp_path(make_canonical_path(imp_path))
    , ctx_path(make_canonical_path(ctx_path))
    , base_path(dir_name())
    {}
};

struct Include {
    Include(const Importer& imp){}
};

int main() {
  const Include & inc = {{"a", "b"}};
}
```

g++-11 generates crashing binaries, g++-10 does not:

```
$ g++-11.0.0 -O2 -std=c++11 main.cc -o a-11; ./a-11; echo $?
free(): invalid pointer
Aborted (core dumped)
134
$ g++-10.2.0 -O2 -std=c++11 main.cc -o a-10; ./a-10; echo $?
0
```

I was not able to easily get rid of std::string as it uses something from
libstdc++.so.

Thus I'm not sure where the bug is. My suspictions are:
1. invalid c++
2. std::string implementation bug
3. g++'s code generation problem around lifetimes of temporary values

I suspect `[3.]`.


```
$ g++-11.0.0 -v
Using built-in specs.
COLLECT_GCC=/usr/bin/g++-11.0.0
COLLECT_LTO_WRAPPER=/usr/libexec/gcc/x86_64-pc-linux-gnu/11.0.0/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with:
/var/tmp/portage/sys-devel/gcc-11.0.0_pre9999/work/gcc-11.0.0_pre9999/configure
--host=x86_64-pc-linux-gnu --build=x86_64-pc-linux-gnu --prefix=/usr
--bindir=/usr/x86_64-pc-linux-gnu/gcc-bin/11.0.0
--includedir=/usr/lib/gcc/x86_64-pc-linux-gnu/11.0.0/include
--datadir=/usr/share/gcc-data/x86_64-pc-linux-gnu/11.0.0
--mandir=/usr/share/gcc-data/x86_64-pc-linux-gnu/11.0.0/man
--infodir=/usr/share/gcc-data/x86_64-pc-linux-gnu/11.0.0/info
--with-gxx-include-dir=/usr/lib/gcc/x86_64-pc-linux-gnu/11.0.0/include/g++-v11
--with-python-dir=/share/gcc-data/x86_64-pc-linux-gnu/11.0.0/python
--enable-languages=c,c++,go,jit,fortran --enable-obsolete --enable-secureplt
--disable-werror --with-system-zlib --enable-nls --without-included-gettext
--enable-checking=release --with-bugurl=https://bugs.gentoo.org/
--with-pkgversion='Gentoo 11.0.0_pre9999 p5, commit
12ae2bc70846a2be8255eaa41322cd1a5a7b7350' --disable-esp --enable-libstdcxx-time
--enable-host-shared --enable-shared --enable-threads=posix
--enable-__cxa_atexit --enable-clocale=gnu --enable-multilib
--with-multilib-list=m32,m64 --disable-fixed-point --enable-targets=all
--enable-libgomp --disable-libssp --disable-libada --disable-systemtap
--enable-valgrind-annotations --enable-vtable-verify --with-zstd --enable-lto
--with-isl --disable-isl-version-check --enable-default-pie
--enable-default-ssp
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 11.0.0 20201228 (experimental) (Gentoo 11.0.0_pre9999 p5, commit
12ae2bc70846a2be8255eaa41322cd1a5a7b7350)
```

[Bug c++/98499] [11 Regression] Possibly bad std::string initialization in constructors

[slyfox at gcc dot gnu.org]

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

--- Comment #1 from Sergei Trofimovich <slyfox at gcc dot gnu.org> ---
Managed to get rid of external <string> dependency:

```
struct string {
  char * _M_buf;
  // local store
  char _M_local_buf[16];

  string(const string &__str) : _M_buf(_M_local_buf) {}

  string(const char *__s) : _M_buf(_M_local_buf) {}

  ~string() {
    if (_M_buf != _M_local_buf)
      __builtin_trap();
  }

  // not defined
  string();
};

// main.cc

__attribute__((noinline)) static string dir_name() { return "c"; }
__attribute__((noinline)) static string make_canonical_path(string path) {
  return path;
}
class Importer {
public:
  string imp_path;
  string ctx_path;
  string base_path;

public:
  __attribute__((noinline)) Importer(string imp_path, string ctx_path)
      : imp_path(make_canonical_path(imp_path)),
        ctx_path(make_canonical_path(ctx_path)), base_path(dir_name()) {}
};
struct Include {
  Include(const Importer &imp) {}
};
int main() { const Include &inc = {{"a", "b"}}; }
```

Note: all our string() constructors shoudl have `_M_buf == _M_local_buf`
invariant. But gcc-11 generates always-crashing program:

```
; g++-11.0.0 -O2 -std=c++11 -S main.cc
main:
        .cfi_startproc
        subq    $152, %rsp
        .cfi_def_cfa_offset 160
        movq    %fs:40, %rax
        movq    %rax, 136(%rsp)
        xorl    %eax, %eax
        leaq    8(%rsp), %rax
        movq    %rsp, %rsi
        leaq    32(%rsp), %rdx
        movq    %rax, (%rsp)
        leaq    64(%rsp), %rdi
        leaq    40(%rsp), %rax
        movq    %rax, 32(%rsp)
        call    _ZN8ImporterC1E6stringS0_
        ud2
```

[Bug c++/98499] [11 Regression] Possibly bad std::string initialization in constructors

[slyfox at gcc dot gnu.org]

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

Sergei Trofimovich <slyfox at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jh at suse dot cz

--- Comment #2 from Sergei Trofimovich <slyfox at gcc dot gnu.org> ---
Bisected down to:

```
commit 520d5ad337eaa15860a5a964daf7ca46cf31c029
Author: Jan Hubicka <jh@suse.cz>
Date:   Sat Nov 14 13:52:36 2020 +0100

    Detect EAF flags in ipa-modref

    A minimal patch for the EAF flags discovery.  It works only in local
ipa-modref
    and gives up on cyclic SSA graphs.  It improves pt_solution_includes
    disambiguations twice.
```

[Bug c++/98499] [11 Regression] Possibly bad std::string initialization in constructors

[slyfox at gcc dot gnu.org]

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

--- Comment #3 from Sergei Trofimovich <slyfox at gcc dot gnu.org> ---
`--param=modref-max-depth=0` makes the bug disappear.

Looking at `-fdump-tree-all` the harmful optimization happens at `107.fre3`
where:

```
  if (&MEM[(struct string *)&D.2237 + 48B]._M_local_buf != _17)
    goto <bb 3>; [0.00%]
  else
    goto <bb 4>; [100.00%]

  <bb 3> [count: 0]:
  __builtin_trap ();
```

is converted into an unconditional `__builtin_trap ();`. `-fdump-tree-all-all`
says it's a constant fold:

```
Value numbering stmt = if (&MEM[(struct string *)&D.2237 + 48B]._M_local_buf !=
_17)
Applying pattern match.pd:4920, gimple-match.c:3373
marking known outgoing edge 2 -> 3 executable
Block 1: BB4 found not executable
...
```

It looks like it's constant-folded into always-true instead of expected
always-false. `match.pd:4920` is a `pta`:

```
/* Simplify pointer equality compares using PTA.  */
(for neeq (ne eq)
 (simplify
  (neeq @0 @1)
  (if (POINTER_TYPE_P (TREE_TYPE (@0))
       && ptrs_compare_unequal (@0, @1))
   { constant_boolean_node (neeq != EQ_EXPR, type); })))
```

`036t.ealias` and `043t.modref1` says `--param=modref-max-depth=0` has the
effect on how many things escape from main(), but I don't understand the
output.

[Bug c++/98499] [11 Regression] Possibly bad std::string initialization in constructors

[slyfox at gcc dot gnu.org]

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

--- Comment #4 from Sergei Trofimovich <slyfox at gcc dot gnu.org> ---
Managed to shrink example even further. Now fails in `037t.fre1`:

```c++
struct string {
  char * _M_buf;
  // local store
  char _M_local_buf[16];

  __attribute__((noinline)) string() : _M_buf(_M_local_buf) {}

  ~string() {
    if (_M_buf != _M_local_buf)
      __builtin_trap();
  }

  string(const string &__str); // no copies
};

// main.cc

__attribute__((noinline)) static string dir_name() { return string(); }
class Importer {
  string base_path;

public:
  __attribute__((noinline)) Importer() : base_path (dir_name()) {}
};

int main() {
  Importer imp;
}
```

```
$ g++-11.0.0 -O2 -std=c++11 main.cc -o a && ./a
Illegal instruction     (core dumped) ./a
$ g++-11.0.0 --param=modref-max-depth=0 -O2 -std=c++11 main.cc -o a && ./a
<ok>
```

It feels like c++'s return-value-optimization somehow misses the PTA.

[Bug c++/98499] [11 Regression] Possibly bad std::string initialization in constructors

[marxin at gcc dot gnu.org]

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

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |marxin at gcc dot gnu.org
     Ever confirmed|0                           |1
   Target Milestone|---                         |11.0
           Priority|P3                          |P1
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2021-01-04

[Bug c++/98499] [11 Regression] Possibly bad std::string initialization in constructors

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |wrong-code
                 CC|                            |rguenth at gcc dot gnu.org

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
Possibly in discovering pure/constness.  See uses of
gimple_call_return_slot_opt_p in tree-ssa-structalias.c

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

[slyfox at gcc dot gnu.org]

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

--- Comment #6 from Sergei Trofimovich <slyfox at gcc dot gnu.org> ---
(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:

```
...
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 }
_6 = { ESCAPED NONLOCAL }


Alias information for int main()

Aliased symbols

imp, UID D.2146, struct Importer, is addressable

Call clobber information

ESCAPED, points-to non-local, points-to vars: { }

Flow-insensitive points-to information

_6, points-to non-local, points-to escaped, points-to NULL, points-to vars: { }

int main ()
{
  struct Importer imp;
  char * _6;

  <bb 2> :
  Importer::Importer (&imp);
  _6 = MEM[(struct string *)&imp]._M_buf;
  if (&MEM[(struct string *)&imp]._M_local_buf != _6)
    goto <bb 3>; [0.00%]
  else
    goto <bb 4>; [100.00%]

  <bb 3> [count: 0]:
  __builtin_trap ();

  <bb 4> :
  imp ={v} {CLOBBER};
  imp ={v} {CLOBBER};
  return 0;
}
```

I think this looks correct. As I understand we care about a few things in the
analysis here:
1. imp.0+64 and _6 both point to the same flow-insensitive classes (both are
ESCAPED NONLOCAL)
2. imp.0+64 and _6 both point to the same field in flow-sensitive analysis
(both do according to `imp.0+64 = { ESCAPED NONLOCAL } same as _6`.

I don't see problems here.

Mechanically looking at incorrect gcc's decision for `imp.0+64 != _6`:

  ptrs_compare_unequal(
    ptr1 = &MEM[(struct string *)&imp]._M_local_buf,
    ptr2 = _6
  )

returns `TRUE` because

  pt_solution_includes(
    info = ptr2->pt,
    obj1 = imp
  )

returns `FALSE`. That seems to be a bug.

Do arguments to `pt_solution_includes` look correct so far? Does it try to
answer "could _6 point at any field of 'imp' type"?

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

[rguenth at gcc dot gnu.org]

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

--- Comment #7 from Richard Biener <rguenth at gcc dot gnu.org> ---
(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 ...

> ```
> ...
> 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)

> 
> Alias information for int main()
> 
> Aliased symbols
> 
> imp, UID D.2146, struct Importer, is addressable
> 
> Call clobber information
> 
> ESCAPED, points-to non-local, points-to vars: { }
> 
> Flow-insensitive points-to information
> 
> _6, points-to non-local, points-to escaped, points-to NULL, points-to vars:
> { }
> 
> int main ()
> {
>   struct Importer imp;
>   char * _6;
> 
>   <bb 2> :
>   Importer::Importer (&imp);
>   _6 = MEM[(struct string *)&imp]._M_buf;
>   if (&MEM[(struct string *)&imp]._M_local_buf != _6)
>     goto <bb 3>; [0.00%]
>   else
>     goto <bb 4>; [100.00%]
> 
>   <bb 3> [count: 0]:
>   __builtin_trap ();
> 
>   <bb 4> :
>   imp ={v} {CLOBBER};
>   imp ={v} {CLOBBER};
>   return 0;
> }
> ```
> 
> I think this looks correct. As I understand we care about a few things in
> the analysis here:
> 1. imp.0+64 and _6 both point to the same flow-insensitive classes (both are
> ESCAPED NONLOCAL)
> 2. imp.0+64 and _6 both point to the same field in flow-sensitive analysis
> (both do according to `imp.0+64 = { ESCAPED NONLOCAL } same as _6`.
> 
> I don't see problems here.
> 
> Mechanically looking at incorrect gcc's decision for `imp.0+64 != _6`:
> 
>   ptrs_compare_unequal(
>     ptr1 = &MEM[(struct string *)&imp]._M_local_buf,
>     ptr2 = _6
>   )
> 
> returns `TRUE` because
> 
>   pt_solution_includes(
>     info = ptr2->pt,
>     obj1 = imp
>   )
> 
> returns `FALSE`. That seems to be a bug.
> 
> Do arguments to `pt_solution_includes` look correct so far? Does it try to
> answer "could _6 point at any field of 'imp' type"?

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

[slyfox at gcc dot 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?

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

[hubicka at gcc dot gnu.org]

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

Jan Hubicka <hubicka at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |hubicka at gcc dot gnu.org
                 CC|                            |hubicka at gcc dot gnu.org
             Status|NEW                         |ASSIGNED

--- Comment #9 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
Thanks for all the detailed analysis and sorry for getting into this late.

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

This is because it is an inter-procedural analysis.  We compile in topological
order and propagate info from function to callers.

Here I think poblem is:

void Importer::Importer (struct Importer * const this)                          
{                                                                               
  struct string * _1;                                                           

  <bb 2> :                                                                      
  *this_3(D) ={v} {CLOBBER};                                                    
  *this_3(D).base_path = dir_name (); [return slot optimization]                
  return;                                                                       

}                                                                               

We get parm 0 flags: direct noescape nodirectescape

While dir_name does:

struct string dir_name ()                                                       
{                                                                               
  <bb 2> :                                                                      
  string::string (_2(D));                                                       
  return _2(D);                                                                 

}
and that gets to                                                                
void string::string (struct string * const this)                                
{                                                                               
  char[16] * _1;                                                                

  <bb 2> :                                                                      
  *this_3(D) ={v} {CLOBBER};                                                    
  _1 = &this_3(D)->_M_local_buf;                                                
  *this_3(D)._M_buf = _1;                                                       
  return;                                                                       

}                                                                               
which indeed conflict with noescape.

So problem here is that return slot optimized variables are behaving kind of
like parameters.  Since modref does not track EAF flags for them I think your
conservative fix makes sense.

It is also relatively easy to track the EAF flags here, I will try to get quick
stats on how often this makes difference (and whether we want to add trakcing
now or next stage1).

Honza

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

[slyfox at gcc dot gnu.org]

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

--- Comment #10 from Sergei Trofimovich <slyfox at gcc dot gnu.org> ---
(In reply to Jan Hubicka from comment #9)
> Thanks for all the detailed analysis and sorry for getting into this late.
> 
> > 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!
> 
> This is because it is an inter-procedural analysis.  We compile in
> topological order and propagate info from function to callers.
> 
> Here I think poblem is:
> 
> void Importer::Importer (struct Importer * const this)                      
> 
> {                                                                           
> 
>   struct string * _1;                                                       
> 
>                                                                             
> 
>   <bb 2> :                                                                  
> 
>   *this_3(D) ={v} {CLOBBER};                                                
> 
>   *this_3(D).base_path = dir_name (); [return slot optimization]            
> 
>   return;                                                                   
> 
>                                                                             
> 
> }                                                                           
> 
> 
> We get parm 0 flags: direct noescape nodirectescape
> 
> While dir_name does:
> 
> struct string dir_name ()                                                   
> 
> {                                                                           
> 
>   <bb 2> :                                                                  
> 
>   string::string (_2(D));                                                   
> 
>   return _2(D);                                                             
> 
>                                                                             
> 
> }
> and that gets to                                                            
> 
> void string::string (struct string * const this)                            
> 
> {                                                                           
> 
>   char[16] * _1;                                                            
> 
>                                                                             
> 
>   <bb 2> :                                                                  
> 
>   *this_3(D) ={v} {CLOBBER};                                                
> 
>   _1 = &this_3(D)->_M_local_buf;                                            
> 
>   *this_3(D)._M_buf = _1;                                                   
> 
>   return;                                                                   
> 
>                                                                             
> 
> }                                                                           
> 
> which indeed conflict with noescape.
> 
> So problem here is that return slot optimized variables are behaving kind of
> like parameters.  Since modref does not track EAF flags for them I think
> your conservative fix makes sense.
> 
> It is also relatively easy to track the EAF flags here, I will try to get
> quick stats on how often this makes difference (and whether we want to add
> trakcing now or next stage1).

Sounds great!

I send a conservative variant of the patch with the test as
https://gcc.gnu.org/pipermail/gcc-patches/2021-January/564579.html. I hope I
will not interfere with your possible improvement.

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

[cvs-commit at gcc dot gnu.org]

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

--- Comment #11 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Sergei Trofimovich <slyfox@gcc.gnu.org>:

https://gcc.gnu.org/g:11056ab7687f7156846e93557c9171b77713bd7e

commit r11-7022-g11056ab7687f7156846e93557c9171b77713bd7e
Author: Sergei Trofimovich <siarheit@google.com>
Date:   Mon Jan 11 18:05:57 2021 +0000

    tree-optimization/98499 - fix modref analysis on RVO statements

    Before the change RVO gimple statements were treated as local
    stores by modres analysis. But in practice RVO escapes target.

    2021-02-01  Sergei Trofimovich  <siarheit@google.com>

    gcc/ChangeLog:

            PR tree-optimization/98499
            * ipa-modref.c (analyze_ssa_name_flags): treat RVO
            conservatively and assume all possible side-effects.

    gcc/testsuite/ChangeLog:

            PR tree-optimization/98499
            * g++.dg/pr98499.C: new test.

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

[slyfox at gcc dot gnu.org]

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

--- Comment #12 from Sergei Trofimovich <slyfox at gcc dot gnu.org> ---
Should be fixed in gcc-11.

(In reply to Jan Hubicka from comment #9)
> It is also relatively easy to track the EAF flags here, I will try to get
> quick stats on how often this makes difference (and whether we want to add
> trakcing now or next stage1).

I filed a PR98925 to track the optimization.

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

[slyfox at gcc dot gnu.org]

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

Sergei Trofimovich <slyfox at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|ASSIGNED                    |RESOLVED

--- Comment #13 from Sergei Trofimovich <slyfox at gcc dot gnu.org> ---
Should be fixed in gcc-11.