[Bug c/48124] New: likely wrong code bug

[Bug c/48124] New: likely wrong code bug

[regehr at cs dot utah.edu]

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

           Summary: likely wrong code bug
           Product: gcc
           Version: 4.7.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: regehr@cs.utah.edu


[regehr@gamow ~]$ current-gcc -v
Using built-in specs.
COLLECT_GCC=current-gcc
COLLECT_LTO_WRAPPER=/uusoc/exports/scratch/regehr/z/compiler-install/gcc-r170949-install/bin/../libexec/gcc/x86_64-unknown-linux-gnu/4.7.0/lto-wrapper
Target: x86_64-unknown-linux-gnu
Configured with: ../configure --with-libelf=/usr/local --enable-lto
--prefix=/home/regehr/z/compiler-install/gcc-r170949-install
--program-prefix=r170949- --enable-languages=c,c++
Thread model: posix
gcc version 4.7.0 20110314 (experimental) (GCC) 


[regehr@gamow ~]$ current-gcc -O1 small.c -o small
[regehr@gamow ~]$ ./small 
g_1 = 2
[regehr@gamow ~]$ current-gcc -Os small.c -o small
[regehr@gamow ~]$ ./small 
g_1 = 1
[regehr@gamow ~]$ cat small.c


struct S0 {
  signed f0 : 26;
  signed f1 : 16;
  signed f2 : 10;
  volatile signed f3 : 14;
};

static int g_1 = 1;
static struct S0 g_2 = {0,0,0,1};

int printf(const char *format, ...);

void func_1(void)
{
   g_2.f3 = 0;
   g_1 = 2;
}

int main (int argc, char* argv[])
{
   func_1();
   printf("g_1 = %d\n", g_1);
   return 0;
}

[Bug middle-end/48124] [4.3/4.4/4.5/4.6/4.7 Regression] likely wrong code bug

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2011.03.14 21:52:28
                 CC|                            |aldyh at gcc dot gnu.org,
                   |                            |jakub at gcc dot gnu.org
          Component|c                           |middle-end
   Target Milestone|---                         |4.3.6
            Summary|likely wrong code bug       |[4.3/4.4/4.5/4.6/4.7
                   |                            |Regression] likely wrong
                   |                            |code bug
     Ever Confirmed|0                           |1

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> 2011-03-14 21:52:28 UTC ---
While on this exact testcase problems started with
http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=159321
after slightly modifying the testcase:

/* { dg-do run } */
/* { dg-options "-Os -fno-toplevel-reorder" } */

extern void abort (void);

struct S
{
  signed a : 26;
  signed b : 16;
  signed c : 10;
  volatile signed d : 14;
};

static struct S e = { 0, 0, 0, 1 };
static int f = 1;

void __attribute__((noinline))
foo (void)
{
  e.d = 0;
  f = 2;
}

int
main ()
{
  if (e.a || e.b || e.c || e.d != 1 || f != 1)
    abort ();
  foo ();
  if (e.a || e.b || e.c || e.d || f != 2)
    abort ();
  return 0;
}

it keeps failing at -Os -fno-toplevel-reorder already in 4.2 and with just -Os
in 4.3.

[Bug middle-end/48124] [4.3/4.4/4.5/4.6/4.7 Regression] likely wrong code bug

[jakub at gcc dot gnu.org]

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

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> 2011-03-14 22:09:29 UTC ---
On the adjusted testcase it started with
http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=115421
Before that the volatile bitfield was accessed just through 32-bit access (and
in 4.1 even with 16-bit access).

[Bug middle-end/48124] [4.3/4.4/4.5/4.6/4.7 Regression] likely wrong code bug

[rguenth at gcc dot gnu.org]

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

--- Comment #3 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-03-15 10:21:04 UTC ---
Hm, e has alignment of 8 bytes and f of 4 but the assembler doesn't pad
e's size to 8 byte multiples appearantly

        .data
        .align 8
        .type   e, @object
        .size   e, 12
e:
        .byte   0
        .byte   0
        .byte   0
        .byte   0
        .value  0
        .byte   0
        .byte   0
        .byte   1
        .byte   0
        .zero   2
        .align 4
        .type   f, @object
        .size   f, 4
f:
        .long   1

this is probably a more general problem also with the vectorizer which
happily increases alignment of global variables independed on if their
size is a multiple of that alignment.

If the asm output machinery does not try to handle padding to multiples
of alignment size then that's the bug I guess.

[Bug middle-end/48124] [4.3/4.4/4.5/4.6/4.7 Regression] likely wrong code bug

[jakub at gcc dot gnu.org]

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

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> 2011-03-15 10:52:44 UTC ---
I think that is not the problem, it is fine if some variables are aligned more
than they need to be for performance reasons.  TYPE_ALIGN is still 32 bits,
while just DECL_ALIGN is 64 bits.

The bug is in expand_assignment/store_bit_field{,_1} I think.
First expand_assignment calls set_mem_attributes_minus_bitpos (to_rtx, to, 0,
bitpos);
which changes
(mem/s/c:BLK (symbol_ref:DI ("e") [flags 0x2] <var_decl 0x7ffff1f4b000 e>) [2
e+0 S12 A64])
into:
(mem/s/v/j/c:BLK (symbol_ref:DI ("e") [flags 0x2] <var_decl 0x7ffff1f4b000 e>)
[2 e+0 S2 A64])
so the information that e is 12 bytes long is harder to find (of course, one
can still look at MEM_EXPR's size).
Then store_bit_field_1 sees i386 has HAVE_insv and op_mode for -m64 insv is
DImode.  The first HAVE_insv if doesn't apply, as i386 (like also at least half
of other targets that HAVE_insv) doesn't allow memory on op0.
But then the if (HAVE_insv && MEM_P (op0)) triggers and it calls get_best_mode,
which for volatile (this case) and !targetm.narrow_bit_fields () decides to
enlarge the mode as much as possible and this "possible" takes into account
just
MEM_ALIGN (which comes from DECL_ALIGN and thus is large here) and the passed
in largest_mode (for insv it comes from insv insn mode).  It doesn't consider
the size of the decl (but see above, expand_assignment made it impossible to
use MEM_SIZE here).  Now, if this if (HAVE_insv && MEM_P (op0)) case is
disabled, it still generates wrong code, as store_fixed_bit_field does
essentially the same, just instead of insv's mode uses word_mode as
largest_mode passed to get_best_mode.

Now, the question is what we want to use as largest_mode in these cases.
Obviously it can't be too large to go beyond end of the object's size here (the
case in this testcase, the reason why sched2 reorders the two stores is because
it sees the first store using SYMBOL_REF ("e") as base and the second store
using SYMBOL_REF ("f") and assumes that those can't alias).  For OpenMP or
C++0x
memory model that isn't enough I guess, even if you have:
struct S
{
  signed a : 26;
  signed b : 16;
  signed c : 10;
  volatile signed d : 14;
  int e;
} s;
I think you can't just modify s.e when writing s.d (I think it is fine to
modify
adjacent bitfields though, Aldy?).  So for C++0x memory model we probably want
to find the next non-bitfield field and use that or something similar.

[Bug middle-end/48124] [4.3/4.4/4.5/4.6/4.7 Regression] likely wrong code bug

[aldyh at redhat dot com]

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

--- Comment #5 from Aldy Hernandez <aldyh at redhat dot com> 2011-03-15 12:42:36 UTC ---
> struct S
> {
>    signed a : 26;
>    signed b : 16;
>    signed c : 10;
>    volatile signed d : 14;
>    int e;
> } s;
> I think you can't just modify s.e when writing s.d (I think it is fine to
> modify
> adjacent bitfields though, Aldy?).

No, you can't modify s.e when writing to s.d.  However, you can modify 
adjacent bitfields.  All contiguous bitfields can be considered a single 
memory location for the purpose of introducing data races.  The only 
exception is when bitfields are separated by a zero-length bitfield, or 
when they happen to be contiguous but occur in different 
structures/unions.  Those conditions force alignments on those fields.

[Bug middle-end/48124] [4.3/4.4/4.5/4.6/4.7 Regression] likely wrong code bug

[jakub at gcc dot gnu.org]

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

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> 2011-03-31 14:27:15 UTC ---
Created attachment 23837
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=23837
gcc46-pr48124.patch

WIP patch, completely untested, that attempts to pass down to get_best_mode
exact offset and type.  The patch just attempts to avoid larger modes crossing
into following objects, but for C++0x model it probably should do more than
that.

[Bug middle-end/48124] [4.3/4.4/4.5/4.6/4.7 Regression] likely wrong code bug

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |wrong-code
           Priority|P3                          |P2

[Bug middle-end/48124] [4.3/4.4/4.5/4.6/4.7 Regression] likely wrong code bug

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |arthur.j.odwyer at gmail
                   |                            |dot com

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> 2011-05-20 05:53:49 UTC ---
*** Bug 49071 has been marked as a duplicate of this bug. ***

[Bug middle-end/48124] [4.3/4.4/4.5/4.6/4.7 Regression] likely wrong code bug

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.3.6                       |4.4.7

--- Comment #8 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-06-27 12:12:23 UTC ---
4.3 branch is being closed, moving to 4.4.7 target.

[Bug middle-end/48124] [4.4/4.5/4.6/4.7 Regression] likely wrong code bug

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|52080                       |

--- Comment #9 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-02-01 12:34:55 UTC ---
Simpler patch I am going to test.  Let's hope the wreckage adjust_address
does to the to_rtx MEM (apart from setting its mode) is harmless.

Index: expr.c
===================================================================
--- expr.c      (revision 183791)
+++ expr.c      (working copy)
@@ -4705,6 +4705,21 @@ expand_assignment (tree to, tree from, b
            to_rtx = adjust_address (to_rtx, mode1, 0);
          else if (GET_MODE (to_rtx) == VOIDmode)
            to_rtx = adjust_address (to_rtx, BLKmode, 0);
+         /* If the alignment of tem is larger than its size and we
+            are performing a bitfield access limit the mode we use
+            for the access to make sure we do not access the decl
+            beyond its end.  See PR48124.  */
+         else if (GET_MODE (to_rtx) == BLKmode
+                  && mode1 == VOIDmode
+                  && DECL_P (tem)
+                  && TREE_CODE (DECL_SIZE (tem)) == INTEGER_CST
+                  && TREE_INT_CST_LOW (DECL_SIZE (tem)) % DECL_ALIGN (tem))
+           {
+             mode1 = mode_for_size (TREE_INT_CST_LOW (DECL_SIZE (tem))
+                                    % DECL_ALIGN (tem),
+                                    MODE_INT, 0);
+             to_rtx = adjust_address (to_rtx, mode1, 0);
+           }
        }

       if (offset != 0)

[Bug middle-end/48124] [4.4/4.5/4.6/4.7 Regression] likely wrong code bug

[rguenth at gcc dot gnu.org]

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

--- Comment #10 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-02-01 12:48:56 UTC ---
(In reply to comment #9)
> Simpler patch I am going to test.  Let's hope the wreckage adjust_address
> does to the to_rtx MEM (apart from setting its mode) is harmless.
> 
> Index: expr.c
> ===================================================================
> --- expr.c      (revision 183791)
> +++ expr.c      (working copy)
> @@ -4705,6 +4705,21 @@ expand_assignment (tree to, tree from, b
>             to_rtx = adjust_address (to_rtx, mode1, 0);
>           else if (GET_MODE (to_rtx) == VOIDmode)
>             to_rtx = adjust_address (to_rtx, BLKmode, 0);
> +         /* If the alignment of tem is larger than its size and we
> +            are performing a bitfield access limit the mode we use
> +            for the access to make sure we do not access the decl
> +            beyond its end.  See PR48124.  */
> +         else if (GET_MODE (to_rtx) == BLKmode
> +                  && mode1 == VOIDmode
> +                  && DECL_P (tem)
> +                  && TREE_CODE (DECL_SIZE (tem)) == INTEGER_CST
> +                  && TREE_INT_CST_LOW (DECL_SIZE (tem)) % DECL_ALIGN (tem))
> +           {
> +             mode1 = mode_for_size (TREE_INT_CST_LOW (DECL_SIZE (tem))
> +                                    % DECL_ALIGN (tem),
> +                                    MODE_INT, 0);
> +             to_rtx = adjust_address (to_rtx, mode1, 0);
> +           }
>         }
> 
>        if (offset != 0)

Or rather

Index: gcc/expr.c
===================================================================
--- gcc/expr.c  (revision 183791)
+++ gcc/expr.c  (working copy)
@@ -4705,6 +4705,22 @@ expand_assignment (tree to, tree from, b
            to_rtx = adjust_address (to_rtx, mode1, 0);
          else if (GET_MODE (to_rtx) == VOIDmode)
            to_rtx = adjust_address (to_rtx, BLKmode, 0);
+         /* If the alignment of tem is larger than its size and we
+            are performing a bitfield access limit the mode we use
+            for the access to make sure we do not access the decl
+            beyond its end.  See PR48124.  */
+         else if (GET_MODE (to_rtx) == BLKmode
+                  && mode1 == VOIDmode
+                  && DECL_P (tem)
+                  && TREE_CODE (DECL_SIZE (tem)) == INTEGER_CST
+                  && (TREE_INT_CST_LOW (DECL_SIZE (tem))
+                      & (DECL_ALIGN (tem) - 1)) != 0)
+           {
+             unsigned HOST_WIDE_INT mis;
+             mis = TREE_INT_CST_LOW (DECL_SIZE (tem)) & (DECL_ALIGN (tem) -
1);
+             mode1 = mode_for_size (mis & -mis, MODE_INT, 0);
+             to_rtx = adjust_address (to_rtx, mode1, 0);
+           }
        }

       if (offset != 0)

[Bug middle-end/48124] [4.4/4.5/4.6/4.7 Regression] likely wrong code bug

[rguenth at gcc dot gnu.org]

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

--- Comment #11 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-02-01 12:57:12 UTC ---
*** Bug 49758 has been marked as a duplicate of this bug. ***

[Bug middle-end/48124] [4.4/4.5/4.6/4.7 Regression] likely wrong code bug

[rguenth at gcc dot gnu.org]

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

--- Comment #12 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-02-01 13:05:33 UTC ---
*** Bug 51176 has been marked as a duplicate of this bug. ***

[Bug middle-end/48124] [4.4/4.5/4.6/4.7 Regression] likely wrong code bug

[rguenth at gcc dot gnu.org]

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

--- Comment #13 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-02-01 13:57:52 UTC ---
Doesn't work.

MEM_SIZE also seems to be somewhat random, because we
re-initialize it via set_mem_attributes_minus_bitpos.  So we can't
fixup the caller to get_best_mode easily without passing down extra
knowledge.

We could pass down fieldmode from store_bit_field_1 to store_fixed_bit_field
and give it some more meaningful mode in the ultimate caller
(expand_assignment).

But then there is the HAVE_insv path which uses mode_for_extraction
which would need to be disabled if that mode is bigger than fieldmode
(but we don't do that at the moment, not sure why).

As usual, bitfield expansion seems to be quite a messy area.

[Bug middle-end/48124] [4.4/4.5/4.6/4.7 Regression] likely wrong code bug

[rguenth at gcc dot gnu.org]

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

--- Comment #14 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-02-01 14:17:05 UTC ---
I remember (bug number?) we have the same issue with multi-word accesses
where the last part uses word_mode (all other pieces as well, of course)
and that accesses the object out-of-bounds.

[Bug middle-end/48124] [4.4/4.5/4.6/4.7 Regression] likely wrong code bug

[rguenth at gcc dot gnu.org]

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

--- Comment #15 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-02-01 14:53:21 UTC ---
*** Bug 50235 has been marked as a duplicate of this bug. ***

[Bug middle-end/48124] [4.4/4.5/4.6/4.7 Regression] likely wrong code bug

[rguenth at gcc dot gnu.org]

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

--- Comment #16 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-02-01 15:48:15 UTC ---
Created attachment 26546
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=26546
another patch

This patch passes bootstrap on x86_64-unknown-linux-gnu but ICEs in
struct-layout tests.

[Bug middle-end/48124] [4.4/4.5/4.6/4.7 Regression] likely wrong code bug

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
         AssignedTo|unassigned at gcc dot       |rguenth at gcc dot gnu.org
                   |gnu.org                     |

--- Comment #17 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-02-21 11:58:53 UTC ---
Mine, at least for 4.8, see PR52080 for the patch.

[Bug middle-end/48124] [4.5/4.6/4.7/4.8 Regression] likely wrong code bug

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.4.7                       |4.5.4

--- Comment #18 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-03-13 12:45:10 UTC ---
4.4 branch is being closed, moving to 4.5.4 target.

[Bug middle-end/48124] [4.5/4.6/4.7 Regression] likely wrong code bug

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |4.8.0
            Summary|[4.5/4.6/4.7/4.8            |[4.5/4.6/4.7 Regression]
                   |Regression] likely wrong    |likely wrong code bug
                   |code bug                    |
      Known to fail|                            |4.7.0

--- Comment #20 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-03-14 10:56:57 UTC ---
Fixed for 4.8.  Don't hold your breath for a backport of this patch (it should
apply cleanly to 4.7 though, but not to anything earlier).

[Bug middle-end/48124] [4.5/4.6/4.7/4.8 Regression] likely wrong code bug

[rguenth at gcc dot gnu.org]

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

--- Comment #19 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-03-14 10:55:17 UTC ---
Author: rguenth
Date: Wed Mar 14 10:55:09 2012
New Revision: 185379

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=185379
Log:
2012-03-14  Richard Guenther  <rguenther@suse.de>

    * tree.h (DECL_BIT_FIELD_REPRESENTATIVE): New define.
    * stor-layout.c (start_bitfield_representative): New function.
    (finish_bitfield_representative): Likewise.
    (finish_bitfield_layout): Likewise.
    (finish_record_layout): Call finish_bitfield_layout.
    * tree.c (free_lang_data_in_decl): Only free DECL_QUALIFIER
    for QUAL_UNION_TYPE fields.
    * tree-streamer-in.c (lto_input_ts_field_decl_tree_pointers):
    Stream DECL_BIT_FIELD_REPRESENTATIVE.
    * tree-streamer-out.c (write_ts_field_decl_tree_pointers): Likewise.

    PR middle-end/52080
    PR middle-end/52097
    PR middle-end/48124
    * expr.c (get_bit_range): Unconditionally extract bitrange
    from DECL_BIT_FIELD_REPRESENTATIVE.
    (expand_assignment): Adjust call to get_bit_range.

    * gcc.dg/torture/pr48124-1.c: New testcase.
    * gcc.dg/torture/pr48124-2.c: Likewise.
    * gcc.dg/torture/pr48124-3.c: Likewise.
    * gcc.dg/torture/pr48124-4.c: Likewise.

Added:
    trunk/gcc/testsuite/gcc.dg/torture/pr48124-1.c
    trunk/gcc/testsuite/gcc.dg/torture/pr48124-2.c
    trunk/gcc/testsuite/gcc.dg/torture/pr48124-3.c
    trunk/gcc/testsuite/gcc.dg/torture/pr48124-4.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/expr.c
    trunk/gcc/stor-layout.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-streamer-in.c
    trunk/gcc/tree-streamer-out.c
    trunk/gcc/tree.c
    trunk/gcc/tree.h

[Bug middle-end/48124] [4.5/4.6/4.7 Regression] likely wrong code bug

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #23837|0                           |1
        is obsolete|                            |
  Attachment #26546|0                           |1
        is obsolete|                            |

--- Comment #21 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-05-30 15:03:11 UTC ---
Created attachment 27525
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=27525
backport of patch and followups from trunk

Bootstrapped on x86_64-unknown-linux-gnu.

[Bug middle-end/48124] [4.5/4.6/4.7 Regression] likely wrong code bug

[ebotcazou at gcc dot gnu.org]

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

--- Comment #22 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2012-05-30 15:28:14 UTC ---
> Bootstrapped on x86_64-unknown-linux-gnu.

This also needs to be tested on 32-bit and strict-alignment platforms.

[Bug middle-end/48124] [4.5/4.6/4.7 Regression] likely wrong code bug

[rguenther at suse dot de]

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

--- Comment #23 from rguenther at suse dot de <rguenther at suse dot de> 2012-05-31 08:41:00 UTC ---
On Wed, 30 May 2012, ebotcazou at gcc dot gnu.org wrote:

> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48124
> 
> --- Comment #22 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2012-05-30 15:28:14 UTC ---
> > Bootstrapped on x86_64-unknown-linux-gnu.
> 
> This also needs to be tested on 32-bit and strict-alignment platforms.

I'll test it on i?85-linux as well.  I don't have access to a 
strict-alignment platform - but the patch is essentially the same
as on trunk.  Can you give it a shot on sparc or do you forsee any
issues and thus would rather not backport this kind of change?  The
idea was that 4.7.1 would still be ok but later this kind of change
is obviously too intrusive.

Thanks,
Richard.

[Bug middle-end/48124] [4.5/4.6/4.7 Regression] likely wrong code bug

[ebotcazou at gcc dot gnu.org]

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

--- Comment #24 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2012-05-31 10:36:10 UTC ---
> I'll test it on i?85-linux as well.  I don't have access to a 
> strict-alignment platform - but the patch is essentially the same
> as on trunk.  Can you give it a shot on sparc or do you forsee any
> issues and thus would rather not backport this kind of change?  The
> idea was that 4.7.1 would still be ok but later this kind of change
> is obviously too intrusive.

Yes, I think it's appropriate for 4.7.1 if all the follow-ups are backported as
well.  I'll give it a whirl on SPARC.

[Bug middle-end/48124] [4.5/4.6/4.7 Regression] likely wrong code bug

[rguenther at suse dot de]

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

--- Comment #24 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2012-05-31 10:36:10 UTC ---
> I'll test it on i?85-linux as well.  I don't have access to a 
> strict-alignment platform - but the patch is essentially the same
> as on trunk.  Can you give it a shot on sparc or do you forsee any
> issues and thus would rather not backport this kind of change?  The
> idea was that 4.7.1 would still be ok but later this kind of change
> is obviously too intrusive.

Yes, I think it's appropriate for 4.7.1 if all the follow-ups are backported as
well.  I'll give it a whirl on SPARC.

--- Comment #25 from rguenther at suse dot de <rguenther at suse dot de> 2012-05-31 10:38:23 UTC ---
On Thu, 31 May 2012, ebotcazou at gcc dot gnu.org wrote:

> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48124
> 
> --- Comment #24 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2012-05-31 10:36:10 UTC ---
> > I'll test it on i?85-linux as well.  I don't have access to a 
> > strict-alignment platform - but the patch is essentially the same
> > as on trunk.  Can you give it a shot on sparc or do you forsee any
> > issues and thus would rather not backport this kind of change?  The
> > idea was that 4.7.1 would still be ok but later this kind of change
> > is obviously too intrusive.
> 
> Yes, I think it's appropriate for 4.7.1 if all the follow-ups are backported as
> well.  I'll give it a whirl on SPARC.

I think I have included them all in the patch I attached.  Meanwhile
multilib testing has finished on x86_64, a pure i?86 bootstrap still
pending (I'm also testing on arm and ppc/ppc64 now, but that may
take a while)

[Bug middle-end/48124] [4.5/4.6/4.7 Regression] likely wrong code bug

[jakub at gcc dot gnu.org]

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

--- Comment #26 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-05-31 10:42:10 UTC ---
(In reply to comment #25)
> On Thu, 31 May 2012, ebotcazou at gcc dot gnu.org wrote:
> 
> > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48124
> > 
> > --- Comment #24 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2012-05-31 10:36:10 UTC ---
> > > I'll test it on i?85-linux as well.  I don't have access to a 
> > > strict-alignment platform - but the patch is essentially the same
> > > as on trunk.  Can you give it a shot on sparc or do you forsee any
> > > issues and thus would rather not backport this kind of change?  The
> > > idea was that 4.7.1 would still be ok but later this kind of change
> > > is obviously too intrusive.
> > 
> > Yes, I think it's appropriate for 4.7.1 if all the follow-ups are backported as
> > well.  I'll give it a whirl on SPARC.
> 
> I think I have included them all in the patch I attached.  Meanwhile
> multilib testing has finished on x86_64, a pure i?86 bootstrap still
> pending (I'm also testing on arm and ppc/ppc64 now, but that may
> take a while)

http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=186819
is kind of a follow-up too.

[Bug middle-end/48124] [4.5/4.6/4.7 Regression] likely wrong code bug

[rguenther at suse dot de]

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

--- Comment #27 from rguenther at suse dot de <rguenther at suse dot de> 2012-05-31 10:48:19 UTC ---
On Thu, 31 May 2012, jakub at gcc dot gnu.org wrote:

> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48124
> 
> --- Comment #26 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-05-31 10:42:10 UTC ---
> (In reply to comment #25)
> > On Thu, 31 May 2012, ebotcazou at gcc dot gnu.org wrote:
> > 
> > > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48124
> > > 
> > > --- Comment #24 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2012-05-31 10:36:10 UTC ---
> > > > I'll test it on i?85-linux as well.  I don't have access to a 
> > > > strict-alignment platform - but the patch is essentially the same
> > > > as on trunk.  Can you give it a shot on sparc or do you forsee any
> > > > issues and thus would rather not backport this kind of change?  The
> > > > idea was that 4.7.1 would still be ok but later this kind of change
> > > > is obviously too intrusive.
> > > 
> > > Yes, I think it's appropriate for 4.7.1 if all the follow-ups are backported as
> > > well.  I'll give it a whirl on SPARC.
> > 
> > I think I have included them all in the patch I attached.  Meanwhile
> > multilib testing has finished on x86_64, a pure i?86 bootstrap still
> > pending (I'm also testing on arm and ppc/ppc64 now, but that may
> > take a while)
> 
> http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=186819
> is kind of a follow-up too.

Yes, that's what motivated me to look again at a possible backport
of the DECL_BIT_FIELD_REPRESENTATIVE patch.  I'd backport that separately
though.

[Bug middle-end/48124] [4.5/4.6/4.7 Regression] likely wrong code bug

[rguenther at suse dot de]

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

--- Comment #28 from rguenther at suse dot de <rguenther at suse dot de> 2012-06-01 10:20:09 UTC ---
On Wed, 30 May 2012, ebotcazou at gcc dot gnu.org wrote:

> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48124
> 
> --- Comment #22 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2012-05-30 15:28:14 UTC ---
> > Bootstrapped on x86_64-unknown-linux-gnu.
> 
> This also needs to be tested on 32-bit and strict-alignment platforms.

I have now successfully tested it on i586-suse-linux-gnu,
powerpc-suse-linux-gnu and powerpc64-suse-linux-gnu.

[Bug middle-end/48124] [4.5/4.6/4.7 Regression] likely wrong code bug

[ebotcazou at gcc dot gnu.org]

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

--- Comment #29 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2012-06-03 08:22:01 UTC ---
> I have now successfully tested it on i586-suse-linux-gnu,
> powerpc-suse-linux-gnu and powerpc64-suse-linux-gnu.

It looks OK on x86/x86-64/PowerPC/SPARC/SPARC64 on my side.

[Bug middle-end/48124] [4.5/4.6/4.7 Regression] likely wrong code bug

[rguenth at gcc dot gnu.org]

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

--- Comment #30 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-06-04 08:43:32 UTC ---
Author: rguenth
Date: Mon Jun  4 08:43:23 2012
New Revision: 188167

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=188167
Log:
2012-06-04  Richard Guenther  <rguenther@suse.de>
    Eric Botcazou  <ebotcazou@adacore.com>

    Backport from mainline
    2012-04-03  Eric Botcazou  <ebotcazou@adacore.com>

        * expr.c (get_bit_range): Add OFFSET parameter and adjust BITPOS.
        Change type of BITOFFSET to signed.  Make sure the lower bound of
        the computed range is non-negative by adjusting OFFSET and BITPOS.
        (expand_assignment): Adjust call to get_bit_range.

    2012-03-27  Eric Botcazou  <ebotcazou@adacore.com>

        * expr.c (get_bit_range): Return the null range if the enclosing record
        is part of a larger bit field.

    2012-03-20  Richard Guenther  <rguenther@suse.de>

        * stor-layout.c (finish_bitfield_representative): Fallback
        to conservative maximum size if the padding up to the next
        field cannot be computed as a constant.
        (finish_bitfield_layout): If we cannot compute the distance
        between the start of the bitfield representative and the
        bitfield member start a new representative.
        * expr.c (get_bit_range): The distance between the start of
        the bitfield representative and the bitfield member is zero
        if the field offsets are not constants.

    2012-03-16  Richard Guenther  <rguenther@suse.de>

        * stor-layout.c (finish_bitfield_representative): Fall back
        to the conservative maximum size if we cannot compute the
        size of the tail padding.

    2012-03-14  Richard Guenther  <rguenther@suse.de>

    * tree.h (DECL_BIT_FIELD_REPRESENTATIVE): New define.
    * stor-layout.c (start_bitfield_representative): New function.
    (finish_bitfield_representative): Likewise.
    (finish_bitfield_layout): Likewise.
    (finish_record_layout): Call finish_bitfield_layout.
    * tree.c (free_lang_data_in_decl): Only free DECL_QUALIFIER
    for QUAL_UNION_TYPE fields.
    * tree-streamer-in.c (lto_input_ts_field_decl_tree_pointers):
    Stream DECL_BIT_FIELD_REPRESENTATIVE.
    * tree-streamer-out.c (write_ts_field_decl_tree_pointers): Likewise.

    PR middle-end/52080
    PR middle-end/52097
    PR middle-end/48124
    * expr.c (get_bit_range): Unconditionally extract bitrange
    from DECL_BIT_FIELD_REPRESENTATIVE.
    (expand_assignment): Adjust call to get_bit_range.

    * gcc.dg/torture/pr48124-1.c: New testcase.
    * gcc.dg/torture/pr48124-2.c: Likewise.
    * gcc.dg/torture/pr48124-3.c: Likewise.
    * gcc.dg/torture/pr48124-4.c: Likewise.
    * gnat.dg/pack16.adb: Likewise.
    * gnat.dg/pack16_pkg.ads: Likewise.
    * gnat.dg/pack17.adb: Likewise.
    * gnat.dg/specs/pack7.ads: Likewise.
    * gnat.dg/specs/pack8.ads: Likewise.
    * gnat.dg/specs/pack8_pkg.ads: Likewise.

Added:
    branches/gcc-4_7-branch/gcc/testsuite/gcc.dg/torture/pr48124-1.c
    branches/gcc-4_7-branch/gcc/testsuite/gcc.dg/torture/pr48124-2.c
    branches/gcc-4_7-branch/gcc/testsuite/gcc.dg/torture/pr48124-3.c
    branches/gcc-4_7-branch/gcc/testsuite/gcc.dg/torture/pr48124-4.c
    branches/gcc-4_7-branch/gcc/testsuite/gnat.dg/pack16.adb
    branches/gcc-4_7-branch/gcc/testsuite/gnat.dg/pack16_pkg.ads
    branches/gcc-4_7-branch/gcc/testsuite/gnat.dg/pack17.adb
    branches/gcc-4_7-branch/gcc/testsuite/gnat.dg/specs/pack7.ads
    branches/gcc-4_7-branch/gcc/testsuite/gnat.dg/specs/pack8.ads
    branches/gcc-4_7-branch/gcc/testsuite/gnat.dg/specs/pack8_pkg.ads
Modified:
    branches/gcc-4_7-branch/gcc/ChangeLog
    branches/gcc-4_7-branch/gcc/expr.c
    branches/gcc-4_7-branch/gcc/stor-layout.c
    branches/gcc-4_7-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_7-branch/gcc/tree-streamer-in.c
    branches/gcc-4_7-branch/gcc/tree-streamer-out.c
    branches/gcc-4_7-branch/gcc/tree.c
    branches/gcc-4_7-branch/gcc/tree.h

[Bug middle-end/48124] [4.5/4.6 Regression] likely wrong code bug

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |NEW
      Known to work|                            |4.7.1
         AssignedTo|rguenth at gcc dot gnu.org  |unassigned at gcc dot
                   |                            |gnu.org
            Summary|[4.5/4.6/4.7 Regression]    |[4.5/4.6 Regression] likely
                   |likely wrong code bug       |wrong code bug

--- Comment #30 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-06-04 08:43:32 UTC ---
Author: rguenth
Date: Mon Jun  4 08:43:23 2012
New Revision: 188167

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=188167
Log:
2012-06-04  Richard Guenther  <rguenther@suse.de>
    Eric Botcazou  <ebotcazou@adacore.com>

    Backport from mainline
    2012-04-03  Eric Botcazou  <ebotcazou@adacore.com>

        * expr.c (get_bit_range): Add OFFSET parameter and adjust BITPOS.
        Change type of BITOFFSET to signed.  Make sure the lower bound of
        the computed range is non-negative by adjusting OFFSET and BITPOS.
        (expand_assignment): Adjust call to get_bit_range.

    2012-03-27  Eric Botcazou  <ebotcazou@adacore.com>

        * expr.c (get_bit_range): Return the null range if the enclosing record
        is part of a larger bit field.

    2012-03-20  Richard Guenther  <rguenther@suse.de>

        * stor-layout.c (finish_bitfield_representative): Fallback
        to conservative maximum size if the padding up to the next
        field cannot be computed as a constant.
        (finish_bitfield_layout): If we cannot compute the distance
        between the start of the bitfield representative and the
        bitfield member start a new representative.
        * expr.c (get_bit_range): The distance between the start of
        the bitfield representative and the bitfield member is zero
        if the field offsets are not constants.

    2012-03-16  Richard Guenther  <rguenther@suse.de>

        * stor-layout.c (finish_bitfield_representative): Fall back
        to the conservative maximum size if we cannot compute the
        size of the tail padding.

    2012-03-14  Richard Guenther  <rguenther@suse.de>

    * tree.h (DECL_BIT_FIELD_REPRESENTATIVE): New define.
    * stor-layout.c (start_bitfield_representative): New function.
    (finish_bitfield_representative): Likewise.
    (finish_bitfield_layout): Likewise.
    (finish_record_layout): Call finish_bitfield_layout.
    * tree.c (free_lang_data_in_decl): Only free DECL_QUALIFIER
    for QUAL_UNION_TYPE fields.
    * tree-streamer-in.c (lto_input_ts_field_decl_tree_pointers):
    Stream DECL_BIT_FIELD_REPRESENTATIVE.
    * tree-streamer-out.c (write_ts_field_decl_tree_pointers): Likewise.

    PR middle-end/52080
    PR middle-end/52097
    PR middle-end/48124
    * expr.c (get_bit_range): Unconditionally extract bitrange
    from DECL_BIT_FIELD_REPRESENTATIVE.
    (expand_assignment): Adjust call to get_bit_range.

    * gcc.dg/torture/pr48124-1.c: New testcase.
    * gcc.dg/torture/pr48124-2.c: Likewise.
    * gcc.dg/torture/pr48124-3.c: Likewise.
    * gcc.dg/torture/pr48124-4.c: Likewise.
    * gnat.dg/pack16.adb: Likewise.
    * gnat.dg/pack16_pkg.ads: Likewise.
    * gnat.dg/pack17.adb: Likewise.
    * gnat.dg/specs/pack7.ads: Likewise.
    * gnat.dg/specs/pack8.ads: Likewise.
    * gnat.dg/specs/pack8_pkg.ads: Likewise.

Added:
    branches/gcc-4_7-branch/gcc/testsuite/gcc.dg/torture/pr48124-1.c
    branches/gcc-4_7-branch/gcc/testsuite/gcc.dg/torture/pr48124-2.c
    branches/gcc-4_7-branch/gcc/testsuite/gcc.dg/torture/pr48124-3.c
    branches/gcc-4_7-branch/gcc/testsuite/gcc.dg/torture/pr48124-4.c
    branches/gcc-4_7-branch/gcc/testsuite/gnat.dg/pack16.adb
    branches/gcc-4_7-branch/gcc/testsuite/gnat.dg/pack16_pkg.ads
    branches/gcc-4_7-branch/gcc/testsuite/gnat.dg/pack17.adb
    branches/gcc-4_7-branch/gcc/testsuite/gnat.dg/specs/pack7.ads
    branches/gcc-4_7-branch/gcc/testsuite/gnat.dg/specs/pack8.ads
    branches/gcc-4_7-branch/gcc/testsuite/gnat.dg/specs/pack8_pkg.ads
Modified:
    branches/gcc-4_7-branch/gcc/ChangeLog
    branches/gcc-4_7-branch/gcc/expr.c
    branches/gcc-4_7-branch/gcc/stor-layout.c
    branches/gcc-4_7-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_7-branch/gcc/tree-streamer-in.c
    branches/gcc-4_7-branch/gcc/tree-streamer-out.c
    branches/gcc-4_7-branch/gcc/tree.c
    branches/gcc-4_7-branch/gcc/tree.h

--- Comment #31 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-06-04 08:44:51 UTC ---
Fixed on the 4.7 branch, too.  Not applicable as is to older branches.

[Bug middle-end/48124] [4.5/4.6 Regression] likely wrong code bug

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.5.4                       |4.6.4

--- Comment #32 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-07-02 11:05:53 UTC ---
The 4.5 branch is being closed, adjusting target milestone.

[Bug middle-end/48124] [4.6 Regression] likely wrong code bug

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED
   Target Milestone|4.6.4                       |4.7.1

--- Comment #33 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-04-12 16:25:41 UTC ---
The 4.6 branch has been closed, fixed in GCC 4.7.1.