Re: [PATCH v1 2/3] compat: address violations of MISRA C Rule 19.1

[Jan Beulich <jbeulich@xxxxxxxx>]

On 26.04.2025 01:42, victorm.lira@xxxxxxx wrote:
> From: Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>
> 
> Rule 19.1 states: "An object shall not be assigned or copied
> to an overlapping object". Since the "call" and "compat_call" are
> fields of the same union, reading from one member and writing to
> the other violates the rule, while not causing Undefined Behavior
> due to their relative sizes. However, a dummy variable is used to
> address the violation and prevent the future possibility of
> incurring in UB.

If there is such a concern, ...

> --- a/xen/common/compat/multicall.c
> +++ b/xen/common/compat/multicall.c
> @@ -15,8 +15,13 @@ typedef int ret_t;
>  static inline void xlat_multicall_entry(struct mc_state *mcs)
>  {
>      int i;
> +    xen_ulong_t arg;
> +
>      for (i=0; i<6; i++)
> -        mcs->compat_call.args[i] = mcs->call.args[i];
> +    {
> +        arg = mcs->call.args[i];
> +        mcs->compat_call.args[i] = arg;
> +    }
>  }

... wouldn't it be of concern as well that the alternating parts of
the union are still accessed in a flip-flop manner? IOW we continue to
rely on the relative placement properties of the individual array
elements. To eliminate such a concern, I think the resulting code would
also want to be correct if iteration was swapped to work downwards.

Also the scope of the temporary could certainly be the loop body rather
than the entire function. I also don't think it needs to be xen_ulong_t,
but maybe using unsigned int instead wouldn't make a difference in
generated code.

Jan