Re: [XEN PATCH 09/13] xen/public: fixed violations of MISRA C:2012 Rule 7.2

[Simone Ballarin <simone.ballarin@xxxxxxxxxxx>]

Hi,

Il giorno mar 20 giu 2023 alle ore 15:00 Jan Beulich <jbeulich@xxxxxxxx> ha scritto:

On 20.06.2023 12:35, Simone Ballarin wrote:
> --- a/xen/include/public/io/ring.h
> +++ b/xen/include/public/io/ring.h
> @@ -36,11 +36,11 @@
>  typedef unsigned int RING_IDX;
> 
>  /* Round a 32-bit unsigned constant down to the nearest power of two. */
> -#define __RD2(_x)  (((_x) & 0x00000002) ? 0x2                  : ((_x) & 0x1))
> -#define __RD4(_x)  (((_x) & 0x0000000c) ? __RD2((_x)>>2)<<2    : __RD2(_x))
> -#define __RD8(_x)  (((_x) & 0x000000f0) ? __RD4((_x)>>4)<<4    : __RD4(_x))
> -#define __RD16(_x) (((_x) & 0x0000ff00) ? __RD8((_x)>>8)<<8    : __RD8(_x))
> -#define __RD32(_x) (((_x) & 0xffff0000) ? __RD16((_x)>>16)<<16 : __RD16(_x))
> +#define __RD2(_x)  (((_x) & 0x00000002U) ? 0x2                  : ((_x) & 0x1))
> +#define __RD4(_x)  (((_x) & 0x0000000cU) ? __RD2((_x)>>2)<<2    : __RD2(_x))
> +#define __RD8(_x)  (((_x) & 0x000000f0U) ? __RD4((_x)>>4)<<4    : __RD4(_x))
> +#define __RD16(_x) (((_x) & 0x0000ff00U) ? __RD8((_x)>>8)<<8    : __RD8(_x))
> +#define __RD32(_x) (((_x) & 0xffff0000U) ? __RD16((_x)>>16)<<16 : __RD16(_x))

While I don't mind the suffixes being added, I'm wondering how
the tool would have spotted the single violation here. Iirc we
don't use this header anywhere in the hypervisor.

In the analyzed build it is used:

aarch64-linux-gnu-gcc-12 -MMD -MP -MF common/.vm_event.o.d -DBUILD_ID -fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes -Wdeclaration-after-statement -Wno-unused-but-set-variable -Wno-unused-local-typedefs -O1 -fno-omit-frame-pointer -nostdinc -fno-builtin -fno-common -Werror -Wredundant-decls -Wno-pointer-arith -Wvla -pipe -D__XEN__ -include ./include/xen/config.h "-Wa,--strip-local-absolute" -g -mcpu=generic -mgeneral-regs-only -mno-outline-atomics -I./include -I./arch/arm/include -fno-pie -fno-stack-protector -fno-exceptions -fno-asynchronous-unwind-tables -Wnested-externs -fprofile-arcs -ftest-coverage -c common/vm_event.c -o common/vm_event.o

xen/common/vm_event.c:

85 FRONT_RING_INIT(&ved->front_ring,

86 (vm_event_sring_t *)ved->ring_page,

87 PAGE_SIZE);

The expansions that lead to __RD32 are:

FRONT_RING_INIT -> FRONT_RING_ATTACH -> __RING_SIZE -> __RD32

Furthermore, if I recall correctly Misra also mandates single
evaluation of macro arguments. While I don't immediately see how
to address that without resorting to compiler extensions, I don't
think it makes sense to address one violation here but not he
other (the more when the code in question doesn't affect the
hypervisor build).

If not clearly connected I suggest discussing one rule at a time.

Jan

--

Simone Ballarin, M.Sc.

Field Application Engineer, BUGSENG (https://bugseng.com)