Re: [Xen-devel] [PATCH] remove dead code from arm/decode.c

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Tue, 13 Dec 2016, Julien Grall wrote:
> Hi Stefano,
> 
> On 10/12/16 01:31, Stefano Stabellini wrote:
> > The rt variable can only be 0 or 7, no need to check if it's 15.
> 
> Be careful, Coverity may point to dead code but it does not mean that deleting
> it is the right thing to do. The code which lead to this conclusion may be
> invalid. In this case, the dead code is happening because the rt variable has
> been miscalculated (see below).
> 
> > 
> > Coverity-ID: 1381835
> > 
> > Signed-off-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> > 
> > diff --git a/xen/arch/arm/decode.c b/xen/arch/arm/decode.c
> > index c6f49a5..decd9dd 100644
> > --- a/xen/arch/arm/decode.c
> > +++ b/xen/arch/arm/decode.c
> > @@ -58,11 +58,6 @@ static int decode_thumb2(register_t pc, struct hsr_dabt
> > *dabt, uint16_t hw1)
> >              /* Undefined opcodes */
> >              goto bad_thumb2;
> > 
> > -        /* Store/Load single data item */
> > -        if ( rt == 15 )
> > -            /* XXX: Rt == 15 is only invalid for store instruction */
> > -            goto bad_thumb2;
> > -
> 
> Looking at the encoding of Thumb2 instruction (see encoding T2 for LDRB
> A8.8.70 in  ARM DDI 0406C.c), this check is correct.
> 
> However, the line 44 "rt = (hw2 >> 12) & 7" will not retrieve the correct
> register as it should be encoded with 4 bits. So the code should have been
> 
> rt = (hw2 >> 12) & 0xf;

Indeed, you are right. I'll resubmit.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel