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

[Simon Marchi <simark@simark.ca>]

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.

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.

Simon