From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 8166938319D5 for ; Fri, 21 Jun 2024 18:07:40 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 8166938319D5 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 8166938319D5 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1718993262; cv=none; b=DWFerrng6881oDMqOBXpKQa026pXOsaco8j1FAFlUrFT7KRnX3xDgem4Hv/DRXi/Jtj3Ao0ngyq/sG63Z+bdNBEt5XvqM+7uU9BRSP4O++Wss7CC6a/4klpMVqorMwTilqAQbUS091Surd9VMPxhsKwgQcIZIlK+1cndG5aeoP0= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1718993262; c=relaxed/simple; bh=bmLqzYLQCXMgM4rlBR1Ui6Quca6b+2wT7+PpZe02xG8=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=R3gGldvmdqqxuPYXLhb3p9Sb1g9dN+F3sac334LaFFZg/uI8Bb+g69FqgVvEXW/qA4hOVCOE1fJkkFlc/7xRcZo50lTZw1x//WWBgvR8LK9cZ2kGwuA5QcZWhQP6HT4kovU9rwGvl8U2dE9uEP8BQ8/qXTfTYIhxFrJewZxyVtQ= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1718993260; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=/DVcXHQ2qyFm+bFhsYX0EJRcjkjt7vHHwnrDBYu+6cY=; b=Qb4Ph+rkxlE8FhGI8IDQbZEGf0jBVpaUlZV54PgnsB78q0sRiJS2fHi7HEwpzEZRE1X9+l 01Dk+1QUA6q8KK+o2PNRrm4Z/ftUbS8XUCVT5XBy363eJJaJr/2PvsONP4t4q2oKQhNsqP qWnsqdu+skDrp7q0bYEsw4cdVo4S4X0= Received: from mx-prod-mc-01.mail-002.prod.us-west-2.aws.redhat.com (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-679-ONHQPSPzMq-ZRtqymiUyvA-1; Fri, 21 Jun 2024 14:07:39 -0400 X-MC-Unique: ONHQPSPzMq-ZRtqymiUyvA-1 Received: from mx-prod-int-03.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-03.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.12]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-01.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 2F5F019560AA for ; Fri, 21 Jun 2024 18:07:37 +0000 (UTC) Received: from fedora.redhat.com (unknown [10.96.134.13]) by mx-prod-int-03.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id C4C6C19560AE; Fri, 21 Jun 2024 18:07:35 +0000 (UTC) From: Guinevere Larsen To: gdb-patches@sourceware.org Cc: Guinevere Larsen Subject: [PATCH] gdb: reintroduce reg_buffer::m_readonly in place of m_has_pseudo Date: Fri, 21 Jun 2024 15:07:30 -0300 Message-ID: <20240621180730.859487-1-blarsen@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.0 on 10.30.177.12 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="US-ASCII"; x-default=true X-Spam-Status: No, score=-10.3 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,RCVD_IN_SBL_CSS,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: A series of commits obfuscated the meaning of reg_buffer:m_has_pseudo, most notably for the current code being commit daf6667d1f94c7e74df4076daf021cd28a2, that changed the checks in regcache::save and regcache::restore from checking if the cache was readonly to checking if the cache had registers. There was no reason provided for that change, but following commits started to use a parameters called "readonly" to set m_has_pseudo, until a following commit only used the literal "true" and "false" values, but still mapping perfectly on whether the register cache will be read only, and not taking the gdbarch's pseudo registers or specific needs of a cache's location into account. The current usages in regcache::save and ::restore were just changed to use m_readonly, making the code more understandable. Two other uses, in readable_regcache::cooked_read and ::cooked_read_value, have just been removed, and the final 2 uses, in reg_buffer's constructor and in reg_buffer::assert_regnum, were changed to always act as if the buffer has pseudo registers. I chose to make reg_buffer always allocate space for pseudo-registers because the cooker_read methods act as if it was possible to cache a pseudo register's value, but as far as I could see, there wasn't any code doing it. Since it would be done in some tdep file and I didn't feel confident in saying no code at all does this caching, and thought it would be safer to allocate space for it. Tested on x86_64 Fedora 40 --- gdb/regcache-dump.c | 18 +++++++----------- gdb/regcache.c | 35 ++++++++++++----------------------- gdb/regcache.h | 19 +++++++++++-------- 3 files changed, 30 insertions(+), 42 deletions(-) diff --git a/gdb/regcache-dump.c b/gdb/regcache-dump.c index bc665dc08a6..6a6633e49dd 100644 --- a/gdb/regcache-dump.c +++ b/gdb/regcache-dump.c @@ -32,8 +32,7 @@ class register_dump_regcache : public register_dump { public: register_dump_regcache (regcache *regcache, bool dump_pseudo) - : register_dump (regcache->arch ()), m_regcache (regcache), - m_dump_pseudo (dump_pseudo) + : register_dump (regcache->arch (), dump_pseudo), m_regcache (regcache) { } @@ -80,9 +79,6 @@ class register_dump_regcache : public register_dump private: regcache *m_regcache; - - /* Dump pseudo registers or not. */ - const bool m_dump_pseudo; }; /* Dump from reg_buffer, used when there is no thread or @@ -92,7 +88,7 @@ class register_dump_reg_buffer : public register_dump, reg_buffer { public: register_dump_reg_buffer (gdbarch *gdbarch, bool dump_pseudo) - : register_dump (gdbarch), reg_buffer (gdbarch, dump_pseudo) + : register_dump (gdbarch, dump_pseudo), reg_buffer (gdbarch, true) { } @@ -101,14 +97,14 @@ class register_dump_reg_buffer : public register_dump, reg_buffer { if (regnum < 0) { - if (m_has_pseudo) + if (m_dump_pseudo) gdb_printf (file, "Cooked value"); else gdb_printf (file, "Raw value"); } else { - if (regnum < gdbarch_num_regs (m_gdbarch) || m_has_pseudo) + if (regnum < gdbarch_num_regs (m_gdbarch) || m_dump_pseudo) { auto size = register_size (m_gdbarch, regnum); @@ -140,7 +136,7 @@ class register_dump_none : public register_dump { public: register_dump_none (gdbarch *arch) - : register_dump (arch) + : register_dump (arch, false) {} protected: @@ -154,7 +150,7 @@ class register_dump_remote : public register_dump { public: register_dump_remote (gdbarch *arch) - : register_dump (arch) + : register_dump (arch, false) {} protected: @@ -181,7 +177,7 @@ class register_dump_groups : public register_dump { public: register_dump_groups (gdbarch *arch) - : register_dump (arch) + : register_dump (arch, true) {} protected: diff --git a/gdb/regcache.c b/gdb/regcache.c index f04354d822f..135238e5be3 100644 --- a/gdb/regcache.c +++ b/gdb/regcache.c @@ -185,8 +185,8 @@ regcache_register_size (const reg_buffer_common *regcache, int n) (gdb::checked_static_cast (regcache)->arch (), n); } -reg_buffer::reg_buffer (gdbarch *gdbarch, bool has_pseudo) - : m_has_pseudo (has_pseudo) +reg_buffer::reg_buffer (gdbarch *gdbarch, bool readonly) + : m_readonly (readonly) { gdb_assert (gdbarch != NULL); m_descr = regcache_descr (gdbarch); @@ -194,18 +194,9 @@ reg_buffer::reg_buffer (gdbarch *gdbarch, bool has_pseudo) /* We don't zero-initialize the M_REGISTERS array, as the bytes it contains aren't meaningful as long as the corresponding register status is not REG_VALID. */ - if (has_pseudo) - { - m_registers.reset (new gdb_byte[m_descr->sizeof_cooked_registers]); - m_register_status.reset - (new register_status[m_descr->nr_cooked_registers] ()); - } - else - { - m_registers.reset (new gdb_byte[m_descr->sizeof_raw_registers]); - m_register_status.reset - (new register_status[gdbarch_num_regs (gdbarch)] ()); - } + m_registers.reset (new gdb_byte[m_descr->sizeof_cooked_registers]); + m_register_status.reset + (new register_status[m_descr->nr_cooked_registers] ()); } regcache::regcache (inferior *inf_for_target_calls, gdbarch *gdbarch) @@ -264,8 +255,9 @@ reg_buffer::save (register_read_ftype cooked_read) { struct gdbarch *gdbarch = m_descr->gdbarch; - /* It should have pseudo registers. */ - gdb_assert (m_has_pseudo); + /* This method should only be called when we're creating a + readonly cache. */ + gdb_assert (m_readonly); /* Clear the dest. */ memset (m_registers.get (), 0, m_descr->sizeof_cooked_registers); memset (m_register_status.get (), REG_UNKNOWN, m_descr->nr_cooked_registers); @@ -297,7 +289,7 @@ regcache::restore (readonly_detached_regcache *src) int regnum; gdb_assert (src != NULL); - gdb_assert (src->m_has_pseudo); + gdb_assert (src->m_readonly); gdb_assert (gdbarch == src->arch ()); @@ -336,10 +328,7 @@ void reg_buffer::assert_regnum (int regnum) const { gdb_assert (regnum >= 0); - if (m_has_pseudo) - gdb_assert (regnum < m_descr->nr_cooked_registers); - else - gdb_assert (regnum < gdbarch_num_regs (arch ())); + gdb_assert (regnum < m_descr->nr_cooked_registers); } /* Type to map a ptid to a list of regcaches (one thread may have multiple @@ -723,7 +712,7 @@ readable_regcache::cooked_read (int regnum, gdb::array_view dst) gdb_assert (dst.size () == m_descr->sizeof_register[regnum]); - if (m_has_pseudo && m_register_status[regnum] != REG_UNKNOWN) + if (m_register_status[regnum] != REG_UNKNOWN) { if (m_register_status[regnum] == REG_VALID) copy (register_buffer (regnum), dst); @@ -774,7 +763,7 @@ readable_regcache::cooked_read_value (int regnum) gdb_assert (regnum < m_descr->nr_cooked_registers); if (regnum < num_raw_registers () - || (m_has_pseudo && m_register_status[regnum] != REG_UNKNOWN) + || (m_register_status[regnum] != REG_UNKNOWN) || !gdbarch_pseudo_register_read_value_p (m_descr->gdbarch)) { value *result = value::allocate_register diff --git a/gdb/regcache.h b/gdb/regcache.h index 2f4b7d94c69..3ecf07020be 100644 --- a/gdb/regcache.h +++ b/gdb/regcache.h @@ -188,7 +188,7 @@ struct cached_reg_t class reg_buffer : public reg_buffer_common { public: - reg_buffer (gdbarch *gdbarch, bool has_pseudo); + reg_buffer (gdbarch *gdbarch, bool readonly); DISABLE_COPY_AND_ASSIGN (reg_buffer); @@ -275,7 +275,7 @@ class reg_buffer : public reg_buffer_common struct regcache_descr *m_descr; - bool m_has_pseudo; + bool m_readonly; /* The register buffers. */ std::unique_ptr m_registers; /* Register cache status. */ @@ -290,8 +290,8 @@ class reg_buffer : public reg_buffer_common class readable_regcache : public reg_buffer { public: - readable_regcache (gdbarch *gdbarch, bool has_pseudo) - : reg_buffer (gdbarch, has_pseudo) + readable_regcache (gdbarch *gdbarch, bool readonly) + : reg_buffer (gdbarch, readonly) {} /* Transfer a raw register [0..NUM_REGS) from core-gdb to this regcache, @@ -349,8 +349,8 @@ class readable_regcache : public reg_buffer class detached_regcache : public readable_regcache { public: - detached_regcache (gdbarch *gdbarch, bool has_pseudo) - : readable_regcache (gdbarch, has_pseudo) + detached_regcache (gdbarch *gdbarch, bool readonly) + : readable_regcache (gdbarch, readonly) {} void raw_update (int regnum) override @@ -537,8 +537,8 @@ class register_dump virtual ~register_dump () = default; protected: - register_dump (gdbarch *arch) - : m_gdbarch (arch) + register_dump (gdbarch *arch, bool dump_pseudo) + : m_gdbarch (arch), m_dump_pseudo (dump_pseudo) {} /* Dump the register REGNUM contents. If REGNUM is -1, print the @@ -546,6 +546,9 @@ class register_dump virtual void dump_reg (ui_file *file, int regnum) = 0; gdbarch *m_gdbarch; + + /* Dump pseudo registers or not. */ + const bool m_dump_pseudo; }; #endif /* REGCACHE_H */ -- 2.45.2