Bug 55137 - [4.8 Regression] Unexpected static structure initialization
Summary: [4.8 Regression] Unexpected static structure initialization
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 4.8.0
: P2 normal
Target Milestone: 4.8.0
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-10-30 12:16 UTC by Sebastian Huber
Modified: 2012-12-06 14:37 UTC (History)
5 users (show)

See Also:
Host:
Target:
Build:
Known to work: 4.7.3
Known to fail:
Last reconfirmed: 2012-10-30 00:00:00


Attachments
Test case. (107 bytes, text/x-c++src)
2012-10-30 12:16 UTC, Sebastian Huber
Details
gcc48-pr55137.patch (803 bytes, patch)
2012-11-06 17:11 UTC, Jakub Jelinek
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sebastian Huber 2012-10-30 12:16:52 UTC
Created attachment 28573 [details]
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}
Comment 1 Paolo Carlini 2012-10-30 12:56:07 UTC
Insane.

struct S {
  int b;
};

struct S s = { -1 + (int)(sizeof(int) - 1) };
Comment 2 Jakub Jelinek 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.
Comment 3 Jakub Jelinek 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.
Comment 4 Sebastian Huber 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?
Comment 5 Sebastian Huber 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.
Comment 6 Jakub Jelinek 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).
Comment 7 Sebastian Huber 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.
Comment 8 Jakub Jelinek 2012-11-06 17:11:21 UTC
Created attachment 28624 [details]
gcc48-pr55137.patch

Untested patch.
Comment 9 Sebastian Huber 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.
Comment 10 Jason Merrill 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...
Comment 11 Jakub Jelinek 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
Comment 12 Jason Merrill 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
Comment 13 Paolo Carlini 2012-11-22 16:23:36 UTC
Fixed then, great.
Comment 14 Jakub Jelinek 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