Re: [PATCH] powerpc/sstep: Fix array out of bound warning

[Ravi Bangoria <ravi.bangoria@linux.ibm.com>]

On 1/28/21 10:50 PM, Naveen N. Rao wrote:
> On 2021/01/15 11:46AM, Ravi Bangoria wrote:
>> Compiling kernel with -Warray-bounds throws below warning:
>>
>>    In function 'emulate_vsx_store':
>>    warning: array subscript is above array bounds [-Warray-bounds]
>>    buf.d[2] = byterev_8(reg->d[1]);
>>    ~~~~~^~~
>>    buf.d[3] = byterev_8(reg->d[0]);
>>    ~~~~~^~~
>>
>> Fix it by converting local variable 'union vsx_reg buf' into an array.
>> Also consider function argument 'union vsx_reg *reg' as array instead
>> of pointer because callers are actually passing an array to it.
> 
> I think you should change the function prototype to reflect this.
> 
> However, while I agree with this change in principle, it looks to be a
> lot of code churn for a fairly narrow use. Perhaps we should just
> address the specific bug. Something like the below (not tested)?

Yes, this would serve the same purpose and it's more compact as well. Sent v2.

> 
> @@ -818,13 +818,15 @@ void emulate_vsx_store(struct instruction_op *op, const union vsx_reg *reg,
>                          break;
>                  if (rev) {
>                          /* reverse 32 bytes */
> -                       buf.d[0] = byterev_8(reg->d[3]);
> -                       buf.d[1] = byterev_8(reg->d[2]);
> -                       buf.d[2] = byterev_8(reg->d[1]);
> -                       buf.d[3] = byterev_8(reg->d[0]);
> -                       reg = &buf;
> +                       union vsx_reg buf32[2];
> +                       buf32[0].d[0] = byterev_8(reg[1].d[1]);
> +                       buf32[0].d[1] = byterev_8(reg[1].d[0]);
> +                       buf32[1].d[0] = byterev_8(reg[0].d[1]);
> +                       buf32[1].d[1] = byterev_8(reg[0].d[0]);
> +                       memcpy(mem, buf32, size);
> +               } else {
> +                       memcpy(mem, reg, size);
>                  }
> -               memcpy(mem, reg, size);
>                  break;
>          case 16:
>                  /* stxv, stxvx, stxvl, stxvll */
> 
> 
> - Naveen
>