Fix PR 67609

Richard Henderson rth@redhat.com
Thu Nov 26 11:02:00 GMT 2015


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~



More information about the Gcc-patches mailing list