[Bug tree-optimization/48784] New: #pragma pack(1) + -fstrict-volatile-bitfields = bad codegen

[Bug tree-optimization/48784] New: #pragma pack(1) + -fstrict-volatile-bitfields = bad codegen

[arthur.j.odwyer at gmail dot com]

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

           Summary: #pragma pack(1) + -fstrict-volatile-bitfields = bad
                    codegen
           Product: gcc
           Version: 4.7.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: arthur.j.odwyer@gmail.com


Created attachment 24109
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=24109
Output of "ajo-gcc -fstrict-volatile-bitfields bug868575724-reduced.c -v"

#pragma pack(1) is incompatible with -fstrict-volatile-bitfields, causing
silently wrong codegen if you mix the two options. It feels like the compiler
really ought to warn the user that bad code will be generated, even if it *is*
ultimately the user's fault.  ...But is it even the user's fault, in this case?

cat >test.c <<EOF
#include <stdio.h>
#pragma pack(1)
volatile struct S0 {
   signed a : 7;
   unsigned b : 28;  /* b can't be fetched with an aligned 32-bit access, */
                     /* but it certainly can be fetched with an unaligned
access */
} g = {0,-1};
int main() {
    printf("%x\n", (unsigned int)g.b);
    return 0;
}
EOF
gcc test.c ; ./a.out  // prints "fffffff"
gcc -fstrict-volatile-bitfields test.c ; ./a.out  // prints "1ffffff"

Without -fstrict-volatile-bitfields, the correct 28-bit number "fffffff" is
printed. With -fstrict-volatile-bitfields, the incorrect 25-bit number
"1ffffff" is printed. 

Bug 43341 is a similar gray-area bug/not-a-bug involving #pragma pack.


This test case is reduced from the output of Csmith
(http://embed.cs.utah.edu/csmith/), using the following command line:
csmith --bitfields --packed-struct -s 868575724 > test868575724.c

[Bug middle-end/48784] #pragma pack(1) + -fstrict-volatile-bitfields = bad codegen

[sandra at codesourcery dot com]

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

Sandra Loosemore <sandra at codesourcery dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |sandra at codesourcery dot
                   |                            |com

--- Comment #1 from Sandra Loosemore <sandra at codesourcery dot com> 2012-08-18 01:11:12 UTC ---
I just checked the test case on mainline head for a couple other builds I have
handy.

ARM EABI prints "1ffffff" whether or not you compile with
-fstrict-volatile-bitfields.

MIPS ELF prints "0" with -fstrict-volatile-bitfields and "fffffff" without.

[Bug middle-end/48784] #pragma pack(1) + -fstrict-volatile-bitfields = bad codegen

[sandra at codesourcery dot com]

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

--- Comment #2 from Sandra Loosemore <sandra at codesourcery dot com> ---
I'm working on a fix for this.

[Bug middle-end/48784] #pragma pack(1) + -fstrict-volatile-bitfields = bad codegen

[sandra at codesourcery dot com]

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

--- Comment #3 from Sandra Loosemore <sandra at codesourcery dot com> ---
Patch posted here:

http://gcc.gnu.org/ml/gcc-patches/2013-06/msg00750.html

[Bug middle-end/48784] #pragma pack(1) + -fstrict-volatile-bitfields = bad codegen

[sandra at codesourcery dot com]

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

--- Comment #4 from Sandra Loosemore <sandra at codesourcery dot com> ---
Updated patch series:

http://gcc.gnu.org/ml/gcc-patches/2013-09/msg02057.html
http://gcc.gnu.org/ml/gcc-patches/2013-09/msg02058.html
http://gcc.gnu.org/ml/gcc-patches/2013-09/msg02059.html

Unfortunately, it seems that fixing bugs with -fstrict-volatile-bitfields has
been blocked by disagreement between global reviewers and target maintainers
who can't agree on whether the C/C++11 memory model should take precedence over
target-specific ABIs by default.  :-(

[Bug middle-end/48784] #pragma pack(1) + -fstrict-volatile-bitfields = bad codegen

[edlinger at gcc dot gnu.org]

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

--- Comment #5 from edlinger at gcc dot gnu.org ---
Author: edlinger
Date: Wed Dec 11 16:50:05 2013
New Revision: 205896

URL: http://gcc.gnu.org/viewcvs?rev=205896&root=gcc&view=rev
Log:
2013-12-11  Sandra Loosemore  <sandra@codesourcery.com>

        PR middle-end/23623
        PR middle-end/48784
        PR middle-end/56341
        PR middle-end/56997

        gcc/
        * expmed.c (strict_volatile_bitfield_p): New function.
        (store_bit_field_1): Don't special-case strict volatile
        bitfields here.
        (store_bit_field): Handle strict volatile bitfields here instead.
        (store_fixed_bit_field): Don't special-case strict volatile
        bitfields here.
        (extract_bit_field_1): Don't special-case strict volatile
        bitfields here.
        (extract_bit_field): Handle strict volatile bitfields here instead.
        (extract_fixed_bit_field): Don't special-case strict volatile
        bitfields here.  Simplify surrounding code to resemble that in
        store_fixed_bit_field.
        * doc/invoke.texi (Code Gen Options): Update
        -fstrict-volatile-bitfields description.

        gcc/testsuite/
        * gcc.dg/pr23623.c: New test.
        * gcc.dg/pr48784-1.c: New test.
        * gcc.dg/pr48784-2.c: New test.
        * gcc.dg/pr56341-1.c: New test.
        * gcc.dg/pr56341-2.c: New test.
        * gcc.dg/pr56997-1.c: New test.
        * gcc.dg/pr56997-2.c: New test.
        * gcc.dg/pr56997-3.c: New test.

Added:
    trunk/gcc/testsuite/gcc.dg/pr23623.c
    trunk/gcc/testsuite/gcc.dg/pr48784-1.c
    trunk/gcc/testsuite/gcc.dg/pr48784-2.c
    trunk/gcc/testsuite/gcc.dg/pr56341-1.c
    trunk/gcc/testsuite/gcc.dg/pr56341-2.c
    trunk/gcc/testsuite/gcc.dg/pr56997-1.c
    trunk/gcc/testsuite/gcc.dg/pr56997-2.c
    trunk/gcc/testsuite/gcc.dg/pr56997-3.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/doc/invoke.texi
    trunk/gcc/expmed.c
    trunk/gcc/testsuite/ChangeLog

[Bug middle-end/48784] #pragma pack(1) + -fstrict-volatile-bitfields = bad codegen

[edlinger at gcc dot gnu.org]

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

--- Comment #6 from edlinger at gcc dot gnu.org ---
Author: edlinger
Date: Wed Dec 11 16:59:24 2013
New Revision: 205897

URL: http://gcc.gnu.org/viewcvs?rev=205897&root=gcc&view=rev
Log:
2013-12-11  Bernd Edlinger  <bernd.edlinger@hotmail.de>
            Sandra Loosemore  <sandra@codesourcery.com>

        PR middle-end/23623
        PR middle-end/48784
        PR middle-end/56341
        PR middle-end/56997
        * expmed.c (strict_volatile_bitfield_p): Add bitregion_start
        and bitregion_end parameters.  Test for compliance with C++
        memory model.
        (store_bit_field): Adjust call to strict_volatile_bitfield_p.
        Add fallback logic for cases where -fstrict-volatile-bitfields
        is supposed to apply, but cannot.
        (extract_bit_field): Likewise. Use narrow_bit_field_mem and
        extract_fixed_bit_field_1 to do the extraction.
        (extract_fixed_bit_field): Revert to previous mode selection algorithm.
        Call extract_fixed_bit_field_1 to do the real work.
        (extract_fixed_bit_field_1): New function.

testsuite:        
        * gcc.dg/pr23623.c: Update to test interaction with C++
        memory model.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/expmed.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gcc.dg/pr23623.c

[Bug middle-end/48784] #pragma pack(1) + -fstrict-volatile-bitfields = bad codegen

[jye2 at gcc dot gnu.org]

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

--- Comment #7 from jye2 at gcc dot gnu.org ---
Author: jye2
Date: Thu Feb 27 07:28:06 2014
New Revision: 208195

URL: http://gcc.gnu.org/viewcvs?rev=208195&root=gcc&view=rev
Log:
2014-02-27  Joey Ye  <joey.ye@arm.com>

        Backport mainline strict-volatile-bitfields fixes
    2013-09-28  Sandra Loosemore  <sandra@codesourcery.com>

        gcc/
        * expr.h (extract_bit_field): Remove packedp parameter.
        * expmed.c (extract_fixed_bit_field): Remove packedp parameter
        from forward declaration.
        (store_split_bit_field): Remove packedp arg from calls to
        extract_fixed_bit_field.
        (extract_bit_field_1): Remove packedp parameter and packedp
        argument from recursive calls and calls to extract_fixed_bit_field.
        (extract_bit_field): Remove packedp parameter and corresponding
        arg to extract_bit_field_1.
        (extract_fixed_bit_field): Remove packedp parameter.  Remove code
        to issue warnings.
        (extract_split_bit_field): Remove packedp arg from call to
        extract_fixed_bit_field.
        * expr.c (emit_group_load_1): Adjust calls to extract_bit_field.
        (copy_blkmode_from_reg): Likewise.
        (copy_blkmode_to_reg): Likewise.
        (read_complex_part): Likewise.
        (store_field): Likewise.
        (expand_expr_real_1): Likewise.
        * calls.c (store_unaligned_arguments_into_pseudos): Adjust call
        to extract_bit_field.
        * config/tilegx/tilegx.c (tilegx_expand_unaligned_load): Adjust
        call to extract_bit_field.
        * config/tilepro/tilepro.c (tilepro_expand_unaligned_load): Adjust
        call to extract_bit_field.
        * doc/invoke.texi (Code Gen Options): Remove mention of warnings
        and special packedp behavior from -fstrict-volatile-bitfields
        documentation.

2013-12-11  Bernd Edlinger  <bernd.edlinger@hotmail.de>

        * expr.c (expand_assignment): Remove dependency on 
        flag_strict_volatile_bitfields. Always set the memory
        access mode.
        (expand_expr_real_1): Likewise.

2013-12-11  Sandra Loosemore  <sandra@codesourcery.com>

        PR middle-end/23623
        PR middle-end/48784
        PR middle-end/56341
        PR middle-end/56997

        gcc/
        * expmed.c (strict_volatile_bitfield_p): New function.
        (store_bit_field_1): Don't special-case strict volatile
        bitfields here.
        (store_bit_field): Handle strict volatile bitfields here instead.
        (store_fixed_bit_field): Don't special-case strict volatile
        bitfields here.
        (extract_bit_field_1): Don't special-case strict volatile
        bitfields here.
        (extract_bit_field): Handle strict volatile bitfields here instead.
        (extract_fixed_bit_field): Don't special-case strict volatile
        bitfields here.  Simplify surrounding code to resemble that in
        store_fixed_bit_field.
        * doc/invoke.texi (Code Gen Options): Update
        -fstrict-volatile-bitfields description.

        gcc/testsuite/
        * gcc.dg/pr23623.c: New test.
        * gcc.dg/pr48784-1.c: New test.
        * gcc.dg/pr48784-2.c: New test.
        * gcc.dg/pr56341-1.c: New test.
        * gcc.dg/pr56341-2.c: New test.
        * gcc.dg/pr56997-1.c: New test.
        * gcc.dg/pr56997-2.c: New test.
        * gcc.dg/pr56997-3.c: New test.

2013-12-11  Bernd Edlinger  <bernd.edlinger@hotmail.de>
             Sandra Loosemore  <sandra@codesourcery.com>

        PR middle-end/23623
        PR middle-end/48784
        PR middle-end/56341
        PR middle-end/56997
        * expmed.c (strict_volatile_bitfield_p): Add bitregion_start
        and bitregion_end parameters.  Test for compliance with C++
        memory model.
        (store_bit_field): Adjust call to strict_volatile_bitfield_p.
        Add fallback logic for cases where -fstrict-volatile-bitfields
        is supposed to apply, but cannot.
        (extract_bit_field): Likewise. Use narrow_bit_field_mem and
        extract_fixed_bit_field_1 to do the extraction.
        (extract_fixed_bit_field): Revert to previous mode selection
algorithm.
        Call extract_fixed_bit_field_1 to do the real work.
        (extract_fixed_bit_field_1): New function.

testsuite:
        * gcc.dg/pr23623.c: Update to test interaction with C++
        memory model.

2013-12-11  Bernd Edlinger  <bernd.edlinger@hotmail.de>

        PR middle-end/59134
        * expmed.c (store_bit_field): Use narrow_bit_field_mem and
        store_fixed_bit_field_1 for -fstrict-volatile-bitfields.
        (store_fixed_bit_field): Split up.  Call store_fixed_bit_field_1
        to do the real work.
        (store_fixed_bit_field_1): New function. 
        (store_split_bit_field): Limit the unit size to the memory mode
size,
        to prevent recursion.

testsuite:
        * gcc.c-torture/compile/pr59134.c: New test.
        * gnat.dg/misaligned_volatile.adb: New test.

Added:
   
branches/ARM/embedded-4_8-branch/gcc/testsuite/gcc.c-torture/compile/pr59134.c
    branches/ARM/embedded-4_8-branch/gcc/testsuite/gcc.dg/pr23623.c
    branches/ARM/embedded-4_8-branch/gcc/testsuite/gcc.dg/pr48784-1.c
    branches/ARM/embedded-4_8-branch/gcc/testsuite/gcc.dg/pr48784-2.c
    branches/ARM/embedded-4_8-branch/gcc/testsuite/gcc.dg/pr56341-1.c
    branches/ARM/embedded-4_8-branch/gcc/testsuite/gcc.dg/pr56341-2.c
    branches/ARM/embedded-4_8-branch/gcc/testsuite/gcc.dg/pr56997-1.c
    branches/ARM/embedded-4_8-branch/gcc/testsuite/gcc.dg/pr56997-2.c
    branches/ARM/embedded-4_8-branch/gcc/testsuite/gcc.dg/pr56997-3.c
   
branches/ARM/embedded-4_8-branch/gcc/testsuite/gnat.dg/misaligned_volatile.adb
Modified:
    branches/ARM/embedded-4_8-branch/gcc/ChangeLog.arm
    branches/ARM/embedded-4_8-branch/gcc/calls.c
    branches/ARM/embedded-4_8-branch/gcc/config/tilegx/tilegx.c
    branches/ARM/embedded-4_8-branch/gcc/config/tilepro/tilepro.c
    branches/ARM/embedded-4_8-branch/gcc/doc/invoke.texi
    branches/ARM/embedded-4_8-branch/gcc/expmed.c
    branches/ARM/embedded-4_8-branch/gcc/expr.c
    branches/ARM/embedded-4_8-branch/gcc/expr.h
    branches/ARM/embedded-4_8-branch/gcc/testsuite/ChangeLog.arm

[Bug middle-end/48784] #pragma pack(1) + -fstrict-volatile-bitfields = bad codegen

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=48784

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
   Target Milestone|---                         |4.9.0
             Status|UNCONFIRMED                 |RESOLVED

--- Comment #8 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Fixed.