[Bug c++/45070] New: Miscompiled c++ class with packed attribute on ARM with -Os optimizations (Qt 4.6.2)

[Bug c++/45070] New: Miscompiled c++ class with packed attribute on ARM with -Os optimizations (Qt 4.6.2)

[siarhei dot siamashka at gmail dot com]

Compilation:
   arm-unknown-linux-gnueabi-g++ -Os -mcpu=cortex-a8 -o test test.cpp

Expected results:
   ./test
   65534 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0

Real results (some garbage data):
   ./test
   544 544 544 544 544 544 544 544 544 544 544 544 544 544 544 544

Note: This is not a big practical issue because Qt 4.7 does not use packed
attribute for QChar anymore (a good idea because using this packed attribute
results in a horribly slow code):
http://qt.gitorious.org/qt/qt/commit/1ec8acd77b6c048f5a68887ac7750b0764ade598


-- 
           Summary: Miscompiled c++ class with packed attribute on ARM with
                    -Os optimizations (Qt 4.6.2)
           Product: gcc
           Version: 4.5.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: siarhei dot siamashka at gmail dot com
GCC target triplet: arm-unknown-linux-gnueabi


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

[Bug c++/45070] Miscompiled c++ class with packed attribute on ARM with -Os optimizations (Qt 4.6.2)

[siarhei dot siamashka at gmail dot com]

------- Comment #1 from siarhei dot siamashka at gmail dot com  2010-07-25 23:25 -------
Created an attachment (id=21308)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=21308&action=view)
packed-testcase.cpp


-- 


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

[Bug c++/45070] Miscompiled c++ class with packed attribute on ARM with -Os optimizations (Qt 4.6.2)

[mikpe at it dot uu dot se]

------- Comment #2 from mikpe at it dot uu dot se  2010-07-26 08:49 -------
With -Os on armv5tel I see a random number repeated 16 times, with -O2 I see
the expected output.  gcc-4.4 and gcc-4.5 are affected.  (Can't test 4.6 or 4.3
right now.)


-- 

mikpe at it dot uu dot se changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mikpe at it dot uu dot se


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

[Bug c++/45070] Miscompiled c++ class with packed attribute on ARM with -Os optimizations (Qt 4.6.2)

[mikpe at it dot uu dot se]

------- Comment #3 from mikpe at it dot uu dot se  2010-07-26 09:33 -------
Created an attachment (id=21312)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=21312&action=view)
reduced test case in C

You don't need C++ to trigger the bug.  Proper C with a function that may
recurse before returning a packed struct suffices.


-- 


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

[Bug c++/45070] Miscompiled c++ class with packed attribute on ARM with -Os optimizations (Qt 4.6.2)

[siarhei dot siamashka at gmail dot com]

------- Comment #4 from siarhei dot siamashka at gmail dot com  2010-07-28 07:16 -------
Could not reproduce the problem with gcc 4.3.5

Disassembly of pr45070.o:

0000000c <next>:
   c:   e92d401f        push    {r0, r1, r2, r3, r4, lr}
  10:   e890000c        ldm     r0, {r2, r3}
  14:   e1a04000        mov     r4, r0
  18:   e1520003        cmp     r2, r3
  1c:   b3a03000        movlt   r3, #0
  20:   ba000014        blt     78 <next+0x6c>
  24:   e5903008        ldr     r3, [r0, #8]
  28:   e3530000        cmp     r3, #0
  2c:   0a00000e        beq     6c <next+0x60>
  30:   e3a03000        mov     r3, #0
  34:   e5803008        str     r3, [r0, #8]
  38:   e2800004        add     r0, r0, #4
  3c:   ebffffef        bl      0 <fetch>
  40:   e1a00004        mov     r0, r4
  44:   ebfffff0        bl      c <next>
  48:   e1a00800        lsl     r0, r0, #16
  4c:   e1a00840        asr     r0, r0, #16
  50:   e5cd0000        strb    r0, [sp]
  54:   e1a00420        lsr     r0, r0, #8
  58:   e5cd0001        strb    r0, [sp, #1]
  5c:   e1dd30b0        ldrh    r3, [sp]
  60:   e1cd30bc        strh    r3, [sp, #12]
  64:   e1dd30bc        ldrh    r3, [sp, #12]
  68:   ea000002        b       78 <next+0x6c>
  6c:   e3a03001        mov     r3, #1
  70:   e5803008        str     r3, [r0, #8]
  74:   e59f3010        ldr     r3, [pc, #16]   ; 8c <next+0x80>
  78:   e1cd30bc        strh    r3, [sp, #12]
  7c:   e5dd300c        ldrb    r3, [sp, #12]
  80:   e5dd000d        ldrb    r0, [sp, #13]
  84:   e1830400        orr     r0, r3, r0, lsl #8
  88:   e8bd801f        pop     {r0, r1, r2, r3, r4, pc}

^^^ POP instruction just overwrites return value in r0 register here

  8c:   0000ffff        .word   0x0000ffff

Looks like the function gets treated as if it were returning 'void'.


-- 

siarhei dot siamashka at gmail dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |wrong-code
      Known to fail|                            |4.5.0
      Known to work|                            |4.3.5


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

[Bug c++/45070] Miscompiled c++ class with packed attribute on ARM with -Os optimizations (Qt 4.6.2)

[siarhei dot siamashka at gmail dot com]

------- Comment #5 from siarhei dot siamashka at gmail dot com  2010-07-28 07:18 -------
The disassembly chunk from the comment above was from gcc 4.5.0, using '-Os
-match=armv5te' options.


-- 


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

[Bug target/45070] Miscompiled c++ class with packed attribute on ARM with -Os optimizations (Qt 4.6.2)

[siarhei dot siamashka at gmail dot com]

------- Comment #6 from siarhei dot siamashka at gmail dot com  2010-07-28 08:37 -------
'arm_size_return_regs()' returns 2 when generating epilogue for 'next' function
here. And as a result, return value not registered in the mask, causing it to
be clobbered.

Would the following patch be the right fix?

Index: gcc/config/arm/arm.c
===================================================================
--- gcc/config/arm/arm.c        (revision 162411)
+++ gcc/config/arm/arm.c        (working copy)
@@ -13705,7 +13705,7 @@
                  && !crtl->tail_call_emit)
                {
                  unsigned long mask;
-                 mask = (1 << (arm_size_return_regs() / 4)) - 1;
+                 mask = (1 << ((arm_size_return_regs() + 3) / 4)) - 1;
                  mask ^= 0xf;
                  mask &= ~saved_regs_mask;
                  reg = 0;


-- 

siarhei dot siamashka at gmail dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|c++                         |target


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

[Bug target/45070] Miscompiled c++ class with packed attribute on ARM with -Os optimizations (Qt 4.6.2)

[ramana at gcc dot gnu dot org]

------- Comment #7 from ramana at gcc dot gnu dot org  2010-07-28 09:01 -------
Thanks for the analysis, yes that appears to be the nub of the problem with the
result being removed . I see the same problem on trunk - 

Patches should however be submitted to gcc-patches@gcc.gnu.org after
appropriate regression testing.

cheers
Ramana


-- 

ramana at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1
      Known to fail|4.5.0                       |4.4.4 4.5.0
   Last reconfirmed|0000-00-00 00:00:00         |2010-07-28 09:01:18
               date|                            |


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

[Bug target/45070] Miscompiled c++ class with packed attribute on ARM with -Os optimizations (Qt 4.6.2)

[ramana at gcc dot gnu dot org]

------- Comment #8 from ramana at gcc dot gnu dot org  2010-07-28 09:22 -------
(In reply to comment #7)
> Thanks for the analysis, yes that appears to be the nub of the problem with the
> result being removed . I see the same problem on trunk - 
> 


I just realized that this is a packed structure and probably need to look up
the semantics of this in the AAPCS. IIRC the AAPCS states that it doesn't
support packed structures or bitfields at exported interfaces. 

Adding Richard for some ABI commentary.

cheers
Ramana


-- 

ramana at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rearnsha at gcc dot gnu dot
                   |                            |org


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

[Bug target/45070] Miscompiled c++ class with packed attribute on ARM with -Os optimizations (Qt 4.6.2)

[rearnsha at gcc dot gnu dot org]

------- Comment #9 from rearnsha at gcc dot gnu dot org  2010-07-28 09:34 -------
(In reply to comment #8)

> I just realized that this is a packed structure and probably need to look up
> the semantics of this in the AAPCS. IIRC the AAPCS states that it doesn't
> support packed structures or bitfields at exported interfaces. 

That just means the ABI doesn't specify the behaviour here.  Portable code
shouldn't use this feature, but that doesn't mean that a compiler can't support
it as an extension.


-- 


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

[Bug target/45070] Miscompiled c++ class with packed attribute on ARM with -Os optimizations (Qt 4.6.2)

[mikpe at it dot uu dot se]

------- Comment #10 from mikpe at it dot uu dot se  2010-07-28 09:45 -------
(In reply to comment #8)
> I just realized that this is a packed structure and probably need to look up
> the semantics of this in the AAPCS. IIRC the AAPCS states that it doesn't
> support packed structures or bitfields at exported interfaces. 

In the C test case the function returning a packed struct is `static' and
called only by name from within the same translation unit.  Surely that's not
an "exported interface".


-- 


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

[Bug target/45070] Miscompiled c++ class with packed attribute on ARM with -Os optimizations (Qt 4.6.2)

[ian dot bolton at arm dot com]

------- Comment #11 from ian dot bolton at arm dot com  2010-08-03 17:13 -------
I have regression-tested the patch for gcc and g++ with target=arm-eabi and
created a regression testsuite case for it, so I expect to be able to submit
both to the mailing list for review in the next day or so.


-- 


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

[Bug target/45070] Miscompiled c++ class with packed attribute on ARM with -Os optimizations (Qt 4.6.2)

[armin76 at gentoo dot org]

------- Comment #12 from armin76 at gentoo dot org  2010-08-14 15:33 -------
Any news? :)


-- 


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

[Bug target/45070] Miscompiled c++ class with packed attribute on ARM with -Os optimizations (Qt 4.6.2)

[siarhei dot siamashka at gmail dot com]

------- Comment #13 from siarhei dot siamashka at gmail dot com  2010-08-14 16:28 -------
(In reply to comment #12)
> Any news? :)

http://gcc.gnu.org/ml/gcc-patches/2010-08/msg00894.html


-- 


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

[Bug target/45070] Miscompiled c++ class with packed attribute on ARM with -Os optimizations (Qt 4.6.2)

[ramana at gcc dot gnu dot org]

------- Comment #14 from ramana at gcc dot gnu dot org  2010-08-19 08:28 -------
Subject: Bug 45070

Author: ramana
Date: Thu Aug 19 08:27:59 2010
New Revision: 163367

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=163367
Log:
For Ian Bolton <ian.bolton@arm.com>

2010-08-19  Ian Bolton  <ian.bolton@arm.com>

        PR target/45070
        * gcc.c-torture/execute/pr45070.c: New.
        * config/arm/arm.c (arm_output_epilogue): Ensure that return
         value of size 1-3 is handled correctly.

Added:
    trunk/gcc/testsuite/gcc.c-torture/execute/pr45070.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/arm/arm.c
    trunk/gcc/testsuite/ChangeLog


-- 


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

[Bug target/45070] Miscompiled c++ class with packed attribute on ARM with -Os optimizations (Qt 4.6.2)

[ibolton at gcc dot gnu dot org]

------- Comment #15 from ibolton at gcc dot gnu dot org  2010-09-02 13:05 -------
Subject: Bug 45070

Author: ibolton
Date: Thu Sep  2 13:05:30 2010
New Revision: 163774

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=163774
Log:
2010-09-02  Ian Bolton  <ian.bolton@arm.com>

        Backport from mainline
        2010-09-01  Ian Bolton  <ian.bolton@arm.com>

        * Makefile.in (tree-switch-conversion.o): Update dependencies.

        2010-08-19  Ian Bolton  <ian.bolton@arm.com>

        PR target/45070
        * config/arm/arm.c (arm_output_epilogue): Ensure that return
        value of size 1-3 is handled correctly.

        * gcc.c-torture/execute/pr45070.c: New.

        2010-08-19  Ian Bolton  <ian.bolton@arm.com>

        * tree-switch-conversion.c (gen_inbound_check): Ensure that the
        type for the conditional has wide enough range.

        * g++.dg/pr44328.C: New test.

        2010-08-07  Marcus Shawcroft  <marcus.shawcroft@arm.com>

        * config/arm/linux-atomic.c (SUBWORD_VAL_CAS): Instantiate with
        'unsigned short' and 'unsigned char' instead of 'short' and 'char'.
        (SUBWORD_BOOL_CAS): Likewise.
        (SUBWORD_SYNC_OP): Likewise.
        (SUBWORD_TEST_AND_SET): Likewise.
        (FETCH_AND_OP_WORD): Parenthesise INF_OP
        (SUBWORD_SYNC_OP): Likewise.
        (OP_AND_FETCH_WORD): Likewise.

        * lib/target-supports.exp: (check_effective_target_sync_int_long):
        Add arm*-*-linux-gnueabi.
        (check_effective_target_sync_char_short): Likewise.

Added:
    branches/gcc-4_5-branch/gcc/testsuite/g++.dg/pr44328.C
      - copied unchanged from r163367, trunk/gcc/testsuite/g++.dg/pr44328.C
    branches/gcc-4_5-branch/gcc/testsuite/gcc.c-torture/execute/pr45070.c
      - copied unchanged from r163367,
trunk/gcc/testsuite/gcc.c-torture/execute/pr45070.c
Modified:
    branches/gcc-4_5-branch/gcc/ChangeLog
    branches/gcc-4_5-branch/gcc/Makefile.in
    branches/gcc-4_5-branch/gcc/config/arm/arm.c
    branches/gcc-4_5-branch/gcc/config/arm/linux-atomic.c
    branches/gcc-4_5-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_5-branch/gcc/testsuite/lib/target-supports.exp
    branches/gcc-4_5-branch/gcc/tree-switch-conversion.c


-- 


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

[Bug target/45070] Miscompiled c++ class with packed attribute on ARM with -Os optimizations (Qt 4.6.2)

[armin76 at gentoo dot org]

------- Comment #16 from armin76 at gentoo dot org  2010-09-04 16:41 -------
I'd like it backported to 4.4 if possible, thanks


-- 


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

[Bug target/45070] Miscompiled c++ class with packed attribute on ARM with -Os optimizations (Qt 4.6.2)

[ibolton at gcc dot gnu dot org]

------- Comment #17 from ibolton at gcc dot gnu dot org  2010-09-07 09:26 -------
(In reply to comment #16)
> I'd like it backported to 4.4 if possible, thanks
> 

Just awaiting approval on the mailing list.  It's ready to go.


-- 

ibolton at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |ibolton at gcc dot gnu dot
                   |dot org                     |org
             Status|NEW                         |ASSIGNED
   Last reconfirmed|2010-07-28 09:01:18         |2010-09-07 09:26:10
               date|                            |


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

[Bug target/45070] Miscompiled c++ class with packed attribute on ARM with -Os optimizations (Qt 4.6.2)

[ibolton at gcc dot gnu dot org]

------- Comment #18 from ibolton at gcc dot gnu dot org  2010-09-07 11:07 -------
Subject: Bug 45070

Author: ibolton
Date: Tue Sep  7 11:07:31 2010
New Revision: 163945

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=163945
Log:
2010-09-07  Ian Bolton  <ian.bolton@arm.com>

        Backport from mainline
        2010-09-01  Ian Bolton  <ian.bolton@arm.com>

        * Makefile.in (tree-switch-conversion.o): Update dependencies.

        2010-08-19  Ian Bolton  <ian.bolton@arm.com>

        PR target/45070
        * config/arm/arm.c (arm_output_epilogue): Ensure that return
        value of size 1-3 is handled correctly.

        * gcc.c-torture/execute/pr45070.c: New.

        2010-08-19  Ian Bolton  <ian.bolton@arm.com>

        * tree-switch-conversion.c (gen_inbound_check): Ensure that the
        type for the conditional has wide enough range.

        * g++.dg/pr44328.C: New test.

        2010-08-07  Marcus Shawcroft  <marcus.shawcroft@arm.com>

        * config/arm/linux-atomic.c (SUBWORD_VAL_CAS): Instantiate with
        'unsigned short' and 'unsigned char' instead of 'short' and 'char'.
        (SUBWORD_BOOL_CAS): Likewise.
        (SUBWORD_SYNC_OP): Likewise.
        (SUBWORD_TEST_AND_SET): Likewise.
        (FETCH_AND_OP_WORD): Parenthesise INF_OP
        (SUBWORD_SYNC_OP): Likewise.
        (OP_AND_FETCH_WORD): Likewise.

        * lib/target-supports.exp: (check_effective_target_sync_int_long):
        Add arm*-*-linux-gnueabi.
        (check_effective_target_sync_char_short): Likewise.


Added:
    branches/gcc-4_4-branch/gcc/testsuite/g++.dg/pr44328.C
      - copied unchanged from r163774,
branches/gcc-4_5-branch/gcc/testsuite/g++.dg/pr44328.C
    branches/gcc-4_4-branch/gcc/testsuite/gcc.c-torture/execute/pr45070.c
      - copied unchanged from r163774,
branches/gcc-4_5-branch/gcc/testsuite/gcc.c-torture/execute/pr45070.c
Modified:
    branches/gcc-4_4-branch/gcc/ChangeLog
    branches/gcc-4_4-branch/gcc/Makefile.in
    branches/gcc-4_4-branch/gcc/config/arm/arm.c
    branches/gcc-4_4-branch/gcc/config/arm/linux-atomic.c
    branches/gcc-4_4-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_4-branch/gcc/testsuite/lib/target-supports.exp
    branches/gcc-4_4-branch/gcc/tree-switch-conversion.c


-- 


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

[Bug target/45070] Miscompiled c++ class with packed attribute on ARM with -Os optimizations (Qt 4.6.2)

[rearnsha at gcc dot gnu dot org]

------- Comment #19 from rearnsha at gcc dot gnu dot org  2010-09-14 14:05 -------
Fixed in all maintained releases.


-- 

rearnsha at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED
   Target Milestone|---                         |4.4.5


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