All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] misra: deviate explicit cast for Rule 11.1
@ 2025-07-27 20:27 Dmytro Prokopchuk1
  2025-07-28  9:56 ` Jan Beulich
  0 siblings, 1 reply; 11+ messages in thread
From: Dmytro Prokopchuk1 @ 2025-07-27 20:27 UTC (permalink / raw)
  To: xen-devel@lists.xenproject.org
  Cc: Dmytro Prokopchuk1, Andrew Cooper, Anthony PERARD, Michal Orzel,
	Jan Beulich, Julien Grall, Roger Pau Monné,
	Stefano Stabellini, Bertrand Marquis, Volodymyr Babchuk

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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] misra: deviate explicit cast for Rule 11.1
  2025-07-27 20:27 [PATCH] misra: deviate explicit cast for Rule 11.1 Dmytro Prokopchuk1
@ 2025-07-28  9:56 ` Jan Beulich
  2025-07-28 10:49   ` Andrew Cooper
  2025-07-28 13:09   ` Dmytro Prokopchuk1
  0 siblings, 2 replies; 11+ messages in thread
From: Jan Beulich @ 2025-07-28  9:56 UTC (permalink / raw)
  To: Dmytro Prokopchuk1, consulting@bugseng.com
  Cc: Andrew Cooper, Anthony PERARD, Michal Orzel, Julien Grall,
	Roger Pau Monné, Stefano Stabellini, Bertrand Marquis,
	Volodymyr Babchuk, xen-devel@lists.xenproject.org

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] misra: deviate explicit cast for Rule 11.1
  2025-07-28  9:56 ` Jan Beulich
@ 2025-07-28 10:49   ` Andrew Cooper
  2025-07-28 17:43     ` Nicola Vetrini
  2025-07-28 13:09   ` Dmytro Prokopchuk1
  1 sibling, 1 reply; 11+ messages in thread
From: Andrew Cooper @ 2025-07-28 10:49 UTC (permalink / raw)
  To: Jan Beulich, Dmytro Prokopchuk1, consulting@bugseng.com
  Cc: Anthony PERARD, Michal Orzel, Julien Grall, Roger Pau Monné,
	Stefano Stabellini, Bertrand Marquis, Volodymyr Babchuk,
	xen-devel@lists.xenproject.org

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] misra: deviate explicit cast for Rule 11.1
  2025-07-28  9:56 ` Jan Beulich
  2025-07-28 10:49   ` Andrew Cooper
@ 2025-07-28 13:09   ` Dmytro Prokopchuk1
  2025-07-28 13:23     ` Jan Beulich
  1 sibling, 1 reply; 11+ messages in thread
From: Dmytro Prokopchuk1 @ 2025-07-28 13:09 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Andrew Cooper, Anthony PERARD, Michal Orzel, Julien Grall,
	Roger Pau Monné, Stefano Stabellini, Bertrand Marquis,
	Volodymyr Babchuk, xen-devel@lists.xenproject.org,
	consulting@bugseng.com



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



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] misra: deviate explicit cast for Rule 11.1
  2025-07-28 13:09   ` Dmytro Prokopchuk1
@ 2025-07-28 13:23     ` Jan Beulich
  0 siblings, 0 replies; 11+ messages in thread
From: Jan Beulich @ 2025-07-28 13:23 UTC (permalink / raw)
  To: Dmytro Prokopchuk1
  Cc: Andrew Cooper, Anthony PERARD, Michal Orzel, Julien Grall,
	Roger Pau Monné, Stefano Stabellini, Bertrand Marquis,
	Volodymyr Babchuk, xen-devel@lists.xenproject.org,
	consulting@bugseng.com

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] misra: deviate explicit cast for Rule 11.1
  2025-07-28 10:49   ` Andrew Cooper
@ 2025-07-28 17:43     ` Nicola Vetrini
  2025-07-28 18:03       ` Dmytro Prokopchuk1
  0 siblings, 1 reply; 11+ messages in thread
From: Nicola Vetrini @ 2025-07-28 17:43 UTC (permalink / raw)
  To: Andrew Cooper
  Cc: Jan Beulich, Dmytro Prokopchuk1, consulting, Anthony PERARD,
	Michal Orzel, Julien Grall, Roger Pau Monné,
	Stefano Stabellini, Bertrand Marquis, Volodymyr Babchuk,
	xen-devel

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] misra: deviate explicit cast for Rule 11.1
  2025-07-28 17:43     ` Nicola Vetrini
@ 2025-07-28 18:03       ` Dmytro Prokopchuk1
  2025-07-28 18:58         ` Dmytro Prokopchuk1
  0 siblings, 1 reply; 11+ messages in thread
From: Dmytro Prokopchuk1 @ 2025-07-28 18:03 UTC (permalink / raw)
  To: Nicola Vetrini, Andrew Cooper
  Cc: Jan Beulich, consulting@bugseng.com, Anthony PERARD, Michal Orzel,
	Julien Grall, Roger Pau Monné, Stefano Stabellini,
	Bertrand Marquis, Volodymyr Babchuk,
	xen-devel@lists.xenproject.org



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.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] misra: deviate explicit cast for Rule 11.1
  2025-07-28 18:03       ` Dmytro Prokopchuk1
@ 2025-07-28 18:58         ` Dmytro Prokopchuk1
  2025-07-28 19:17           ` Nicola Vetrini
  0 siblings, 1 reply; 11+ messages in thread
From: Dmytro Prokopchuk1 @ 2025-07-28 18:58 UTC (permalink / raw)
  To: Nicola Vetrini
  Cc: Jan Beulich, consulting@bugseng.com, Anthony PERARD, Michal Orzel,
	Julien Grall, Roger Pau Monné, Stefano Stabellini,
	Bertrand Marquis, Volodymyr Babchuk,
	xen-devel@lists.xenproject.org, Andrew Cooper



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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] misra: deviate explicit cast for Rule 11.1
  2025-07-28 18:58         ` Dmytro Prokopchuk1
@ 2025-07-28 19:17           ` Nicola Vetrini
  2025-07-29  7:26             ` Jan Beulich
  0 siblings, 1 reply; 11+ messages in thread
From: Nicola Vetrini @ 2025-07-28 19:17 UTC (permalink / raw)
  To: Dmytro Prokopchuk1
  Cc: Jan Beulich, consulting, Anthony PERARD, Michal Orzel,
	Julien Grall, Roger Pau Monné, Stefano Stabellini,
	Bertrand Marquis, Volodymyr Babchuk, xen-devel, Andrew Cooper

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] misra: deviate explicit cast for Rule 11.1
  2025-07-28 19:17           ` Nicola Vetrini
@ 2025-07-29  7:26             ` Jan Beulich
  2025-07-29 11:03               ` Nicola Vetrini
  0 siblings, 1 reply; 11+ messages in thread
From: Jan Beulich @ 2025-07-29  7:26 UTC (permalink / raw)
  To: Nicola Vetrini
  Cc: consulting, Anthony PERARD, Michal Orzel, Julien Grall,
	Roger Pau Monné, Stefano Stabellini, Bertrand Marquis,
	Volodymyr Babchuk, xen-devel, Andrew Cooper, Dmytro Prokopchuk1

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] misra: deviate explicit cast for Rule 11.1
  2025-07-29  7:26             ` Jan Beulich
@ 2025-07-29 11:03               ` Nicola Vetrini
  0 siblings, 0 replies; 11+ messages in thread
From: Nicola Vetrini @ 2025-07-29 11:03 UTC (permalink / raw)
  To: Jan Beulich
  Cc: consulting, Anthony PERARD, Michal Orzel, Julien Grall,
	Roger Pau Monné, Stefano Stabellini, Bertrand Marquis,
	Volodymyr Babchuk, xen-devel, Andrew Cooper, Dmytro Prokopchuk1

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2025-07-29 11:11 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-27 20:27 [PATCH] misra: deviate explicit cast for Rule 11.1 Dmytro Prokopchuk1
2025-07-28  9:56 ` Jan Beulich
2025-07-28 10:49   ` Andrew Cooper
2025-07-28 17:43     ` Nicola Vetrini
2025-07-28 18:03       ` Dmytro Prokopchuk1
2025-07-28 18:58         ` Dmytro Prokopchuk1
2025-07-28 19:17           ` Nicola Vetrini
2025-07-29  7:26             ` Jan Beulich
2025-07-29 11:03               ` Nicola Vetrini
2025-07-28 13:09   ` Dmytro Prokopchuk1
2025-07-28 13:23     ` Jan Beulich

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.