On Mon, Nov 20, 2023 at 08:33:45AM -0600, Gustavo A. R. Silva wrote:
>
>
> 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?
Please see git history to understand the commit that introduced that
code. You can do that by starting with a 'git blame' on the file. You'll
find that the commit that introduced the above hunk was af99da74333b
("powerpc/sstep: Support VSX vector paired storage access
instructions").
The above code path is hit via
do_vsx_load()->emulate_vsx_load()->do_byte_reverse()
- Naveen
