[Bug c++/86669] New: [5/6/7/8/9 regression] Complete object constructor clone omits length for a c++11 braced initialiser

classic Classic list List threaded Threaded
12 messages Options
Reply | Threaded
Open this post in threaded view
|

[Bug c++/86669] New: [5/6/7/8/9 regression] Complete object constructor clone omits length for a c++11 braced initialiser

mpolacek at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86669

            Bug ID: 86669
           Summary: [5/6/7/8/9 regression] Complete object constructor
                    clone omits length for a c++11 braced initialiser
           Product: gcc
           Version: 9.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: iains at gcc dot gnu.org
  Target Milestone: ---

Created attachment 44435
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=44435&action=edit
pre-processed and reduced code

This applies to targets that do not allow (or have) alias support for
constructor clones.

c-reduce has obfuscated the variable names in the reduced code, of course ...

The original code looks like this:

====

AArch64LegalizerInfo::AArch64LegalizerInfo(const AArch64Subtarget &ST)
  : LegalizerInfo()  {
  const LLT p0 = LLT::pointer(0, 64);
  const LLT s1 = LLT::scalar(1);
  const LLT s8 = LLT::scalar(8);
  const LLT s16 = LLT::scalar(16);
  const LLT s32 = LLT::scalar(32);
  const LLT s64 = LLT::scalar(64);
  const LLT s128 = LLT::scalar(128);
  const LLT v2s32 = LLT::vector(2, 32);
  const LLT v4s32 = LLT::vector(4, 32);
  const LLT v2s64 = LLT::vector(2, 64);

  for (auto Ty : {p0, s1, s8, s16, s32, s64})
    setAction({/*G_IMPLICIT_DEF*/41, Ty}, Legal);

...
 }
=====

The gimple for the base constructor contains:

"

                try
                  {
                    D.2730 = {};
                    D.2730.i = 6; <<<<< OK _M_len is set
                    D.2729[0] = e;
                    D.2729[1] = f;
                    D.2729[2] = g;
                    D.2729[3] = h;
                    D.2729[4] = i;
                    D.2729[5] = j;
                    D.2730.a = &D.2729;
                    __for_range = &D.2730;
                    __for_begin = std::initializer_list<llvm::J>::begin
(__for_range);
                    __for_end = std::initializer_list<llvm::J>::end
(__for_range);

"

but for the complete constructor:

"
                try
                  {
                    D.2745 = {}; <<< _M_len init missing
                    D.2744[0] = e;
                    D.2744[1] = f;
                    D.2744[2] = g;
                    D.2744[3] = h;
                    D.2744[4] = i;
                    D.2744[5] = j;
                    D.2745.a = &D.2744;
                    __for_range = &D.2745;
                    __for_begin = std::initializer_list<llvm::J>::begin
(__for_range);
                    __for_end = std::initializer_list<llvm::J>::end
(__for_range);
                    <D.2808>:

"

========

With clang
.../bin/clang -cc1 -triple x86_64-apple-macosx10.12.0 -x c++-cpp-output
AArch64LegalizerInfo-fmt.ii -O0 -S -std=c++11 -fvisibility-inlines-hidden
-fno-rtti -o tc.s

The complete constructor is rendered as a thunk calling the base constructor
(not sure why we conclude this is not viable in GCC)

======

With GCC (all versions I've tried 5...trunk) we get the effect above.

.../libexec/gcc/x86_64-apple-darwin14/7.3.0/cc1plus -fpreprocessed
AArch64LegalizerInfo-fmt.ii -fPIC -quiet  -mmacosx-version-min=10.10
-mtune=core2  -g3 -O0 -Werror=date-time -Wall -Wextra -Wno-unused-parameter
-Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -Wpedantic
-Wno-long-long -Wno-maybe-uninitialized -Wdelete-non-virtual-dtor -Wno-comment
-std=c++11 -fPIC -fvisibility-inlines-hidden -fno-exceptions -fno-rtti  -o tg.s

======

I took a look at what was happening in cp/optimize.c:maybe_clone_body() but
couldn't see any special difference between the operations carried out to clone
the base and the complete constructors.
Reply | Threaded
Open this post in threaded view
|

[Bug c++/86669] [6/7/8/9 regression] Complete object constructor clone omits length for a c++11 braced initialiser

mpolacek at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86669

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |wrong-code
   Target Milestone|---                         |6.5
            Summary|[5/6/7/8/9 regression]      |[6/7/8/9 regression]
                   |Complete object constructor |Complete object constructor
                   |clone omits length for a    |clone omits length for a
                   |c++11 braced initialiser    |c++11 braced initialiser
Reply | Threaded
Open this post in threaded view
|

[Bug c++/86669] [6/7/8/9 regression] Complete object constructor clone omits length for a c++11 braced initialiser

mpolacek at gcc dot gnu.org
In reply to this post by mpolacek at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86669

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|6.5                         |7.4

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 6 branch is being closed
Reply | Threaded
Open this post in threaded view
|

[Bug c++/86669] [7/8/9 regression] Complete object constructor clone omits length for a c++11 braced initialiser

mpolacek at gcc dot gnu.org
In reply to this post by mpolacek at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86669

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P2

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
It sounds like invalid tree sharing.
Reply | Threaded
Open this post in threaded view
|

[Bug c++/86669] [7/8/9 regression] Complete object constructor clone omits length for a c++11 braced initialiser

mpolacek at gcc dot gnu.org
In reply to this post by mpolacek at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86669

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 45105
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=45105&action=edit
gcc9-pr86669.patch

Indeed, it does.  clone_body has code to unshare DECL_INITIAL, but it does so
only for FOR_EACH_LOCAL_DECL decls, the artificial temporary for
std::initializer_list in this case isn't tracked among those.

The following patch fixes it at least when looking at the testcase using a
cross-compiler, but I have no way to 1) test it on darwin 2) check whether the
testcase in the patch actually fails at runtime without the patch and is fixed
with it.
I'll bootstrap/regtest the patch on x86_64-linux and i686-linux, can somebody
please do the 1) and 2)?  Thanks.
Reply | Threaded
Open this post in threaded view
|

[Bug c++/86669] [7/8/9 regression] Complete object constructor clone omits length for a c++11 braced initialiser

mpolacek at gcc dot gnu.org
In reply to this post by mpolacek at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86669

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2018-11-27
                 CC|                            |jakub at gcc dot gnu.org
           Assignee|unassigned at gcc dot gnu.org      |jakub at gcc dot gnu.org
     Ever confirmed|0                           |1
Reply | Threaded
Open this post in threaded view
|

[Bug c++/86669] [7/8/9 regression] Complete object constructor clone omits length for a c++11 braced initialiser

mpolacek at gcc dot gnu.org
In reply to this post by mpolacek at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86669

--- Comment #4 from Iain Sandoe <iains at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #3)

> Created attachment 45105 [details]
> gcc9-pr86669.patch
>
> Indeed, it does.  clone_body has code to unshare DECL_INITIAL, but it does
> so only for FOR_EACH_LOCAL_DECL decls, the artificial temporary for
> std::initializer_list in this case isn't tracked among those.
>
> The following patch fixes it at least when looking at the testcase using a
> cross-compiler, but I have no way to 1) test it on darwin 2) check whether
> the testcase in the patch actually fails at runtime without the patch and is
> fixed with it.
> I'll bootstrap/regtest the patch on x86_64-linux and i686-linux, can
> somebody please do the 1) and 2)?  Thanks.

thanks for the patch, I will do 1 and 2
Reply | Threaded
Open this post in threaded view
|

[Bug c++/86669] [7/8/9 regression] Complete object constructor clone omits length for a c++11 braced initialiser

mpolacek at gcc dot gnu.org
In reply to this post by mpolacek at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86669

--- Comment #5 from Iain Sandoe <iains at gcc dot gnu.org> ---
(In reply to Iain Sandoe from comment #4)
> (In reply to Jakub Jelinek from comment #3)
> > Created attachment 45105 [details]
> > gcc9-pr86669.patch

> > I'll bootstrap/regtest the patch on x86_64-linux and i686-linux, can
> > somebody please do the 1) and 2)?  Thanks.
>
> thanks for the patch, I will do 1 and 2

Works for me, tested on x86_64-darwin, and the test-case fails before the patch
and passes after it.

It applies cleanly to 7.x as well, I built 7.3.1 + the patch and built the
original failing code with that; also passed.
Reply | Threaded
Open this post in threaded view
|

[Bug c++/86669] [7/8/9 regression] Complete object constructor clone omits length for a c++11 braced initialiser

mpolacek at gcc dot gnu.org
In reply to this post by mpolacek at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86669

--- Comment #6 from Iain Sandoe <iains at gcc dot gnu.org> ---
(In reply to Iain Sandoe from comment #5)

> (In reply to Iain Sandoe from comment #4)
> > (In reply to Jakub Jelinek from comment #3)
> > > Created attachment 45105 [details]
> > > gcc9-pr86669.patch
>
> > > I'll bootstrap/regtest the patch on x86_64-linux and i686-linux, can
> > > somebody please do the 1) and 2)?  Thanks.
> >
> > thanks for the patch, I will do 1 and 2
>
> Works for me, tested on x86_64-darwin, and the test-case fails before the
> patch and passes after it.

NOTE: regtesting still in progress.
Reply | Threaded
Open this post in threaded view
|

[Bug c++/86669] [7/8/9 regression] Complete object constructor clone omits length for a c++11 braced initialiser

mpolacek at gcc dot gnu.org
In reply to this post by mpolacek at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86669

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|7.4                         |7.5
Reply | Threaded
Open this post in threaded view
|

[Bug c++/86669] [7/8/9 regression] Complete object constructor clone omits length for a c++11 braced initialiser

mpolacek at gcc dot gnu.org
In reply to this post by mpolacek at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86669

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Author: jakub
Date: Fri Dec  7 15:20:04 2018
New Revision: 266893

URL: https://gcc.gnu.org/viewcvs?rev=266893&root=gcc&view=rev
Log:
        PR c++/86669
        * call.c (make_temporary_var_for_ref_to_temp): Call pushdecl even for
        automatic vars.

        * g++.dg/cpp0x/initlist105.C: New test.
        * g++.dg/cpp0x/initlist106.C: New test.
        * g++.dg/other/pr86669.C: New test.

Added:
    trunk/gcc/testsuite/g++.dg/cpp0x/initlist105.C
    trunk/gcc/testsuite/g++.dg/cpp0x/initlist106.C
    trunk/gcc/testsuite/g++.dg/other/pr86669.C
Modified:
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/call.c
    trunk/gcc/testsuite/ChangeLog
Reply | Threaded
Open this post in threaded view
|

[Bug c++/86669] [7/8 regression] Complete object constructor clone omits length for a c++11 braced initialiser

mpolacek at gcc dot gnu.org
In reply to this post by mpolacek at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86669

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[7/8/9 regression] Complete |[7/8 regression] Complete
                   |object constructor clone    |object constructor clone
                   |omits length for a c++11    |omits length for a c++11
                   |braced initialiser          |braced initialiser

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed for GCC9+ so far.