Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
From:   Michael Ellerman <mpe@ellerman.id.au>
To:     Bagas Sanjaya <bagasdotme@gmail.com>, linuxppc-dev@lists.ozlabs.org
Cc:     linux-kernel@vger.kernel.org,
        Benjamin Herrenschmidt <benh@kernel.crashing.org>,
        Paul Mackerras <paulus@samba.org>,
        Nicholas Piggin <npiggin@gmail.com>,
        Christophe Leroy <christophe.leroy@csgroup.eu>,
        Anders Roxell <anders.roxell@linaro.org>,
        Arnd Bergmann <arnd@arndb.de>,
        Yang Li <yang.lee@linux.alibaba.com>
Subject: Re: outside array bounds error on ppc64_defconfig, GCC 12.1.0
In-Reply-To: <YpbUcPrm61RLIiZF@debian.me>
References: <YpbUcPrm61RLIiZF@debian.me>
Date:   Tue, 07 Jun 2022 12:05:18 +1000
Message-ID: <87mtepns81.fsf@mpe.ellerman.id.au>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

Bagas Sanjaya <bagasdotme@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 :/

cheers