Bug 68949 - [5/6 Regression] Implicit initialization of array member silently miscompiling.
Summary: [5/6 Regression] Implicit initialization of array member silently miscompiling.
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 5.3.0
: P2 normal
Target Milestone: 5.4
Assignee: Jason Merrill
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2015-12-16 22:13 UTC by Wouter van Kesteren
Modified: 2016-01-28 15:53 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2016-01-22 00:00:00


Attachments
ii (217 bytes, text/plain)
2015-12-16 22:13 UTC, Wouter van Kesteren
Details
cc (370 bytes, text/x-csrc)
2015-12-16 22:15 UTC, Wouter van Kesteren
Details
zero-cout.cc (419 bytes, text/x-csrc)
2015-12-17 03:31 UTC, Wouter van Kesteren
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Wouter van Kesteren 2015-12-16 22:13:49 UTC
Created attachment 37056 [details]
ii

Hello, i believe that i have found a bug in gcc. This is my first time submitting a bug to this project so if i'm missing something important please let me know so i can provide it. I've tried to follow the guidelines best i could.

More specifically, i think there is a bug in the way it handles the implicit default initialization of an array member when that member has a constexpr constructor and a non-inline move constructor that delegates.

I can imagine that doesn't really paint a clear picture so ill just let the code do the talking instead:

"""
// % $CXX -std=c++11 -Wall -Wextra -o zero zero.cc && ./zero; echo $?
//
// expected return
// return of g++ 4.9.2
// return of clang++ 3.7.0
// 0
//
// return of g++ 5.2.0
// return of g++ 5.3.0
// 1

struct Sub {
    int i;

    constexpr Sub() : i(-1) {} // remove constexpr and it works as expected
    Sub(Sub&& rhs); // remove this constructor and it works as epxected.
};

// v-- move this inline and it works as expected
// v-- remove ': Sub()' and it works as expected
Sub::Sub(Sub&& rhs) : Sub() { int tmp = i; i = rhs.i; rhs.i = tmp; }

struct Class {
    // v-- remove '[1]' and it works as expected
    // v-- add '= {}' and it works as expected
    Sub s[1];

    // v-- add ': s{}' and it works as expected
    // v-- removing this constructor makes it work as expected
    Class() {}
};

int main() {
    Class c;
    return c.s[0].i != -1;
}
"""

Instead of the expected '-1', 'i' is actually '0' on 5.2 and 5.3 causing it to return 1 on those.

Ive attached the .ii that this file produced. And for completion here is the -v output of the gcc's i had at my disposal (the gcc-5.2 behavior was given by someone else):

% x86_64-pc-linux-gnu-gcc-5.1 -v
Using built-in specs.
COLLECT_GCC=x86_64-pc-linux-gnu-gcc-5.1
COLLECT_LTO_WRAPPER=/usr/x86_64-pc-linux-gnu/libexec/gcc/x86_64-pc-linux-gnu/5.3.0/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: /var/tmp/paludis/build/sys-devel-gcc-5.3.0/work/gcc-5.3.0/configure --cache-file=config.cache --build=x86_64-pc-linux-gnu --host=x86_64-pc-linux-gnu --target=x86_64-pc-linux-gnu --prefix=/usr/x86_64-pc-linux-gnu --datarootdir=/usr/share --localstatedir=/var --sysconfdir=/etc --disable-dependency-tracking --enable-fast-install --enable-serial-configure --disable-bootstrap --disable-decimal-float --disable-install-libiberty --disable-libada --disable-libatomic --disable-libcilkrts --disable-libffi --disable-libgfortran --disable-libgo --disable-libgomp --disable-libitm --disable-libjava --disable-libmpx --disable-libobjc --disable-liboffloadmic --disable-libquadmath --disable-libsanitizer --disable-libssp --disable-libstdcxx --disable-libstdc++-v3 --disable-libvtv --disable-vtable-verify --disable-multilib --disable-nls --disable-shared --enable-lto --disable-plugin --enable-threads --enable-languages=c,c++,fortran,objc,obj-c++ --with-sysroot= --with-gxx-include-dir=/usr/x86_64-pc-linux-gnu/include/c++/5.3.0 --with-isl --program-transform='s,$,-5.1,' --with-lib-path=/usr/x86_64-pc-linux-gnu/lib --with-as=/usr/x86_64-pc-linux-gnu/bin/x86_64-pc-linux-gnu-as --with-ld=/usr/x86_64-pc-linux-gnu/bin/x86_64-pc-linux-gnu-ld --with-system-zlib --with-glibc-version=2.11 --enable-linker-build-id --with-multilib-list=
Thread model: posix
gcc version 5.3.0 (GCC)

% x86_64-pc-linux-gnu-gcc-4.9 -v
Using built-in specs.
COLLECT_GCC=x86_64-pc-linux-gnu-gcc-4.9
COLLECT_LTO_WRAPPER=/usr/x86_64-pc-linux-gnu/libexec/gcc/x86_64-pc-linux-gnu/4.9.2/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: /var/tmp/paludis/build/sys-devel-gcc-4.9.2-r11/work/gcc-4.9.2/configure --cache-file=config.cache --build=x86_64-pc-linux-gnu --host=x86_64-pc-linux-gnu --target=x86_64-pc-linux-gnu --prefix=/usr/x86_64-pc-linux-gnu --datarootdir=/usr/share --localstatedir=/var --sysconfdir=/etc --disable-dependency-tracking --enable-fast-install --enable-serial-configure --disable-bootstrap --disable-decimal-float --disable-install-libiberty --disable-libada --disable-libatomic --disable-libcilkrts --disable-libffi --disable-libgfortran --disable-libgo --disable-libgomp --disable-libitm --disable-libjava --disable-libobjc --disable-libquadmath --disable-libsanitizer --disable-libssp --disable-libstdcxx --disable-libstdc++-v3 --disable-libvtv --disable-vtable-verify --disable-multilib --disable-nls --disable-shared --enable-lto --disable-plugin --enable-threads --enable-languages=c,c++,fortran,objc,obj-c++ --with-sysroot= --with-gxx-include-dir=/usr/x86_64-pc-linux-gnu/include/c++/4.9.2 --without-cloog --program-transform='s,$,-4.9,' --with-lib-path=/usr/x86_64-pc-linux-gnu/lib --with-as=/usr/x86_64-pc-linux-gnu/bin/x86_64-pc-linux-gnu-as --with-ld=/usr/x86_64-pc-linux-gnu/bin/x86_64-pc-linux-gnu-ld --with-system-zlib --with-glibc-version=2.11 --enable-linker-build-id --with-multilib-list=
Thread model: posix
gcc version 4.9.2 (GCC)
Comment 1 Wouter van Kesteren 2015-12-16 22:15:46 UTC
Created attachment 37057 [details]
cc
Comment 2 Drea Pinski 2015-12-17 02:22:51 UTC
Works correctly on aarch64-linux-gnu with GCC 6:
apinski@arm64:~/src/local$ g++ t.cc
apinski@arm64:~/src/local$ ./a.out ;echo $?
1
apinski@arm64:~/src/local$ g++ -v
Using built-in specs.
COLLECT_GCC=g++
COLLECT_LTO_WRAPPER=/home/apinski/local-gcc/libexec/gcc/aarch64-unknown-linux-gnu/6.0.0/lto-wrapper
Target: aarch64-unknown-linux-gnu
Configured with: /home/apinski/src/local/gcc/configure --prefix=/home/apinski/local-gcc --enable-languages=c,c++,fortran --disable-sanitizer
Thread model: posix
gcc version 6.0.0 20151217 (experimental) [trunk revision 231731] (GCC)
Comment 3 Wouter van Kesteren 2015-12-17 03:16:07 UTC
The program is (imo atleast) supposed to exit with success (0), not with 1, which is a failure. So i would say it is NOT working in gcc6 based on the output you showed.
Comment 4 Wouter van Kesteren 2015-12-17 03:30:06 UTC
I realise that maybe in my effort to make a tiny testcase without headers so that the .ii wasnt hundreds of lines the testcase might have become a bit hard to grasp. I'll attach a second cc with a iostream include and a cout to hopefully clarify things up a bit:

% x86_64-pc-linux-gnu-g++-5.3 -std=c++11 -Wall -Wextra -o zero-cout zero-cout.cc && ./zero-cout
Expected c.s[0].i to be '-1', got '0'

% x86_64-pc-linux-gnu-g++-4.9 -std=c++11 -Wall -Wextra -o zero-cout zero-cout.cc && ./zero-cout
Expected c.s[0].i to be '-1', got '-1'

% clang++ -std=c++11 -Wall -Wextra -o zero-cout zero-cout.cc && ./zero-cout
Expected c.s[0].i to be '-1', got '-1'
Comment 5 Wouter van Kesteren 2015-12-17 03:31:04 UTC
Created attachment 37058 [details]
zero-cout.cc
Comment 6 Jakub Jelinek 2015-12-17 12:26:11 UTC
This is really weird, can't really bisect this, as the compilers pretty randomly sometimes emit
    this->s = {};
in Class::Class constructor and other times
    this->s[0].i = -1;
(in the gimple dump).
Say our bisect seed r231644 emits the latter, but my current trunk or week old trunk on another box gives the former.  Even in the bisect seed, most revisions give the correct this->s[0].i = -1; version, but some exceptions don't, even when clearly the changes in those revisions don't touch anything that should affect this.
Comment 7 Jakub Jelinek 2015-12-17 13:35:58 UTC
So, build_vec_init calls maybe_constant_init on
<<< Unknown tree: aggr_init_expr
  4
  __comp_ctor 
  D.2114
  (struct Sub *) <<< Unknown tree: void_cst >>> >>>;
During that, we call
cxx_eval_call_expression
on the ctor (__comp_ctor in particular).
That function does:
  if (DECL_CLONED_FUNCTION_P (fun))
    fun = DECL_CLONED_FUNCTION (fun);
so loses the notion on whether we wanted to call the base or comp ctor.
And then we run into:
          if (DECL_SAVED_TREE (fun) == NULL_TREE
              && (DECL_CONSTRUCTOR_P (fun) || DECL_DESTRUCTOR_P (fun)))
            /* The maybe-in-charge 'tor had its DECL_SAVED_TREE
               cleared, try a clone.  */
            for (fun = DECL_CHAIN (fun);
                 fun && DECL_CLONED_FUNCTION_P (fun);
                 fun = DECL_CHAIN (fun))
              if (DECL_SAVED_TREE (fun))
                break;
          gcc_assert (DECL_SAVED_TREE (fun));
and just pick randomly one of the ctors, and depending on which one we pick we either get the base or comp ctor behavior.
Now, for whatever reason, in the "good" compiler the __base_ctor (which is the first clone) has DECL_SAVED_TREE non-NULL and __comp_ctor DECL_SAVED_TREE NULL
(the former being:
<<cleanup_point <<< Unknown tree: expr_stmt
  (void) (*(struct 
  {
    int i;
  } &) this = {CLOBBER}) >>>>>;
<<cleanup_point <<< Unknown tree: expr_stmt
  (void) (*(struct 
  {
    int i;
  } &) this = {CLOBBER}) >>>>>;
{
  <<cleanup_point <<< Unknown tree: expr_stmt
  (void) (((struct Sub *) this)->i = -1) >>>>>;
}
) and in the "bad" compiler the __base_ctor has DECL_SAVED_TREE NULL and __comp_ctor has DECL_SAVED_TREE non-NULL,
<<cleanup_point <<< Unknown tree: expr_stmt
  (void) (*(struct 
  {
    int i;
  } &) this = {CLOBBER}) >>>>>;
Comment 8 Jakub Jelinek 2015-12-17 13:42:03 UTC
And, the reason why it is supposedly different is that this all happens during gimplification of the Class::Class() ctor, and DECL_SAVED_TREE of both __base_ctor and __comp_ctor of Sub::Sub() has been initially non-NULL, but during gimplification of those functions has been cleared.  And between the good and bad compiler the difference is just in the cgraph decision which function should be gimplified first and which should be gimplified later.
Comment 9 Jason Merrill 2016-01-26 21:34:48 UTC
Author: jason
Date: Tue Jan 26 21:34:16 2016
New Revision: 232848

URL: https://gcc.gnu.org/viewcvs?rev=232848&root=gcc&view=rev
Log:
	PR c++/68949

	* constexpr.c (register_constexpr_fundef): Keep the un-massaged body.
	(cxx_eval_call_expression): Don't look through clones.
	* optimize.c (maybe_clone_body): Clear DECL_SAVED_TREE of the alias.
	* semantics.c (expand_or_defer_fn_1): Keep DECL_SAVED_TREE of
	maybe-in-charge *tor.

Added:
    trunk/gcc/testsuite/g++.dg/cpp0x/constexpr-array15.C
Modified:
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/constexpr.c
    trunk/gcc/cp/optimize.c
    trunk/gcc/cp/semantics.c
Comment 10 Jason Merrill 2016-01-27 19:19:05 UTC
Author: jason
Date: Wed Jan 27 19:18:33 2016
New Revision: 232896

URL: https://gcc.gnu.org/viewcvs?rev=232896&root=gcc&view=rev
Log:
	PR c++/68949

	* optimize.c (maybe_clone_body): Clear DECL_SAVED_TREE of the alias.
	* semantics.c (expand_or_defer_fn_1): Keep DECL_SAVED_TREE of
	maybe-in-charge *tor.

Added:
    branches/gcc-5-branch/gcc/testsuite/g++.dg/cpp0x/constexpr-array15.C
Modified:
    branches/gcc-5-branch/gcc/cp/ChangeLog
    branches/gcc-5-branch/gcc/cp/optimize.c
    branches/gcc-5-branch/gcc/cp/semantics.c
Comment 11 Jason Merrill 2016-01-28 15:53:11 UTC
Fixed.