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

[Richard Henderson]

On 7/4/23 18:00, Alex Bennée wrote:
> Peter Maydell <peter.maydell@linaro.org> writes:
>
> > 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.
>
> Hmm that does make me worry we could have more land mines waiting to be
> found. Would converting MAKE_64BIT_MASK into an inline function and
> asserting be a better solution?

I'd be tempted to keep a macro, and use __builtin_constant_p to make sure this expands to 
a constant if possible.  Ideally constants would be diagnosed at compile-time and runtime 
values get runtime asserts.


r~