From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id UkggKx62KWqINAAAWB0awg (envelope-from ) for ; Wed, 10 Jun 2026 15:08:14 -0400 Authentication-Results: simark.ca; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=iSI0Eptt; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 9F6251E098; Wed, 10 Jun 2026 15:08:14 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-6.4 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED 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 F3A811E070 for ; Wed, 10 Jun 2026 15:08:12 -0400 (EDT) Received: from vm01.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 9CE0242FD4CE for ; Wed, 10 Jun 2026 19:08:11 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 9CE0242FD4CE Authentication-Results: sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=iSI0Eptt Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTP id 50B364BA2E07 for ; Wed, 10 Jun 2026 19:07:28 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 50B364BA2E07 Authentication-Results: sourceware.org; dmarc=pass (p=quarantine 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 50B364BA2E07 Authentication-Results: sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1781118448; cv=none; b=usFvCuDmJr0856qIF+Z0PoG46KvdDh/6u5qr7OZxiJjKbCe+DZije12NvZxYpHrl+hLUDrA35JSEv0vVO6Huy9Pbtqe5dmcSEhv9icEyJYI5+V6RufgRpE3whCdd0J5pUeD92dvTqNBE8bHcB3cAvNTTlAXsL2CC/4H/SimA4V8= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1781118448; c=relaxed/simple; bh=JysbfuMK09KyvhpiN3PUxngHe0cpyWodX7p2uS7fylE=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=VXPfkQJX6WOzd7YMsDir54+nDBRdSQg3dfn2kbJH+2BU/V5c6192lOy1fP1zp5AQgCemBa6O6iT+sANwPhSUDynxWvQjFfiQ/S3Cu32vJgEdgA/K67GWp8orfhzDO60b2oEomIsnyIB0BJeLmhLX2QES1jMvCZVI6N8Jdmi54uI= ARC-Authentication-Results: i=1; sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=iSI0Eptt DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 50B364BA2E07 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1781118447; 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: in-reply-to:in-reply-to:references:references; bh=fr01juaF/6tliM/6+eaaNI3/qw6hahqoBlO98viqr4Y=; b=iSI0EpttbtRmsvM6AaM4ekXPVosHeBlObGcO4rfAqkKJAqrgBAoFZ1UFU4t8cBQUxYZxpu 1GHs9ptWOI2ME1wcuyGz8xhqH4k8No2I5kRBnLszzoH1R3ZyXQIUCSXOofzz+pFoabCdwk ikD4k/o33BTSjNpsQZJUp4IZTPrbzqI= Received: from mail-dy1-f198.google.com (mail-dy1-f198.google.com [74.125.82.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-644-hxJXeRfXM4aFILFD2KUx0w-1; Wed, 10 Jun 2026 15:07:26 -0400 X-MC-Unique: hxJXeRfXM4aFILFD2KUx0w-1 X-Mimecast-MFC-AGG-ID: hxJXeRfXM4aFILFD2KUx0w_1781118445 Received: by mail-dy1-f198.google.com with SMTP id 5a478bee46e88-304ea42b025so131215eec.0 for ; Wed, 10 Jun 2026 12:07:26 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1781118445; x=1781723245; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=fr01juaF/6tliM/6+eaaNI3/qw6hahqoBlO98viqr4Y=; b=RVTGGED39IXDg98f0NmVyjaUifcXCIgJMLdBJ2U8tG9lNPVJErT8e0YNMxvh3E6J5/ 0D0dsXfn3XYCoJqsncAzjmeHqX3HnrmgxtHI7YpHzhqOKSkpbZpQaHN7jAN07Z9/SQg9 cFgriklERn7LKySKsGxBk3DcvUjnQ0dYdhCLQaRqLnz1cTL2aYLKB8Uj+TR5LHZsd85u 7tGLknUqvhR6ddpEJo8c9RWqxBz3RirSnN/whRLBCESxu869uqrS8xEYK+2Z/mr7tkT7 //zhg7VlsDhkNRVFM33TfxqX1lTvyCDbxSUFkjoGVI4fse8AKhj0+rKNJO938YwlLFYS 8Qxg== X-Forwarded-Encrypted: i=1; AFNElJ9svYQjIlzELu1Qg3QQaDumZhYCbVQWUk4xg4i6pUFcIeE/zhajifUFw0hFTF21A7PVAPq7ZDxv6ZT5Mg==@sourceware.org X-Gm-Message-State: AOJu0YxZI2CPQh5lrwvMZQ6uN0GIXFcmg5LoTfRtVrumKJZbT6c/Xb/K tmfy76iwfX2V5bMdP2F/D7aGBCPX7C4t7TZ9zS0kYXsit3NoJUEogHtCciGUr/yrtFAgqz4vIX4 Rygwyue7l/DIEoizXEK6C9vRBeb4pN6jJV85XTmjLwBcnxkCacEYSnHfbDF3z/Q1ZtuvtNq4= X-Gm-Gg: Acq92OEs43fEyt/3+0/pmspmM85jibKd9NoekU5F+QKwcCiT57KU1xah4YuSYE7lvhs iADzYIByWBOcFfsIKKM+Uu29C1/o+PVffina2i62j14pmkINHEIkWOM1T6onQ6947GAmUov8ciV 8lq+hC/QUxg/R2VRLx1mGKXXIZU6RK8Qu0w7Q8pdt1WVK3y6X3WzAyEKqQs37Q671a+iiMhaF0l bNW6YHmQpEXsEGO2potjEJWLguot00cgulsFR6h1xYebcEEewTCwDC+xzcaEtVc0cZKxbJ5M0M2 SCOzl7WEsUQxin45TSGvV2sNNPOiN55rQNTdHCdSu4TH1QP0tIs4niD6YlQptC50bbGjS50sco0 iVgAKw7zuewXJ9/5ruFcIHO/N5ATE/+sY7ih7v8oj4hNFRlFknZqGuGr8x5jWaBkUW9IhINSU1N 9fEd0= X-Received: by 2002:a05:7301:52c3:b0:2f0:ee2:f6c6 with SMTP id 5a478bee46e88-3078011f9d4mr9504818eec.19.1781118444716; Wed, 10 Jun 2026 12:07:24 -0700 (PDT) X-Received: by 2002:a05:7301:52c3:b0:2f0:ee2:f6c6 with SMTP id 5a478bee46e88-3078011f9d4mr9504793eec.19.1781118444006; Wed, 10 Jun 2026 12:07:24 -0700 (PDT) Received: from ?IPV6:2804:14d:8084:993e:2d5d:7adb:2b6:a50e? ([2804:14d:8084:993e:2d5d:7adb:2b6:a50e]) by smtp.gmail.com with ESMTPSA id 5a478bee46e88-3074df102a1sm25481066eec.20.2026.06.10.12.07.21 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 10 Jun 2026 12:07:23 -0700 (PDT) Message-ID: <8ec6ee8f-b4e2-4cb1-a238-5f200dd512a4@redhat.com> Date: Wed, 10 Jun 2026 16:07:18 -0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v4 4/8] gdb/record: c++ify internal structures of record-full.c To: "Schimpe, Christina" , "gdb-patches@sourceware.org" Cc: Thiago Jung Bauermann References: <20260602143342.12245-1-guinevere@redhat.com> <20260602143342.12245-5-guinevere@redhat.com> From: Guinevere Larsen In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: DhgTog-k6K-tigzM5dyEcSiC7yUTLR0TKkm3mC9gaoE_1781118445 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit 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 On 6/10/26 11:46 AM, Schimpe, Christina wrote: >> -----Original Message----- >> From: Guinevere Larsen >> Sent: Dienstag, 2. Juni 2026 16:34 >> To: gdb-patches@sourceware.org >> Cc: Guinevere Larsen ; Thiago Jung Bauermann >> >> Subject: [PATCH v4 4/8] gdb/record: c++ify internal structures of record-full.c >> >> This commit adds a constructor, destructor, and some methods to the >> structures record_full_entry, record_full_reg_entry and >> record_full_mem_entry. This is a move to disentangle the internal >> representation of the data and how record-full manipulates it for replaying. >> >> Along with this change, record_full_entry is changed to use an std::variant, >> since it was basically doing that already, but now we have the stdlibc++ error >> checking to make sure we're only accessing elements we're allowed to. >> >> Reviewed-by: Thiago Jung Bauermann >> --- >> gdb/record-full.c | 613 ++++++++++++++++++++++++---------------------- >> 1 file changed, 326 insertions(+), 287 deletions(-) >> >> diff --git a/gdb/record-full.c b/gdb/record-full.c index >> a3a31e7276f..73de84372b0 100644 >> --- a/gdb/record-full.c >> +++ b/gdb/record-full.c >> @@ -51,6 +51,7 @@ >> #include >> #include >> #include >> +#include >> >> /* This module implements "target record-full", also known as "process >> record and replay". This target sits on top of a "normal" target @@ -92,13 >> +93,116 @@ struct record_full_mem_entry >> CORE_ADDR addr; >> int len; >> /* Set this flag if target memory for this entry >> - can no longer be accessed. */ >> - int mem_entry_not_accessible; >> + can be accessed. */ >> + bool mem_entry_accessible; >> union >> { >> gdb_byte *ptr; >> gdb_byte buf[sizeof (gdb_byte *)]; >> } u; >> + >> + record_full_mem_entry () : addr (0), len (0), mem_entry_accessible >> + (true) { } >> + >> + record_full_mem_entry (CORE_ADDR mem_addr, int mem_len) { >> + addr = mem_addr; >> + len = mem_len; >> + if (len > sizeof (u.buf)) >> + u.ptr = new gdb_byte[len]; >> + mem_entry_accessible = true; >> + } >> + >> + record_full_mem_entry (record_full_mem_entry &&other) { >> + addr = other.addr; >> + len = other.len; >> + memcpy (u.buf, other.u.buf, sizeof (u.buf)); >> + mem_entry_accessible = other.mem_entry_accessible; >> + /* With len == 0, OTHER is guaranteed to not try to free the >> + memory when it is destructed. */ >> + other.len = 0; >> + } >> + >> + ~record_full_mem_entry () >> + { >> + if (len > sizeof (u.buf)) >> + delete[] u.ptr; >> + } >> + >> + record_full_mem_entry &operator= (record_full_mem_entry &&other) { >> + gdb_assert (this != &other); >> + addr = other.addr; >> + len = other.len; >> + memcpy (u.buf, other.u.buf, sizeof (u.buf)); >> + mem_entry_accessible = other.mem_entry_accessible; >> + /* With len == 0, OTHER is guaranteed to not try to free the >> + memory when it is destructed. */ >> + other.len = 0; >> + return *this; >> + } >> + >> + DISABLE_COPY_AND_ASSIGN (record_full_mem_entry); >> + >> + gdb_byte *get_loc () >> + { >> + if (len > sizeof (u.buf)) >> + return u.ptr; >> + else >> + return u.buf; >> + } >> + >> + bool execute (gdbarch *gdbarch) >> + { >> + /* Nothing to do if the memory is flagged not_accessible. */ >> + if (!mem_entry_accessible) >> + return false; >> + >> + gdb::byte_vector buf (len); >> + >> + if (record_debug > 1) >> + gdb_printf (gdb_stdlog, >> + "Process record: record_full_mem %s to " >> + "inferior addr = %s len = %d.\n", >> + host_address_to_string (this), >> + paddress (gdbarch, addr), >> + len); >> + >> + if (record_read_memory (gdbarch, addr, buf.data (), len)) >> + mem_entry_accessible = false; >> + else >> + { >> + if (target_write_memory (addr, >> + get_loc (), >> + len)) >> + { >> + mem_entry_accessible = false; >> + if (record_debug) >> + warning (_("Process record: error writing memory at " >> + "addr = %s len = %d."), >> + paddress (gdbarch, addr), >> + len); >> + } >> + else >> + { >> + memcpy (get_loc (), buf.data (), len); >> + >> + /* We've changed memory --- check if a hardware >> + watchpoint should trap. Note that this >> + presently assumes the target beneath supports >> + continuable watchpoints. On non-continuable >> + watchpoints target, we'll want to check this >> + _before_ actually doing the memory change, and >> + not doing the change at all if the watchpoint >> + traps. */ >> + if (hardware_watchpoint_inserted_in_range >> + (current_inferior ()->aspace.get (), addr, len)) >> + return true; >> + } >> + } >> + return false; >> + } >> }; >> >> struct record_full_reg_entry >> @@ -110,6 +214,71 @@ struct record_full_reg_entry >> gdb_byte *ptr; >> gdb_byte buf[2 * sizeof (gdb_byte *)]; >> } u; >> + >> + record_full_reg_entry () : num (0), len (0) { } >> + >> + record_full_reg_entry (gdbarch *gdbarch, int regnum) { >> + num = regnum; >> + len = register_size (gdbarch, regnum); >> + if (len > sizeof (u.buf)) >> + u.ptr = new gdb_byte[len]; >> + } >> + >> + record_full_reg_entry (record_full_reg_entry &&other) { >> + num = other.num; >> + len = other.len; >> + memcpy (u.buf, other.u.buf, sizeof (u.buf)); >> + /* With len == 0, OTHER is guaranteed to not try to free the >> + memory when it is destructed. */ >> + other.len = 0; >> + } >> + >> + ~record_full_reg_entry () >> + { >> + if (len > sizeof (u.buf)) >> + delete[] u.ptr; >> + } >> + >> + record_full_reg_entry &operator=(record_full_reg_entry &&other) { >> + gdb_assert (this != &other); >> + num = other.num; >> + len = other.len; >> + memcpy (u.buf, other.u.buf, sizeof (u.buf)); >> + /* With len == 0, OTHER is guaranteed to not try to free the >> + memory when it is destructed. */ >> + other.len = 0; >> + return *this; >> + } >> + >> + DISABLE_COPY_AND_ASSIGN (record_full_reg_entry); >> + >> + gdb_byte *get_loc () >> + { >> + if (len > sizeof (u.buf)) >> + return u.ptr; >> + else >> + return u.buf; >> + } >> + >> + bool execute (regcache *regcache) >> + { >> + gdb::byte_vector buf (len); >> + >> + if (record_debug > 1) >> + gdb_printf (gdb_stdlog, >> + "Process record: record_full_reg %s to " >> + "inferior num = %d.\n", >> + host_address_to_string (this), >> + num); >> + >> + regcache->cooked_read (num, buf); >> + regcache->cooked_write (num, get_loc ()); >> + memcpy (get_loc (), buf.data (), len); >> + return false; >> + } >> }; >> >> enum record_full_type >> @@ -118,16 +287,88 @@ enum record_full_type >> record_full_mem >> }; >> >> -struct record_full_entry >> +class record_full_entry >> { >> - enum record_full_type type; >> - union >> + std::variant entry; >> + >> +public: >> + record_full_entry () : entry (record_full_reg_entry ()) {} >> + >> + /* Constructor for a register entry. Type is here to make it >> + easier to recognize it in the constructor calls, it isn't >> + actually important. */ >> + record_full_entry (record_full_type reg_type, gdbarch *gdbarch, >> + int regnum) >> + : entry(record_full_reg_entry (gdbarch, regnum)) >> { >> - /* reg */ >> - struct record_full_reg_entry reg; >> - /* mem */ >> - struct record_full_mem_entry mem; >> - } u; >> + gdb_assert (reg_type == record_full_reg); } >> + >> + record_full_entry (record_full_type mem_type, CORE_ADDR addr, int >> + len) >> + : entry(record_full_mem_entry (addr, len)) { >> + gdb_assert (mem_type == record_full_mem); } >> + >> + record_full_entry (record_full_entry &&other) >> + : entry (std::move (other.entry)) >> + { >> + } >> + >> + DISABLE_COPY_AND_ASSIGN (record_full_entry); >> + >> + record_full_reg_entry& reg () >> + { >> + gdb_assert (type () == record_full_reg); >> + return std::get (entry); } >> + >> + record_full_mem_entry& mem () >> + { >> + gdb_assert (type () == record_full_mem); >> + return std::get (entry); } >> + >> + record_full_type type () const >> + { >> + switch (entry.index ()) >> + { >> + case 0: >> + return record_full_reg; >> + case 1: >> + return record_full_mem; >> + } >> + gdb_assert_not_reached ("Impossible variant index"); } >> + >> + /* Get the pointer to the data stored by this entry. */ gdb_byte >> + *get_loc () { >> + switch (type ()) >> + { >> + case record_full_reg: >> + return reg ().get_loc (); >> + case record_full_mem: >> + return mem ().get_loc (); >> + } >> + gdb_assert_not_reached ("Impossible entry type"); } >> + >> + /* Execute this entry, swapping the appropriate values from memory or >> + register and the recorded ones. Returns TRUE if the execution was >> + stopped by a watchpoint. */ >> + >> + bool execute (regcache *regcache) >> + { >> + switch (type ()) >> + { >> + case record_full_reg: >> + return reg ().execute (regcache); >> + case record_full_mem: >> + return mem ().execute (regcache->arch ()); >> + } >> + return false; >> + } >> }; >> >> /* This is the main structure that comprises the execution log. >> @@ -386,91 +627,12 @@ static struct cmd_list_element >> *record_full_cmdlist; static void record_full_goto_insn (size_t target_insn, >> enum exec_direction_kind dir); >> >> -/* Initialization and cleanup functions for record_full_reg and >> - record_full_mem entries. */ >> - >> -/* Init a record_full_reg record entry. */ >> - >> -static inline record_full_entry >> -record_full_reg_init (struct regcache *regcache, int regnum) -{ >> - record_full_entry rec; >> - struct gdbarch *gdbarch = regcache->arch (); >> - >> - rec.type = record_full_reg; >> - rec.u.reg.num = regnum; >> - rec.u.reg.len = register_size (gdbarch, regnum); >> - if (rec.u.reg.len > sizeof (rec.u.reg.u.buf)) >> - rec.u.reg.u.ptr = (gdb_byte *) xmalloc (rec.u.reg.len); >> - >> - return rec; >> -} >> - >> -/* Cleanup a record_full_reg record entry. */ >> - >> -static inline void >> -record_full_reg_cleanup (record_full_entry rec) -{ >> - gdb_assert (rec.type == record_full_reg); >> - if (rec.u.reg.len > sizeof (rec.u.reg.u.buf)) >> - xfree (rec.u.reg.u.ptr); >> -} >> - >> -/* Init a record_full_mem record entry. */ >> - >> -static inline record_full_entry >> -record_full_mem_init (CORE_ADDR addr, int len) -{ >> - record_full_entry rec; >> - >> - rec.type = record_full_mem; >> - rec.u.mem.addr = addr; >> - rec.u.mem.len = len; >> - if (rec.u.mem.len > sizeof (rec.u.mem.u.buf)) >> - rec.u.mem.u.ptr = (gdb_byte *) xmalloc (len); >> - rec.u.mem.mem_entry_not_accessible = 0; >> - >> - return rec; >> -} >> - >> -/* Cleanup a record_full_mem record entry. */ >> - >> -static inline void >> -record_full_mem_cleanup (record_full_entry rec) -{ >> - gdb_assert (rec.type == record_full_mem); >> - if (rec.u.mem.len > sizeof (rec.u.mem.u.buf)) >> - xfree (rec.u.mem.u.ptr); >> -} >> - >> -/* Free one record entry, any type. */ >> - >> -static inline void >> -record_full_entry_cleanup (record_full_entry rec) -{ >> - >> - switch (rec.type) { >> - case record_full_reg: >> - record_full_reg_cleanup (rec); >> - break; >> - case record_full_mem: >> - record_full_mem_cleanup (rec); >> - break; >> - } >> -} >> - >> static void >> record_full_reset_history () >> { >> record_full_insn_count = 0; >> record_full_next_insn = 0; >> >> - for (auto &insn : record_full_list) >> - { >> - for (auto &entry : insn.effects) >> - record_full_entry_cleanup (entry); >> - } >> - >> record_full_list.clear (); >> } >> >> @@ -480,11 +642,7 @@ static void >> record_full_list_release_following (int index) { >> for (int i = record_full_list.size () - 1; i > index; i--) >> - { >> - for (auto &entry : record_full_list[i].effects) >> - record_full_entry_cleanup (entry); >> - record_full_list.pop_back (); >> - } >> + record_full_list.pop_back (); >> /* Set the next instruction to be past the end of the log so we >> start recording if the user moves forward again. */ >> record_full_next_insn = index; >> @@ -513,9 +671,6 @@ record_full_list_release_first (void) >> if (record_full_list.empty ()) >> return; >> >> - for (auto &entry : record_full_list[0].effects) >> - record_full_entry_cleanup (entry); >> - >> record_full_list.pop_front (); >> --record_full_next_insn; >> } >> @@ -525,28 +680,7 @@ record_full_list_release_first (void) static void >> record_full_arch_list_add (record_full_entry &rec) { >> - record_full_incomplete_instruction.effects.push_back (rec); -} >> - >> -/* Return the value storage location of a record entry. */ -static inline >> gdb_byte * -record_full_get_loc (struct record_full_entry *rec) -{ >> - switch (rec->type) { >> - case record_full_mem: >> - if (rec->u.mem.len > sizeof (rec->u.mem.u.buf)) >> - return rec->u.mem.u.ptr; >> - else >> - return rec->u.mem.u.buf; >> - case record_full_reg: >> - if (rec->u.reg.len > sizeof (rec->u.reg.u.buf)) >> - return rec->u.reg.u.ptr; >> - else >> - return rec->u.reg.u.buf; >> - default: >> - gdb_assert_not_reached ("unexpected record_full_entry type"); >> - return NULL; >> - } >> + record_full_incomplete_instruction.effects.push_back (std::move >> + (rec)); >> } >> >> /* Record the value of a register NUM to record_full_arch_list. */ @@ -554,7 >> +688,7 @@ record_full_get_loc (struct record_full_entry *rec) int >> record_full_arch_list_add_reg (struct regcache *regcache, int regnum) { >> - record_full_entry rec; >> + record_full_entry rec (record_full_reg, regcache->arch (), regnum); >> >> if (record_debug > 1) >> gdb_printf (gdb_stdlog, >> @@ -562,9 +696,7 @@ record_full_arch_list_add_reg (struct regcache >> *regcache, int regnum) >> "record list.\n", >> regnum); >> >> - rec = record_full_reg_init (regcache, regnum); >> - >> - regcache->cooked_read (regnum, record_full_get_loc (&rec)); >> + regcache->cooked_read (regnum, rec.get_loc ()); >> >> record_full_arch_list_add (rec); >> >> @@ -577,7 +709,7 @@ record_full_arch_list_add_reg (struct regcache >> *regcache, int regnum) int record_full_arch_list_add_mem (CORE_ADDR >> addr, int len) { >> - record_full_entry rec; >> + record_full_entry rec (record_full_mem, addr, len); >> >> if (record_debug > 1) >> gdb_printf (gdb_stdlog, >> @@ -588,14 +720,9 @@ record_full_arch_list_add_mem (CORE_ADDR addr, >> int len) >> if (!addr) /* FIXME: Why? Some arch must permit it... */ >> return 0; >> >> - rec = record_full_mem_init (addr, len); >> - >> if (record_read_memory (current_inferior ()->arch (), addr, >> - record_full_get_loc (&rec), len)) >> - { >> - record_full_mem_cleanup (rec); >> - return -1; >> - } >> + rec.get_loc (), len)) >> + return -1; >> >> record_full_arch_list_add (rec); >> >> @@ -723,91 +850,6 @@ record_full_gdb_operation_disable_set (void) static >> enum target_stop_reason record_full_stop_reason >> = TARGET_STOPPED_BY_NO_REASON; >> >> -/* Execute one instruction from the record log. Each instruction in >> - the log will be represented by an arbitrary sequence of register >> - entries and memory entries, followed by an 'end' entry. */ >> - >> -static inline void >> -record_full_exec_entry (regcache *regcache, >> - gdbarch *gdbarch, >> - record_full_entry *entry) >> -{ >> - switch (entry->type) >> - { >> - case record_full_reg: /* reg */ >> - { >> - gdb::byte_vector reg (entry->u.reg.len); >> - >> - if (record_debug > 1) >> - gdb_printf (gdb_stdlog, >> - "Process record: record_full_reg %s to " >> - "inferior num = %d.\n", >> - host_address_to_string (entry), >> - entry->u.reg.num); >> - >> - regcache->cooked_read (entry->u.reg.num, reg.data ()); >> - regcache->cooked_write (entry->u.reg.num, record_full_get_loc >> (entry)); >> - memcpy (record_full_get_loc (entry), reg.data (), entry->u.reg.len); >> - } >> - break; >> - >> - case record_full_mem: /* mem */ >> - { >> - /* Nothing to do if the entry is flagged not_accessible. */ >> - if (!entry->u.mem.mem_entry_not_accessible) >> - { >> - gdb::byte_vector mem (entry->u.mem.len); >> - >> - if (record_debug > 1) >> - gdb_printf (gdb_stdlog, >> - "Process record: record_full_mem %s to " >> - "inferior addr = %s len = %d.\n", >> - host_address_to_string (entry), >> - paddress (gdbarch, entry->u.mem.addr), >> - entry->u.mem.len); >> - >> - if (record_read_memory (gdbarch, >> - entry->u.mem.addr, mem.data (), >> - entry->u.mem.len)) >> - entry->u.mem.mem_entry_not_accessible = 1; >> - else >> - { >> - if (target_write_memory (entry->u.mem.addr, >> - record_full_get_loc (entry), >> - entry->u.mem.len)) >> - { >> - entry->u.mem.mem_entry_not_accessible = 1; >> - if (record_debug) >> - warning (_("Process record: error writing memory at " >> - "addr = %s len = %d."), >> - paddress (gdbarch, entry->u.mem.addr), >> - entry->u.mem.len); >> - } >> - else >> - { >> - memcpy (record_full_get_loc (entry), mem.data (), >> - entry->u.mem.len); >> - >> - /* We've changed memory --- check if a hardware >> - watchpoint should trap. Note that this >> - presently assumes the target beneath supports >> - continuable watchpoints. On non-continuable >> - watchpoints target, we'll want to check this >> - _before_ actually doing the memory change, and >> - not doing the change at all if the watchpoint >> - traps. */ >> - if (hardware_watchpoint_inserted_in_range >> - (current_inferior ()->aspace.get (), >> - entry->u.mem.addr, entry->u.mem.len)) >> - record_full_stop_reason = >> TARGET_STOPPED_BY_WATCHPOINT; >> - } >> - } >> - } >> - } >> - break; >> - } >> -} >> - >> /* Execute one entry in the log by executing all the effects. */ >> >> static inline void >> @@ -816,7 +858,8 @@ record_full_exec_insn (regcache *regcache, >> record_full_instruction &insn) { >> for (auto &entry : insn.effects) >> - record_full_exec_entry (regcache, gdbarch, &entry); >> + if (entry.execute (regcache)) >> + record_full_stop_reason = TARGET_STOPPED_BY_WATCHPOINT; >> } >> >> static void record_full_restore (struct bfd &cbfd); @@ -2193,21 +2236,19 >> @@ record_full_read_entry_from_bfd (bfd *cbfd, asection *osec, int >> *bfd_offset) >> bfdcore_read (cbfd, osec, ®num, sizeof (regnum), bfd_offset); >> regnum = netorder32 (regnum); >> >> - record_full_entry rec; >> - >> - rec = record_full_reg_init (cache, regnum); >> + record_full_entry rec (record_full_reg, cache->arch (), regnum); >> >> /* Get val. */ >> - bfdcore_read (cbfd, osec, record_full_get_loc (&rec), >> - rec.u.reg.len, bfd_offset); >> + bfdcore_read (cbfd, osec, rec.get_loc (), >> + rec.reg ().len, bfd_offset); >> >> if (record_debug) >> gdb_printf (gdb_stdlog, >> " Reading register %d (1 " >> "plus %lu plus %d bytes)\n", >> - rec.u.reg.num, >> + rec.reg ().num, >> (unsigned long) sizeof (regnum), >> - rec.u.reg.len); >> + rec.reg ().len); >> >> record_full_arch_list_add (rec); >> break; >> @@ -2223,22 +2264,20 @@ record_full_read_entry_from_bfd (bfd *cbfd, >> asection *osec, int *bfd_offset) >> bfdcore_read (cbfd, osec, &addr, sizeof (addr), bfd_offset); >> addr = netorder64 (addr); >> >> - record_full_entry rec; >> - rec = record_full_mem_init (addr, len); >> + record_full_entry rec (record_full_mem, addr, len); >> >> /* Get val. */ >> - bfdcore_read (cbfd, osec, record_full_get_loc (&rec), >> - len, bfd_offset); >> + bfdcore_read (cbfd, osec, rec.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 (), >> - rec.u.mem.addr), >> + rec.mem ().addr), >> (unsigned long) sizeof (addr), >> (unsigned long) sizeof (len), >> - rec.u.mem.len); >> + rec.mem ().len); >> >> record_full_arch_list_add (rec); >> break; >> @@ -2385,57 +2424,56 @@ record_full_write_entry_to_bfd >> (record_full_entry &entry, >> uint32_t regnum, len; >> uint64_t addr; >> >> - type = entry.type; >> + type = entry.type (); >> bfdcore_write (obfd.get (), osec, &type, sizeof (type), bfd_offset); >> >> - switch (entry.type) >> + switch (type) >> { >> case record_full_reg: /* reg */ >> - if (record_debug) >> - gdb_printf (gdb_stdlog, >> - " Writing register %d (1 " >> - "plus %lu plus %d bytes)\n", >> - entry.u.reg.num, >> - (unsigned long) sizeof (regnum), >> - entry.u.reg.len); >> - >> - /* Write regnum. */ >> - regnum = netorder32 (entry.u.reg.num); >> - bfdcore_write (obfd.get (), osec, ®num, >> - sizeof (regnum), bfd_offset); >> - >> - /* Write regval. */ >> - bfdcore_write (obfd.get (), osec, >> - record_full_get_loc (&entry), >> - entry.u.reg.len, bfd_offset); >> - break; >> + { >> + auto ® = 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; >> + } >> >> case record_full_mem: /* mem */ >> - if (record_debug) >> - gdb_printf (gdb_stdlog, >> - " Writing memory %s (1 plus " >> - "%lu plus %lu plus %d bytes)\n", >> - paddress (gdbarch, >> - entry.u.mem.addr), >> - (unsigned long) sizeof (addr), >> - (unsigned long) sizeof (len), >> - entry.u.mem.len); >> - >> - /* Write memlen. */ >> - len = netorder32 (entry.u.mem.len); >> - bfdcore_write (obfd.get (), osec, &len, sizeof (len), >> - bfd_offset); >> - >> - /* Write memaddr. */ >> - addr = netorder64 (entry.u.mem.addr); >> - bfdcore_write (obfd.get (), osec, &addr, >> - sizeof (addr), bfd_offset); >> - >> - /* Write memval. */ >> - bfdcore_write (obfd.get (), osec, >> - record_full_get_loc (&entry), >> - entry.u.mem.len, bfd_offset); >> - break; >> + { >> + 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; >> + } >> } >> } >> >> @@ -2482,13 +2520,13 @@ record_full_base_target::save_record (const >> char *recfilename) >> /* Number of effects of an instruction. */ >> save_size += sizeof (uint32_t) + sizeof (uint8_t) + sizeof (uint32_t); >> for (auto &entry : record_full_list[i].effects) >> - switch (entry.type) >> + switch (entry.type ()) >> { >> case record_full_reg: >> - save_size += 1 + 4 + entry.u.reg.len; >> + save_size += 1 + 4 + entry.reg ().len; >> break; >> case record_full_mem: >> - save_size += 1 + 4 + 8 + entry.u.mem.len; >> + save_size += 1 + 4 + 8 + entry.mem ().len; >> break; >> } >> } >> @@ -2621,18 +2659,18 @@ maintenance_print_record_instruction (const >> char *args, int from_tty) >> >> gdbarch *arch = current_inferior ()->arch (); >> >> - for (auto entry : to_print->effects) >> + for (auto &entry : to_print->effects) >> { >> - switch (entry.type) >> + switch (entry.type ()) >> { >> case record_full_reg: >> { >> - type *regtype = gdbarch_register_type (arch, entry.u.reg.num); >> + type *regtype = gdbarch_register_type (arch, entry.reg ().num); >> value *val >> = value_from_contents (regtype, >> - record_full_get_loc (&entry)); >> + entry.get_loc ()); >> gdb_printf ("Register %s changed: ", >> - gdbarch_register_name (arch, entry.u.reg.num)); >> + gdbarch_register_name (arch, entry.reg ().num)); >> struct value_print_options opts; >> get_user_print_options (&opts); >> opts.raw = true; >> @@ -2642,11 +2680,12 @@ maintenance_print_record_instruction (const >> char *args, int from_tty) >> } >> case record_full_mem: >> { >> - gdb_byte *b = record_full_get_loc (&entry); >> + record_full_mem_entry& mem = entry.mem (); >> + gdb_byte *b = entry.get_loc (); >> gdb_printf ("%d bytes of memory at address %s changed from:", >> - entry.u.mem.len, >> - print_core_address (arch, entry.u.mem.addr)); >> - for (int i = 0; i < entry.u.mem.len; i++) >> + mem.len, >> + print_core_address (arch, mem.addr)); >> + for (int i = 0; i < mem.len; i++) >> gdb_printf (" %02x", b[i]); >> gdb_printf ("\n"); >> break; >> -- >> 2.54.0 >> > It seems that you did not change the code as discussed here: > https://sourceware.org/pipermail/gdb-patches/2026-May/227699.html > > Would you mind sharing the reason for that? > In my opinion this is necessary. I blame the weekend. Leave on friday thinking "I'll do it on monday", arrive on monday thinking "I did it last friday". I made sure to make the change now. Added the asserts that the length is 0 in the move operator, which means we won't need to worry about freeing the memory. -- Cheers, Guinevere Larsen It/she > > Christina > > > > > Intel Deutschland GmbH > > Registered Address: Dornacher Strasse 1, 85622 Feldkirchen, Germany > Tel: +49 89 991 430, www.intel.de > Managing Directors: Harry Demas, Jeffrey Schneiderman, Yin Chong Sorrell > Chairperson of the Supervisory Board: Nicole Lau > Registered Seat: Munich > Commercial Register: Amtsgericht Muenchen HRB 186928 >