Re: [PATCH] target/arm: Avoid over-length shift in arm_cpu_sve_finalize() error case

[Peter Maydell]

On Tue, 4 Jul 2023 at 16:52, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> On 4/7/23 17:43, Peter Maydell wrote:
> > If you build QEMU with the clang sanitizer enabled, you can see it
> > fire when running the arm-cpu-features test:
> >
> > $ QTEST_QEMU_BINARY=./build/arm-clang/qemu-system-aarch64 
> > ./build/arm-clang/tests/qtest/arm-cpu-features
> > [...]
> > ../../target/arm/cpu64.c:125:19: runtime error: shift exponent 64 is too 
> > large for 64-bit type 'unsigned long long'
> > [...]
> >
> > This happens because the user can specify some incorrect SVE
> > properties that result in our calculating a max_vq of 0.  We catch
> > this and error out, but before we do that we calculate
> >
> >   vq_mask = MAKE_64BIT_MASK(0, max_vq);$
> >
> > and the MAKE_64BIT_MASK() call is only valid for lengths that are
> > greater than zero, so we hit the undefined behaviour.
>
> Can we fix it generically?
>
> -- >8 --
> --- a/include/qemu/bitops.h
> +++ b/include/qemu/bitops.h
> @@ -28,3 +28,3 @@
>   #define MAKE_64BIT_MASK(shift, length) \
> -    (((~0ULL) >> (64 - (length))) << (shift))
> +    ((length) ? (((~0ULL) >> (64 - (length))) << (shift)) : 0)
>
> ---

Only by introducing a conditional in the case where the length
isn't a compile time constant.
Like the extract and deposit functions, the assumption is that
you're operating on a field that actually exists and isn't
zero-width.

thanks
-- PMM