[14/24] gdb: read pseudo register through frame

  Change gdbarch_pseudo_register_read_value to take a frame instead of a
regcache.  The frame (and formerly the regcache) is used to read raw
registers needed to make up the pseudo register value.  The problem with
using the regcache is that it always provides raw register values for
the current frame (frame 0).

Let's say the user wants to read the ebx register on amd64.  ebx is a pseudo
register, obtained by reading the bottom half (bottom 4 bytes) of the
rbx register, which is a raw register.  If the currently selected frame
is frame 0, it works fine:

    (gdb) frame 0
    #0  break_here_asm () at /home/smarchi/src/binutils-gdb/gdb/testsuite/gdb.arch/amd64-pseudo-unwind-asm.S:36
    36      in /home/smarchi/src/binutils-gdb/gdb/testsuite/gdb.arch/amd64-pseudo-unwind-asm.S
    (gdb) p/x $ebx
    $1 = 0x24252627
    (gdb) p/x $rbx
    $2 = 0x2021222324252627

But if the user is looking at another frame, and the raw register behind
the pseudo register has been saved at some point in the call stack, then
we get a wrong answer:

    (gdb) frame 1
    #1  0x000055555555517d in caller () at /home/smarchi/src/binutils-gdb/gdb/testsuite/gdb.arch/amd64-pseudo-unwind-asm.S:56
    56      in /home/smarchi/src/binutils-gdb/gdb/testsuite/gdb.arch/amd64-pseudo-unwind-asm.S
    (gdb) p/x $ebx
    $3 = 0x24252627
    (gdb) p/x $rbx
    $4 = 0x1011121314151617

Here, the value of ebx was computed using the value of rbx in frame 0
(through the regcache), it should have been computed using the value of
rbx in frame 1.

In other to make this work properly, make the following changes:

 - Make dwarf2_frame_prev_register return nullptr if it doesn't know how
   to unwind a register and that register is a pseudo register.
   Previously, it returned `frame_unwind_got_register`, meaning, in our
   example, "the value of ebx in frame 1 is the same as the value of ebx
   in frame 0", which is obviously false.  Return nullptr as a way to
   say "I don't know".

 - In frame_unwind_register_value, when prev_register (for instance
   dwarf2_frame_prev_register) returns nullptr, and we are trying to
   read a pseudo register, try to get the register value through
   gdbarch_pseudo_register_read_value or gdbarch_pseudo_register_read.
   If using gdbarch_pseudo_register_read, the behavior is known to be
   broken.  Implementations should be migrated to use
   gdbarch_pseudo_register_read_value to fix that.

 - Change gdbarch_pseudo_register_read_value to take a frame_info
   instead of a regcache, update implementations (aarch64, amd64, i386).
   In i386-tdep.c, I made a copy of i386_mmx_regnum_to_fp_regnum that
   uses a frame instead of a regcache.  The version using the regcache
   is still used by i386_pseudo_register_write.  It will get removed in
   a subsequent patch.

 - Add some helpers in value.{c,h} to implement the common cases of
   pseudo registers: taking part of a raw register and concatenating
   multiple raw registers.

 - Update readable_regcache::{cooked_read,cooked_read_value} to pass the
   current frame to gdbarch_pseudo_register_read_value.  Passing the
   current frame will give the same behavior as before: for frame 0, raw
   registers will be read from the current thread's regcache.

Notes:

 - I do not plan on changing gdbarch_pseudo_register_read to receive a
   frame instead of a regcache. That method is considered deprecated.
   Instead, we should be working on migrating implementations to use
   gdbarch_pseudo_register_read_value instead.

 - In frame_unwind_register_value, we still ask the unwinder to try to
   unwind pseudo register values.  It's apparently possible for the
   debug info to provide information about [1] pseudo registers, so we
   want to try that first, before falling back to computing them
   ourselves.

[1] https://inbox.sourceware.org/gdb-patches/20180528174715.A954AD804AD@oc3748833570.ibm.com/

Change-Id: Id6ef1c64e19090a183dec050e4034d8c2394e7ca
Reviewed-by: John Baldwin <jhb@FreeBSD.org>
---
 gdb/aarch64-tdep.c        | 148 +++++++++++--------------
 gdb/amd64-tdep.c          |  43 ++------
 gdb/dwarf2/frame.c        |   5 +-
 gdb/frame.c               |  36 +++++-
 gdb/gdbarch-gen.h         |   4 +-
 gdb/gdbarch.c             |   4 +-
 gdb/gdbarch_components.py |   2 +-
 gdb/i386-tdep.c           | 224 ++++++++++++++------------------------
 gdb/i386-tdep.h           |   7 +-
 gdb/regcache.c            |  38 +++++--
 gdb/value.c               |  70 ++++++++++++
 gdb/value.h               |  26 +++++
 12 files changed, 322 insertions(+), 285 deletions(-)

Message ID	20231201162751.741751-15-simon.marchi@efficios.com
State	New
Headers	DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 8EA24385C6F3 From: Simon Marchi <simon.marchi@efficios.com> To: gdb-patches@sourceware.org Cc: Luis Machado <luis.machado@arm.com>, John Baldwin <jhb@freebsd.org>, "Aktemur, Tankut Baris" <tankut.baris.aktemur@intel.com>, Simon Marchi <simon.marchi@efficios.com>, John Baldwin <jhb@FreeBSD.org> Subject: [PATCH 14/24] gdb: read pseudo register through frame Date: Fri, 1 Dec 2023 11:27:27 -0500 Message-ID: <20231201162751.741751-15-simon.marchi@efficios.com> In-Reply-To: <20231201162751.741751-1-simon.marchi@efficios.com> References: <20231201162751.741751-1-simon.marchi@efficios.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Precedence: list Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org
Series	Fix reading and writing pseudo registers in non-current frames \| [00/24] Fix reading and writing pseudo registers in non-current frames [01/24] gdb: don't handle i386 k registers as pseudo registers [02/24] gdb: use reg_buffer_common throughout gdbsupport/common-regcache.h [03/24] gdb: make store_integer take an array_view [04/24] gdb: simplify conditions in regcache::{read, write, raw_collect, raw_supply}_part [05/24] gdb: change regcache interface to use array_view [06/24] gdb: fix bugs in {get,put}_frame_register_bytes [07/24] gdb: make put_frame_register take an array_view [08/24] gdb: change value_of_register and value_of_register_lazy to take the next frame [09/24] gdb: remove frame_register [10/24] gdb: make put_frame_register take the next frame [11/24] gdb: make put_frame_register_bytes take the next frame [12/24] gdb: make get_frame_register_bytes take the next frame [13/24] gdb: add value::allocate_register [14/24] gdb: read pseudo register through frame [15/24] gdb: change parameter name in frame_unwind_register_unsigned declaration [16/24] gdb: rename gdbarch_pseudo_register_write to gdbarch_deprecated_pseudo_register_write [17/24] gdb: add gdbarch_pseudo_register_write that takes a frame [18/24] gdb: migrate i386 and amd64 to the new gdbarch_pseudo_register_write [19/24] gdb: make aarch64_za_offsets_from_regnum return za_offsets [20/24] gdb: add missing raw register read in aarch64_sme_pseudo_register_write [21/24] gdb: migrate aarch64 to new gdbarch_pseudo_register_write [22/24] gdb: migrate arm to gdbarch_pseudo_register_read_value [23/24] gdb: migrate arm to new gdbarch_pseudo_register_write [24/24] gdb/testsuite: add tests for unwinding of pseudo registers

Context	Check	Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64	success	Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm	success	Testing passed

[14/24] gdb: read pseudo register through frame

Checks

Commit Message

Patch