From: Michael Ellerman > Sent: 07 June 2022 03:05 > > Bagas Sanjaya <bagasdo...@gmail.com> writes: > > Hi, > > > > I'm trying to verify Drop ppc_inst_as_str() patch on [1] by performing > > ppc64_defconfig build with powerpc64-unknown-linux-gnu-gcc (GCC 12.1.0). > > The patch is applied on top of powerpc tree, next branch. > > Yeah I see it too. > > > I got outside array bounds error: > > > > CC arch/powerpc/kernel/dbell.o > > In function 'do_byte_reverse', > > inlined from 'do_vec_store' at arch/powerpc/lib/sstep.c:722:3, > > inlined from 'emulate_loadstore' at arch/powerpc/lib/sstep.c:3509:9: > > arch/powerpc/lib/sstep.c:286:25: error: array subscript [3, 4] is outside > > array bounds of 'union > <anonymous>[1]' [-Werror=array-bounds] > > 286 | up[0] = byterev_8(up[3]); > > | ^~~~~~~~~~~~~~~~ > > > > arch/owerpc/lib/sstep.c: In function 'emulate_loadstore': > > arch/powerpc/lib/sstep.c:708:11: note: at offset [24, 39] into object 'u' > > of size 16 > > 708 | } u; > > | ^ > > In function 'do_byte_reverse', > > inlined from 'do_vec_store' at arch/powerpc/lib/sstep.c:722:3, > > inlined from 'emulate_loadstore' at arch/powerpc/lib/sstep.c:3509:9: > > arch/powerpc/lib/sstep.c:287:23: error: array subscript [3, 4] is outside > > array bounds of 'union > <anonymous>[1]' [-Werror=array-bounds] > > 287 | up[3] = tmp; > > | ~~~~~~^~~~~ > > This happens because we have a generic byte reverse function > (do_byte_reverse()), that takes a size as a parameter. So it will > reverse 8, 16, 32 bytes etc. > > In some cases the compiler can see that we're passing a pointer to > storage that is smaller than 32 bytes, but it isn't convinced that the > size parameter is also smaller than 32 bytes. > > Which I think is reasonable, the code that sets the size is separate > from this code, so the compiler can't really deduce that it's safe. > > I don't see a really simple fix. I tried clamping the size parameter to > do_byte_reverse() with max(), but that didn't work :/
I had a quick look at the code - it is somewhat horrid! Not really surprising the compiler is confused. Although it shouldn't be outputting that error message unless it is certain. Could it be re-written to read the data into an __u128 (or whatever the compiler type is). Optionally byteswap the entire thing (swap the words and then byteswap each word). The do a put_user_8/16/32/64() to write out the value. I think that would remove all the memory accesses and make it a lot faster as well. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)