[Bug c++/55137] New: [4.8 Regression] Unexpected static structure initialization

[Bug c++/55137] New: [4.8 Regression] Unexpected static structure initialization

[sebastian.huber@embedded-brains.de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55137

             Bug #: 55137
           Summary: [4.8 Regression] Unexpected static structure
                    initialization
    Classification: Unclassified
           Product: gcc
           Version: 4.8.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: sebastian.huber@embedded-brains.de


Created attachment 28573
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=28573
Test case.

The test case:

struct S {
  int a;
  int b;
};

struct S s = {
  sizeof(int),
  (((1024 * 8) * 2) - (1024 * 8))
    + ((int)(((1024 * 8)) + (2 * sizeof(int)) + 8 - 1))
};

yields:

gcc -xc++ -g -c test.cc -o test.o && gdb test.o -ex "p s" -ex quit
[...]
$1 = {a = 0, b = 0}

Correct is (sizeof(int) == 4):

gcc -xc -g -c test.cc -o test.o && gdb test.o -ex "p s" -ex quit
[...]
$1 = {a = 4, b = 16399}

[Bug c++/55137] [4.8 Regression] Unexpected static structure initialization

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55137

Paolo Carlini <paolo.carlini at oracle dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2012-10-30
     Ever Confirmed|0                           |1

--- Comment #1 from Paolo Carlini <paolo.carlini at oracle dot com> 2012-10-30 12:56:07 UTC ---
Insane.

struct S {
  int b;
};

struct S s = { -1 + (int)(sizeof(int) - 1) };

[Bug c++/55137] [4.8 Regression] Unexpected static structure initialization

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55137

Paolo Carlini <paolo.carlini at oracle dot com> changed:

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

[Bug c++/55137] [4.8 Regression] Unexpected static structure initialization

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55137

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org,
                   |                            |jason at gcc dot gnu.org

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-11-06 15:04:10 UTC ---
The change is that now the struct is dynamically initialized rather than
statically as it used to.
So it is not wrong, just worse code quality.
And the reason for that is that maybe_constant_value doesn't fold it into a
constant, because there is TREE_OVERFLOW.  sizeof result is size_t, unsigned
type and TREE_OVERFLOW comes probably from - 1 at the end converted to -1U and
then converted to int.

[Bug c++/55137] [4.8 Regression] Unexpected static structure initialization

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55137

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.7.4

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-11-06 15:21:25 UTC ---
enum E { E1 = -1 + (int) (sizeof (int) - 1) };
errors while it used to be accepted before.
Dunno if that is valid or not.
If it is valid, the series of foldings that result into the overflow are first
folding sizeof (int) - 1UL into sizeof (int) + 18446744073709551615UL and later
on conversion of -1 + (int) (sizeof (int) + 18446744073709551615UL) to
-1 + (int) sizeof (int) + (int) 18446744073709551615UL.

[Bug c++/55137] [4.8 Regression] Unexpected static structure initialization

[sebastian.huber@embedded-brains.de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55137

--- Comment #4 from Sebastian Huber <sebastian.huber@embedded-brains.de> 2012-11-06 15:31:42 UTC ---
(In reply to comment #2)
> The change is that now the struct is dynamically initialized rather than
> statically as it used to.

What do you mean with dynamically initialized?

> So it is not wrong, just worse code quality.

You mean that the result of Paolo Carlini's test case is correct?

[Bug c++/55137] [4.8 Regression] Unexpected static structure initialization

[sebastian.huber@embedded-brains.de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55137

--- Comment #5 from Sebastian Huber <sebastian.huber@embedded-brains.de> 2012-11-06 15:34:57 UTC ---
(In reply to comment #3)
> enum E { E1 = -1 + (int) (sizeof (int) - 1) };
> errors while it used to be accepted before.
> Dunno if that is valid or not.
> If it is valid, the series of foldings that result into the overflow are first
> folding sizeof (int) - 1UL into sizeof (int) + 18446744073709551615UL and later
> on conversion of -1 + (int) (sizeof (int) + 18446744073709551615UL) to
> -1 + (int) sizeof (int) + (int) 18446744073709551615UL.

Why is the ( ) ignored?  The sizeof (int) - 1 is clearly an unsigned integer
which can be converted to a signed integer.

[Bug c++/55137] [4.8 Regression] Unexpected static structure initialization

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55137

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-11-06 15:37:09 UTC ---
What I mean that for your testcase while you have s: .zero 8
instead of s: .long 4, 16399, there is also dynamic initialization:
        movl    $4, s(%rip)
        movl    $16399, s+4(%rip)
(at -O2, at -O0 worse).

[Bug c++/55137] [4.8 Regression] Unexpected static structure initialization

[sebastian.huber@embedded-brains.de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55137

--- Comment #7 from Sebastian Huber <sebastian.huber@embedded-brains.de> 2012-11-06 15:50:38 UTC ---
(In reply to comment #6)
> What I mean that for your testcase while you have s: .zero 8
> instead of s: .long 4, 16399, there is also dynamic initialization:
>         movl    $4, s(%rip)
>         movl    $16399, s+4(%rip)
> (at -O2, at -O0 worse).

Ok, I should have looked at the assembler output.

I hit this problem on the RTEMS operating system.  It uses a supposed to be
statically initialized configuration structure for system initialization.  The
global constructors run in the initialization task (which would dynamically
initialize the configuration structure now).  To be able to start the
initialization task, we need the configuration.

>From my point of view this dynamic initialization is pretty bad here.

[Bug c++/55137] [4.8 Regression] Unexpected static structure initialization

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55137

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-11-06 17:11:21 UTC ---
Created attachment 28624
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=28624
gcc48-pr55137.patch

Untested patch.

[Bug c++/55137] [4.8 Regression] Unexpected static structure initialization

[sebastian.huber@embedded-brains.de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55137

--- Comment #9 from Sebastian Huber <sebastian.huber@embedded-brains.de> 2012-11-08 10:19:33 UTC ---
(In reply to comment #8)
> Created attachment 28624 [details]
> gcc48-pr55137.patch
> 
> Untested patch.

I tried this patch and GCC Git version 5838c24a86ac8a8afe77285f224a3ce50596954e
and I get this:

internal compiler error: in compute_antic, at tree-ssa-pre.c:2511

With an earlier snapshot 4.8-20120923 your patch solves my problem.

[Bug c++/55137] [4.8 Regression] Unexpected static structure initialization

[jason at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55137

--- Comment #10 from Jason Merrill <jason at gcc dot gnu.org> 2012-11-08 20:10:09 UTC ---
(In reply to comment #3)
> If it is valid, the series of foldings that result into the overflow are first
> folding sizeof (int) - 1UL into sizeof (int) + 18446744073709551615UL and later
> on conversion of -1 + (int) (sizeof (int) + 18446744073709551615UL) to
> -1 + (int) sizeof (int) + (int) 18446744073709551615UL.

Under C++ semantics, this folding introduces implementation-defined behavior,
not undefined behavior, so it's still a constant-expression; conversion to a
signed integer is different from overflow in the language.  I have been working
around this difference between back end and language semantics in
cxx_eval_constant_expression by unsetting TREE_OVERFLOW after folding NOP_EXPR,
but that doesn't help this example.

The semantics of overflow and conversion to signed integer are different in C
as well; the only difference from C++ is that C allows the implementation to
trap *or* produce an implementation-defined value.  Perhaps we could
distinguish them in the compiler as well...

[Bug c++/55137] [4.8 Regression] Unexpected static structure initialization

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55137

--- Comment #11 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-11-08 20:16:54 UTC ---
(In reply to comment #10)
> (In reply to comment #3)
> > If it is valid, the series of foldings that result into the overflow are first
> > folding sizeof (int) - 1UL into sizeof (int) + 18446744073709551615UL and later
> > on conversion of -1 + (int) (sizeof (int) + 18446744073709551615UL) to
> > -1 + (int) sizeof (int) + (int) 18446744073709551615UL.
> 
> Under C++ semantics, this folding introduces implementation-defined behavior,
> not undefined behavior, so it's still a constant-expression; conversion to a
> signed integer is different from overflow in the language.  I have been working
> around this difference between back end and language semantics in
> cxx_eval_constant_expression by unsetting TREE_OVERFLOW after folding NOP_EXPR,
> but that doesn't help this example.
> 
> The semantics of overflow and conversion to signed integer are different in C
> as well; the only difference from C++ is that C allows the implementation to
> trap *or* produce an implementation-defined value.  Perhaps we could
> distinguish them in the compiler as well...

In this case I think it is really a bug in fold-const.c, as can be seen on the
miscompiled testcase there (provided it is valid C resp. C++).
See http://gcc.gnu.org/ml/gcc-patches/2012-11/msg00636.html

[Bug c++/55137] [4.8 Regression] Unexpected static structure initialization

[jason at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55137

--- Comment #12 from Jason Merrill <jason at gcc dot gnu.org> 2012-11-22 14:42:06 UTC ---
Author: jason
Date: Thu Nov 22 14:42:00 2012
New Revision: 193727

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=193727
Log:
    PR c++/55137
    * semantics.c (verify_constant): Track overflow separately.
    (reduced_constant_expression_p): Don't check it here.
    (cxx_eval_constant_expression): Check it on CSTs.
    (cxx_eval_outermost_constant_expr): Treat overflows as non-constant
    at this point, but still return the folded version.
    (potential_constant_expression_1): Don't check overflow.

Added:
    trunk/gcc/testsuite/g++.dg/init/static-init3.C
Modified:
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/semantics.c

[Bug c++/55137] [4.8 Regression] Unexpected static structure initialization

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55137

Paolo Carlini <paolo.carlini at oracle dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED
   Target Milestone|4.7.4                       |4.8.0
      Known to fail|4.8.0                       |

--- Comment #13 from Paolo Carlini <paolo.carlini at oracle dot com> 2012-11-22 16:23:36 UTC ---
Fixed then, great.

[Bug c++/55137] [4.8 Regression] Unexpected static structure initialization

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55137

--- Comment #14 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-12-06 14:37:16 UTC ---
Author: jakub
Date: Thu Dec  6 14:37:09 2012
New Revision: 194250

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=194250
Log:
    PR c++/55137
    * fold-const.c (fold_binary_loc) <associate>: Don't introduce
    TREE_OVERFLOW through reassociation.  If type doesn't have defined
    overflow, but one or both of the operands do, use the wrapping type
    for reassociation and only convert to type at the end.

    * g++.dg/opt/pr55137.C: New test.
    * gcc.c-torture/execute/pr55137.c: New test.

Added:
    trunk/gcc/testsuite/g++.dg/opt/pr55137.C
    trunk/gcc/testsuite/gcc.c-torture/execute/pr55137.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/fold-const.c
    trunk/gcc/testsuite/ChangeLog