Re: [PATCH] misra: deviate explicit cast for Rule 11.1

[Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>]

On 2025-07-28 20:58, Dmytro Prokopchuk1 wrote:
> On 7/28/25 21:03, Dmytro Prokopchuk wrote:
> > On 7/28/25 20:43, Nicola Vetrini wrote:
> > > On 2025-07-28 12:49, Andrew Cooper wrote:
> > > > On 28/07/2025 10:56 am, Jan Beulich wrote:
> > > > > On 27.07.2025 22:27, Dmytro Prokopchuk1 wrote:
> > > > > > Explicitly cast 'halt_this_cpu' when passing it
> > > > > > to 'smp_call_function' to match the required
> > > > > > function pointer type '(void (*)(void *info))'.
> > > > > >
> > > > > > Document and justify a MISRA C R11.1 deviation
> > > > > > (explicit cast).
> > > > > >
> > > > > > Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@xxxxxxxx>
> > > > > All you talk about is the rule that you violate by adding a cast.
> > > > > But what is
> > > > > the problem you're actually trying to resolve by adding a cast?
> > > > >
> > > > > > --- a/xen/arch/arm/shutdown.c
> > > > > > +++ b/xen/arch/arm/shutdown.c
> > > > > > @@ -25,7 +25,8 @@ void machine_halt(void)
> > > > > >      watchdog_disable();
> > > > > >      console_start_sync();
> > > > > >      local_irq_enable();
> > > > > > -    smp_call_function(halt_this_cpu, NULL, 0);
> > > > > > +    /* SAF-15-safe */
> > > > > > +    smp_call_function((void (*)(void *))halt_this_cpu, NULL, 0);
> > > > > Now this is the kind of cast that is very dangerous. The function's
> > > > > signature
> > > > > changing will go entirely unnoticed (by the compiler) with such a
> > > > > cast in place.
> > > > I agree.  This code is *far* safer in practice without the cast, 
> > > > than
> > > > with it.
> > > >
> > > > > If Misra / Eclair are unhappy about such an extra (benign here)
> > > > > attribute, I'd
> > > > > be interested to know what their suggestion is to deal with the
> > > > > situation
> > > > > without making the code worse (as in: more risky). I first thought
> > > > > about having
> > > > > a new helper function that then simply chains to halt_this_cpu(),
> > > > > yet that
> > > > > would result in a function which can't return, but has no noreturn
> > > > > attribute.
> > > > I guess that Eclair cannot know what an arbitrary attribute does and
> > > > whether it impacts the ABI, but it would be lovely if Eclair could 
> > > > be
> > > > told "noreturn is a safe attribute to differ on"?
> > > I'm convinced it can do that. Perhaps something like
> > >
> > > -config=MC3A2.R11.1,casts+={safe,
> > > "kind(bitcast)&&to(type(pointer(inner(return(builtin(void))&&all_param(1, 
> > > pointer(builtin(void)))))))&&from(expr(skip(!syntactic(), 
> > > ref(property(noreturn)))))"}
> > > which is a mess but decodes to that, more or less.
> > >
> > > I haven't tested it yet, though, but on a toy example [1] it works.
> > >
> > > [1]
> > > void __attribute__((noreturn)) f(void *p) {
> > >    __builtin_abort();
> > > }
> > >
> > > void g(int x, void (*fp)(void *p)) {
> > >    if (x < 3) {
> > >      f((void*)x);
> > >    }
> > > }
> > >
> > > int main(int argc, char **argv) {
> > >    g(argc, f);
> > >    return 0;
> > > }
> > Thanks, Nicola.
> > I will check this.
> >
> > Dmytro.
> It works.
> The violation "non-compliant cast: implicit cast from `void(*)(void*)'
> to `void(*)(void*)'" is gone.
Great. Now what would be really useful is a way to abstract this more 
nicely (I was able to write this only by looking at the AST). However 
noreturn is probably about the only attribute that has a repercussion on 
the decl and is safe to cast away, unless I'm mistaken.
Thanks,
 Nicola

--
Nicola Vetrini, B.Sc.
Software Engineer
BUGSENG (https://bugseng.com)
LinkedIn: https://www.linkedin.com/in/nicola-vetrini-a42471253