| Message ID | 1796361.XWXQsmNen7@polaris |
|---|---|
| State | New |
| Headers | show |
On Sat, Oct 15, 2016 at 12:11 PM, Eric Botcazou <ebotcazou@adacore.com> wrote: > Hi, > > gcc.dg/atomic/c11-atomic-exec-1.c has been failing at -O2 -mcpu=v8 on SPARC > since the fix for PR middle-end/61268 was installed, i.e. with all 5.x, 6.x > and 7.0 compilers. The failure mode is as follows: > > emit_cstore is called on: > > (mem/c:DF (plus:SI (plus:SI (reg/f:SI 104 virtual-stack-vars) > (reg:SI 1753)) > (const_int 8 [0x8])) [2 D.1567+8 S8 A64]) > > and does: > > last = get_last_insn (); > > then it calls prepare_operand, which calls copy_to_mode_reg, which calls > emit_move_insn, which calls validize_mem, which modifies the MEM in-place: > > (mem/c:DF (plus:SI (reg/f:SI 1757) > (const_int 8 [0x8])) [2 D.1567+8 S8 A64]) > > after emitting an addition: > > (set (reg/f:SI 1757) > (plus:SI (reg/f:SI 104 virtual-stack-vars) (reg:SI 1753))) > > But the call to maybe_expand_insn from emit_cstore fails so: > > delete_insns_since (last); > > is invoked and the addition is lost, i.e. (reg/f:SI 1757) will be emitted > uninitialized in the RTL stream, leading to a SIGSEGV at run time. > > IMO this shows that the explow.c hunk of the fix for PR middle-end/61268 was > ill-advised and should not have been accepted. Having copy_to_mode_reg or > emit_move_insn silently modify their argument is IMO a recipe for problems. Agreed. > So the attached patch backs it out nd I suggest installing on all branches. Ok for trunk (and branches after a while). Richard. > Tested on x86_64-suse-linux. > > > 2016-10-15 Eric Botcazou <ebotcazou@adacore.com> > > * explow.c (validize_mem): Do not modify the argument in-place. > > -- > Eric Botcazou
Index: explow.c =================================================================== --- explow.c (revision 241147) +++ explow.c (working copy) @@ -496,9 +496,8 @@ memory_address_addr_space (machine_mode return x; } -/* If REF is a MEM with an invalid address, change it into a valid address. - Pass through anything else unchanged. REF must be an unshared rtx and - the function may modify it in-place. */ +/* Convert a mem ref into one with a valid memory address. + Pass through anything else unchanged. */ rtx validize_mem (rtx ref) @@ -510,7 +509,8 @@ validize_mem (rtx ref) MEM_ADDR_SPACE (ref))) return ref; - return replace_equiv_address (ref, XEXP (ref, 0), true); + /* Don't alter REF itself, since that is probably a stack slot. */ + return replace_equiv_address (ref, XEXP (ref, 0)); } /* If X is a memory reference to a member of an object block, try rewriting
Hi, gcc.dg/atomic/c11-atomic-exec-1.c has been failing at -O2 -mcpu=v8 on SPARC since the fix for PR middle-end/61268 was installed, i.e. with all 5.x, 6.x and 7.0 compilers. The failure mode is as follows: emit_cstore is called on: (mem/c:DF (plus:SI (plus:SI (reg/f:SI 104 virtual-stack-vars) (reg:SI 1753)) (const_int 8 [0x8])) [2 D.1567+8 S8 A64]) and does: last = get_last_insn (); then it calls prepare_operand, which calls copy_to_mode_reg, which calls emit_move_insn, which calls validize_mem, which modifies the MEM in-place: (mem/c:DF (plus:SI (reg/f:SI 1757) (const_int 8 [0x8])) [2 D.1567+8 S8 A64]) after emitting an addition: (set (reg/f:SI 1757) (plus:SI (reg/f:SI 104 virtual-stack-vars) (reg:SI 1753))) But the call to maybe_expand_insn from emit_cstore fails so: delete_insns_since (last); is invoked and the addition is lost, i.e. (reg/f:SI 1757) will be emitted uninitialized in the RTL stream, leading to a SIGSEGV at run time. IMO this shows that the explow.c hunk of the fix for PR middle-end/61268 was ill-advised and should not have been accepted. Having copy_to_mode_reg or emit_move_insn silently modify their argument is IMO a recipe for problems. So the attached patch backs it out and I suggest installing on all branches. Tested on x86_64-suse-linux. 2016-10-15 Eric Botcazou <ebotcazou@adacore.com> * explow.c (validize_mem): Do not modify the argument in-place.