[Bug target/52080] Stores to bitfields introduce a store-data-race on adjacent data
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52080 Jackie Rosen jackie.rosen at hushmail dot com changed: What|Removed |Added CC||jackie.rosen at hushmail dot com --- Comment #16 from Jackie Rosen jackie.rosen at hushmail dot com --- *** Bug 260998 has been marked as a duplicate of this bug. *** Seen from the domain http://volichat.com Page where seen: http://volichat.com/adult-chat-rooms Marked for reference. Resolved as fixed @bugzilla.
[Bug target/52080] Stores to bitfields introduce a store-data-race on adjacent data
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52080 --- Comment #15 from Richard Biener rguenth at gcc dot gnu.org 2012-06-04 08:43:31 UTC --- Author: rguenth Date: Mon Jun 4 08:43:23 2012 New Revision: 188167 URL: http://gcc.gnu.org/viewcvs?root=gccview=revrev=188167 Log: 2012-06-04 Richard Guenther rguent...@suse.de Eric Botcazou ebotca...@adacore.com Backport from mainline 2012-04-03 Eric Botcazou ebotca...@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 ebotca...@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 rguent...@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 rguent...@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 rguent...@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 target/52080] Stores to bitfields introduce a store-data-race on adjacent data
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52080 --- Comment #15 from Richard Guenther rguenth at gcc dot gnu.org 2012-06-04 08:43:31 UTC --- Author: rguenth Date: Mon Jun 4 08:43:23 2012 New Revision: 188167 URL: http://gcc.gnu.org/viewcvs?root=gccview=revrev=188167 Log: 2012-06-04 Richard Guenther rguent...@suse.de Eric Botcazou ebotca...@adacore.com Backport from mainline 2012-04-03 Eric Botcazou ebotca...@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 ebotca...@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 rguent...@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 rguent...@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 rguent...@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 target/52080] Stores to bitfields introduce a store-data-race on adjacent data
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52080 Richard Guenther rguenth at gcc dot gnu.org changed: What|Removed |Added Status|ASSIGNED|RESOLVED Known to work||4.8.0 Resolution||FIXED Target Milestone|--- |4.8.0 --- Comment #14 from Richard Guenther rguenth at gcc dot gnu.org 2012-03-14 10:57:32 UTC --- Fixed for 4.8.
[Bug target/52080] Stores to bitfields introduce a store-data-race on adjacent data
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52080 --- Comment #13 from Richard Guenther rguenth at gcc dot gnu.org 2012-03-14 10:55:16 UTC --- Author: rguenth Date: Wed Mar 14 10:55:09 2012 New Revision: 185379 URL: http://gcc.gnu.org/viewcvs?root=gccview=revrev=185379 Log: 2012-03-14 Richard Guenther rguent...@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 target/52080] Stores to bitfields introduce a store-data-race on adjacent data
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52080 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 #12 from Richard Guenther rguenth at gcc dot gnu.org 2012-02-21 11:58:00 UTC --- Created attachment 26712 -- http://gcc.gnu.org/bugzilla/attachment.cgi?id=26712 candidate patch Here is a candidate patch (it ICEs one Ada testcase, see PR52134). It enforces the C++11 memory model (as far as bitfields are concerned) upon everyone and builds on the machinery that was implemented for it (changing the implementation for when we compute an underlying object and permanently store it, to make it possible to use this information for optimization purposes). It fixes the related PR48124 as well. It's an invasive change that will change code generation on STRICT_ALIGNMENT platforms quite severely for bitfield accesses. So it is not an appropriate fix for GCC 4.7 at this point of the development cycyle, nor would it be appropriate to backport it. Queued for GCC 4.8.
[Bug target/52080] Stores to bitfields introduce a store-data-race on adjacent data
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52080
Petr Tesarik gcc-bugs at tesarici dot cz changed:
What|Removed |Added
CC||gcc-bugs at tesarici dot cz
--- Comment #9 from Petr Tesarik gcc-bugs at tesarici dot cz 2012-02-02
11:00:50 UTC ---
OK, my minimal test case removed the volatile keyword by mistake.
The real code in BTRFS has the volatile for the lock value which precedes the
bitfield, so the corresponding structure would be:
struct x {
long a;
volatile unsigned int lock; /* - note the volatile here */
unsigned int full : 1;
};
Now, GCC should honour that the value of lock can change any time, so it must
not assume that writing back the same value a few cycles later is safe.
[Bug target/52080] Stores to bitfields introduce a store-data-race on adjacent data
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52080
--- Comment #10 from Richard Guenther rguenth at gcc dot gnu.org 2012-02-02
11:08:56 UTC ---
(In reply to comment #9)
OK, my minimal test case removed the volatile keyword by mistake.
The real code in BTRFS has the volatile for the lock value which precedes the
bitfield, so the corresponding structure would be:
struct x {
long a;
volatile unsigned int lock; /* - note the volatile here */
unsigned int full : 1;
};
Now, GCC should honour that the value of lock can change any time, so it
must
not assume that writing back the same value a few cycles later is safe.
volatiles on single structure members is of course under- (or even
un-)specified. Consider
struct x {
int i : 1;
volatile int j : 1;
};
Where we clearly cannot access i without modifying j (but it's still
valid C). So I don't think that a volatile member inside a non-volatile
struct guarantees anything.
Now, with
struct x {
long a;
volatile unsigned int lock;
unsigned int full : 1;
};
void
wrong(volatile struct x *ptr)
{
ptr-full = 1;
}
IA64 uses
.mmi
ld8.acq r14 = [r32]
;;
nop 0
dep r14 = r15, r14, 32, 1
;;
.mib
st8.rel [r32] = r14
which seems to be an attempt to work around this issue (albeit a
possibly very slow one).
[Bug target/52080] Stores to bitfields introduce a store-data-race on adjacent data
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52080 --- Comment #11 from Petr Tesarik gcc-bugs at tesarici dot cz 2012-02-02 12:39:25 UTC --- (In reply to comment #10) IA64 uses .mmi ld8.acq r14 = [r32] ;; nop 0 dep r14 = r15, r14, 32, 1 ;; .mib st8.rel [r32] = r14 which seems to be an attempt to work around this issue (albeit a possibly very slow one). Are you referring to the .acq and .rel forms? That doesn't change the situation at all. All it does is ensure correct memory ordering with respect to external visibility, but it does nothing to avoid the race condition.
[Bug target/52080] Stores to bitfields introduce a store-data-race on adjacent data
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52080 Richard Guenther rguenth at gcc dot gnu.org changed: What|Removed |Added Target|ia64-*-linux|ia64-*-linux, ||sparc64-*-linux CC||ebotcazou at gcc dot ||gnu.org --- Comment #1 from Richard Guenther rguenth at gcc dot gnu.org 2012-02-01 10:22:51 UTC --- SPARC64 is also affected. ;; ptr_1(D)-full = 1; (insn 6 5 7 (set (reg:DI 110) (mem/j:DI (plus:DI (reg/v/f:DI 109 [ ptr ]) (const_int 8 [0x8])) [0+8 S8 A64])) t.c:10 -1 (nil)) (insn 7 6 8 (set (reg:DI 112) (const_int 2147483648 [0x8000])) t.c:10 -1 (nil)) (insn 8 7 9 (set (reg:DI 111) (ior:DI (reg:DI 110) (reg:DI 112))) t.c:10 -1 (nil)) (insn 9 8 0 (set (mem/j:DI (plus:DI (reg/v/f:DI 109 [ ptr ]) (const_int 8 [0x8])) [0+8 S8 A64]) (reg:DI 111)) t.c:10 -1 (nil)) wrong: ldx [%o0+8], %g2 sethi %hi(2147483648), %g1 or %g2, %g1, %g1 jmp %o7+8 stx%g1, [%o0+8] At least IA64 also can do 4-byte loads/stores (but not sure the HW wouldn't re-introduce the data race).
[Bug target/52080] Stores to bitfields introduce a store-data-race on adjacent data
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52080
--- Comment #2 from Richard Guenther rguenth at gcc dot gnu.org 2012-02-01
10:25:21 UTC ---
SPARC64 also can do 32bit loads/stores as the following testcase shows:
struct x {
long a;
unsigned int lock;
unsigned int full;
};
void
wrong(struct x *ptr)
{
ptr-full = 1;
}
here we simply use a 32bit store.
[Bug target/52080] Stores to bitfields introduce a store-data-race on adjacent data
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52080 Richard Guenther rguenth at gcc dot gnu.org changed: What|Removed |Added Depends on||48124 --- Comment #3 from Richard Guenther rguenth at gcc dot gnu.org 2012-02-01 10:38:58 UTC --- For SPARC64 optimize_bitfield_assignment_op fails so we fall into the store_field path with mode1 == VOIDmode (what get_inner_refrence says). to_rtx is (mem/j:BLK (reg/v/f:DI 109 [ ptr ]) [2 *ptr_1(D)+0 S1 A64]) Related bug: PR48124.
[Bug target/52080] Stores to bitfields introduce a store-data-race on adjacent data
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52080 --- Comment #4 from Richard Guenther rguenth at gcc dot gnu.org 2012-02-01 11:12:41 UTC --- Btw, offset = bitnum / unit; bitpos = bitnum % unit; byte_offset = (bitnum % BITS_PER_WORD) / BITS_PER_UNIT + (offset * UNITS_PER_WORD); byte_offset is bollocks (or has a really poor name). On SPARC64 I see (gdb) p unit $11 = 8 (gdb) p offset $12 = 12 (gdb) p bitpos $13 = 0 (gdb) p byte_offset $14 = 100 Other than that we are falling into the generic store_fixed_bit_field routine which at the top does unsigned int total_bits = BITS_PER_WORD; and mode = GET_MODE (op0); if (GET_MODE_BITSIZE (mode) == 0 || GET_MODE_BITSIZE (mode) GET_MODE_BITSIZE (word_mode)) mode = word_mode; (now, why we use a BLKmode mem here and not a QImode mem may be surprising) get_best_mode still returns DImode because that's the largest mode the given alignment supports. After that we're lost. So it seems that to fix this case we'd need to figure out some other largest mode we can pass to get_best_mode. The only hint would be from providing a different mode for the initial MEM we create, like with Index: gcc/expr.c === --- gcc/expr.c (revision 183791) +++ gcc/expr.c (working copy) @@ -4705,6 +4705,12 @@ 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); + else if (TREE_CODE (to) == COMPONENT_REF + DECL_BIT_FIELD (TREE_OPERAND (to, 1)) + DECL_MODE (TREE_OPERAND (to, 1)) != BLKmode) + to_rtx = adjust_address (to_rtx, +TYPE_MODE (DECL_BIT_FIELD_TYPE + (TREE_OPERAND (to, 1))), 0); } if (offset != 0) That avoids the use of QImode we have on the field-decl but also adjusts MEM_SIZE ...
[Bug target/52080] Stores to bitfields introduce a store-data-race on adjacent data
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52080 Eric Botcazou ebotcazou at gcc dot gnu.org changed: What|Removed |Added Status|UNCONFIRMED |NEW Last reconfirmed||2012-02-01 Ever Confirmed|0 |1 --- Comment #5 from Eric Botcazou ebotcazou at gcc dot gnu.org 2012-02-01 17:11:10 UTC --- You probably need to add a 'volatile' somewhere to really have wrong code.
[Bug target/52080] Stores to bitfields introduce a store-data-race on adjacent data
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52080 --- Comment #6 from Eric Botcazou ebotcazou at gcc dot gnu.org 2012-02-01 17:45:08 UTC --- get_best_mode returns DImode because of: /* Nonzero if access to memory by bytes is slow and undesirable. For RISC chips, it means that access to memory by bytes is no better than access by words when possible, so grab a whole word and maybe make use of that. */ #define SLOW_BYTE_ACCESS 1
[Bug target/52080] Stores to bitfields introduce a store-data-race on adjacent data
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52080 Michael Matz matz at gcc dot gnu.org changed: What|Removed |Added CC||matz at gcc dot gnu.org --- Comment #7 from Michael Matz matz at gcc dot gnu.org 2012-02-01 17:49:45 UTC --- Yeah. Which is okay for reading, but doing the same when writing is problematic.
[Bug target/52080] Stores to bitfields introduce a store-data-race on adjacent data
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52080 Peter Bergner bergner at gcc dot gnu.org changed: What|Removed |Added Target|ia64-*-linux, |ia64-*-linux, |sparc64-*-linux |sparc64-*-linux, ||powerpc64-*-linux CC||bergner at gcc dot gnu.org --- Comment #8 from Peter Bergner bergner at gcc dot gnu.org 2012-02-01 18:52:06 UTC --- This fails on powerpc64-linux as well (-m64), where we generate: ld 9,8(3) li 10,1 rldimi 9,10,31,32 std 9,8(3) blr
