In the file common/softirq macro set_bit is called with argument
smp_processor_id.
Once expanded this set_bit's argument is used in sizeof operations
and thus 'smp_processor_id', being a macro that expands to a
function call with potential side effects, generates a violation.

To address this violation the value of smp_processor_id is therefore
stored in a variable called 'cpu' before passing it to macro set_bit.

No functional change.

Signed-off-by: Alessandro Zucchelli <alessandro.zucchelli@bugseng.com>
---
 xen/common/softirq.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/xen/common/softirq.c b/xen/common/softirq.c
index bee4a82009..c5f3870534 100644
--- a/xen/common/softirq.c
+++ b/xen/common/softirq.c
@@ -139,7 +139,8 @@ void cpu_raise_softirq_batch_finish(void)
 
 void raise_softirq(unsigned int nr)
 {
-    set_bit(nr, &softirq_pending(smp_processor_id()));
+    unsigned int cpu = smp_processor_id();
+    set_bit(nr, &softirq_pending(cpu));
 }
 
 /*
-- 
2.34.1

On 25.06.2024 12:14, Alessandro Zucchelli wrote:
> In the file common/softirq macro set_bit is called with argument
> smp_processor_id.
> Once expanded this set_bit's argument is used in sizeof operations
> and thus 'smp_processor_id', being a macro that expands to a
> function call with potential side effects, generates a violation.

Noticing only now, but applicable also to patch 2: "expands" isn't quite
right, is it? That's true for x86, but apparently not for Arm. Unless I
managed to overlook something there. So perhaps "may expand" instead?

> --- a/xen/common/softirq.c
> +++ b/xen/common/softirq.c
> @@ -139,7 +139,8 @@ void cpu_raise_softirq_batch_finish(void)
>  
>  void raise_softirq(unsigned int nr)
>  {
> -    set_bit(nr, &softirq_pending(smp_processor_id()));
> +    unsigned int cpu = smp_processor_id();
> +    set_bit(nr, &softirq_pending(cpu));
>  }

Nit (style): Blank line between declaration(s) and statement(s) please.

I guess both aspects could be taken care of while committing.

Jan

On Tue, 25 Jun 2024, Alessandro Zucchelli wrote:
> In the file common/softirq macro set_bit is called with argument
> smp_processor_id.
> Once expanded this set_bit's argument is used in sizeof operations
> and thus 'smp_processor_id', being a macro that expands to a
> function call with potential side effects, generates a violation.
> 
> To address this violation the value of smp_processor_id is therefore
> stored in a variable called 'cpu' before passing it to macro set_bit.
> 
> No functional change.
> 
> Signed-off-by: Alessandro Zucchelli <alessandro.zucchelli@bugseng.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>