Re: [PATCH v3 7/7] amd64_analyze_prologue: use target_read_code instead of read_code

Simon Marchi
2020-08-06T15:01:40Z

On 2020-06-23 9:28 p.m., Victor Collod via Gdb-patches wrote:
> Using target_read_code enables gracefuly handling error cases.

Can you expand a bit why you think it's more graceful?

>
> 2020-06-23 Victor Collod <vcollod@nvidia.com>
>
> * amd64-tdep.c (amd64_analyze_prologue): Use target_read_code
> instead of read_code.
> ---
> gdb/amd64-tdep.c | 19 ++++++++++++++-----
> 1 file changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
> index 901733cf443..7f70c1d0d8d 100644
> --- a/gdb/amd64-tdep.c
> +++ b/gdb/amd64-tdep.c
> @@ -2394,9 +2394,15 @@ amd64_analyze_prologue (struct gdbarch *gdbarch,
> else
> pc = amd64_analyze_stack_align (pc, current_pc, cache);
>
> - read_code (pc, buf, 4);
> - /* Check for the `endbr64' instruction and skip it if found. */
> - if (memcmp (buf, endbr64, sizeof (endbr64)) == 0)
> + /* Try to read enough bytes to check for `endbr64'. */
> + if (target_read_code (pc, buf, 4) != 0)
> + {
> + /* If it fails, read just enough data for `pushq %rbp'. */
> + if (target_read_code (pc, buf, 1) != 0)
> + return pc;
> + }
> + /* If reading succeeded, check for the `endbr64' instruction and skip it if found. */
> + else if (memcmp (buf, endbr64, sizeof (endbr64)) == 0)

Personally, I find this "try to read 4 else try to read 1" approach less clear
and less intuitive than the "read 1 byte and read more if needed approach".

Simon