[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH] misra: deviate explicit cast for Rule 11.1
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.
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |