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

[Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>]

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;
}

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