Re: [PATCH v2 1/2] x86: x86_emulate: address violations of MISRA C Rule 19.1

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Fri, 2 May 2025, 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". In the function like macro "get_rep_prefix",
> one member of a union is assigned the value of another member. Reading from 
> one
> member and writing to the other violates the rule, while not causing Undefined
> Behavior due to their relative sizes. Instead, use casts combined with exactly
> overlapping accesses to address violations.
> 
> No functional change.
> 
> Signed-off-by: Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>
> Signed-off-by: Federico Serafini <federico.serafini@xxxxxxxxxxx>
> Signed-off-by: Victor Lira <victorm.lira@xxxxxxx>
> ---
> Changes in v2:
> - Use casts combined with exactly overlapping accesses to address
>   violations
> - fix commit message
> ---
> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Cc: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> Cc: Michal Orzel <michal.orzel@xxxxxxx>
> Cc: Jan Beulich <jbeulich@xxxxxxxx>
> Cc: Julien Grall <julien@xxxxxxx>
> Cc: Roger Pau Monn?? <roger.pau@xxxxxxxxxx>
> Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> Cc: Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>
> Cc: Federico Serafini <federico.serafini@xxxxxxxxxxx>
> Cc: Bertrand Marquis <bertrand.marquis@xxxxxxx>
> ---
>  xen/arch/x86/x86_emulate/x86_emulate.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c 
> b/xen/arch/x86/x86_emulate/x86_emulate.c
> index 8e14ebb35b..d678855238 100644
> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
> @@ -527,8 +527,8 @@ static inline void put_loop_count(
>          if ( !amd_like(ctxt) && mode_64bit() && ad_bytes == 4 )         \
>          {                                                               \
>              _regs.r(cx) = 0;                                            \
> -            if ( extend_si ) _regs.r(si) = _regs.esi;                   \
> -            if ( extend_di ) _regs.r(di) = _regs.edi;                   \
> +            if ( extend_si ) _regs.r(si) = (uint32_t)_regs.r(si);        \
> +            if ( extend_di ) _regs.r(di) = (uint32_t)_regs.r(di);        \

NIT: code style, alignment of the \

Can be fixed on commit.

Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>


>          }                                                               \
>          goto complete_insn;                                             \
>      }                                                                   \
> @@ -2029,7 +2029,7 @@ x86_emulate(
>          switch ( op_bytes )
>          {
>          case 2: _regs.ax = (int8_t)_regs.ax; break; /* cbw */
> -        case 4: _regs.r(ax) = (uint32_t)(int16_t)_regs.ax; break; /* cwde */
> +        case 4: _regs.r(ax) = (uint32_t)(int16_t)_regs.r(ax); break; /* cwde 
> */
>          case 8: _regs.r(ax) = (int32_t)_regs.r(ax); break; /* cdqe */
>          }
>          break;
> --
> 2.25.1
>