On Thu, 14 Aug 2025, Teddy Astie wrote:
> 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_prokopch...@epam.com>
> > ---
> > 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.
You are right, we should need to update the deviation instead here:
-doc_begin="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."
-config=MC3A2.R11.1,casts+={safe,
"from(type(canonical(__function_pointer_types)))
&&to(type(canonical(builtin(unsigned long)||pointer(builtin(void)))))
&&relation(definitely_preserves_value)"
}
-doc_end
