Re: [PATCH] Record ARM THUMB2 PLD/PLI cache instructions

[Trent Piepho <tpiepho at impinj dot com>]

On Sun, 2018-09-30 at 10:21 -0400, Simon Marchi wrote:
> On 2018-09-28 7:04 p.m., Trent Piepho wrote:
> > These weren't decoded correctly and trigger an unknown instruction error
> > when recording.  The ARM format was handled, but not the 32-bit THUMB2
> > format.
> > 
> > Since they are only hints that may affect cache state, there is nothing
> > to record.
> > 
> > gdb/ChangeLog
> > 2018-09-28  Trent Piepho  <tpiepho@impinj.com>
> > 
> >         PR gdb/23725
> >         * gdb/arm-tdep.c (thumb2_record_ld_mem_hints): Decode thumb2 PLD/PLI
> > ---
> >  gdb/arm-tdep.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
> > index c3280ee211..90936ada8e 100644
> > --- a/gdb/arm-tdep.c
> > +++ b/gdb/arm-tdep.c
> > @@ -12683,6 +12683,14 @@ thumb2_record_ld_mem_hints (insn_decode_record *thumb2_insn_r)
> >                  record_buf);
> >        return ARM_RECORD_SUCCESS;
> >      }
> > +  else
> > +    {
> > +      if (bits (thumb2_insn_r->arm_insn, 20, 22) == 0x1)
> > +	{
> > +	  /* Handle PLD, PLI affect only caches, so nothing to record */
> > +	  return ARM_RECORD_SUCCESS;
> > +	}
> > +    }
> >  
> >    return ARM_RECORD_FAILURE;
> >  }
> > 
> 
> Hi Trent,
> 
> Thanks for the patch.  After staring at the ARM architecture reference manual enough, I
> think this is fine.
> 
> In the manual, however, in table "Table A5-20 Load byte, memory hints", some encodings with
> Rt == 0b1111 decode to "UNPREDICTABLE".  Should the record fail for those?  I think currently
> with your patch we will accept them.  I am thinking it would be good to fail, because since
> we can't know the side effects of such instruction, we risk showing some false information if
> we just assume nothing has changed.

I'm not sure what document this is from, but in https://static.docs.arm
.com/ddi0406/c/DDI0406C_C_arm_architecture_reference_manual.pdf

Table A6-20 is titled as above.

Rather than this, I used the thumb2 supplement I found here: http://her
mes.wings.cs.wisc.edu/files/Thumb-2SupplementReferenceManual.pdf

Section 3.3.3 had the most useful table and exhaustive list of possible
encodings for this type of thumb2 instruction.

I see now that not every possible addressing mode is supported for
PLD/PLI, and there are ways to encode a reserved addressing mode for
all instructions of this type.

I've prepared a follow on patch that should provide an exhaustive check
for PLD and PLI instructions.  It also enhances the check for other
instructions of this general format, but I've not verified that the
code is exhaustive there.  It is at least better than it was.

> If you are motivated, it would be nice to add a test for this instruction in arm_record_test,
> but I won't require it, since the current state is that this test isn't meant to test all
> possible instruction, and I don't want to impose that burden on you.

I might that be THAT motivated, since I've never even used that test
feature.