Re: [PATCH 9/9] xen/arm32: vfp: Add missing U for shifted constant

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

Hi Julien,

> On 25 Jun 2023, at 22:49, Julien Grall <julien@xxxxxxx> wrote:
> 
> From: Julien Grall <jgrall@xxxxxxxxxx>
> 
> When enabling UBSAN on arm32, the following splat will be printed:
> 
> (XEN) 
> ================================================================================
> (XEN) UBSAN: Undefined behaviour in arch/arm/arm32/vfp.c:75:22
> (XEN) left shift of 255 by 24 places cannot be represented in type 'int'
> 
> This is referring to the shift in FPSID_IMPLEMENTER_MASK. While we could
> only add the U to the value shift there, it would be better to be
> consistent and also add it for every value shifted.
> 
> This should also addressing MISRA Rule 7.2:
> 
>    A "u" or "U" suffix shall be applied to all integer constants that
>    are represented in an unsigned type
> 
> Signed-off-by: Julien Grall <jgrall@xxxxxxxxxx>

Reviewed-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>

Cheers
Bertrand

> ---
> xen/arch/arm/include/asm/arm32/vfp.h | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/xen/arch/arm/include/asm/arm32/vfp.h 
> b/xen/arch/arm/include/asm/arm32/vfp.h
> index bade3bc66e1f..2f2e4b24bb40 100644
> --- a/xen/arch/arm/include/asm/arm32/vfp.h
> +++ b/xen/arch/arm/include/asm/arm32/vfp.h
> @@ -1,23 +1,23 @@
> #ifndef _ARM_ARM32_VFP_H
> #define _ARM_ARM32_VFP_H
> 
> -#define FPEXC_EX                (1u << 31)
> -#define FPEXC_EN                (1u << 30)
> -#define FPEXC_FP2V              (1u << 28)
> +#define FPEXC_EX                (1U << 31)
> +#define FPEXC_EN                (1U << 30)
> +#define FPEXC_FP2V              (1U << 28)
> 
> -#define MVFR0_A_SIMD_MASK       (0xf << 0)
> +#define MVFR0_A_SIMD_MASK       (0xfU << 0)
> 
> 
> #define FPSID_IMPLEMENTER_BIT   (24)
> -#define FPSID_IMPLEMENTER_MASK  (0xff << FPSID_IMPLEMENTER_BIT)
> +#define FPSID_IMPLEMENTER_MASK  (0xffU << FPSID_IMPLEMENTER_BIT)
> #define FPSID_ARCH_BIT          (16)
> -#define FPSID_ARCH_MASK         (0xf << FPSID_ARCH_BIT)
> +#define FPSID_ARCH_MASK         (0xfU << FPSID_ARCH_BIT)
> #define FPSID_PART_BIT          (8)
> -#define FPSID_PART_MASK         (0xff << FPSID_PART_BIT)
> +#define FPSID_PART_MASK         (0xffU << FPSID_PART_BIT)
> #define FPSID_VARIANT_BIT       (4)
> -#define FPSID_VARIANT_MASK      (0xf << FPSID_VARIANT_BIT)
> +#define FPSID_VARIANT_MASK      (0xfU << FPSID_VARIANT_BIT)
> #define FPSID_REV_BIT           (0)
> -#define FPSID_REV_MASK          (0xf << FPSID_REV_BIT)
> +#define FPSID_REV_MASK          (0xfU << FPSID_REV_BIT)
> 
> struct vfp_state
> {
> -- 
> 2.40.1
>