From patchwork Wed Oct 24 01:43:32 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Marchi X-Patchwork-Id: 29858 Received: (qmail 89187 invoked by alias); 24 Oct 2018 01:43:50 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 87477 invoked by uid 89); 24 Oct 2018 01:43:46 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.2 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_SOFTFAIL autolearn=ham version=3.3.2 spammy=sk:user_re X-HELO: barracuda.ebox.ca Received: from barracuda.ebox.ca (HELO barracuda.ebox.ca) (96.127.255.19) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 24 Oct 2018 01:43:43 +0000 Received: from smtp.ebox.ca (smtp.electronicbox.net [96.127.255.82]) by barracuda.ebox.ca with ESMTP id nBWXayLnxdxFUdLe (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Tue, 23 Oct 2018 21:43:39 -0400 (EDT) Received: from simark.lan (unknown [192.222.164.54]) by smtp.ebox.ca (Postfix) with ESMTP id 56759441B21; Tue, 23 Oct 2018 21:43:39 -0400 (EDT) From: Simon Marchi To: gdb-patches@sourceware.org Cc: Ulrich Weigand , Tom Tromey , Simon Marchi Subject: [PATCH v2 2/3] Read pseudo registers from frame instead of regcache Date: Tue, 23 Oct 2018 21:43:32 -0400 Message-Id: <20181024014333.14143-3-simon.marchi@polymtl.ca> In-Reply-To: <20181024014333.14143-1-simon.marchi@polymtl.ca> References: <20181024014333.14143-1-simon.marchi@polymtl.ca> MIME-Version: 1.0 X-IsSubscribed: yes From: Simon Marchi Reading pseudo registers (made of one or more raw register) from an unwound stack frame (frame number > #0) does not work. The raw registers needed to compose the pseudo registers are always read from the current thread's regcache, which is effectively frame #0. Here's an example using the x86-64 test included in this patch (with some output remove for brevity): (gdb) tb break_here_asm (gdb) r (gdb) info registers rbx ebx rbx 0x2021222324252627 2315169217770759719 ebx 0x24252627 606414375 (gdb) up (gdb) info registers rbx ebx rbx 0x1011121314151617 1157726452361532951 ebx 0x24252627 606414375 We would expect the last ebx value to be 0x14151617, half of the rbx value in the same frame. The problem comes from the fact that the gdbarch hooks: - pseudo_register_read - pseudo_register_read_value - pseudo_register_write receive a regcache as a parameter, as a way to get the values of the raw registers required to compute the pseudo register value. However, the regcache object always contains the current register values, not the values unwound to the frame we're interested in. That's why ebx in frame #1 appears to have the value it has in frame #0. This patch introduces new implementations of the register_reader and register_readwriter interfaces which access register values by unwinding them from a particular frame. It therefore allows computing pseudo register values using the raw register values of that frame. My initial implementation [1] made it so raw registers were always unwound using the frame unwinder (generally goes to dwarf2_frame_prev_register) and pseudo registers were always unwound through gdbarch_pseudo_register_read. The problem with this, as pointed out by Ulrich, is that some DWARF info may contain unwind info for pseudo registers. An example of this are the S (single-precision) registers on ARM, where two 32-bits S registers form one 64-bits D (double-precision) register. Today, according to the "DWARF for the ARM architecture" document [2], producers should describe the location of the saved D registers in the DWARF info and the value of the S registers should be inferred from them. In GDB words, S are pseudo registers built from D, which are raw registers. So we would unwind D registers using the frame unwinder (i.e. DWARF info) and unwind S registers using gdbarch_pseudo_register_read, which will actually read the right D register and derive the S register value from that. However, before the introduction of the D registers, the S registers had register numbers assigned (64-95). So it's possible to find binaries in the wild whose DWARF info have rules to unwind S registers. Using my initial approach described previously wouldn't work then. When trying to read an S register, we would try to read the matching D register, for which we don't have any unwind information, and conclude its value hasn't been saved. We would miss the unwind info specific to the S register, and end up with a wrong result. So what this patch does is first try to unwind pseudo registers using the frame unwinder, for both raw and pseudo registers. If we specifically have info for that register, great, we use that. Typically, for pseudo registers, we won't find anything. So we'll fall back to going through gdbarch_pseudo_register_read. [1] https://sourceware.org/ml/gdb-patches/2018-05/msg00705.html [2] http://infocenter.arm.com/help/topic/com.arm.doc.ihi0040b/IHI0040B_aadwarf.pdf --- gdb/dwarf2-frame.c | 12 ++++- gdb/frame.c | 122 ++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 124 insertions(+), 10 deletions(-) diff --git a/gdb/dwarf2-frame.c b/gdb/dwarf2-frame.c index 2201c636590d..2b698fbf6caa 100644 --- a/gdb/dwarf2-frame.c +++ b/gdb/dwarf2-frame.c @@ -1290,8 +1290,16 @@ dwarf2_frame_prev_register (struct frame_info *this_frame, void **this_cache, registers are actually undefined (which is different to CFI "undefined"). Code above issues a complaint about this. Here just fudge the books, assume GCC, and that the value is - more inner on the stack. */ - return frame_unwind_got_register (this_frame, regnum, regnum); + more inner on the stack. + + If the register is a pseudo one, it's possible that we don't have + unwind info for it, but we have unwind info for the raw registers + that compose it. Return nullptr, so that frame_unwind_register_value + attempts reconstructing the value from raw registers. */ + if (regnum < gdbarch_num_regs (gdbarch)) + return frame_unwind_got_register (this_frame, regnum, regnum); + else + return nullptr; case DWARF2_FRAME_REG_SAME_VALUE: return frame_unwind_got_register (this_frame, regnum, regnum); diff --git a/gdb/frame.c b/gdb/frame.c index 4624eada87b2..cd7149ee38b2 100644 --- a/gdb/frame.c +++ b/gdb/frame.c @@ -1180,14 +1180,74 @@ get_frame_register (struct frame_info *frame, frame_unwind_register (frame->next, regnum, buf); } +/* Implement the register_reader interface, but read the registers of a given + frame. */ + +struct frame_register_reader : public virtual register_reader +{ + frame_register_reader (frame_info *next_frame) + : m_next_frame (next_frame) + {} + + gdbarch *arch () const override + { + return get_frame_arch (m_next_frame); + } + + register_status raw_read (int regnum, gdb_byte *buf) override + { + return cooked_read (regnum, buf); + } + + register_status cooked_read (int regnum, gdb_byte *buf) override + { + TRY + { + frame_unwind_register (m_next_frame, regnum, buf); + return REG_VALID; + } + CATCH (ex, RETURN_MASK_ALL) + { + return REG_UNAVAILABLE; + } + END_CATCH + } + +protected: + frame_info *m_next_frame; +}; + +/* Implement the register_readwriter interface, but read/write the registers of + a given frame. */ + +struct frame_register_readwriter : public frame_register_reader, + public register_readwriter +{ + frame_register_readwriter (frame_info *next_frame) + : frame_register_reader (next_frame) + {} + + void raw_write (int regnum, const gdb_byte *buf) override + { + cooked_write (regnum, buf); + } + + void cooked_write (int regnum, const gdb_byte *buf) override + { + frame_info *this_frame = get_prev_frame (m_next_frame); + put_frame_register (this_frame, regnum, buf); + } +}; + struct value * frame_unwind_register_value (frame_info *next_frame, int regnum) { - struct gdbarch *gdbarch; - struct value *value; - gdb_assert (next_frame != NULL); - gdbarch = frame_unwind_arch (next_frame); + + struct gdbarch *gdbarch = frame_unwind_arch (next_frame); + + gdb_assert (regnum >= 0); + gdb_assert (regnum < gdbarch_num_cooked_regs (gdbarch)); if (frame_debug) { @@ -1203,9 +1263,41 @@ frame_unwind_register_value (frame_info *next_frame, int regnum) frame_unwind_find_by_frame (next_frame, &next_frame->prologue_cache); /* Ask this frame to unwind its register. */ - value = next_frame->unwind->prev_register (next_frame, - &next_frame->prologue_cache, - regnum); + struct value *value + = next_frame->unwind->prev_register (next_frame, + &next_frame->prologue_cache, regnum); + + if (value == nullptr) + { + frame_register_reader reg_read (next_frame); + + if (gdbarch_pseudo_register_read_value_p (gdbarch)) + { + /* This is a pseudo register, we don't know how how what raw registers + this pseudo register is made of. Ask the gdbarch to read the + value, it will itself ask the next frame to unwind the values of + the raw registers it needs to compose the value of the pseudo + register. */ + value + = gdbarch_pseudo_register_read_value (gdbarch, ®_read, regnum); + VALUE_LVAL (value) = not_lval; + } + else if (gdbarch_pseudo_register_read_p (gdbarch)) + { + value = allocate_value (register_type (gdbarch, regnum)); + VALUE_LVAL (value) = not_lval; + + register_status st + = gdbarch_pseudo_register_read (gdbarch, ®_read, regnum, + value_contents_raw (value)); + if (st == REG_UNAVAILABLE) + mark_value_bytes_unavailable (value, 0, + TYPE_LENGTH (value_type (value))); + } + else + error (_("Can't unwind value of register %d (%s)"), regnum, + user_reg_map_regnum_to_name (gdbarch, regnum)); + } if (frame_debug) { @@ -1353,10 +1445,14 @@ put_frame_register (struct frame_info *frame, int regnum, enum lval_type lval; CORE_ADDR addr; + gdb_assert (regnum >= 0); + gdb_assert (regnum < gdbarch_num_cooked_regs (gdbarch)); + frame_register (frame, regnum, &optim, &unavail, &lval, &addr, &realnum, NULL); if (optim) error (_("Attempt to assign to a register that was not saved.")); + switch (lval) { case lval_memory: @@ -1368,7 +1464,17 @@ put_frame_register (struct frame_info *frame, int regnum, get_current_regcache ()->cooked_write (realnum, buf); break; default: - error (_("Attempt to assign to an unmodifiable value.")); + { + if (regnum < gdbarch_num_regs (gdbarch)) + error (_("Attempt to assign to an unmodifiable value.")); + + frame_info *next_frame = get_next_frame_sentinel_okay (frame); + frame_register_readwriter frame_readwriter (next_frame); + + /* This is a pseudo-register, the arch will find out which raw registers + to modify and update them. */ + gdbarch_pseudo_register_write (gdbarch, &frame_readwriter, regnum, buf); + } } }