[PATCH] misra: deviate explicit cast for Rule 11.1

[PATCH] misra: deviate explicit cast for Rule 11.1

[Dmytro Prokopchuk1]

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@epam.com>
---
 docs/misra/safe.json    | 8 ++++++++
 xen/arch/arm/shutdown.c | 6 ++++--
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/docs/misra/safe.json b/docs/misra/safe.json
index 3584cb90c6..26a04ec521 100644
--- a/docs/misra/safe.json
+++ b/docs/misra/safe.json
@@ -124,6 +124,14 @@
         },
         {
             "id": "SAF-15-safe",
+            "analyser": {
+                "eclair": "MC3A2.R11.1"
+            },
+            "name": "Rule 11.1: conversions shall not be performed between a pointer to a function and any other type",
+            "text": "The explicit cast from 'void noreturn (*)(void *)' to 'void (*)(void *)' is safe because the semantics of the 'noreturn' attribute do not alter the calling convention or behavior of the resulting code."
+        },
+        {
+            "id": "SAF-16-safe",
             "analyser": {},
             "name": "Sentinel",
             "text": "Next ID to be used"
diff --git a/xen/arch/arm/shutdown.c b/xen/arch/arm/shutdown.c
index c9778e5786..57a5583820 100644
--- 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);
     local_irq_disable();
 
     /* Wait at most another 10ms for all other CPUs to go offline. */
@@ -50,7 +51,8 @@ void machine_restart(unsigned int delay_millisecs)
     spin_debug_disable();
 
     local_irq_enable();
-    smp_call_function(halt_this_cpu, NULL, 0);
+    /* SAF-15-safe */
+    smp_call_function((void (*)(void *))halt_this_cpu, NULL, 0);
     local_irq_disable();
 
     mdelay(delay_millisecs);
-- 
2.43.0

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

[Jan Beulich]

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@epam.com>

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.

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.

Jan

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

[Andrew Cooper]

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@epam.com>
> 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"?

~Andrew

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

[Nicola Vetrini]

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@epam.com>
>> 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

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

[Dmytro Prokopchuk1]

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@epam.com>
>>> 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.

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

[Dmytro Prokopchuk1]

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@epam.com>
>>>> 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

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

[Nicola Vetrini]

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@epam.com>
>>>>> 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

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

[Jan Beulich]

On 28.07.2025 21:17, Nicola Vetrini wrote:
> On 2025-07-28 20:58, Dmytro Prokopchuk1 wrote:
>> 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.

Not sure what you mean by "repercussion" here, and hence my remark may be
entirely meaningless, but: const and pure are also examples of
attributes which are safe to cast away, aiui. In fact, given the sheer
number of attributes, I would be surprised if there weren't more.

Jan

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

[Nicola Vetrini]

On 2025-07-29 09:26, Jan Beulich wrote:
> On 28.07.2025 21:17, Nicola Vetrini wrote:
>> On 2025-07-28 20:58, Dmytro Prokopchuk1 wrote:
>>> 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.
> 
> Not sure what you mean by "repercussion" here, and hence my remark may 
> be
> entirely meaningless, but: const and pure are also examples of
> attributes which are safe to cast away, aiui. In fact, given the sheer
> number of attributes, I would be surprised if there weren't more.
> 

Not all attributes influence compatibility of declarations. noreturn is 
a bit special in the sense that gcc and clang don't quite agree on the 
semantics of its various forms, where we follow clang's perspective. See 
https://github.com/llvm/llvm-project/issues/113511 for instance, in 
particular

> So I think the only way in which Clang's behavior is diverging from 
> GCC's here is that we don't support an implicit conversion between 
> pointer-to-function and pointer-to-noreturn function. This should 
> presumably behave somewhat like the implicit conversion between pointer 
> to-function and pointer-to-noexcept-function except that it can 
> apparently be performed implicitly in either direction.

But in any case my remark was more about the future than current plans, 
so there should be no bug here.

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

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

[Dmytro Prokopchuk1]

On 7/28/25 12:56, 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@epam.com>
> 
> 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.
> 
> 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.
> 
> Jan

Yes, Misra doesn't like cast.

Initially Misra reported about non-compliant implicit cast due to 
'noreturn' attribute:
smp_call_function(halt_this_cpu, NULL, 0);

I thought that in this case explicit cast is better, telling compiler 
exact type.
But, Misra reported about non-compliant c-style (explicit) cast.
So, I decided to deviate explicit cast.

I tried to write wrapper function to resolve this.
Example:
static void halt_this_cpu_2(void *arg)
{
     halt_this_cpu(arg);
}
void machine_halt(void)
{
     ...
     smp_call_function(halt_this_cpu_2, NULL, 0);
     ...

Unfortunately new R2.1 violation was observed.
"function definition `halt_this_cpu_2(void*)' (unit 
`xen/arch/arm/shutdown.c' with target `xen/arch/arm/shutdown.o') will 
never return"

Maybe it's better to have such violation....instead of R11.1 
"non-compliant cast"


I can remove cast and re-write deviation justification.
Are you OK with that, Jan?

Dmytro

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

[Jan Beulich]

On 28.07.2025 15:09, Dmytro Prokopchuk1 wrote:
> 
> 
> On 7/28/25 12:56, 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@epam.com>
>>
>> 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.
>>
>> 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.
>>
>> Jan
> 
> Yes, Misra doesn't like cast.
> 
> Initially Misra reported about non-compliant implicit cast due to 
> 'noreturn' attribute:
> smp_call_function(halt_this_cpu, NULL, 0);
> 
> I thought that in this case explicit cast is better, telling compiler 
> exact type.
> But, Misra reported about non-compliant c-style (explicit) cast.
> So, I decided to deviate explicit cast.
> 
> I tried to write wrapper function to resolve this.
> Example:
> static void halt_this_cpu_2(void *arg)
> {
>      halt_this_cpu(arg);
> }
> void machine_halt(void)
> {
>      ...
>      smp_call_function(halt_this_cpu_2, NULL, 0);
>      ...
> 
> Unfortunately new R2.1 violation was observed.
> "function definition `halt_this_cpu_2(void*)' (unit 
> `xen/arch/arm/shutdown.c' with target `xen/arch/arm/shutdown.o') will 
> never return"
> 
> Maybe it's better to have such violation....instead of R11.1 
> "non-compliant cast"
> 
> 
> I can remove cast and re-write deviation justification.
> Are you OK with that, Jan?

I expect so, as a temporary measure. In the longer run I would hope Eclair
can be adjusted to accept such cases without complaint.

Jan