Re: [RFC - is this a bug?] powerpc/lib/sstep: Asking for some light on this, please. :)

["Gustavo A. R. Silva" <gustavo@embeddedor.com>]

On 11/20/23 08:25, Naveen N Rao wrote:
> On Fri, Nov 17, 2023 at 12:36:01PM -0600, Gustavo A. R. Silva wrote:
>> Hi all,
>>
>> I'm trying to fix the following -Wstringop-overflow warnings, and I'd like
>> to get your feedback on this, please:
>>
>> In function 'do_byte_reverse',
>>      inlined from 'do_vec_store' at /home/gus/linux/arch/powerpc/lib/sstep.c:722:3,
>>      inlined from 'emulate_loadstore' at /home/gus/linux/arch/powerpc/lib/sstep.c:3510:9:
>> arch/powerpc/lib/sstep.c:287:23: error: writing 8 bytes into a region of size 0 [-Werror=stringop-overflow=]
>>    287 |                 up[3] = tmp;
>>        |                 ~~~~~~^~~~~
>> arch/powerpc/lib/sstep.c: In function 'emulate_loadstore':
>> arch/powerpc/lib/sstep.c:708:11: note: at offset [24, 39] into destination object 'u' of size 16
>>    708 |         } u;
>>        |           ^
>> In function 'do_byte_reverse',
>>      inlined from 'do_vec_store' at /home/gus/linux/arch/powerpc/lib/sstep.c:722:3,
>>      inlined from 'emulate_loadstore' at /home/gus/linux/arch/powerpc/lib/sstep.c:3510:9:
>> arch/powerpc/lib/sstep.c:289:23: error: writing 8 bytes into a region of size 0 [-Werror=stringop-overflow=]
>>    289 |                 up[2] = byterev_8(up[1]);
>>        |                 ~~~~~~^~~~~~~~~~~~~~~~~~
>> arch/powerpc/lib/sstep.c: In function 'emulate_loadstore':
>> arch/powerpc/lib/sstep.c:708:11: note: at offset 16 into destination object 'u' of size 16
>>    708 |         } u;
>>        |           ^
>> In function 'do_byte_reverse',
>>      inlined from 'do_vec_load' at /home/gus/linux/arch/powerpc/lib/sstep.c:691:3,
>>      inlined from 'emulate_loadstore' at /home/gus/linux/arch/powerpc/lib/sstep.c:3439:9:
>> arch/powerpc/lib/sstep.c:287:23: error: writing 8 bytes into a region of size 0 [-Werror=stringop-overflow=]
>>    287 |                 up[3] = tmp;
>>        |                 ~~~~~~^~~~~
>> arch/powerpc/lib/sstep.c: In function 'emulate_loadstore':
>> arch/powerpc/lib/sstep.c:681:11: note: at offset [24, 39] into destination object 'u' of size 16
>>    681 |         } u = {};
>>        |           ^
>> arch/powerpc/lib/sstep.c:681:11: note: at offset [24, 39] into destination object 'u' of size 16
>> arch/powerpc/lib/sstep.c:681:11: note: at offset [24, 39] into destination object 'u' of size 16
>>
>> The origing of the issue seems to be the following calls to function `do_vec_store()`, when
>> `size > 16`, or more precisely when `size == 32`
>>
>> arch/powerpc/lib/sstep.c:
>> 3436         case LOAD_VMX:
>> 3437                 if (!(regs->msr & MSR_PR) && !(regs->msr & MSR_VEC))
>> 3438                         return 0;
>> 3439                 err = do_vec_load(op->reg, ea, size, regs, cross_endian);
>> 3440                 break;
>> ...
>> 3507         case STORE_VMX:
>> 3508                 if (!(regs->msr & MSR_PR) && !(regs->msr & MSR_VEC))
>> 3509                         return 0;
>> 3510                 err = do_vec_store(op->reg, ea, size, regs, cross_endian);
>> 3511                 break;
> 
> LOAD_VMX and STORE_VMX are set in analyse_instr() and size does not
> exceed 16. So, the warning looks incorrect to me.

Does that mean that this piece of code is never actually executed:

  281         case 32: {
  282                 unsigned long *up = (unsigned long *)ptr;
  283                 unsigned long tmp;
  284
  285                 tmp = byterev_8(up[0]);
  286                 up[0] = byterev_8(up[3]);
  287                 up[3] = tmp;
  288                 tmp = byterev_8(up[2]);
  289                 up[2] = byterev_8(up[1]);
  290                 up[1] = tmp;
  291                 break;
  292         }

or rather, under which conditions the above code is executed, if any?

Thanks!
--
Gustavo