From patchwork Fri Dec 1 16:27:15 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Marchi X-Patchwork-Id: 81137 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 D70E33860002 for ; Fri, 1 Dec 2023 16:28:32 +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 A596E3856DD4 for ; Fri, 1 Dec 2023 16:27:54 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org A596E3856DD4 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 A596E3856DD4 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=1701448077; cv=none; b=D7dOaZc+inAsbwBZ8yKQPuYQ/6VuhMJvQ21BwI9vM7MBlxN5AloVYjeyn59XKRi2gAumgGgeyRQKnJRVTVk0Jshhf6IWvRadq1UkdK9jnV+p4Vg8Ryg+Wl0AAPwGF9Ifw4Z9tYNOJlrRbizq9YoIExefwM0uCZuBUyPXRp951Z4= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1701448077; c=relaxed/simple; bh=4aq1iGf6v+dIt5l6l8HFkQIQ7m3ORp1ahIk7iJbHHmo=; h=From:To:Subject:Date:Message-ID:MIME-Version; b=VnX4bJDkYep594zP+bBmAd3YUJhjk3MbLFTLXZDc452EOEV3wgfOcsFGTGpz4z0RaXmUivY2W31TEw09t7Y2OnMu5Dju8Ii6CrD56WQVUAeqqoKPE25Ibhfg8C7JBMXEBEx2FslgDeZxtn09v71gUguRiM8ZY6fr/vCRXhIAun0= 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 BCF721E0C0; Fri, 1 Dec 2023 11:27:53 -0500 (EST) From: Simon Marchi To: gdb-patches@sourceware.org Cc: Luis Machado , John Baldwin , "Aktemur, Tankut Baris" , Simon Marchi , John Baldwin Subject: [PATCH 02/24] gdb: use reg_buffer_common throughout gdbsupport/common-regcache.h Date: Fri, 1 Dec 2023 11:27:15 -0500 Message-ID: <20231201162751.741751-3-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.7 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 New in v3: - remove unrelated whitespace change - formatting fixes - use regcache local var instead of self->regcache Right now, gdbsupport/common-regcache.h contains two abstractons for a regcache. An opaque type `regcache` (gdb and gdbserver both have their own regcache that is the concrete version of this) and an abstract base class `reg_buffer_common`, that is the base of regcaches on both sides. These abstractions allow code to be written for both gdb and gdbserver, for instance in the gdb/arch sub-directory. However, having two different abstractions is impractical. If some common code has a regcache, and wants to use an operation defined on reg_buffer_common, it can't. It would be better to have just one. Change all instances of `regcache *` in gdbsupport/common-regcache.h to be `reg_buffer_common *`, then fix fallouts. Implementations in gdb and gdbserver now need to down-cast (using gdb::checked_static_cast) from reg_buffer_common to their concrete regcache type. Some of them could be avoided by changing free functions (like regcache_register_size) to be virtual methods on reg_buffer_common. I tried it, it seems to work, but I did not include it in this series to avoid adding unnecessary changes. Change-Id: Ia5503adb6b5509a0f4604bd2a68b4642cc5283fd Reviewed-by: John Baldwin --- gdb/arch/arm-get-next-pcs.c | 8 ++++---- gdb/arch/arm-get-next-pcs.h | 5 +++-- gdb/arch/arm.c | 2 +- gdb/arch/arm.h | 4 ++-- gdb/arm-linux-tdep.c | 11 ++++++----- gdb/arm-tdep.c | 5 +++-- gdb/nat/aarch64-hw-point.c | 3 +-- gdb/nat/linux-btrace.c | 3 +-- gdb/regcache.c | 17 ++++++++++------- gdbserver/linux-arm-low.cc | 4 +++- gdbserver/regcache.cc | 17 +++++++++++------ gdbsupport/common-regcache.cc | 2 +- gdbsupport/common-regcache.h | 17 ++++++++++------- 13 files changed, 56 insertions(+), 42 deletions(-) diff --git a/gdb/arch/arm-get-next-pcs.c b/gdb/arch/arm-get-next-pcs.c index dcbb5a5e2e69..528753aff2f7 100644 --- a/gdb/arch/arm-get-next-pcs.c +++ b/gdb/arch/arm-get-next-pcs.c @@ -32,7 +32,7 @@ arm_get_next_pcs_ctor (struct arm_get_next_pcs *self, int byte_order, int byte_order_for_code, int has_thumb2_breakpoint, - struct regcache *regcache) + reg_buffer_common *regcache) { self->ops = ops; self->byte_order = byte_order; @@ -268,12 +268,12 @@ thumb_get_next_pcs_raw (struct arm_get_next_pcs *self) { int byte_order = self->byte_order; int byte_order_for_code = self->byte_order_for_code; + reg_buffer_common *regcache = self->regcache; CORE_ADDR pc = regcache_read_pc (self->regcache); unsigned long pc_val = ((unsigned long) pc) + 4; /* PC after prefetch */ unsigned short inst1; CORE_ADDR nextpc = pc + 2; /* Default is next instruction. */ ULONGEST status, itstate; - struct regcache *regcache = self->regcache; std::vector next_pcs; nextpc = MAKE_THUMB_ADDR (nextpc); @@ -653,8 +653,8 @@ arm_get_next_pcs_raw (struct arm_get_next_pcs *self) unsigned long this_instr = 0; unsigned long status; CORE_ADDR nextpc; - struct regcache *regcache = self->regcache; - CORE_ADDR pc = regcache_read_pc (self->regcache); + reg_buffer_common *regcache = self->regcache; + CORE_ADDR pc = regcache_read_pc (regcache); std::vector next_pcs; pc_val = (unsigned long) pc; diff --git a/gdb/arch/arm-get-next-pcs.h b/gdb/arch/arm-get-next-pcs.h index e6bb8d832286..ec347f01b4fd 100644 --- a/gdb/arch/arm-get-next-pcs.h +++ b/gdb/arch/arm-get-next-pcs.h @@ -24,6 +24,7 @@ /* Forward declaration. */ struct arm_get_next_pcs; +struct reg_buffer_common; /* get_next_pcs operations. */ struct arm_get_next_pcs_ops @@ -50,7 +51,7 @@ struct arm_get_next_pcs not. */ int has_thumb2_breakpoint; /* Registry cache. */ - struct regcache *regcache; + reg_buffer_common *regcache; }; /* Initialize arm_get_next_pcs. */ @@ -59,7 +60,7 @@ void arm_get_next_pcs_ctor (struct arm_get_next_pcs *self, int byte_order, int byte_order_for_code, int has_thumb2_breakpoint, - struct regcache *regcache); + reg_buffer_common *regcache); /* Find the next possible PCs after the current instruction executes. */ std::vector arm_get_next_pcs (struct arm_get_next_pcs *self); diff --git a/gdb/arch/arm.c b/gdb/arch/arm.c index 6fc4c0e11cd6..5c41b26483bb 100644 --- a/gdb/arch/arm.c +++ b/gdb/arch/arm.c @@ -322,7 +322,7 @@ thumb2_instruction_changes_pc (unsigned short inst1, unsigned short inst2) /* See arm.h. */ unsigned long -shifted_reg_val (struct regcache *regcache, unsigned long inst, +shifted_reg_val (reg_buffer_common *regcache, unsigned long inst, int carry, unsigned long pc_val, unsigned long status_reg) { unsigned long res, shift; diff --git a/gdb/arch/arm.h b/gdb/arch/arm.h index c64a15600de3..b6c316191877 100644 --- a/gdb/arch/arm.h +++ b/gdb/arch/arm.h @@ -188,7 +188,7 @@ enum system_register_address : CORE_ADDR ((CORE_ADDR) (((unsigned long) (addr)) + 8 + (sbits (instr, 0, 23) << 2))) /* Forward declaration. */ -struct regcache; +struct reg_buffer_common; /* Return the size in bytes of the complete Thumb instruction whose first halfword is INST1. */ @@ -213,7 +213,7 @@ int thumb_advance_itstate (unsigned int itstate); /* Decode shifted register value. */ -unsigned long shifted_reg_val (struct regcache *regcache, +unsigned long shifted_reg_val (reg_buffer_common *regcache, unsigned long inst, int carry, unsigned long pc_val, diff --git a/gdb/arm-linux-tdep.c b/gdb/arm-linux-tdep.c index 8117d35a4d37..05538251612b 100644 --- a/gdb/arm-linux-tdep.c +++ b/gdb/arm-linux-tdep.c @@ -903,8 +903,10 @@ static CORE_ADDR arm_linux_get_next_pcs_syscall_next_pc (struct arm_get_next_pcs *self) { CORE_ADDR next_pc = 0; - CORE_ADDR pc = regcache_read_pc (self->regcache); - int is_thumb = arm_is_thumb (self->regcache); + regcache *regcache + = gdb::checked_static_cast (self->regcache); + CORE_ADDR pc = regcache_read_pc (regcache); + int is_thumb = arm_is_thumb (regcache); ULONGEST svc_number = 0; if (is_thumb) @@ -914,7 +916,7 @@ arm_linux_get_next_pcs_syscall_next_pc (struct arm_get_next_pcs *self) } else { - struct gdbarch *gdbarch = self->regcache->arch (); + struct gdbarch *gdbarch = regcache->arch (); enum bfd_endian byte_order_for_code = gdbarch_byte_order_for_code (gdbarch); unsigned long this_instr = @@ -937,8 +939,7 @@ arm_linux_get_next_pcs_syscall_next_pc (struct arm_get_next_pcs *self) { /* SIGRETURN or RT_SIGRETURN may affect the arm thumb mode, so update IS_THUMB. */ - next_pc = arm_linux_sigreturn_next_pc (self->regcache, svc_number, - &is_thumb); + next_pc = arm_linux_sigreturn_next_pc (regcache, svc_number, &is_thumb); } /* Addresses for calling Thumb functions have the bit 0 set. */ diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c index 23fecf7cf666..83fce2abd508 100644 --- a/gdb/arm-tdep.c +++ b/gdb/arm-tdep.c @@ -7255,7 +7255,8 @@ CORE_ADDR arm_get_next_pcs_addr_bits_remove (struct arm_get_next_pcs *self, CORE_ADDR val) { - return gdbarch_addr_bits_remove (self->regcache->arch (), val); + return gdbarch_addr_bits_remove + (gdb::checked_static_cast (self->regcache)->arch (), val); } /* Wrapper over syscall_next_pc for use in get_next_pcs. */ @@ -7271,7 +7272,7 @@ arm_get_next_pcs_syscall_next_pc (struct arm_get_next_pcs *self) int arm_get_next_pcs_is_thumb (struct arm_get_next_pcs *self) { - return arm_is_thumb (self->regcache); + return arm_is_thumb (gdb::checked_static_cast (self->regcache)); } /* single_step() is called just before we want to resume the inferior, diff --git a/gdb/nat/aarch64-hw-point.c b/gdb/nat/aarch64-hw-point.c index 6747e61e0265..8e9a532cd2a4 100644 --- a/gdb/nat/aarch64-hw-point.c +++ b/gdb/nat/aarch64-hw-point.c @@ -137,8 +137,7 @@ aarch64_point_is_aligned (ptid_t ptid, int is_watchpoint, CORE_ADDR addr, alignment = AARCH64_HWP_ALIGNMENT; else { - struct regcache *regcache - = get_thread_regcache_for_ptid (ptid); + reg_buffer_common *regcache = get_thread_regcache_for_ptid (ptid); /* Set alignment to 2 only if the current process is 32-bit, since thumb instruction can be 2-byte aligned. Otherwise, set diff --git a/gdb/nat/linux-btrace.c b/gdb/nat/linux-btrace.c index 89bf28b5133f..c4f4cbd77115 100644 --- a/gdb/nat/linux-btrace.c +++ b/gdb/nat/linux-btrace.c @@ -284,13 +284,12 @@ perf_event_read_bts (btrace_target_info *tinfo, const uint8_t *begin, struct perf_event_sample sample; size_t read = 0; struct btrace_block block = { 0, 0 }; - struct regcache *regcache; gdb_assert (begin <= start); gdb_assert (start <= end); /* The first block ends at the current pc. */ - regcache = get_thread_regcache_for_ptid (tinfo->ptid); + reg_buffer_common *regcache = get_thread_regcache_for_ptid (tinfo->ptid); block.end = regcache_read_pc (regcache); /* The buffer may contain a partial record as its last entry (i.e. when the diff --git a/gdb/regcache.c b/gdb/regcache.c index e46a0b58f505..6c986b740dfd 100644 --- a/gdb/regcache.c +++ b/gdb/regcache.c @@ -180,9 +180,10 @@ register_size (struct gdbarch *gdbarch, int regnum) /* See gdbsupport/common-regcache.h. */ int -regcache_register_size (const struct regcache *regcache, int n) +regcache_register_size (const reg_buffer_common *regcache, int n) { - return register_size (regcache->arch (), n); + return register_size + (gdb::checked_static_cast (regcache)->arch (), n); } reg_buffer::reg_buffer (gdbarch *gdbarch, bool has_pseudo) @@ -417,7 +418,7 @@ get_thread_regcache (thread_info *thread) /* See gdbsupport/common-regcache.h. */ -struct regcache * +reg_buffer_common * get_thread_regcache_for_ptid (ptid_t ptid) { /* This function doesn't take a process_stratum_target parameter @@ -630,11 +631,12 @@ readable_regcache::raw_read (int regnum, T *val) } enum register_status -regcache_raw_read_unsigned (struct regcache *regcache, int regnum, +regcache_raw_read_unsigned (reg_buffer_common *regcache, int regnum, ULONGEST *val) { gdb_assert (regcache != NULL); - return regcache->raw_read (regnum, val); + return gdb::checked_static_cast (regcache)->raw_read + (regnum, val); } void @@ -1314,8 +1316,9 @@ reg_buffer::raw_compare (int regnum, const void *buf, int offset) const /* Special handling for register PC. */ CORE_ADDR -regcache_read_pc (struct regcache *regcache) +regcache_read_pc (reg_buffer_common *reg_buf) { + regcache *regcache = gdb::checked_static_cast (reg_buf); struct gdbarch *gdbarch = regcache->arch (); CORE_ADDR pc_val; @@ -1342,7 +1345,7 @@ regcache_read_pc (struct regcache *regcache) /* See gdbsupport/common-regcache.h. */ CORE_ADDR -regcache_read_pc_protected (regcache *regcache) +regcache_read_pc_protected (reg_buffer_common *regcache) { CORE_ADDR pc; try diff --git a/gdbserver/linux-arm-low.cc b/gdbserver/linux-arm-low.cc index 5975b44af0ae..0a6f3622695a 100644 --- a/gdbserver/linux-arm-low.cc +++ b/gdbserver/linux-arm-low.cc @@ -24,6 +24,7 @@ #include "linux-aarch32-low.h" #include "linux-aarch32-tdesc.h" #include "linux-arm-tdesc.h" +#include "gdbsupport/gdb-checked-static-cast.h" #include /* Don't include elf.h if linux/elf.h got included by gdb_proc_service.h. @@ -913,7 +914,8 @@ get_next_pcs_syscall_next_pc (struct arm_get_next_pcs *self) CORE_ADDR pc = regcache_read_pc (self->regcache); int is_thumb = arm_is_thumb_mode (); ULONGEST svc_number = 0; - struct regcache *regcache = self->regcache; + regcache *regcache + = gdb::checked_static_cast (self->regcache); if (is_thumb) { diff --git a/gdbserver/regcache.cc b/gdbserver/regcache.cc index 2e75a948a198..823ea1f015d3 100644 --- a/gdbserver/regcache.cc +++ b/gdbserver/regcache.cc @@ -21,6 +21,8 @@ #include "gdbthread.h" #include "tdesc.h" #include "gdbsupport/rsp-low.h" +#include "gdbsupport/gdb-checked-static-cast.h" + #ifndef IN_PROCESS_AGENT struct regcache * @@ -64,7 +66,7 @@ get_thread_regcache (struct thread_info *thread, int fetch) /* See gdbsupport/common-regcache.h. */ -struct regcache * +reg_buffer_common * get_thread_regcache_for_ptid (ptid_t ptid) { return get_thread_regcache (find_thread_ptid (ptid), 1); @@ -307,9 +309,10 @@ register_size (const struct target_desc *tdesc, int n) /* See gdbsupport/common-regcache.h. */ int -regcache_register_size (const struct regcache *regcache, int n) +regcache_register_size (const reg_buffer_common *regcache, int n) { - return register_size (regcache->tdesc, n); + return register_size + (gdb::checked_static_cast (regcache)->tdesc, n); } static unsigned char * @@ -437,10 +440,11 @@ regcache::raw_collect (int n, void *buf) const } enum register_status -regcache_raw_read_unsigned (struct regcache *regcache, int regnum, +regcache_raw_read_unsigned (reg_buffer_common *reg_buf, int regnum, ULONGEST *val) { int size; + regcache *regcache = gdb::checked_static_cast (reg_buf); gdb_assert (regcache != NULL); @@ -486,9 +490,10 @@ collect_register_by_name (struct regcache *regcache, /* Special handling for register PC. */ CORE_ADDR -regcache_read_pc (struct regcache *regcache) +regcache_read_pc (reg_buffer_common *regcache) { - return the_target->read_pc (regcache); + return the_target->read_pc + (gdb::checked_static_cast (regcache)); } void diff --git a/gdbsupport/common-regcache.cc b/gdbsupport/common-regcache.cc index 3515bedb3830..b6f02bac16e2 100644 --- a/gdbsupport/common-regcache.cc +++ b/gdbsupport/common-regcache.cc @@ -23,7 +23,7 @@ /* Return the register's value or throw if it's not available. */ ULONGEST -regcache_raw_get_unsigned (struct regcache *regcache, int regnum) +regcache_raw_get_unsigned (reg_buffer_common *regcache, int regnum) { ULONGEST value; enum register_status status; diff --git a/gdbsupport/common-regcache.h b/gdbsupport/common-regcache.h index e462f532407b..6d98ca8c92ed 100644 --- a/gdbsupport/common-regcache.h +++ b/gdbsupport/common-regcache.h @@ -20,6 +20,8 @@ #ifndef COMMON_COMMON_REGCACHE_H #define COMMON_COMMON_REGCACHE_H +struct reg_buffer_common; + /* This header is a stopgap until we have an independent regcache. */ enum register_status : signed char @@ -44,28 +46,29 @@ enum register_status : signed char thread specified by PTID. This function must be provided by the client. */ -extern struct regcache *get_thread_regcache_for_ptid (ptid_t ptid); +extern reg_buffer_common *get_thread_regcache_for_ptid (ptid_t ptid); /* Return the size of register numbered N in REGCACHE. This function must be provided by the client. */ -extern int regcache_register_size (const struct regcache *regcache, int n); +extern int regcache_register_size (const reg_buffer_common *regcache, int n); /* Read the PC register. This function must be provided by the client. */ -extern CORE_ADDR regcache_read_pc (struct regcache *regcache); +extern CORE_ADDR regcache_read_pc (reg_buffer_common *regcache); /* Read the PC register. If PC cannot be read, return 0. This is a wrapper around 'regcache_read_pc'. */ -extern CORE_ADDR regcache_read_pc_protected (regcache *regcache); +extern CORE_ADDR regcache_read_pc_protected (reg_buffer_common *regcache); /* Read a raw register into a unsigned integer. */ -extern enum register_status regcache_raw_read_unsigned - (struct regcache *regcache, int regnum, ULONGEST *val); +extern enum register_status +regcache_raw_read_unsigned (reg_buffer_common *regcache, int regnum, + ULONGEST *val); -ULONGEST regcache_raw_get_unsigned (struct regcache *regcache, int regnum); +ULONGEST regcache_raw_get_unsigned (reg_buffer_common *regcache, int regnum); struct reg_buffer_common {