Created attachment 24109 [details] 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
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.
I'm working on a fix for this.
Patch posted here: http://gcc.gnu.org/ml/gcc-patches/2013-06/msg00750.html
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. :-(
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
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
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
Fixed.