Re: [PATCH] misra: address Rule 11.1 violation in cmpxchgptr()

["Teddy Astie" <teddy.astie@xxxxxxxxxx>]

Hello,

Le 13/08/2025 à 20:07, Dmytro Prokopchuk1 a écrit :
> Misra Rule 11.1 states: "Conversions shall not be performed between a
> pointer to a function and any other type."
>
> The violation occurs in the macro:
>      __typeof__(**(ptr)) *const o_ = (o);                                \
>      __typeof__(**(ptr)) *n_ = (n);                                      \
>      ((__typeof__(*(ptr)))__cmpxchg(ptr, (unsigned long)o_,              \
>                                     (unsigned long)n_, sizeof(*(ptr)))); \
> })
> when it is used for handling function pointers of type:
> typedef void (*)(struct vcpu *, unsigned int).
> The issue happens because the '__cmpxchg()' function returns an 'unsigned
> long', which is then converted back into a function pointer, causing a
> violation of Rule 11.1. In this particular usage, the return value of the
> macro 'cmpxchgptr()' is not required. To address the violation, the macro
> has been replaced to discard the return value of '__cmpxchg()', preventing
> the conversion.
>
> Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@xxxxxxxx>
> ---
> Probably separate macro is too much for this single case.
>
> And the following will be enought:
> __cmpxchg(&xen_consumers[i], (unsigned long)NULL, (unsigned long)fn, 
> sizeof(*(&xen_consumers[i])));
> ---
>   xen/common/event_channel.c | 15 +++++++++++++--
>   1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c
> index 67700b050a..2094338b28 100644
> --- a/xen/common/event_channel.c
> +++ b/xen/common/event_channel.c
> @@ -93,6 +93,17 @@ static void cf_check default_xen_notification_fn(
>           vcpu_wake(v);
>   }
>
> +/*
> + * A slightly more updated variant of cmpxchgptr() where old value
> + * is not returned.
> + */
> +#define cmpxchgptr_noret(ptr, o, n) ({                  \
> +    __typeof__(**(ptr)) *const o_ = (o);                \
> +    __typeof__(**(ptr)) *n_ = (n);                      \
> +    (void)__cmpxchg(ptr, (unsigned long)o_,             \
> +                    (unsigned long)n_, sizeof(*(ptr))); \
> +})
> +
>   /*
>    * Given a notification function, return the value to stash in
>    * the evtchn->xen_consumer field.
> @@ -106,9 +117,9 @@ static uint8_t 
> get_xen_consumer(xen_event_channel_notification_t fn)
>
>       for ( i = 0; i < ARRAY_SIZE(xen_consumers); i++ )
>       {
> -        /* Use cmpxchgptr() in lieu of a global lock. */
> +        /* Use cmpxchgptr_noret() in lieu of a global lock. */
>           if ( xen_consumers[i] == NULL )
> -            cmpxchgptr(&xen_consumers[i], NULL, fn);
> +            cmpxchgptr_noret(&xen_consumers[i], NULL, fn);
>           if ( xen_consumers[i] == fn )
>               break;
>       }

AFAICS, Rule 11.1 has a deviation which allows this specific case.

In docs/misra/deviations.rst
> * - R11.1
>   - The conversion from a function pointer to unsigned long or (void \*) does
>     not lose any information, provided that the target type has enough bits
>     to store it.
>   - Tagged as `safe` for ECLAIR.

Here, we are constructing a function pointer from a unsigned long. I
assume this rule goes the other way it says, and allow converting a
unsigned long into a function pointer as long as its value is a valid
function pointer.

Teddy


Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech