| Message ID | 20230625204907.57291-10-julien@xen.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xen/arm: Enable UBSAN support | expand |
Hi Julien, > -----Original Message----- > Subject: [PATCH 9/9] xen/arm32: vfp: Add missing U for shifted constant > > From: Julien Grall <jgrall@amazon.com> > > 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@amazon.com> Reviewed-by: Henry Wang <Henry.Wang@arm.com> Kind regards, Henry
Hi Julien, > On 25 Jun 2023, at 22:49, Julien Grall <julien@xen.org> wrote: > > From: Julien Grall <jgrall@amazon.com> > > 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@amazon.com> Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com> 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 >
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 {