This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: Fix PR 67609
- From: Richard Henderson <rth at redhat dot com>
- To: James Greenhalgh <james dot greenhalgh at arm dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>, Richard Biener <richard dot guenther at gmail dot com>, Marcus Shawcroft <marcus dot shawcroft at arm dot com>, Richard Earnshaw <richard dot earnshaw at arm dot com>, vmakarov at redhat dot com, law at redhat dot com, david dot sherwood at arm dot com, Tejas dot Belagod at arm dot com
- Date: Thu, 26 Nov 2015 11:55:06 +0100
- Subject: Re: Fix PR 67609
- Authentication-results: sourceware.org; auth=none
- References: <562FDCDF dot 4030501 at redhat dot com> <20151125171428 dot GA23219 at arm dot com>
On 11/25/2015 06:14 PM, James Greenhalgh wrote:
On Tue, Oct 27, 2015 at 01:21:51PM -0700, Richard Henderson wrote:
* aarch64 is almost certainly vulnerable, since it deleted its
CANNOT_CHANGE_MODE_CLASS implementation in January. I haven't tried to create
a test case that fails for it, but I'm certain it's possible.
The best I've come up with so far needs some union-hackery that I'm not
convinced is legal, and looks like:
typedef union
{
double v[2];
double s __attribute__ ((vector_size (16)));
} data;
data reg;
void
set_lower (double b)
{
dodgy_data stack_var;
double __attribute__ ((vector_size (16))) one = { 1.0, 1.0 };
stack_var.s = reg.s;
stack_var.s += one;
stack_var.v[0] += b;
reg.s = stack_var.s;
}
Modulo the one typo, this is a valid test case.
This shows the issue going back to GCC 4.9, the code we generate for
AArch64 looks like:
set_lower:
fmov v2.2d, 1.0e+0
adrp x0, reg
ldr q1, [x0, #:lo12:reg]
fadd v1.2d, v1.2d, v2.2d
orr v2.16b, v1.16b, v1.16b
fadd d2, d0, d1 // <----- Clobbered stack_var.v[1].
str q2, [x0, #:lo12:reg] // <----- Wrote zeroes to the top half of reg
ret
And yes, we mis-compile it, for the same reason:
(insn 13 12 14 2 (set (subreg:DF (reg/v:TI 78 [ stack_var ]) 0)
(reg:DF 85)) /localhome/devel/rth/z.c:16 54 {*movdf_aarch64}
(nil))
becomes
(insn 13 11 16 2 (set (subreg:DF (reg/v:TI 78 [ stack_var ]) 0)
(plus:DF (reg:DF 32 v0 [ b ])
(subreg:DF (reg:V2DF 82) 0))) /localhome/devel/rth/z.c:16 804 {adddf3}
(expr_list:REG_DEAD (reg:V2DF 82)
(expr_list:REG_DEAD (reg:DF 32 v0 [ b ])
(nil))))
becomes
(insn:TI 13 11 17 (set (reg:DF 34 v2 [orig:78 stack_var ] [78])
(plus:DF (reg:DF 32 v0 [ b ])
(reg:DF 33 v1 [82]))) /localhome/devel/rth/z.c:16 804 {adddf3}
(expr_list:REG_DEAD (reg:DF 33 v1 [82])
(expr_list:REG_DEAD (reg:DF 32 v0 [ b ])
(nil))))
Reading the documentation you add below, we'd need to bring back
CANNOT_CHANGE_MODE_CLASS for AArch64 and forbid changes from wide registers
to 64-bit (and larger) values. Is this right?
Not exactly -- forbid BITS_PER_WORD (64-bit) subregs of hard registers >
BITS_PER_WORD. See the verbiage I added to the i386 backend for this.
Are these workarounds intended
to be temporary, or is the midend bug likely to be fixed?
Not in the near term, no. We'd need to replace subreg, which that does 3 jobs
simultaneously, with something else that's less ambiguous.
r~