[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 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. Dmytro
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |