From patchwork Fri Dec 1 16:27:21 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Marchi X-Patchwork-Id: 81143 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 6F73E3860002 for ; Fri, 1 Dec 2023 16:30:25 +0000 (GMT) X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id 2BD84385B53E for ; Fri, 1 Dec 2023 16:29:57 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 2BD84385B53E Authentication-Results: sourceware.org; dmarc=fail (p=none dis=none) header.from=efficios.com Authentication-Results: sourceware.org; spf=fail smtp.mailfrom=efficios.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 2BD84385B53E Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=158.69.221.121 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1701448200; cv=none; b=Ia/DW67jo92ifkcIFoMjpixAzvvWMLCuH1UQfgeQaV/KuAifZs/4EYzXpk7aAOj4JIGBC6JNZiz4hHIoiy59Z9q0yU5XHbnIeseolBFKrYACNIZyJ7S++JycLQwxtreIGuckVui7YgrG0eqrhij8DB1kcfx8cGB7BvJP89lBRQw= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1701448200; c=relaxed/simple; bh=iaaVJJ08UHd1zVElD2L75RPGtKQipy7DhubyKnidLuM=; h=From:To:Subject:Date:Message-ID:MIME-Version; b=YvIL4+pr3r4oejZjypyhfYP2TyjT6BGuYnaZvKeJp2ZPH8Yk+hynfc7l4xqoEVrcFbnMhdAak8yexqiVEqoSv9t8dG40MYFnuedAddWtWgW87QQioeO/dr6AQ8Ypaz6Hu+kXvYxplpRm5CsT0PaN5pO55Xeuk5aWw3rBdUbFFGU= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from smarchi-efficios.internal.efficios.com (192-222-143-198.qc.cable.ebox.net [192.222.143.198]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 04CF01E091; Fri, 1 Dec 2023 11:29:56 -0500 (EST) From: Simon Marchi To: gdb-patches@sourceware.org Cc: Luis Machado , John Baldwin , "Aktemur, Tankut Baris" , Simon Marchi , John Baldwin Subject: [PATCH 08/24] gdb: change value_of_register and value_of_register_lazy to take the next frame Date: Fri, 1 Dec 2023 11:27:21 -0500 Message-ID: <20231201162751.741751-9-simon.marchi@efficios.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20231201162751.741751-1-simon.marchi@efficios.com> References: <20231201162751.741751-1-simon.marchi@efficios.com> MIME-Version: 1.0 X-Spam-Status: No, score=-3496.6 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_NONE, KAM_DMARC_STATUS, SPF_HELO_PASS, SPF_SOFTFAIL, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org Some functions related to the handling of registers in frames accept "this frame", for which we want to read or write the register values, while other functions accept "the next frame", which is the frame next to that. The later is needed because we sometimes need to read register values for a frame that does not exist yet (usually when trying to unwind that frame-to-be). value_of_register and value_of_register_lazy both take "this frame", even if what they ultimately want internally is "the next frame". This is annoying if you are in a spot that currently has "the next frame" and need to call one of these functions (which happens later in this series). You need to get the previous frame only for those functions to get the next frame again. This is more manipulations, more chances of mistake. I propose to change these functions (and a few more functions in the subsequent patches) to operate on "the next frame". Things become a bit less awkward when all these functions agree on which frame they take. So, in this patch, change value_of_register_lazy and value_of_register to take "the next frame" instead of "this frame". This adds a lot of get_next_frame_sentinel_okay, but if we convert the user registers API to also use "the next frame" instead of "this frame", it will get simple again. Change-Id: Iaa24815e648fbe5ae3c214c738758890a91819cd Reviewed-By: John Baldwin --- gdb/aarch64-tdep.c | 3 +-- gdb/arm-tdep.c | 4 ++-- gdb/eval.c | 3 ++- gdb/findvar.c | 34 ++++++++++++---------------------- gdb/frame-unwind.c | 3 ++- gdb/guile/scm-frame.c | 3 ++- gdb/infcmd.c | 6 +++--- gdb/loongarch-tdep.c | 3 ++- gdb/mi/mi-main.c | 3 ++- gdb/mips-tdep.c | 2 +- gdb/nds32-tdep.c | 5 +++-- gdb/python/py-frame.c | 3 ++- gdb/python/py-unwind.c | 4 ++-- gdb/riscv-tdep.c | 4 ++-- gdb/s12z-tdep.c | 2 +- gdb/std-regs.c | 11 +++++++---- gdb/value.h | 9 +++++++-- 17 files changed, 53 insertions(+), 49 deletions(-) diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c index 4a7ce68e85a9..b0aee191d71f 100644 --- a/gdb/aarch64-tdep.c +++ b/gdb/aarch64-tdep.c @@ -3449,9 +3449,8 @@ value_of_aarch64_user_reg (frame_info_ptr frame, const void *baton) { const int *reg_p = (const int *) baton; - return value_of_register (*reg_p, frame); + return value_of_register (*reg_p, get_next_frame_sentinel_okay (frame)); } - /* Implement the "software_single_step" gdbarch method, needed to single step through atomic sequences on AArch64. */ diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c index 83fce2abd508..1ee8bdf46159 100644 --- a/gdb/arm-tdep.c +++ b/gdb/arm-tdep.c @@ -9954,9 +9954,9 @@ static struct value * value_of_arm_user_reg (frame_info_ptr frame, const void *baton) { const int *reg_p = (const int *) baton; - return value_of_register (*reg_p, frame); + return value_of_register (*reg_p, get_next_frame_sentinel_okay (frame)); } - + static enum gdb_osabi arm_elf_osabi_sniffer (bfd *abfd) { diff --git a/gdb/eval.c b/gdb/eval.c index 8192aeba3640..e075cc3138d6 100644 --- a/gdb/eval.c +++ b/gdb/eval.c @@ -1134,7 +1134,8 @@ eval_op_register (struct type *expect_type, struct expression *exp, && regno < gdbarch_num_cooked_regs (exp->gdbarch)) val = value::zero (register_type (exp->gdbarch, regno), not_lval); else - val = value_of_register (regno, get_selected_frame (NULL)); + val = value_of_register + (regno, get_next_frame_sentinel_okay (get_selected_frame ())); if (val == NULL) error (_("Value of register %s not available."), name); else diff --git a/gdb/findvar.c b/gdb/findvar.c index c7a681f8a890..e5513d00dea4 100644 --- a/gdb/findvar.c +++ b/gdb/findvar.c @@ -243,42 +243,32 @@ copy_integer_to_size (gdb_byte *dest, int dest_size, const gdb_byte *source, } } -/* Return a `value' with the contents of (virtual or cooked) register - REGNUM as found in the specified FRAME. The register's type is - determined by register_type (). */ +/* See value.h. */ -struct value * -value_of_register (int regnum, frame_info_ptr frame) +value * +value_of_register (int regnum, frame_info_ptr next_frame) { - struct gdbarch *gdbarch = get_frame_arch (frame); - struct value *reg_val; + gdbarch *gdbarch = frame_unwind_arch (next_frame); /* User registers lie completely outside of the range of normal registers. Catch them early so that the target never sees them. */ if (regnum >= gdbarch_num_cooked_regs (gdbarch)) - return value_of_user_reg (regnum, frame); + return value_of_user_reg (regnum, get_prev_frame_always (next_frame)); - reg_val = value_of_register_lazy (frame, regnum); + value *reg_val = value_of_register_lazy (next_frame, regnum); reg_val->fetch_lazy (); return reg_val; } -/* Return a `value' with the contents of (virtual or cooked) register - REGNUM as found in the specified FRAME. The register's type is - determined by register_type (). The value is not fetched. */ +/* See value.h. */ -struct value * -value_of_register_lazy (frame_info_ptr frame, int regnum) +value * +value_of_register_lazy (frame_info_ptr next_frame, int regnum) { - struct gdbarch *gdbarch = get_frame_arch (frame); - struct value *reg_val; - frame_info_ptr next_frame; + gdbarch *gdbarch = frame_unwind_arch (next_frame); gdb_assert (regnum < gdbarch_num_cooked_regs (gdbarch)); - - gdb_assert (frame != NULL); - - next_frame = get_next_frame_sentinel_okay (frame); + gdb_assert (next_frame != nullptr); /* In some cases NEXT_FRAME may not have a valid frame-id yet. This can happen if we end up trying to unwind a register as part of the frame @@ -291,7 +281,7 @@ value_of_register_lazy (frame_info_ptr frame, int regnum) /* We should have a valid next frame. */ gdb_assert (frame_id_p (get_frame_id (next_frame))); - reg_val = value::allocate_lazy (register_type (gdbarch, regnum)); + value *reg_val = value::allocate_lazy (register_type (gdbarch, regnum)); reg_val->set_lval (lval_register); VALUE_REGNUM (reg_val) = regnum; VALUE_NEXT_FRAME_ID (reg_val) = get_frame_id (next_frame); diff --git a/gdb/frame-unwind.c b/gdb/frame-unwind.c index 927b6256b0b3..89ae3d70b200 100644 --- a/gdb/frame-unwind.c +++ b/gdb/frame-unwind.c @@ -278,7 +278,8 @@ struct value * frame_unwind_got_register (frame_info_ptr frame, int regnum, int new_regnum) { - return value_of_register_lazy (frame, new_regnum); + return value_of_register_lazy (get_next_frame_sentinel_okay (frame), + new_regnum); } /* Return a value which indicates that FRAME saved REGNUM in memory at diff --git a/gdb/guile/scm-frame.c b/gdb/guile/scm-frame.c index 45863c587c1d..830acb8b53ed 100644 --- a/gdb/guile/scm-frame.c +++ b/gdb/guile/scm-frame.c @@ -835,7 +835,8 @@ gdbscm_frame_read_register (SCM self, SCM register_scm) register_str, strlen (register_str)); if (regnum >= 0) - value = value_of_register (regnum, frame); + value = value_of_register (regnum, + get_next_frame_sentinel_okay (frame)); } } catch (const gdb_exception &ex) diff --git a/gdb/infcmd.c b/gdb/infcmd.c index ec818bc6936f..20e5fde55ee1 100644 --- a/gdb/infcmd.c +++ b/gdb/infcmd.c @@ -2319,9 +2319,9 @@ default_print_registers_info (struct gdbarch *gdbarch, if (*(gdbarch_register_name (gdbarch, i)) == '\0') continue; - default_print_one_register_info (file, - gdbarch_register_name (gdbarch, i), - value_of_register (i, frame)); + default_print_one_register_info + (file, gdbarch_register_name (gdbarch, i), + value_of_register (i, get_next_frame_sentinel_okay (frame))); } } diff --git a/gdb/loongarch-tdep.c b/gdb/loongarch-tdep.c index f7d08c60a582..12865b99e13f 100644 --- a/gdb/loongarch-tdep.c +++ b/gdb/loongarch-tdep.c @@ -392,7 +392,8 @@ loongarch_software_single_step (struct regcache *regcache) static struct value * value_of_loongarch_user_reg (frame_info_ptr frame, const void *baton) { - return value_of_register ((long long) baton, frame); + return value_of_register ((long long) baton, + get_next_frame_sentinel_okay (frame)); } /* Implement the frame_align gdbarch method. */ diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c index 487b0a920254..21f2c72cfb90 100644 --- a/gdb/mi/mi-main.c +++ b/gdb/mi/mi-main.c @@ -1101,7 +1101,8 @@ output_register (frame_info_ptr frame, int regnum, int format, int skip_unavailable) { struct ui_out *uiout = current_uiout; - struct value *val = value_of_register (regnum, frame); + value *val + = value_of_register (regnum, get_next_frame_sentinel_okay (frame)); struct value_print_options opts; if (skip_unavailable && !val->entirely_available ()) diff --git a/gdb/mips-tdep.c b/gdb/mips-tdep.c index 16edfdb0f1b8..adaa091bda14 100644 --- a/gdb/mips-tdep.c +++ b/gdb/mips-tdep.c @@ -8074,7 +8074,7 @@ static struct value * value_of_mips_user_reg (frame_info_ptr frame, const void *baton) { const int *reg_p = (const int *) baton; - return value_of_register (*reg_p, frame); + return value_of_register (*reg_p, get_next_frame_sentinel_okay (frame)); } static struct gdbarch * diff --git a/gdb/nds32-tdep.c b/gdb/nds32-tdep.c index a81b5fd1fae1..7618a81fc256 100644 --- a/gdb/nds32-tdep.c +++ b/gdb/nds32-tdep.c @@ -267,9 +267,10 @@ static const struct static struct value * value_of_nds32_reg (frame_info_ptr frame, const void *baton) { - return value_of_register ((int) (intptr_t) baton, frame); + return value_of_register ((int) (intptr_t) baton, + get_next_frame_sentinel_okay (frame)); } - + /* Implement the "frame_align" gdbarch method. */ static CORE_ADDR diff --git a/gdb/python/py-frame.c b/gdb/python/py-frame.c index 0a7e10f09ff2..f38bf5d8237f 100644 --- a/gdb/python/py-frame.c +++ b/gdb/python/py-frame.c @@ -260,7 +260,8 @@ frapy_read_register (PyObject *self, PyObject *args, PyObject *kw) return nullptr; gdb_assert (regnum >= 0); - struct value *val = value_of_register (regnum, frame); + value *val + = value_of_register (regnum, get_next_frame_sentinel_okay (frame)); if (val == NULL) PyErr_SetString (PyExc_ValueError, _("Can't read register.")); diff --git a/gdb/python/py-unwind.c b/gdb/python/py-unwind.c index ee50c51b531d..f1162f22290f 100644 --- a/gdb/python/py-unwind.c +++ b/gdb/python/py-unwind.c @@ -467,8 +467,8 @@ pending_framepy_read_register (PyObject *self, PyObject *args, PyObject *kw) which maps to a real register. In the past, get_frame_register_value() was used here, which did not handle the user register case. */ - struct value *val = value_of_register (regnum, - pending_frame->frame_info); + value *val = value_of_register + (regnum, get_next_frame_sentinel_okay (pending_frame->frame_info)); if (val == NULL) PyErr_Format (PyExc_ValueError, "Cannot read register %d from frame.", diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c index 2b4b9e2a02ac..8110a8deedbf 100644 --- a/gdb/riscv-tdep.c +++ b/gdb/riscv-tdep.c @@ -168,7 +168,7 @@ static struct value * value_of_riscv_user_reg (frame_info_ptr frame, const void *baton) { const int *reg_p = (const int *) baton; - return value_of_register (*reg_p, frame); + return value_of_register (*reg_p, get_next_frame_sentinel_okay (frame)); } /* Information about a register alias that needs to be set up for this @@ -1149,7 +1149,7 @@ riscv_print_one_register_info (struct gdbarch *gdbarch, try { - val = value_of_register (regnum, frame); + val = value_of_register (regnum, get_next_frame_sentinel_okay (frame)); regtype = val->type (); } catch (const gdb_exception_error &ex) diff --git a/gdb/s12z-tdep.c b/gdb/s12z-tdep.c index 4781eab083eb..42fd1fb1d6ca 100644 --- a/gdb/s12z-tdep.c +++ b/gdb/s12z-tdep.c @@ -494,7 +494,7 @@ s12z_print_ccw_info (struct gdbarch *gdbarch, frame_info_ptr frame, int reg) { - struct value *v = value_of_register (reg, frame); + value *v = value_of_register (reg, get_next_frame_sentinel_okay (frame)); const char *name = gdbarch_register_name (gdbarch, reg); uint32_t ccw = value_as_long (v); gdb_puts (name, file); diff --git a/gdb/std-regs.c b/gdb/std-regs.c index 54cf9018e426..221b15f1de9f 100644 --- a/gdb/std-regs.c +++ b/gdb/std-regs.c @@ -39,7 +39,7 @@ value_of_builtin_frame_fp_reg (frame_info_ptr frame, const void *baton) register can do so by adding "fp" to register name table (mind you, doing this is probably a dangerous thing). */ return value_of_register (gdbarch_deprecated_fp_regnum (gdbarch), - frame); + get_next_frame_sentinel_okay (frame)); else { struct type *data_ptr_type = builtin_type (gdbarch)->builtin_data_ptr; @@ -58,7 +58,8 @@ value_of_builtin_frame_pc_reg (frame_info_ptr frame, const void *baton) struct gdbarch *gdbarch = get_frame_arch (frame); if (gdbarch_pc_regnum (gdbarch) >= 0) - return value_of_register (gdbarch_pc_regnum (gdbarch), frame); + return value_of_register (gdbarch_pc_regnum (gdbarch), + get_next_frame_sentinel_okay (frame)); else { struct type *func_ptr_type = builtin_type (gdbarch)->builtin_func_ptr; @@ -77,7 +78,8 @@ value_of_builtin_frame_sp_reg (frame_info_ptr frame, const void *baton) struct gdbarch *gdbarch = get_frame_arch (frame); if (gdbarch_sp_regnum (gdbarch) >= 0) - return value_of_register (gdbarch_sp_regnum (gdbarch), frame); + return value_of_register (gdbarch_sp_regnum (gdbarch), + get_next_frame_sentinel_okay (frame)); error (_("Standard register ``$sp'' is not available for this target")); } @@ -87,7 +89,8 @@ value_of_builtin_frame_ps_reg (frame_info_ptr frame, const void *baton) struct gdbarch *gdbarch = get_frame_arch (frame); if (gdbarch_ps_regnum (gdbarch) >= 0) - return value_of_register (gdbarch_ps_regnum (gdbarch), frame); + return value_of_register (gdbarch_ps_regnum (gdbarch), + get_next_frame_sentinel_okay (frame)); error (_("Standard register ``$ps'' is not available for this target")); } diff --git a/gdb/value.h b/gdb/value.h index 6d91e8eee7a9..3f9b35b589bd 100644 --- a/gdb/value.h +++ b/gdb/value.h @@ -1123,9 +1123,14 @@ extern struct value *value_of_variable (struct symbol *var, extern struct value *address_of_variable (struct symbol *var, const struct block *b); -extern struct value *value_of_register (int regnum, frame_info_ptr frame); +/* Return a value with the contents of register REGNUM as found in the frame + previous to NEXT_FRAME. */ -struct value *value_of_register_lazy (frame_info_ptr frame, int regnum); +extern value *value_of_register (int regnum, frame_info_ptr next_frame); + +/* Same as the above, but the value is not fetched. */ + +extern value *value_of_register_lazy (frame_info_ptr next_frame, int regnum); /* Return the symbol's reading requirement. */