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

[Gustavo A. R. Silva]

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;

Inside `do_vec_store()`, we have that the size of `union u` is 16 bytes:

  702                                         int size, struct pt_regs *regs,
  703                                         bool cross_endian)
  704 {
  705         union {
  706                 __vector128 v;
  707                 u8 b[sizeof(__vector128)];
  708         } u;

and then function `do_byte_reverse()` is called

  721         if (unlikely(cross_endian))
  722                 do_byte_reverse(&u.b[ea & 0xf], size);

and if `size == 32`, the following piece of code tries to access
`ptr` outside of its boundaries:

  260 static nokprobe_inline void do_byte_reverse(void *ptr, int nb)
  261 {
  262         switch (nb) {
...
  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         }

The following patch is merely a test to illustrate how the value in `size` initially appears
to influence the warnings:

diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
index a4ab8625061a..86786957b736 100644
--- a/arch/powerpc/lib/sstep.c
+++ b/arch/powerpc/lib/sstep.c
@@ -3436,7 +3436,7 @@ int emulate_loadstore(struct pt_regs *regs, struct instruction_op *op)
         case LOAD_VMX:
                 if (!(regs->msr & MSR_PR) && !(regs->msr & MSR_VEC))
                         return 0;
-               err = do_vec_load(op->reg, ea, size, regs, cross_endian);
+               err = do_vec_load(op->reg, ea, (size > 16) ? 16 : size, regs, cross_endian);
                 break;
  #endif
  #ifdef CONFIG_VSX
@@ -3507,7 +3507,7 @@ int emulate_loadstore(struct pt_regs *regs, struct instruction_op *op)
         case STORE_VMX:
                 if (!(regs->msr & MSR_PR) && !(regs->msr & MSR_VEC))
                         return 0;
-               err = do_vec_store(op->reg, ea, size, regs, cross_endian);
+               err = do_vec_store(op->reg, ea, (size > 16) ? 16 : size, regs, cross_endian);
                 break;
  #endif
  #ifdef CONFIG_VSX

Is there something that I may be overlooking here? :)

Thanks!
--
Gustavo