From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id L6TmHtYT42ml7iMAWB0awg (envelope-from ) for ; Sat, 18 Apr 2026 01:17:10 -0400 Authentication-Results: simark.ca; dkim=pass (2048-bit key; unprotected) header.d=linaro.org header.i=@linaro.org header.a=rsa-sha256 header.s=google header.b=DPb6mfz7; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 58AF61E093; Sat, 18 Apr 2026 01:17:10 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-2.4 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,RCVD_IN_VALIDITY_CERTIFIED_BLOCKED, RCVD_IN_VALIDITY_RPBL_BLOCKED,RCVD_IN_VALIDITY_SAFE_BLOCKED autolearn=ham autolearn_force=no version=4.0.1 Received: from vm01.sourceware.org (vm01.sourceware.org [38.145.34.32]) (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 ESMTPS id 726F31E093 for ; Sat, 18 Apr 2026 01:17:08 -0400 (EDT) Received: from vm01.sourceware.org (localhost [127.0.0.1]) by sourceware.org (Postfix) with ESMTP id 623BA4AA394F for ; Sat, 18 Apr 2026 05:17:07 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 623BA4AA394F Authentication-Results: sourceware.org; dkim=pass (2048-bit key, unprotected) header.d=linaro.org header.i=@linaro.org header.a=rsa-sha256 header.s=google header.b=DPb6mfz7 Received: from mail-dy1-x132c.google.com (mail-dy1-x132c.google.com [IPv6:2607:f8b0:4864:20::132c]) by sourceware.org (Postfix) with ESMTPS id DDAB44C318A4 for ; Sat, 18 Apr 2026 05:16:41 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org DDAB44C318A4 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linaro.org ARC-Filter: OpenARC Filter v1.0.0 sourceware.org DDAB44C318A4 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::132c ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1776489402; cv=none; b=PIFE5m7PdKr8q98HVSqKYZDgSES/x5ueUepQpS/5DksZd2YWPaSixEqOY68B/7WKF8XFBqMBeaCkz74EurJww3Op04LuOORFO2vIRrcnfd63kzRJunxLpQHD1nZayUz0t/607I6quzggWRS8u5UjXu8EhXRkcykCtcIbzCGIkig= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1776489402; c=relaxed/simple; bh=91X8NxCGpEFGrq/FsgvOlVzGyAydIJ2m9Ru3dnGuCzs=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=smbpkdFrwnq31h0rK9UjNIK7NUZ3na+Zm2FVz1OHTAip03hRDyIyKe7mjJy0a8skucxLe8WTx+zi5bj5dvmg8lstqeIfE6h9PMa7eRppMDNzOaMgxDczEMYunH+nIGEkgXcm6ZliSFjrziaHvq8LjaoISBytddCxsEJL31c7iUU= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org DDAB44C318A4 Received: by mail-dy1-x132c.google.com with SMTP id 5a478bee46e88-2bd9a485bd6so2669843eec.1 for ; Fri, 17 Apr 2026 22:16:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1776489401; x=1777094201; darn=sourceware.org; h=mime-version:message-id:date:user-agent:references:in-reply-to :subject:cc:to:from:from:to:cc:subject:date:message-id:reply-to; bh=dhLAjl0Ayz1kaB15F4L48d7XhxPp+KlaEXbBmzVUGOA=; b=DPb6mfz7YpLg6TCJDD7lAHp4epNHPPVle/YZBjbXEg28+Ipo9J7jtapDhYOJUo3uQk F6ulKkNjPWp4si4MNrdXZifaGLIgkMECWIfJ3rZO3oyYvpP+d6RSu+Akc052Lqu3Q7Nu jIp1BSXb7GRUUzyoVW/sozKUT82iis2ZtptDbBeHIPHzDH8XpoLCt8ElyLt0p6k+9G+N F27ctkwGQ2fBvDGcanyHPGJr0yiPFFjX0v7IL9pOhdT7p34EJaeJEZ9Gop32+BrhJcye exl0ToX0MvxfuIyYQZ7EN4yQgp27AfyEijI6s2yVeTdZUEIhF4dqZadv/6DB2HmuiNWd 4e0Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1776489401; x=1777094201; h=mime-version:message-id:date:user-agent:references:in-reply-to :subject:cc:to:from:x-gm-gg:x-gm-message-state:from:to:cc:subject :date:message-id:reply-to; bh=dhLAjl0Ayz1kaB15F4L48d7XhxPp+KlaEXbBmzVUGOA=; b=fSAulxNIppFT1wN9u5FQSHrIyZlO+vLa98QaVEVKrKsfEufPyGbD8qGotFvqKfKo42 K+kg2tW4Ihd/Wj/2RYNL/PLRkGSEfB6OLuehuGkTn46arWtnqzsbB1yOUq5zbhnCOnq7 vCfPxHh4rA1w6fnS8k5haidUeIvKuge+RgXDWw6C2tb363KCmX7TKrDJHnvtfRplpYFK UxxaKau6xzsFCDyE5td/Es3D/SCjBxvVc5CfitnbnTTevstbgr7g8Z8KBy7ur/xqHwVR v5hj4wcZjQSI1zdZ5heJdohFdKL7RAgPUZmJ+hkdkdzAyp0zyPQ9Vq3fT77UUoVmzgSo I0lw== X-Gm-Message-State: AOJu0YzGEGBW51w8qsyHJXvKgsdPiIut0rRN11/fJTsZ4fqN2vLy9Ns2 7URHzY0d/OY1M9P8BtfKvJKySozB6mjF3BTfUacVqf/ME1rPMhqN+5yBDc3IZCLwD+iARIB/x7+ 3EGOY X-Gm-Gg: AeBDietofXDU4pbpd6fozmSagUR2d0EcK3Lzy2C5fc+C89uyLkjFH0TP+NVvHvaiT6m FZ5nvuBD9g5TOd9eYA53b/4Qmy2Da6M/SSzroEiBHuXwBic2KECG63KxZ6ufCgkxOgqE8LFgMKo d39tWID2MQPruIxBy2zXPYfjOKQnCgmUeZE4Ukym+9y39ulJQWxJN5PrPM/VFsXEvti+7hDnZq+ ZbTVDyxPt6+fwVwbkrv1kppvO29D5MPDElQgSpymNVFi5jX5kZSsa4oKp9uCmyGeHyWiJ+I8KVJ FqYHGYCcOykRqzLuxK2ky4vCJAhgm2EU4sB+10ltKwISRqAXNAW9TDvuMKp1EIJoEeTbWLEVn8W 3EYINIn7UPZzx/lGo0LCGvlB1Oy3EX8y/6UasfTM4CvNY2ulFIGbmJCuGliIcBabmo3wqYYepPy 0WvudNPryy0r0P8SeQmRp2oU6MF4T8tO1C99rZCcYqRBGRFYTHqDNgOSw= X-Received: by 2002:a05:7300:ec17:b0:2d8:df01:d9f7 with SMTP id 5a478bee46e88-2e478a2f193mr2986758eec.15.1776489400623; Fri, 17 Apr 2026 22:16:40 -0700 (PDT) Received: from localhost ([2804:14d:7e39:8083:f04c:42e3:5943:38f6]) by smtp.gmail.com with ESMTPSA id 5a478bee46e88-2e539fa3cecsm5198494eec.6.2026.04.17.22.16.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 17 Apr 2026 22:16:40 -0700 (PDT) From: Thiago Jung Bauermann To: Guinevere Larsen Cc: gdb-patches@sourceware.org Subject: Re: [PATCH 5/6] gdb/record: extract the PC to record_full_instruction In-Reply-To: <20260415185836.2732968-6-guinevere@redhat.com> (Guinevere Larsen's message of "Wed, 15 Apr 2026 15:58:35 -0300") References: <20260415185836.2732968-1-guinevere@redhat.com> <20260415185836.2732968-6-guinevere@redhat.com> User-Agent: mu4e 1.14.0; emacs 30.2 Date: Sat, 18 Apr 2026 02:16:37 -0300 Message-ID: <877bq4alvu.fsf@linaro.org> MIME-Version: 1.0 Content-Type: text/plain 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~public-inbox=simark.ca@sourceware.org Guinevere Larsen writes: > diff --git a/gdb/record-full.c b/gdb/record-full.c > index c9e66fac578..297b5b76ae7 100644 > --- a/gdb/record-full.c > +++ b/gdb/record-full.c > @@ -250,6 +250,16 @@ class record_full_entry > gdb_assert (mem_type == record_full_mem); > } > > + void set_entry (record_full_reg_entry reg) > + { > + entry = reg; > + } > + > + void set_entry (record_full_mem_entry mem) > + { > + entry = mem; > + } > + > record_full_reg_entry& reg () > { > gdb_assert (type () == record_full_reg); > @@ -325,6 +335,7 @@ struct record_full_instruction > uint32_t insn_num; > std::optional sigval; > std::vector effects; > + record_full_reg_entry pc; Maybe mention pc in the doc comment above struct record_full_instruction? > > /* Execute the full instruction. As a side effect, set > record_full_stop_reason. */ > @@ -684,7 +695,10 @@ record_full_arch_list_add_reg (struct regcache *regcache, int regnum) > > regcache->cooked_read (regnum, rec.get_loc ()); > > - record_full_arch_list_add (rec); > + if (regnum == gdbarch_pc_regnum (regcache->arch ())) > + record_full_incomplete_instruction.pc = rec.reg (); > + else > + record_full_arch_list_add (rec); > > return 0; > } > @@ -841,6 +855,7 @@ static enum target_stop_reason record_full_stop_reason > > void record_full_instruction::exec_insn (regcache *regcache, gdbarch *gdbarch) > { > + pc.execute (regcache, gdbarch); > for (auto &entry : effects) > if (entry.execute (regcache, gdbarch)) > record_full_stop_reason = TARGET_STOPPED_BY_WATCHPOINT; > @@ -2184,13 +2199,73 @@ netorder32 (uint32_t input) > return ret; > } > > +static record_full_reg_entry > +record_full_read_reg_from_bfd (bfd *cbfd, asection *osec, int *bfd_offset) > +{ > + uint32_t regnum; > + regcache *cache = get_thread_regcache (inferior_thread ()); > + > + /* Get register number to regnum. */ > + bfdcore_read (cbfd, osec, ®num, sizeof (regnum), > + bfd_offset); > + regnum = netorder32 (regnum); > + > + record_full_reg_entry reg (cache->arch (), regnum); > + > + /* Get val. */ > + bfdcore_read (cbfd, osec, reg.get_loc (), > + reg.len, bfd_offset); > + > + if (record_debug) > + gdb_printf (gdb_stdlog, > + " Reading register %d (1 " > + "plus %lu plus %d bytes)\n", > + reg.num, > + (unsigned long) sizeof (regnum), > + reg.len); > + return reg; > + > +} > + > +static record_full_mem_entry > +record_full_read_mem_from_bfd (bfd *cbfd, asection *osec, int *bfd_offset) > +{ > + uint32_t len; > + uint64_t addr; > + > + /* Get len. */ > + bfdcore_read (cbfd, osec, &len, sizeof (len), bfd_offset); > + len = netorder32 (len); > + > + /* Get addr. */ > + bfdcore_read (cbfd, osec, &addr, sizeof (addr), > + bfd_offset); > + addr = netorder64 (addr); > + > + record_full_mem_entry mem (addr, len); > + > + /* Get val. */ > + bfdcore_read (cbfd, osec, mem.get_loc (), > + len, bfd_offset); > + > + if (record_debug) > + gdb_printf (gdb_stdlog, > + " Reading memory %s (1 plus " > + "%lu plus %lu plus %d bytes)\n", > + paddress (get_current_arch (), > + mem.addr), > + (unsigned long) sizeof (addr), > + (unsigned long) sizeof (len), > + len); > + > + return mem; > +} > + As a suggestion to further C++fy this code (feel free to adopt them or not), you could make the functions above constructors of record_full_reg_entry and record_full_mem_entry or static methods record_full_reg_entry::from_bfd and record_full_mem_entry::from_bfd Admittedly it wouldn't be a big difference, just slightly better code organization. > static void > record_full_read_entry_from_bfd (bfd *cbfd, asection *osec, int *bfd_offset) > { > uint8_t rectype; > - uint32_t regnum, len; > - uint64_t addr; > - regcache *cache = get_thread_regcache (inferior_thread ()); > + record_full_entry rec; > > bfdcore_read (cbfd, osec, &rectype, sizeof (rectype), > bfd_offset); Same here, record_full_entry::from_bfd. Except for the call to record_full_arch_list_add, which would then be moved to the caller. > @@ -2387,6 +2422,67 @@ cmd_record_full_restore (const char *args, int from_tty) > record_full_open (nullptr, from_tty); > } > > +static void > +record_full_write_reg_to_bfd (record_full_reg_entry ®, > + gdb_bfd_ref_ptr obfd, > + asection *osec, int *bfd_offset, > + gdbarch *gdbarch) > +{ > + uint32_t regnum; > + > + if (record_debug) > + gdb_printf (gdb_stdlog, > + " Writing register %d (1 " > + "plus %lu plus %d bytes)\n", > + reg.num, > + (unsigned long) sizeof (regnum), > + reg.len); > + > + /* Write regnum. */ > + regnum = netorder32 (reg.num); > + bfdcore_write (obfd.get (), osec, ®num, > + sizeof (regnum), bfd_offset); > + > + /* Write regval. */ > + bfdcore_write (obfd.get (), osec, > + reg.get_loc (), > + reg.len, bfd_offset); > +} > + > +static void > +record_full_write_mem_to_bfd (record_full_mem_entry &mem, > + gdb_bfd_ref_ptr obfd, > + asection *osec, int *bfd_offset, > + gdbarch *gdbarch) > +{ > + uint32_t len; > + uint64_t addr; > + > + if (record_debug) > + gdb_printf (gdb_stdlog, > + " Writing memory %s (1 plus " > + "%lu plus %lu plus %d bytes)\n", > + paddress (gdbarch, mem.addr), > + (unsigned long) sizeof (addr), > + (unsigned long) sizeof (len), > + mem.len); > + > + /* Write memlen. */ > + len = netorder32 (mem.len); > + bfdcore_write (obfd.get (), osec, &len, sizeof (len), > + bfd_offset); > + > + /* Write memaddr. */ > + addr = netorder64 (mem.addr); > + bfdcore_write (obfd.get (), osec, &addr, > + sizeof (addr), bfd_offset); > + > + /* Write memval. */ > + bfdcore_write (obfd.get (), osec, > + mem.get_loc (), > + mem.len, bfd_offset); > +} > + These could be write_to_bfd methods in record_full_reg_entry and record_full_mem_entry. > static void > record_full_write_entry_to_bfd (record_full_entry &entry, > gdb_bfd_ref_ptr obfd, And this could be a non-virtual method in record_full_entry ... > @@ -2395,8 +2491,6 @@ record_full_write_entry_to_bfd (record_full_entry &entry, > { > /* Save entry. */ > uint8_t type; > - uint32_t regnum, len; > - uint64_t addr; > > type = entry.type (); > bfdcore_write (obfd.get (), osec, &type, sizeof (type), bfd_offset ... which would do this part and then call the write_to_bfd virtual method instead of the switch below. > @@ -2404,56 +2498,14 @@ record_full_write_entry_to_bfd (record_full_entry &entry, > switch (type) > { > case record_full_reg: /* reg */ > - { > - auto reg = entry.reg (); > - if (record_debug) > - gdb_printf (gdb_stdlog, > - " Writing register %d (1 " > - "plus %lu plus %d bytes)\n", > - reg.num, > - (unsigned long) sizeof (regnum), > - reg.len); > - > - /* Write regnum. */ > - regnum = netorder32 (reg.num); > - bfdcore_write (obfd.get (), osec, ®num, > - sizeof (regnum), bfd_offset); > - > - /* Write regval. */ > - bfdcore_write (obfd.get (), osec, > - entry.get_loc (), > - reg.len, bfd_offset); > - break; > - } > + record_full_write_reg_to_bfd (entry.reg (), obfd, osec, > + bfd_offset, gdbarch); > + break; > > case record_full_mem: /* mem */ > - { > - auto mem = entry.mem (); > - if (record_debug) > - gdb_printf (gdb_stdlog, > - " Writing memory %s (1 plus " > - "%lu plus %lu plus %d bytes)\n", > - paddress (gdbarch, mem.addr), > - (unsigned long) sizeof (addr), > - (unsigned long) sizeof (len), > - mem.len); > - > - /* Write memlen. */ > - len = netorder32 (mem.len); > - bfdcore_write (obfd.get (), osec, &len, sizeof (len), > - bfd_offset); > - > - /* Write memaddr. */ > - addr = netorder64 (mem.addr); > - bfdcore_write (obfd.get (), osec, &addr, > - sizeof (addr), bfd_offset); > - > - /* Write memval. */ > - bfdcore_write (obfd.get (), osec, > - entry.get_loc (), > - mem.len, bfd_offset); > - break; > - } > + record_full_write_mem_to_bfd (entry.mem (), obfd, osec, > + bfd_offset, gdbarch); > + break; > } > > } -- Thiago