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 > > > > 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.&j!z޶םwib֫rnr