Re: [XEN PATCH 09/12] x86/emul: address violations of MISRA C Rule 16.3

[Federico Serafini <federico.serafini@xxxxxxxxxxx>]

On 11/09/24 14:42, Jan Beulich wrote:
> On 10.09.2024 12:09, Federico Serafini wrote:
> > --- a/xen/arch/x86/x86_emulate/fpu.c
> > +++ b/xen/arch/x86/x86_emulate/fpu.c
> > @@ -218,6 +218,7 @@ int x86emul_fpu(struct x86_emulate_state *s,
> >                */
> >               if ( dst->type == OP_MEM && !s->fpu_ctrl && !fpu_check_write() )
> >                   dst->type = OP_NONE;
> > +            break;
> >           }
> >           break;
> >   
> > @@ -296,6 +297,7 @@ int x86emul_fpu(struct x86_emulate_state *s,
> >               default:
> >                   generate_exception(X86_EXC_UD);
> >               }
> > +            break;
> >           }
> >           break;
> >   
> > @@ -386,6 +388,7 @@ int x86emul_fpu(struct x86_emulate_state *s,
> >                */
> >               if ( dst->type == OP_MEM && !s->fpu_ctrl && !fpu_check_write() )
> >                   dst->type = OP_NONE;
> > +            break;
> >           }
> >           break;
> >   
> > @@ -457,6 +460,7 @@ int x86emul_fpu(struct x86_emulate_state *s,
> >               case 7: /* fistp m64i */
> >                   goto fpu_memdst64;
> >               }
> > +            break;
> Aren't you swapping one violation for another here? Unlike in the earlier
> three cases, this new break is unreachable, because of the nature of the
> preceding switch() statement (cases being exhaustive and every case ending
> in "goto"; this is something even a static analyzer can [in principle]
> spot).
You are right, but the resulting violation of Rule 2.1
("A project shall not contain unreachable code") is deviated with the
following justification:
"The compiler implementation guarantees that the unreachable code is
removed. Constant expressions and unreachable branches of if and switch
statements are expected."

--
Federico Serafini,

Ph.D. Student, Ca' Foscari University of Venice
Software Engineer, BUGSENG