From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id +xiiMmXO/GlDrB8AWB0awg (envelope-from ) for ; Thu, 07 May 2026 13:39:49 -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=NRFtiiHF; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id B1C1F1E067; Thu, 07 May 2026 13:39:49 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-3.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,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 8F67A1E067 for ; Thu, 07 May 2026 13:39:46 -0400 (EDT) Received: from vm01.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 7C6924BA7982 for ; Thu, 7 May 2026 17:39:38 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 7C6924BA7982 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=NRFtiiHF 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 1062B4BA23FB for ; Thu, 7 May 2026 17:38:11 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 1062B4BA23FB 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 1062B4BA23FB 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=1778175491; cv=none; b=Rr6H3c89FT5lYgnTQnERCNVurLIDbsDU+PeTOlrkoUgHOm+0K+YfxRMWM0dccgzZTUsqnPxLd0Mu0eAIkteoqE0XLbwbgkuqFKxTB3CXvtbBZMlDxFMW/eicaWuPyCA3P+1Y3VGzeGsY6wp9JN0mXWXNY5TcBbpBFS1XV5pN2cQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1778175491; c=relaxed/simple; bh=GR61mW1OMI4vIqLld18EZS920gZsIm7QrQ3EnWkA9u8=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=Qf34gGLccKNZ7yeXLWDNsNxgMXePwvDJAVd6XCUMO0wK68DBqKbapCez/OjcGphbuXM9xmWYkHCSjWekkbCE2pTqfh3U8myN0CZOvl/Gg1qzW0QhpHcvEwMAOlR1A2S+LjiUTxuasLJr7ZW7qO9Ey1KElLnWPsqCsy8Y/iaAsIk= 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=NRFtiiHF DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 1062B4BA23FB DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1778175490; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=r4NBzQ5dSeJDeYIX8deHfHgwE6ticWRl4ZdutV1h3pQ=; b=NRFtiiHFvHNaaNRw3izjwzABAtHcF3/pzfh5dgCpMwudyIgMkVZDLsC/v+UpxFkUB+uvGm h7ie6IcmKLkGaz2OGp60ZJWzDZ/chOYDke7twcAsbmrgO/eOLKPAQeaplWFBbtD7j+LBgb L/GXt4R6EJwp6OC4HKRoUJ1EnisA7BM= Received: from mail-qk1-f199.google.com (mail-qk1-f199.google.com [209.85.222.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-616-rotIQc6AO5KohIbFaSaOog-1; Thu, 07 May 2026 13:38:08 -0400 X-MC-Unique: rotIQc6AO5KohIbFaSaOog-1 X-Mimecast-MFC-AGG-ID: rotIQc6AO5KohIbFaSaOog_1778175488 Received: by mail-qk1-f199.google.com with SMTP id af79cd13be357-8ebe2c5977aso229981485a.0 for ; Thu, 07 May 2026 10:38:08 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1778175487; x=1778780287; h=content-transfer-encoding:in-reply-to:from:content-language :references: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=r4NBzQ5dSeJDeYIX8deHfHgwE6ticWRl4ZdutV1h3pQ=; b=FacLPF9t6z9WuLBV6L9n0fiZ9lfo3NH7rkewiwnX4qkXV4krvOsshqPMyafpKMBgSB 05Z4x6qR+YSMZbJtkBqD2jjj1LsVvvQZ/lvkDLTnsrP9fo4rhzcTK4OHMkUjohnRQkdI dKuQl/vu7/9S7CBp8iB4+7hL54i1lgwysJ1VDzkRh7QX0kJkUI64f/uLX78O4droWn2w utemLUYJ+qBUTGv8/MBrWZmX0+9w0YkaZYVq8QDIRLV+ghkSHRcJ3OjT92bL1iuuxGd+ 1aCE6WxZKYryBry6YZPDy6xE4HisHH/AD4L+Scoq4YEnvKIe9XWenjHflKSvG+TtFCpJ 6FDA== X-Forwarded-Encrypted: i=1; AFNElJ9/r6/WO5c5+NqrKLFU+oIB6m7n88bDNQXbRTna4g57/Oyu7FzIGP7p81stk97FMjL3CpmFMvBm4bbXFA==@sourceware.org X-Gm-Message-State: AOJu0YxgzkODTiXGNhdmrw9LKIMaGnlfsJAUbUlaQ5EOCk841ltYE8Ud xnOEPeLTZUKVDfwrjPVJh6l0R1tuD/CdHOI51CLfhN5sXQ7EjXO0HAe/GMMJlkPFYsXJo311Y0C UUvkAlBWPvKNeB5JttkjYc8ASOWHu68ShjjbdlI8WIeBv5vwTtTeE9CvRLxcUpjavthJkIXc= X-Gm-Gg: AeBDiesCXAhX9xXT0mLZxr1RQ2PjSJSNCvXib5Nj1l2dYOcM9GZrfc+JkhZlXfwAP3U jsINbUFTPvvS9EeWQF+AbZU4v9evGSgZTUFyx7xBfVYS/VgUYndg/2ZrSVVMfrLPR2nnljqjmyo oI6csNUpN7ZlelRkhVF35MisjXPjhPCnc6YuIbkOOAbK04JpRs55VI//8tJ/+C7VrPVlsKgy4yz Io8J/3yCsluGq6NlbwvCtH1VW445O3OHa3bybf9aJAnWBNSYIv2aGKFdSEAEtiiWoatdFDJB2nD 1eS3YjS7R7h1A2lxEn4kZni/tgJi2VqttH511Urj3+FOBzpPuJN5UTVT6kq8jZ0CPoqViwjn2jE AOdT0GFaGjMBSd8mjpsKdI85bmfmcQuI= X-Received: by 2002:a05:620a:371e:b0:8d6:d217:96a0 with SMTP id af79cd13be357-9064c8b5943mr540369185a.0.1778175487035; Thu, 07 May 2026 10:38:07 -0700 (PDT) X-Received: by 2002:a05:620a:371e:b0:8d6:d217:96a0 with SMTP id af79cd13be357-9064c8b5943mr540359185a.0.1778175486064; Thu, 07 May 2026 10:38:06 -0700 (PDT) Received: from ?IPV6:2804:14d:8084:993e::75d? ([2804:14d:8084:993e::75d]) by smtp.gmail.com with ESMTPSA id af79cd13be357-8fc2c91c807sm2198985585a.32.2026.05.07.10.38.04 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 07 May 2026 10:38:05 -0700 (PDT) Message-ID: <9de15785-0595-417b-b14d-fe5820738231@redhat.com> Date: Thu, 7 May 2026 14:38:02 -0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2 1/6] gdb/record: Refactor record history To: "Schimpe, Christina" , "gdb-patches@sourceware.org" References: <20260423152604.171356-1-guinevere@redhat.com> <20260423152604.171356-2-guinevere@redhat.com> From: Guinevere Larsen In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: PMNeAYjxnZX-BFR1tOfP_HAnJztdQEfHNgkFvkMK99A_1778175488 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit 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 5/6/26 6:16 PM, Schimpe, Christina wrote: > Hi Guinevere, > > Thank you for working on this. > I found two items that might be worth looking at in more detail, but > apart from that it’s mostly minor nits. Please find my feedback below. > > I don’t have much experience with the record feature, so my review is > a bit slow here. To fully understand this patch, I probably need more time, > but I wanted to share the comments I already have. Sorry for chiming in > so late. I’ll try again tomorrow, or by Friday at the latest. Hi! Thank you for noting all the inconsistencies, I'll fix almost all of them before pushing! The one thing I don't think it's a good idea is to make the change from record_full_list -> record_full_log in this commit... mostly because it will introduce untold amounts of rebase conflicts. I'll introduce a following commit to this series that will make that renaming, though :) -- Cheers, Guinevere Larsen It/she >> -----Original Message----- >> From: Guinevere Larsen >> Sent: Donnerstag, 23. April 2026 17:26 >> To: gdb-patches@sourceware.org >> Cc: Guinevere Larsen >> Subject: [PATCH v2 1/6] gdb/record: Refactor record history >> >> This is the first step in a large refactor in how GDB keeps execution >> history. Rather than using a linked list where multiple entries can >> describe a single instruction, the history will now be stored in an >> std::deque, each instruction being one entry in the deque. >> >> The choice was initially to use an std::vector, but it would become >> unwieldy because it needs all the memory to be consecutive, which is >> hard for 200 thousand entries. Deque was picked because it was a nice >> midpoint between vector (maximum cache cohesion) and linked list >> (maximum ease of finding space to store more). >> >> Each instruction in memory will be now one record_full_instruction >> entry, which for this commit just contains a vector of >> record_full_entry for the effects of the instruction, and the data that >> was stored in the record_full_end entry (that is, the instruction number >> and the signal, if any). >> >> This change introduced a minimal performance improvement (what's >> important is that it isn't a degradation) and a reduction in the total >> memory footprint of roughly 20% if the entire history is used. >> --- >> gdb/aarch64-tdep.c | 2 - >> gdb/amd64-linux-tdep.c | 3 - >> gdb/arm-tdep.c | 2 - >> gdb/i386-linux-tdep.c | 3 - >> gdb/i386-tdep.c | 4 - >> gdb/loongarch-tdep.c | 2 - >> gdb/moxie-tdep.c | 2 - >> gdb/ppc-linux-tdep.c | 3 - >> gdb/record-full.c | 1180 ++++++++++++++++------------------------ >> gdb/record-full.h | 1 - >> gdb/riscv-tdep.c | 3 - >> gdb/rs6000-tdep.c | 4 - >> gdb/s390-linux-tdep.c | 3 - >> gdb/s390-tdep.c | 2 - >> 14 files changed, 476 insertions(+), 738 deletions(-) >> >> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c >> index 4befaa2720d..a532992b1d8 100644 >> --- a/gdb/aarch64-tdep.c >> +++ b/gdb/aarch64-tdep.c >> @@ -6210,8 +6210,6 @@ aarch64_process_record (struct gdbarch *gdbarch, >> struct regcache *regcache, >> aarch64_record.aarch64_mems[rec_no].len)) >> ret = -1; >> >> - if (record_full_arch_list_add_end ()) >> - ret = -1; >> } >> >> deallocate_reg_mem (&aarch64_record); >> diff --git a/gdb/amd64-linux-tdep.c b/gdb/amd64-linux-tdep.c >> index a5ac26654cf..9b23db72bbe 100644 >> --- a/gdb/amd64-linux-tdep.c >> +++ b/gdb/amd64-linux-tdep.c >> @@ -1591,9 +1591,6 @@ amd64_linux_record_signal (struct gdbarch >> *gdbarch, >> + AMD64_LINUX_frame_size)) >> return -1; >> >> - if (record_full_arch_list_add_end ()) >> - return -1; >> - >> return 0; >> } >> >> diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c >> index 08cce9dbad8..1ce0927f26b 100644 >> --- a/gdb/arm-tdep.c >> +++ b/gdb/arm-tdep.c >> @@ -14911,8 +14911,6 @@ arm_process_record (struct gdbarch *gdbarch, >> struct regcache *regcache, >> } >> } >> >> - if (record_full_arch_list_add_end ()) >> - ret = -1; >> } >> >> >> diff --git a/gdb/i386-linux-tdep.c b/gdb/i386-linux-tdep.c >> index 23aeccac9fc..4f33766fcdd 100644 >> --- a/gdb/i386-linux-tdep.c >> +++ b/gdb/i386-linux-tdep.c >> @@ -949,9 +949,6 @@ i386_linux_record_signal (struct gdbarch *gdbarch, >> I386_LINUX_xstate + >> I386_LINUX_frame_size)) >> return -1; >> >> - if (record_full_arch_list_add_end ()) >> - return -1; >> - >> return 0; >> } >> >> >> diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c >> index fa935b5fcdb..fee36b46a73 100644 >> --- a/gdb/i386-tdep.c >> +++ b/gdb/i386-tdep.c >> @@ -5201,8 +5201,6 @@ i386_record_vex (struct i386_record_s *ir, uint8_t >> vex_w, uint8_t vex_r, >> } >> >> record_full_arch_list_add_reg (ir->regcache, ir- >>> regmap[X86_RECORD_REIP_REGNUM]); >> - if (record_full_arch_list_add_end ()) >> - return -1; >> >> return 0; >> } >> @@ -8359,8 +8357,6 @@ Do you want to stop the program?"), >> >> /* In the future, maybe still need to deal with need_dasm. */ >> I386_RECORD_FULL_ARCH_LIST_ADD_REG (X86_RECORD_REIP_REGNUM); >> - if (record_full_arch_list_add_end ()) >> - return -1; >> >> return 0; >> >> diff --git a/gdb/loongarch-tdep.c b/gdb/loongarch-tdep.c >> index 225b1abb703..5ec6b2a48d0 100644 >> --- a/gdb/loongarch-tdep.c >> +++ b/gdb/loongarch-tdep.c >> @@ -2789,8 +2789,6 @@ loongarch_process_record (struct gdbarch >> *gdbarch, struct regcache *regcache, >> LOONGARCH_PC_REGNUM)) >> return -1; >> >> - if (record_full_arch_list_add_end ()) >> - return -1; >> } >> >> return ret; >> diff --git a/gdb/moxie-tdep.c b/gdb/moxie-tdep.c >> index c8a04d21f59..2f8f0186488 100644 >> --- a/gdb/moxie-tdep.c >> +++ b/gdb/moxie-tdep.c >> @@ -1040,8 +1040,6 @@ moxie_process_record (struct gdbarch *gdbarch, >> struct regcache *regcache, >> >> if (record_full_arch_list_add_reg (regcache, MOXIE_PC_REGNUM)) >> return -1; >> - if (record_full_arch_list_add_end ()) >> - return -1; >> return 0; >> } >> >> diff --git a/gdb/ppc-linux-tdep.c b/gdb/ppc-linux-tdep.c >> index 19698eaacbe..e2f53551709 100644 >> --- a/gdb/ppc-linux-tdep.c >> +++ b/gdb/ppc-linux-tdep.c >> @@ -1565,9 +1565,6 @@ ppc_linux_record_signal (struct gdbarch *gdbarch, >> struct regcache *regcache, >> if (record_full_arch_list_add_mem (sp, SIGNAL_FRAMESIZE + >> sizeof_rt_sigframe)) >> return -1; >> >> - if (record_full_arch_list_add_end ()) >> - return -1; >> - >> return 0; >> } >> >> diff --git a/gdb/record-full.c b/gdb/record-full.c >> index 69d2c100e57..aac69cee17e 100644 >> --- a/gdb/record-full.c >> +++ b/gdb/record-full.c > I think we should include vector, optional and deque directly. > >> @@ -72,20 +72,17 @@ >> #define DEFAULT_RECORD_FULL_INSN_MAX_NUM 200000 >> >> #define RECORD_FULL_IS_REPLAY \ >> - (record_full_list->next || ::execution_direction == EXEC_REVERSE) >> + ( (record_full_next_insn != record_full_list.size ()) \ > Nit: No whitespace after the first bracket: > => ((record_full_next_insn != record_full_list.size ()) \ > >> + || ::execution_direction == EXEC_REVERSE) >> >> #define RECORD_FULL_FILE_MAGIC netorder32(0x20091016) >> >> /* These are the core structs of the process record functionality. >> >> A record_full_entry is a record of the value change of a register >> - ("record_full_reg") or a part of memory ("record_full_mem"). And each >> - instruction must have a struct record_full_entry ("record_full_end") >> - that indicates that this is the last struct record_full_entry of this >> - instruction. >> - >> - Each struct record_full_entry is linked to "record_full_list" by "prev" >> - and "next" pointers. */ >> + ("record_full_reg") or a part of memory ("record_full_mem"). >> + These are saved on the record_full_instruction struct, which also >> + contains some extra information, such as delivered signals. */ >> >> struct record_full_mem_entry >> { >> @@ -112,47 +109,14 @@ struct record_full_reg_entry >> } u; >> }; >> >> -struct record_full_end_entry >> -{ >> - enum gdb_signal sigval; >> - ULONGEST insn_num; >> -}; >> - >> enum record_full_type >> { >> - record_full_end = 0, >> record_full_reg, >> record_full_mem >> }; >> >> -/* This is the data structure that makes up the execution log. >> - >> - The execution log consists of a single linked list of entries >> - of type "struct record_full_entry". It is doubly linked so that it >> - can be traversed in either direction. >> - >> - The start of the list is anchored by a struct called >> - "record_full_first". The pointer "record_full_list" either points >> - to the last entry that was added to the list (in record mode), or to >> - the next entry in the list that will be executed (in replay mode). >> - >> - Each list element (struct record_full_entry), in addition to next >> - and prev pointers, consists of a union of three entry types: mem, >> - reg, and end. A field called "type" determines which entry type is >> - represented by a given list element. >> - >> - Each instruction that is added to the execution log is represented >> - by a variable number of list elements ('entries'). The instruction >> - will have one "reg" entry for each register that is changed by >> - executing the instruction (including the PC in every case). It >> - will also have one "mem" entry for each memory change. Finally, >> - each instruction will have an "end" entry that separates it from >> - the changes associated with the next instruction. */ >> - >> struct record_full_entry >> { >> - struct record_full_entry *prev; >> - struct record_full_entry *next; >> enum record_full_type type; >> union >> { >> @@ -160,11 +124,31 @@ struct record_full_entry >> struct record_full_reg_entry reg; >> /* mem */ >> struct record_full_mem_entry mem; >> - /* end */ >> - struct record_full_end_entry end; >> } u; >> }; >> >> +/* This is the main structure that comprises the execution log. >> + Each instruction is comprised of: >> + * The instruction number: How many instructions were recorded before >> + this one; >> + * sigval: Whether the inferior received a signal while the following >> + instruction was being recorded; >> + * effects: A list of record_full_entry structures, each of which >> + describing one effect that the instruction has on the inferior. >> + >> + Note, the signal is stored in the previous instruction for historical >> + reasons. This is how it was first implemented, and no one has gotten >> + around to changing it yet. */ >> + >> +struct record_full_instruction >> +{ >> + /* This might be different from the index if >> + we had to remove the first few instructions. */ >> + uint32_t insn_num; >> + std::optional sigval; >> + std::vector effects; >> +}; >> + >> /* If true, query if PREC cannot record memory >> change of next instruction. */ >> bool record_full_memory_query = false; >> @@ -181,27 +165,25 @@ static detached_regcache >> *record_full_core_regbuf = NULL; >> static std::vector record_full_core_sections; >> static struct record_full_core_buf_entry *record_full_core_buf_list = NULL; >> >> -/* The following variables are used for managing the linked list that >> - represents the execution log. >> +/* The following variables are used for managing the history of executed >> + instructions from the inferior. >> >> - record_full_first is the anchor that holds down the beginning of >> - the list. >> + record_full_list contains all instructions that were fully executed and >> + saved to the log, so that we can replay the execution. >> >> - record_full_list serves two functions: >> - 1) In record mode, it anchors the end of the list. >> - 2) In replay mode, it traverses the list and points to >> - the next instruction that must be emulated. >> + record_full_next_insn always points to the next instruction that would >> + be executed if the inferior executes forward. In the special case when >> + the inferior is not replaying, record_full_next_insn points past the >> + end of the history. >> >> - record_full_arch_list_head and record_full_arch_list_tail are used >> - to manage a separate list, which is used to build up the change >> - elements of the currently executing instruction during record mode. >> - When this instruction has been completely annotated in the "arch >> - list", it will be appended to the main execution log. */ >> + record_full_incomplete_instruction holds a partial instruction, while >> + the lower target is disassembling the instruction, or as partial xfers are >> + happening. It is manipulated by the "arch list" functions for historical >> + reasons. */ >> >> -static struct record_full_entry record_full_first; >> -static struct record_full_entry *record_full_list = &record_full_first; >> -static struct record_full_entry *record_full_arch_list_head = NULL; >> -static struct record_full_entry *record_full_arch_list_tail = NULL; >> +static std::deque record_full_list; > Given that this is not a list anymore I wonder if we should better call this sth. like > record_full_history or record_full_log or even simply recorded_instructions? > >> +static record_full_instruction record_full_incomplete_instruction; >> +int record_full_next_insn; >> >> /* true ask user. false auto delete the last struct record_full_entry. */ >> static bool record_full_stop_at_limit = true; >> @@ -361,6 +343,14 @@ record_full_target::kill () >> record_kill (this); >> } >> >> +static void >> +record_full_reset_incomplete () >> +{ >> + record_full_incomplete_instruction.effects.clear (); >> + record_full_incomplete_instruction.sigval.reset (); >> + record_full_incomplete_instruction.insn_num = 0; >> +} >> + >> /* See record-full.h. */ >> >> int >> @@ -390,156 +380,120 @@ static struct cmd_list_element >> *show_record_full_cmdlist; >> /* Command list for "record full". */ >> static struct cmd_list_element *record_full_cmdlist; >> >> -static void record_full_goto_insn (struct record_full_entry *entry, >> +static void record_full_goto_insn (size_t target_insn, >> enum exec_direction_kind dir); >> >> -/* Alloc and free functions for record_full_reg, record_full_mem, and >> - record_full_end entries. */ >> +/* Initialization and cleanup functions for record_full_reg and >> + record_full_mem entries. */ >> >> -/* Alloc a record_full_reg record entry. */ >> +/* Init a record_full_reg record entry. */ >> >> -static inline struct record_full_entry * >> -record_full_reg_alloc (struct regcache *regcache, int regnum) >> +static inline struct record_full_entry >> +record_full_reg_init (struct regcache *regcache, int regnum) >> { >> - struct record_full_entry *rec; >> + struct record_full_entry rec; >> struct gdbarch *gdbarch = regcache->arch (); >> >> - rec = XCNEW (struct record_full_entry); >> - 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); >> + 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; >> } >> >> -/* Free a record_full_reg record entry. */ >> +/* Cleanup a record_full_reg record entry. */ >> >> static inline void >> -record_full_reg_release (struct record_full_entry *rec) >> +record_full_reg_cleanup (struct 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); >> - xfree (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); >> } >> >> -/* Alloc a record_full_mem record entry. */ >> +/* Init a record_full_mem record entry. */ >> >> -static inline struct record_full_entry * >> -record_full_mem_alloc (CORE_ADDR addr, int len) >> +static inline struct record_full_entry >> +record_full_mem_init (CORE_ADDR addr, int len) >> { >> - struct record_full_entry *rec; >> + struct record_full_entry rec; >> >> - rec = XCNEW (struct record_full_entry); >> - 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.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; >> } >> >> -/* Free a record_full_mem record entry. */ >> +/* Cleanup a record_full_mem record entry. */ >> >> static inline void >> -record_full_mem_release (struct 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); >> - xfree (rec); >> -} >> - >> -/* Alloc a record_full_end record entry. */ >> - >> -static inline struct record_full_entry * >> -record_full_end_alloc (void) >> +record_full_mem_cleanup (struct record_full_entry rec) >> { >> - struct record_full_entry *rec; >> - >> - rec = XCNEW (struct record_full_entry); >> - rec->type = record_full_end; >> - >> - return 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 a record_full_end record entry. */ >> +/* Free one record entry, any type. */ >> >> static inline void >> -record_full_end_release (struct record_full_entry *rec) >> -{ >> - xfree (rec); >> -} >> - >> -/* Free one record entry, any type. >> - Return entry->type, in case caller wants to know. */ >> - >> -static inline enum record_full_type >> -record_full_entry_release (struct record_full_entry *rec) >> +record_full_entry_cleanup (struct record_full_entry rec) >> { >> - enum record_full_type type = rec->type; >> >> - switch (type) { >> + switch (rec.type) { >> case record_full_reg: >> - record_full_reg_release (rec); >> + record_full_reg_cleanup (rec); >> break; >> case record_full_mem: >> - record_full_mem_release (rec); >> - break; >> - case record_full_end: >> - record_full_end_release (rec); >> + record_full_mem_cleanup (rec); >> break; >> } >> - return type; >> } >> >> -/* Free all record entries in list pointed to by REC. */ >> - >> static void >> -record_full_list_release (struct record_full_entry *rec) >> +record_full_reset_history () >> { >> - if (!rec) >> - return; >> - >> - while (rec->next) >> - rec = rec->next; >> + record_full_insn_num = 0; >> + record_full_insn_count = 0; >> + record_full_next_insn = 0; >> >> - while (rec->prev) >> + for (auto &insn : record_full_list) >> { >> - rec = rec->prev; >> - record_full_entry_release (rec->next); >> + for (auto &entry : insn.effects) >> + record_full_entry_cleanup (entry); >> } >> >> - if (rec == &record_full_first) >> - { >> - record_full_insn_num = 0; >> - record_full_first.next = NULL; >> - } >> - else >> - record_full_entry_release (rec); >> + record_full_list.clear (); >> } >> >> -/* Free all record entries forward of the given list position. */ >> - >> static void >> -record_full_list_release_following (struct record_full_entry *rec) >> +record_full_list_release_following (int index) >> { >> - struct record_full_entry *tmp = rec->next; >> - >> - rec->next = NULL; >> - while (tmp) >> + for (int i = record_full_list.size () - 1; i > index; i--) >> { >> - rec = tmp->next; >> - if (record_full_entry_release (tmp) == record_full_end) >> - { >> - record_full_insn_num--; >> - record_full_insn_count--; >> - } >> - tmp = rec; >> + for (auto &entry : record_full_list[i].effects) >> + record_full_entry_cleanup (entry); >> + 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; >> +} >> + >> +static void >> +record_full_save_instruction () >> +{ >> + ++record_full_insn_count; >> + record_full_incomplete_instruction.insn_num = record_full_insn_count; >> + record_full_incomplete_instruction.effects.shrink_to_fit (); >> + record_full_list.push_back (record_full_incomplete_instruction); >> + record_full_next_insn++; >> } >> >> /* Delete the first instruction from the beginning of the log, to make >> @@ -550,52 +504,22 @@ record_full_list_release_following (struct >> record_full_entry *rec) >> static void >> record_full_list_release_first (void) >> { >> - struct record_full_entry *tmp; >> - >> - if (!record_full_first.next) >> + if (record_full_list.empty ()) >> return; >> >> - /* Loop until a record_full_end. */ >> - while (1) >> - { >> - /* Cut record_full_first.next out of the linked list. */ >> - tmp = record_full_first.next; >> - record_full_first.next = tmp->next; >> - tmp->next->prev = &record_full_first; >> - >> - /* tmp is now isolated, and can be deleted. */ >> - if (record_full_entry_release (tmp) == record_full_end) >> - break; /* End loop at first record_full_end. */ >> + for (auto &entry : record_full_list[0].effects) >> + record_full_entry_cleanup (entry); >> >> - if (!record_full_first.next) >> - { >> - gdb_assert (record_full_insn_num == 1); >> - break; /* End loop when list is empty. */ >> - } >> - } >> + record_full_list.pop_front (); >> + --record_full_next_insn; >> } >> >> /* Add a struct record_full_entry to record_full_arch_list. */ >> >> static void >> -record_full_arch_list_add (struct record_full_entry *rec) >> +record_full_arch_list_add (struct record_full_entry &rec) >> { >> - if (record_debug > 1) >> - gdb_printf (gdb_stdlog, >> - "Process record: record_full_arch_list_add %s.\n", >> - host_address_to_string (rec)); >> - >> - if (record_full_arch_list_tail) >> - { >> - record_full_arch_list_tail->next = rec; >> - rec->prev = record_full_arch_list_tail; >> - record_full_arch_list_tail = rec; >> - } >> - else >> - { >> - record_full_arch_list_head = rec; >> - record_full_arch_list_tail = rec; >> - } >> + record_full_incomplete_instruction.effects.push_back (rec); >> } >> >> /* Return the value storage location of a record entry. */ >> @@ -613,7 +537,6 @@ record_full_get_loc (struct record_full_entry *rec) >> return rec->u.reg.u.ptr; >> else >> return rec->u.reg.u.buf; >> - case record_full_end: >> default: >> gdb_assert_not_reached ("unexpected record_full_entry type"); >> return NULL; >> @@ -625,7 +548,7 @@ record_full_get_loc (struct record_full_entry *rec) >> int >> record_full_arch_list_add_reg (struct regcache *regcache, int regnum) >> { >> - struct record_full_entry *rec; >> + struct record_full_entry rec; > I think we can omit the struct keyword here (there are some more occurrences in this patch). > >> if (record_debug > 1) >> gdb_printf (gdb_stdlog, >> @@ -633,9 +556,9 @@ record_full_arch_list_add_reg (struct regcache >> *regcache, int regnum) >> "record list.\n", >> regnum); >> >> - rec = record_full_reg_alloc (regcache, regnum); >> + rec = record_full_reg_init (regcache, regnum); >> >> - regcache->cooked_read (regnum, record_full_get_loc (rec)); >> + regcache->cooked_read (regnum, record_full_get_loc (&rec)); >> >> record_full_arch_list_add (rec); >> >> @@ -648,7 +571,7 @@ record_full_arch_list_add_reg (struct regcache >> *regcache, int regnum) >> int >> record_full_arch_list_add_mem (CORE_ADDR addr, int len) >> { >> - struct record_full_entry *rec; >> + struct record_full_entry rec; >> >> if (record_debug > 1) >> gdb_printf (gdb_stdlog, >> @@ -659,12 +582,12 @@ 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_alloc (addr, len); >> + rec = record_full_mem_init (addr, len); >> >> if (record_read_memory (current_inferior ()->arch (), addr, >> - record_full_get_loc (rec), len)) >> + record_full_get_loc (&rec), len)) >> { >> - record_full_mem_release (rec); >> + record_full_mem_cleanup (rec); >> return -1; >> } >> >> @@ -673,27 +596,6 @@ record_full_arch_list_add_mem (CORE_ADDR addr, >> int len) >> return 0; >> } >> >> -/* Add a record_full_end type struct record_full_entry to >> - record_full_arch_list. */ >> - >> -int >> -record_full_arch_list_add_end (void) >> -{ >> - struct record_full_entry *rec; >> - >> - if (record_debug > 1) >> - gdb_printf (gdb_stdlog, >> - "Process record: add end to arch list.\n"); >> - >> - rec = record_full_end_alloc (); >> - rec->u.end.sigval = GDB_SIGNAL_0; >> - rec->u.end.insn_num = ++record_full_insn_count; >> - >> - record_full_arch_list_add (rec); >> - >> - return 0; >> -} >> - >> static void >> record_full_check_insn_num (void) >> { >> @@ -725,8 +627,7 @@ record_full_message (struct regcache *regcache, >> enum gdb_signal signal) >> >> try >> { >> - record_full_arch_list_head = NULL; >> - record_full_arch_list_tail = NULL; >> + record_full_reset_incomplete (); >> >> /* Check record_full_insn_num. */ >> record_full_check_insn_num (); >> @@ -746,20 +647,16 @@ record_full_message (struct regcache *regcache, >> enum gdb_signal signal) >> the user says something different, like "deliver this signal" >> during the replay mode). >> >> - User should understand that nothing he does during the replay >> - mode will change the behavior of the child. If he tries, >> + User should understand that nothing they do during the replay >> + mode will change the behavior of the child. If they try, >> then that is a user error. >> >> But we should still deliver the signal to gdb during the replay, >> if we delivered it during the recording. Therefore we should >> record the signal during record_full_wait, not >> record_full_resume. */ >> - if (record_full_list != &record_full_first) /* FIXME better way >> - to check */ >> - { >> - gdb_assert (record_full_list->type == record_full_end); >> - record_full_list->u.end.sigval = signal; >> - } >> + if (signal != GDB_SIGNAL_0) >> + record_full_list[record_full_next_insn - 1].sigval = signal; >> >> if (signal == GDB_SIGNAL_0 >> || !gdbarch_process_record_signal_p (gdbarch)) >> @@ -778,13 +675,11 @@ record_full_message (struct regcache *regcache, >> enum gdb_signal signal) >> } >> catch (const gdb_exception &ex) >> { >> - record_full_list_release (record_full_arch_list_tail); >> + record_full_reset_incomplete (); >> throw; >> } >> >> - record_full_list->next = record_full_arch_list_head; >> - record_full_arch_list_head->prev = record_full_list; >> - record_full_list = record_full_arch_list_tail; >> + record_full_save_instruction (); >> >> if (record_full_insn_num == record_full_insn_max_num) >> record_full_list_release_first (); >> @@ -829,9 +724,9 @@ static enum target_stop_reason >> record_full_stop_reason >> entries and memory entries, followed by an 'end' entry. */ >> >> static inline void >> -record_full_exec_insn (struct regcache *regcache, >> - struct gdbarch *gdbarch, >> - struct record_full_entry *entry) >> +record_full_exec_entry (struct regcache *regcache, >> + struct gdbarch *gdbarch, >> + struct record_full_entry *entry) > Nit: omit struct keywords > And there are some more places in this series/patch, but I will stop commenting > now on all of them. > >> { >> switch (entry->type) >> { >> @@ -909,6 +804,15 @@ record_full_exec_insn (struct regcache *regcache, >> } >> } >> >> +static inline void >> +record_full_exec_insn (struct regcache *regcache, >> + struct gdbarch *gdbarch, >> + record_full_instruction &insn) >> +{ >> + for (auto &entry : insn.effects) >> + record_full_exec_entry (regcache, gdbarch, &entry); >> +} >> + >> static void record_full_restore (struct bfd &cbfd); >> >> /* Asynchronous signal handle registered as event loop source for when >> @@ -983,10 +887,7 @@ record_full_open (const char *args, int from_tty) >> record_preopen (); >> >> /* Reset */ >> - record_full_insn_num = 0; >> - record_full_insn_count = 0; >> - record_full_list = &record_full_first; >> - record_full_list->next = NULL; >> + record_full_reset_history (); >> >> bfd *cbfd = get_inferior_core_bfd (current_inferior ()); >> if (cbfd != nullptr) >> @@ -1014,7 +915,7 @@ record_full_base_target::close () >> if (record_debug) >> gdb_printf (gdb_stdlog, "Process record: record_full_close\n"); >> >> - record_full_list_release (record_full_list); >> + record_full_reset_history (); >> >> /* Release record_full_core_regbuf. */ >> if (record_full_core_regbuf) >> @@ -1313,7 +1214,6 @@ record_full_wait_1 (struct target_ops *ops, >> struct gdbarch *gdbarch = regcache->arch (); >> const address_space *aspace = current_inferior ()->aspace.get (); >> int continue_flag = 1; >> - int first_record_full_end = 1; >> >> try >> { >> @@ -1322,7 +1222,6 @@ record_full_wait_1 (struct target_ops *ops, >> record_full_stop_reason = TARGET_STOPPED_BY_NO_REASON; >> status->set_stopped (GDB_SIGNAL_0); >> >> - /* Check breakpoint when forward execute. */ >> if (execution_direction == EXEC_FORWARD) >> { >> tmp_pc = regcache_read_pc (regcache); >> @@ -1330,8 +1229,7 @@ record_full_wait_1 (struct target_ops *ops, >> >> &record_full_stop_reason)) >> { >> if (record_debug) >> - gdb_printf (gdb_stdlog, >> - "Process record: break at %s.\n", >> + gdb_printf (gdb_stdlog, "Process record: break at %s.\n", >> paddress (gdbarch, tmp_pc)); >> goto replay_out; >> } >> @@ -1344,10 +1242,10 @@ record_full_wait_1 (struct target_ops *ops, >> the signal. */ >> target_terminal::ours (); >> >> - /* In EXEC_FORWARD mode, record_full_list points to the tail of prev >> - instruction. */ >> - if (execution_direction == EXEC_FORWARD && record_full_list->next) >> - record_full_list = record_full_list->next; >> + /* In EXEC_FORWARD mode, record_full_next_insn is the next >> + instruction to be executed. */ >> + if (execution_direction == EXEC_REVERSE) >> + record_full_next_insn--; >> >> /* Loop over the record_full_list, looking for the next place to >> stop. */ >> @@ -1355,108 +1253,92 @@ record_full_wait_1 (struct target_ops *ops, >> { >> /* Check for beginning and end of log. */ >> if (execution_direction == EXEC_REVERSE >> - && record_full_list == &record_full_first) >> + && record_full_next_insn < 0) >> { >> /* Hit beginning of record log in reverse. */ >> status->set_no_history (); >> + record_full_next_insn = 0; >> break; >> } >> if (execution_direction != EXEC_REVERSE >> - && !record_full_list->next) >> + && record_full_next_insn == record_full_list.size ()) >> { >> /* Hit end of record log going forward. */ >> status->set_no_history (); >> break; >> } >> >> - record_full_exec_insn (regcache, gdbarch, record_full_list); >> + record_full_exec_insn >> + (regcache, gdbarch, >> + record_full_list[record_full_next_insn]); >> >> - if (record_full_list->type == record_full_end) >> + /* step */ >> + if (record_full_resume_step) >> { >> if (record_debug > 1) >> - gdb_printf >> - (gdb_stdlog, >> - "Process record: record_full_end %s to " >> - "inferior.\n", >> - host_address_to_string (record_full_list)); >> - >> - if (first_record_full_end >> - && execution_direction == EXEC_REVERSE) >> - { >> - /* When reverse execute, the first >> - record_full_end is the part of current >> - instruction. */ >> - first_record_full_end = 0; >> - } >> - else >> - { >> - /* In EXEC_REVERSE mode, this is the >> - record_full_end of prev instruction. In >> - EXEC_FORWARD mode, this is the >> - record_full_end of current instruction. */ >> - /* step */ >> - if (record_full_resume_step) >> - { >> - if (record_debug > 1) >> - gdb_printf (gdb_stdlog, >> - "Process record: step.\n"); >> - continue_flag = 0; >> - } >> - >> - /* check breakpoint */ >> - tmp_pc = regcache_read_pc (regcache); >> - if (record_check_stopped_by_breakpoint >> - (aspace, tmp_pc, &record_full_stop_reason)) >> - { >> - if (record_debug) >> - gdb_printf (gdb_stdlog, >> - "Process record: break " >> - "at %s.\n", >> - paddress (gdbarch, tmp_pc)); >> + gdb_printf (gdb_stdlog, >> + "Process record: step.\n"); >> + continue_flag = 0; >> + } >> >> - continue_flag = 0; >> - } >> + /* check breakpoint */ >> + tmp_pc = regcache_read_pc (regcache); >> + if (record_check_stopped_by_breakpoint >> + (aspace, tmp_pc, &record_full_stop_reason)) >> + { >> + if (record_debug) >> + gdb_printf (gdb_stdlog, >> + "Process record: break " >> + "at %s.\n", >> + paddress (gdbarch, tmp_pc)); >> >> - if (record_full_stop_reason >> - == TARGET_STOPPED_BY_WATCHPOINT) >> - { >> - if (record_debug) >> - gdb_printf (gdb_stdlog, >> - "Process record: hit hw " >> - "watchpoint.\n"); >> - continue_flag = 0; >> - } >> - /* Check target signal */ >> - if (record_full_list->u.end.sigval != GDB_SIGNAL_0) >> - /* FIXME: better way to check */ >> - continue_flag = 0; >> - } >> + continue_flag = 0; >> } >> >> - if (continue_flag) >> + if (record_full_stop_reason >> + == TARGET_STOPPED_BY_WATCHPOINT) >> { >> - if (execution_direction == EXEC_REVERSE) >> - { >> - if (record_full_list->prev) >> - record_full_list = record_full_list->prev; >> - } >> - else >> - { >> - if (record_full_list->next) >> - record_full_list = record_full_list->next; >> - } >> + if (record_debug) >> + gdb_printf (gdb_stdlog, >> + "Process record: hit hw " >> + "watchpoint.\n"); >> + continue_flag = 0; >> } >> + if (record_full_list[record_full_next_insn].sigval.has_value ()) >> + continue_flag = 0; >> + >> + if (execution_direction == EXEC_REVERSE) >> + record_full_next_insn--; >> + else >> + record_full_next_insn++; >> } >> while (continue_flag); >> >> + if (record_full_next_insn < 0) >> + { >> + gdb_assert (execution_direction == EXEC_REVERSE); >> + record_full_next_insn = 0; >> + } >> + else if (record_full_next_insn > record_full_list.size ()) >> + { >> + gdb_assert (execution_direction == EXEC_FORWARD); >> + record_full_next_insn = record_full_list.size (); >> + } >> + /* Reset the current instruciton to point to the one to be replayed > Nit: instruction > >> + moving forward. */ >> + else if (execution_direction == EXEC_REVERSE) >> + record_full_next_insn++; >> + >> replay_out: >> if (status->kind () == TARGET_WAITKIND_STOPPED) >> { >> + int insn = (execution_direction == EXEC_FORWARD) >> + ? record_full_next_insn - 1 : record_full_next_insn; >> if (record_full_get_sig) >> status->set_stopped (GDB_SIGNAL_INT); >> - else if (record_full_list->u.end.sigval != GDB_SIGNAL_0) >> - /* FIXME: better way to check */ >> - status->set_stopped (record_full_list->u.end.sigval); >> + else if (record_full_list[insn].sigval.has_value ()) >> + status->set_stopped >> + (record_full_list[insn].sigval.value ()); >> else >> status->set_stopped (GDB_SIGNAL_TRAP); >> } >> @@ -1464,12 +1346,9 @@ record_full_wait_1 (struct target_ops *ops, >> catch (const gdb_exception &ex) >> { >> if (execution_direction == EXEC_REVERSE) >> - { >> - if (record_full_list->next) >> - record_full_list = record_full_list->next; >> - } >> + record_full_next_insn++; >> else >> - record_full_list = record_full_list->prev; >> + record_full_next_insn--; >> >> throw; >> } >> @@ -1557,8 +1436,7 @@ record_full_registers_change (struct regcache >> *regcache, int regnum) >> /* Check record_full_insn_num. */ >> record_full_check_insn_num (); >> >> - record_full_arch_list_head = NULL; >> - record_full_arch_list_tail = NULL; >> + record_full_reset_incomplete (); >> >> if (regnum < 0) >> { >> @@ -1568,7 +1446,7 @@ record_full_registers_change (struct regcache >> *regcache, int regnum) >> { >> if (record_full_arch_list_add_reg (regcache, i)) >> { >> - record_full_list_release (record_full_arch_list_tail); >> + record_full_reset_incomplete (); >> error (_("Process record: failed to record execution log.")); >> } >> } >> @@ -1577,18 +1455,11 @@ record_full_registers_change (struct regcache >> *regcache, int regnum) >> { >> if (record_full_arch_list_add_reg (regcache, regnum)) >> { >> - record_full_list_release (record_full_arch_list_tail); >> + record_full_reset_incomplete (); >> error (_("Process record: failed to record execution log.")); >> } >> } >> - if (record_full_arch_list_add_end ()) >> - { >> - record_full_list_release (record_full_arch_list_tail); >> - error (_("Process record: failed to record execution log.")); >> - } >> - record_full_list->next = record_full_arch_list_head; >> - record_full_arch_list_head->prev = record_full_list; >> - record_full_list = record_full_arch_list_tail; >> + record_full_save_instruction (); >> >> if (record_full_insn_num == record_full_insn_max_num) >> record_full_list_release_first (); >> @@ -1607,7 +1478,7 @@ record_full_target::store_registers (struct >> regcache *regcache, int regno) >> { >> int n; >> >> - /* Let user choose if he wants to write register or not. */ >> + /* Let user choose if they want to write register or not. */ >> if (regno < 0) >> n = >> query (_("Because GDB is in replay mode, changing the " >> @@ -1642,7 +1513,7 @@ record_full_target::store_registers (struct >> regcache *regcache, int regno) >> } >> >> /* Destroy the record from here forward. */ >> - record_full_list_release_following (record_full_list); >> + record_full_list_release_following (record_full_next_insn); >> } >> >> record_full_registers_change (regcache, regno); >> @@ -1675,36 +1546,24 @@ record_full_target::xfer_partial (enum >> target_object object, >> error (_("Process record canceled the operation.")); >> >> /* Destroy the record from here forward. */ >> - record_full_list_release_following (record_full_list); >> + record_full_list_release_following (record_full_next_insn); >> } >> >> /* Check record_full_insn_num */ >> record_full_check_insn_num (); >> >> /* Record registers change to list as an instruction. */ >> - record_full_arch_list_head = NULL; >> - record_full_arch_list_tail = NULL; >> + record_full_reset_incomplete (); >> if (record_full_arch_list_add_mem (offset, len)) >> { >> - record_full_list_release (record_full_arch_list_tail); >> + record_full_reset_incomplete (); >> if (record_debug) >> gdb_printf (gdb_stdlog, >> "Process record: failed to record " >> "execution log."); >> return TARGET_XFER_E_IO; >> } >> - if (record_full_arch_list_add_end ()) >> - { >> - record_full_list_release (record_full_arch_list_tail); >> - if (record_debug) >> - gdb_printf (gdb_stdlog, >> - "Process record: failed to record " >> - "execution log."); >> - return TARGET_XFER_E_IO; >> - } >> - record_full_list->next = record_full_arch_list_head; >> - record_full_arch_list_head->prev = record_full_list; >> - record_full_list = record_full_arch_list_tail; >> + record_full_save_instruction (); >> >> if (record_full_insn_num == record_full_insn_max_num) >> record_full_list_release_first (); >> @@ -1866,8 +1725,7 @@ record_full_base_target::get_bookmark (const char >> *args, int from_tty) >> char *ret = NULL; >> >> /* Return stringified form of instruction count. */ >> - if (record_full_list && record_full_list->type == record_full_end) >> - ret = xstrdup (pulongest (record_full_list->u.end.insn_num)); >> + ret = xstrdup (pulongest (record_full_list[record_full_next_insn].insn_num)); > I think we can get here with an empty record_full_list - in case we are in recording mode, > no instructions have been logged and this function is called: > > ~~~ > (gdb) info record > Active record target: record-full > Record mode: > No instructions have been logged. > Max logged instructions is 200000. > (gdb) bookmark > > Thread 1 "gdb-up" hit Breakpoint 1, record_full_base_target::get_bookmark (this=0x555558392900 , args=0x0, from_tty=1) at /tmp/record-full.c:1774 > 1774 char *ret = NULL; > (gdb) p record_full_next_insn > $1 = 0 > (gdb) p record_full_list.empty () > $2 = true > ~~~ > >> if (record_debug) >> { >> @@ -1923,30 +1781,22 @@ record_full_base_target::record_method (ptid_t >> ptid) >> void >> record_full_base_target::info_record () >> { >> - struct record_full_entry *p; >> - >> if (RECORD_FULL_IS_REPLAY) >> gdb_printf (_("Replay mode:\n")); >> else >> gdb_printf (_("Record mode:\n")); >> >> - /* Find entry for first actual instruction in the log. */ >> - for (p = record_full_first.next; >> - p != NULL && p->type != record_full_end; >> - p = p->next) >> - ; >> - >> /* Do we have a log at all? */ >> - if (p != NULL && p->type == record_full_end) >> + if (!record_full_list.empty ()) >> { >> /* Display instruction number for first instruction in the log. */ >> - gdb_printf (_("Lowest recorded instruction number is %s.\n"), >> - pulongest (p->u.end.insn_num)); >> + gdb_printf (_("Lowest recorded instruction number is %u.\n"), >> + record_full_list[0].insn_num); >> >> /* If in replay mode, display where we are in the log. */ >> if (RECORD_FULL_IS_REPLAY) >> - gdb_printf (_("Current instruction number is %s.\n"), >> - pulongest (record_full_list->u.end.insn_num)); >> + gdb_printf (_("Current instruction number is %u.\n"), >> + record_full_list[record_full_next_insn].insn_num); >> >> /* Display instruction number for last instruction in the log. */ >> gdb_printf (_("Highest recorded instruction number is %s.\n"), >> @@ -1975,7 +1825,7 @@ record_full_base_target::supports_delete_record () >> void >> record_full_base_target::delete_record () >> { >> - record_full_list_release_following (record_full_list); >> + record_full_reset_history (); >> } >> >> /* The "record_is_replaying" target method. */ >> @@ -2001,23 +1851,23 @@ record_full_base_target::record_will_replay >> (ptid_t ptid, int dir) >> /* Go to a specific entry. */ >> >> static void >> -record_full_goto_entry (struct record_full_entry *p) >> +record_full_goto_entry (size_t target_insn) >> { >> - if (p == NULL) >> + if (target_insn >= record_full_list.size ()) >> error (_("Target insn not found.")); >> - else if (p == record_full_list) >> + else if (target_insn == record_full_next_insn) >> error (_("Already at target insn.")); >> - else if (p->u.end.insn_num > record_full_list->u.end.insn_num) >> + else if (target_insn > record_full_next_insn) >> { >> gdb_printf (_("Go forward to insn number %s\n"), >> - pulongest (p->u.end.insn_num)); >> - record_full_goto_insn (p, EXEC_FORWARD); >> + pulongest (record_full_list[target_insn].insn_num)); >> + record_full_goto_insn (target_insn, EXEC_FORWARD); >> } >> else >> { >> gdb_printf (_("Go backward to insn number %s\n"), >> - pulongest (p->u.end.insn_num)); >> - record_full_goto_insn (p, EXEC_REVERSE); >> + pulongest (target_insn)); >> + record_full_goto_insn (target_insn, EXEC_REVERSE); >> } >> >> registers_changed (); >> @@ -2033,13 +1883,7 @@ record_full_goto_entry (struct record_full_entry >> *p) >> void >> record_full_base_target::goto_record_begin () >> { >> - struct record_full_entry *p = NULL; >> - >> - for (p = &record_full_first; p != NULL; p = p->next) >> - if (p->type == record_full_end) >> - break; >> - >> - record_full_goto_entry (p); >> + record_full_goto_entry (0); >> } >> >> /* The "goto_record_end" target method. */ >> @@ -2047,15 +1891,7 @@ record_full_base_target::goto_record_begin () >> void >> record_full_base_target::goto_record_end () >> { >> - struct record_full_entry *p = NULL; >> - >> - for (p = record_full_list; p->next != NULL; p = p->next) >> - ; >> - for (; p!= NULL; p = p->prev) >> - if (p->type == record_full_end) >> - break; >> - >> - record_full_goto_entry (p); >> + record_full_goto_entry (record_full_list.size () -1); > Nit: whitespace before 1 => "record_full_list.size () - 1" > > And in case the record_full_list is empty, won't we call > record_full_goto_entry with an invalid value for target_insn ? > >> } >> >> /* The "goto_record" target method. */ >> @@ -2063,13 +1899,7 @@ record_full_base_target::goto_record_end () >> void >> record_full_base_target::goto_record (ULONGEST target_insn) >> { >> - struct record_full_entry *p = NULL; >> - >> - for (p = &record_full_first; p != NULL; p = p->next) >> - if (p->type == record_full_end && p->u.end.insn_num == target_insn) >> - break; >> - >> - record_full_goto_entry (p); >> + record_full_goto_entry (target_insn); >> } >> >> /* The "record_stop_replaying" target method. */ >> @@ -2339,19 +2169,96 @@ netorder32 (uint32_t input) >> return ret; >> } >> >> +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 ()); >> + >> + bfdcore_read (cbfd, osec, &rectype, sizeof (rectype), bfd_offset); >> + switch (rectype) >> + { >> + case record_full_reg: /* reg */ >> + { >> + /* Get register number to regnum. */ >> + bfdcore_read (cbfd, osec, ®num, sizeof (regnum), bfd_offset); >> + regnum = netorder32 (regnum); >> + >> + record_full_entry rec; >> + >> + rec = record_full_reg_init (cache, regnum); >> + >> + /* Get val. */ >> + bfdcore_read (cbfd, osec, record_full_get_loc (&rec), >> + rec.u.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, >> + (unsigned long) sizeof (regnum), >> + rec.u.reg.len); >> + >> + record_full_arch_list_add (rec); >> + break; >> + } >> + >> + case record_full_mem: /* mem */ >> + { >> + /* 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_entry rec; >> + rec = record_full_mem_init (addr, len); >> + >> + /* Get val. */ >> + bfdcore_read (cbfd, osec, record_full_get_loc (&rec), 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), >> + (unsigned long) sizeof (addr), >> + (unsigned long) sizeof (len), >> + len); >> + >> + record_full_arch_list_add (rec); >> + break; >> + } >> + >> + default: >> + error (_("Bad entry type %d, offset %d of %lu in core file %ps."), >> + rectype, *bfd_offset, bfd_section_size (osec), >> + styled_string (file_name_style.style (), >> + bfd_get_filename (cbfd))); >> + break; >> + } >> +} >> + >> /* Restore the execution log from core file CBFD. */ >> >> static void >> record_full_restore (struct bfd &cbfd) >> { >> uint32_t magic; >> - struct record_full_entry *rec; >> asection *osec; >> uint32_t osec_size; >> int bfd_offset = 0; >> >> /* "record_full_restore" can only be called when record list is empty. */ >> - gdb_assert (record_full_first.next == NULL); >> + gdb_assert (record_full_list.empty ()); >> >> if (record_debug) >> gdb_printf (gdb_stdlog, "Restoring recording from core file.\n"); >> @@ -2379,124 +2286,47 @@ record_full_restore (struct bfd &cbfd) >> "RECORD_FULL_FILE_MAGIC (0x%s)\n", >> phex_nz (netorder32 (magic), 4)); >> >> - /* Restore the entries in recfd into record_full_arch_list_head and >> - record_full_arch_list_tail. */ >> - record_full_arch_list_head = NULL; >> - record_full_arch_list_tail = NULL; >> record_full_insn_num = 0; >> >> try >> { >> - regcache *regcache = get_thread_regcache (inferior_thread ()); >> - >> - while (1) >> + while (bfd_offset < osec_size) >> { >> - uint8_t rectype; >> - uint32_t regnum, len, signal, count; >> - uint64_t addr; >> - >> - /* We are finished when offset reaches osec_size. */ >> - if (bfd_offset >= osec_size) >> - break; >> - bfdcore_read (&cbfd, osec, &rectype, sizeof (rectype), &bfd_offset); >> - >> - switch (rectype) >> + record_full_reset_incomplete (); >> + uint32_t eff_count = 0; >> + uint8_t sigval; >> + uint32_t insn_num; >> + >> + /* First read the generic information for an instruction. */ >> + bfdcore_read (&cbfd, osec, &sigval, sizeof (uint8_t), &bfd_offset); >> + bfdcore_read (&cbfd, osec, &eff_count, sizeof (uint32_t), >> + &bfd_offset); >> + bfdcore_read (&cbfd, osec, &insn_num, sizeof (uint32_t), >> + &bfd_offset); >> + >> + record_full_incomplete_instruction.insn_num = netorder32 >> (insn_num); >> + if (sigval != GDB_SIGNAL_0) >> + record_full_incomplete_instruction.sigval = (gdb_signal) sigval; >> + >> + eff_count = netorder32 (eff_count); >> + >> + /* This deals with all the side effects. */ >> + while (eff_count > 0) >> { > There is one space too much indent. > >> - case record_full_reg: /* reg */ >> - /* Get register number to regnum. */ >> - bfdcore_read (&cbfd, osec, ®num, sizeof (regnum), >> - &bfd_offset); >> - regnum = netorder32 (regnum); >> - >> - rec = record_full_reg_alloc (regcache, regnum); >> - >> - /* Get val. */ >> - bfdcore_read (&cbfd, osec, record_full_get_loc (rec), >> - rec->u.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, >> - (unsigned long) sizeof (regnum), >> - rec->u.reg.len); >> - break; >> + eff_count--; >> >> - case record_full_mem: /* mem */ >> - /* 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); >> - >> - rec = record_full_mem_alloc (addr, len); >> - >> - /* Get val. */ >> - bfdcore_read (&cbfd, osec, record_full_get_loc (rec), >> - rec->u.mem.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), >> - (unsigned long) sizeof (addr), >> - (unsigned long) sizeof (len), >> - rec->u.mem.len); >> - break; >> - >> - case record_full_end: /* end */ >> - rec = record_full_end_alloc (); >> - record_full_insn_num ++; >> - >> - /* Get signal value. */ >> - bfdcore_read (&cbfd, osec, &signal, sizeof (signal), >> - &bfd_offset); >> - signal = netorder32 (signal); >> - rec->u.end.sigval = (enum gdb_signal) signal; >> - >> - /* Get insn count. */ >> - bfdcore_read (&cbfd, osec, &count, sizeof (count), &bfd_offset); >> - count = netorder32 (count); >> - rec->u.end.insn_num = count; >> - record_full_insn_count = count + 1; >> - if (record_debug) >> - gdb_printf (gdb_stdlog, >> - " Reading record_full_end (1 + " >> - "%lu + %lu bytes), offset == %s\n", >> - (unsigned long) sizeof (signal), >> - (unsigned long) sizeof (count), >> - paddress (get_current_arch (), >> - bfd_offset)); >> - break; >> - >> - default: >> - error (_("Bad entry type in core file %ps."), >> - styled_string (file_name_style.style (), >> - bfd_get_filename (&cbfd))); >> - break; >> + record_full_read_entry_from_bfd (&cbfd, osec, &bfd_offset); >> } >> >> - /* Add rec to record arch list. */ >> - record_full_arch_list_add (rec); >> + record_full_save_instruction (); >> } >> } >> catch (const gdb_exception &ex) >> { >> - record_full_list_release (record_full_arch_list_tail); >> + record_full_reset_incomplete (); >> throw; >> } >> >> - /* Add record_full_arch_list_head to the end of record list. */ >> - record_full_first.next = record_full_arch_list_head; >> - record_full_arch_list_head->prev = &record_full_first; >> - record_full_arch_list_tail->next = NULL; >> - record_full_list = &record_full_first; >> - >> /* Update record_full_insn_max_num. */ >> if (record_full_insn_num > record_full_insn_max_num) >> { >> @@ -2505,6 +2335,10 @@ record_full_restore (struct bfd &cbfd) >> record_full_insn_max_num); >> } >> >> + /* When loading a recording, we'll always start at the oldest possible >> + instruction, no matter where the original recording was stopped. */ >> + record_full_next_insn = 0; >> + >> /* Succeeded. */ >> gdb_printf (_("Restored records from core file %s.\n"), >> bfd_get_filename (&cbfd)); >> @@ -2538,13 +2372,73 @@ cmd_record_full_restore (const char *args, int >> from_tty) >> record_full_open (nullptr, from_tty); >> } >> >> +static void >> +record_full_write_entry_to_bfd (record_full_entry &entry, >> + gdb_bfd_ref_ptr obfd, >> + asection *osec, int *bfd_offset, >> + struct gdbarch *gdbarch) >> +{ >> + /* 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); >> + >> + switch (entry.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; >> + >> + 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; >> + } >> + >> +} >> + >> /* Save the execution log to a file. We use a modified elf corefile >> format, with an extra section for our data. */ >> >> void >> record_full_base_target::save_record (const char *recfilename) >> { >> - struct record_full_entry *cur_record_full_list; >> uint32_t magic; >> struct gdbarch *gdbarch; >> int save_size = 0; >> @@ -2562,9 +2456,6 @@ record_full_base_target::save_record (const char >> *recfilename) >> /* Arrange to remove the output file on failure. */ >> gdb::unlinker unlink_file (recfilename); >> >> - /* Save the current record entry to "cur_record_full_list". */ >> - cur_record_full_list = record_full_list; >> - >> /* Get the values of regcache and gdbarch. */ >> regcache *regcache = get_thread_regcache (inferior_thread ()); >> gdbarch = regcache->arch (); >> @@ -2574,34 +2465,27 @@ record_full_base_target::save_record (const >> char *recfilename) >> = record_full_gdb_operation_disable_set (); >> >> /* Reverse execute to the begin of record list. */ >> - while (1) >> - { >> - /* Check for beginning and end of log. */ >> - if (record_full_list == &record_full_first) >> - break; >> - >> - record_full_exec_insn (regcache, gdbarch, record_full_list); >> - >> - if (record_full_list->prev) >> - record_full_list = record_full_list->prev; >> - } >> + for (int i = record_full_next_insn - 1; i >= 0; i--) >> + record_full_exec_insn (regcache, gdbarch, >> + record_full_list[i]); >> >> /* Compute the size needed for the extra bfd section. */ >> save_size = 4; /* magic cookie */ >> - for (record_full_list = record_full_first.next; record_full_list; >> - record_full_list = record_full_list->next) >> - switch (record_full_list->type) >> - { >> - case record_full_end: >> - save_size += 1 + 4 + 4; >> - break; >> - case record_full_reg: >> - save_size += 1 + 4 + record_full_list->u.reg.len; >> - break; >> - case record_full_mem: >> - save_size += 1 + 4 + 8 + record_full_list->u.mem.len; >> - break; >> - } >> + for (int i = record_full_list.size () - 1; i >= 0; i--) >> + { >> + /* 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) >> + { >> + case record_full_reg: >> + save_size += 1 + 4 + entry.u.reg.len; >> + break; >> + case record_full_mem: >> + save_size += 1 + 4 + 8 + entry.u.mem.len; >> + break; >> + } >> + } >> >> /* Make the new bfd section. */ >> osec = bfd_make_section_anyway_with_flags (obfd.get (), "precord", >> @@ -2630,108 +2514,32 @@ record_full_base_target::save_record (const >> char *recfilename) >> >> /* Save the entries to recfd and forward execute to the end of >> record list. */ >> - record_full_list = &record_full_first; >> - while (1) >> + for (int i = 0; i < record_full_list.size (); i++) >> { >> - /* Save entry. */ >> - if (record_full_list != &record_full_first) >> + uint32_t eff_count = (uint32_t) record_full_list[i].effects.size (); >> + uint32_t insn_num = record_full_list[i].insn_num; >> + uint8_t sigval = (record_full_list[i].sigval.has_value ()) >> + ? record_full_list[i].sigval.value () >> + : GDB_SIGNAL_0; >> + >> + /* Signal. */ >> + bfdcore_write (obfd.get (), osec, &sigval, sizeof (sigval), &bfd_offset); >> + eff_count = netorder32 (eff_count); >> + /* Number of effects. */ >> + bfdcore_write (obfd.get (), osec, &eff_count, sizeof (eff_count), >> + &bfd_offset); >> + /* Instruction number. */ >> + bfdcore_write (obfd.get (), osec, &insn_num, sizeof (uint32_t), >> + &bfd_offset); >> + >> + for (auto &entry : record_full_list[i].effects) >> { >> - uint8_t type; >> - uint32_t regnum, len, signal, count; >> - uint64_t addr; >> - >> - type = record_full_list->type; >> - bfdcore_write (obfd.get (), osec, &type, sizeof (type), &bfd_offset); >> - >> - switch (record_full_list->type) >> - { >> - case record_full_reg: /* reg */ >> - if (record_debug) >> - gdb_printf (gdb_stdlog, >> - " Writing register %d (1 " >> - "plus %lu plus %d bytes)\n", >> - record_full_list->u.reg.num, >> - (unsigned long) sizeof (regnum), >> - record_full_list->u.reg.len); >> - >> - /* Write regnum. */ >> - regnum = netorder32 (record_full_list->u.reg.num); >> - bfdcore_write (obfd.get (), osec, ®num, >> - sizeof (regnum), &bfd_offset); >> - >> - /* Write regval. */ >> - bfdcore_write (obfd.get (), osec, >> - record_full_get_loc (record_full_list), >> - record_full_list->u.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, >> - record_full_list->u.mem.addr), >> - (unsigned long) sizeof (addr), >> - (unsigned long) sizeof (len), >> - record_full_list->u.mem.len); >> - >> - /* Write memlen. */ >> - len = netorder32 (record_full_list->u.mem.len); >> - bfdcore_write (obfd.get (), osec, &len, sizeof (len), >> - &bfd_offset); >> - >> - /* Write memaddr. */ >> - addr = netorder64 (record_full_list->u.mem.addr); >> - bfdcore_write (obfd.get (), osec, &addr, >> - sizeof (addr), &bfd_offset); >> - >> - /* Write memval. */ >> - bfdcore_write (obfd.get (), osec, >> - record_full_get_loc (record_full_list), >> - record_full_list->u.mem.len, &bfd_offset); >> - break; >> - >> - case record_full_end: >> - if (record_debug) >> - gdb_printf (gdb_stdlog, >> - " Writing record_full_end (1 + " >> - "%lu + %lu bytes)\n", >> - (unsigned long) sizeof (signal), >> - (unsigned long) sizeof (count)); >> - /* Write signal value. */ >> - signal = netorder32 (record_full_list->u.end.sigval); >> - bfdcore_write (obfd.get (), osec, &signal, >> - sizeof (signal), &bfd_offset); >> - >> - /* Write insn count. */ >> - count = netorder32 (record_full_list->u.end.insn_num); >> - bfdcore_write (obfd.get (), osec, &count, >> - sizeof (count), &bfd_offset); >> - break; >> - } >> + record_full_write_entry_to_bfd (entry, obfd, osec, &bfd_offset, >> + gdbarch); >> } >> - >> /* Execute entry. */ >> - record_full_exec_insn (regcache, gdbarch, record_full_list); >> - >> - if (record_full_list->next) >> - record_full_list = record_full_list->next; >> - else >> - break; >> - } >> - >> - /* Reverse execute to cur_record_full_list. */ >> - while (1) >> - { >> - /* Check for beginning and end of log. */ >> - if (record_full_list == cur_record_full_list) >> - break; >> - >> - record_full_exec_insn (regcache, gdbarch, record_full_list); >> - >> - if (record_full_list->prev) >> - record_full_list = record_full_list->prev; >> + if (i < record_full_next_insn) >> + record_full_exec_insn (regcache, gdbarch, record_full_list[i]); >> } >> >> unlink_file.keep (); >> @@ -2746,7 +2554,7 @@ record_full_base_target::save_record (const char >> *recfilename) >> correspondingly. */ >> >> static void >> -record_full_goto_insn (struct record_full_entry *entry, >> +record_full_goto_insn (size_t target_insn, >> enum exec_direction_kind dir) > You could explain the new parameter in the function description. > >> { >> scoped_restore restore_operation_disable >> @@ -2757,17 +2565,12 @@ record_full_goto_insn (struct record_full_entry >> *entry, >> /* Assume everything is valid: we will hit the entry, >> and we will not hit the end of the recording. */ >> >> - if (dir == EXEC_FORWARD) >> - record_full_list = record_full_list->next; >> - >> - do >> - { >> - record_full_exec_insn (regcache, gdbarch, record_full_list); >> - if (dir == EXEC_REVERSE) >> - record_full_list = record_full_list->prev; >> - else >> - record_full_list = record_full_list->next; >> - } while (record_full_list != entry); >> + if (dir == EXEC_REVERSE) >> + for (int i = record_full_next_insn; i > target_insn; i--) >> + record_full_exec_insn (regcache, gdbarch, record_full_list[i - 1]); >> + else >> + for (int i = record_full_next_insn; i < target_insn; i++) >> + record_full_exec_insn (regcache, gdbarch, record_full_list[i]); >> } >> >> /* Alias for "target record-full". */ >> @@ -2798,61 +2601,36 @@ set_record_full_insn_max_num (const char >> *args, int from_tty, >> static void >> maintenance_print_record_instruction (const char *args, int from_tty) >> { >> - struct record_full_entry *to_print = record_full_list; >> + if (record_full_list.empty ()) >> + error (_("Not enough recorded history")); >> >> + int offset = record_full_next_insn - 1; >> + /* Reduce the offset by 1 if the record_full_next_insn is after the end >> + so that we show the last recorded instruction instead of crashing. */ >> + if (offset == record_full_list.size ()) >> + offset--; >> if (args != nullptr) >> { >> - int offset = value_as_long (parse_and_eval (args)); >> - if (offset > 0) >> - { >> - /* Move forward OFFSET instructions. We know we found the >> - end of an instruction when to_print->type is record_full_end. */ >> - while (to_print->next != nullptr && offset > 0) >> - { >> - to_print = to_print->next; >> - if (to_print->type == record_full_end) >> - offset--; >> - } >> - if (offset != 0) >> - error (_("Not enough recorded history")); >> - } >> - else >> - { >> - while (to_print->prev != nullptr && offset < 0) >> - { >> - to_print = to_print->prev; >> - if (to_print->type == record_full_end) >> - offset++; >> - } >> - if (offset != 0) >> - error (_("Not enough recorded history")); >> - } >> + offset += value_as_long (parse_and_eval (args)); >> + if (offset >= record_full_list.size () || offset < 0) >> + error (_("Not enough recorded history")); >> } >> - gdb_assert (to_print != nullptr); >> + auto to_print = record_full_list.begin () + offset; >> >> gdbarch *arch = current_inferior ()->arch (); >> >> - /* Go back to the start of the instruction. */ >> - while (to_print->prev != nullptr && to_print->prev->type != record_full_end) >> - to_print = to_print->prev; >> - >> - /* if we're in the first record, there are no actual instructions >> - recorded. Warn the user and leave. */ >> - if (to_print == &record_full_first) >> - error (_("Not enough recorded history")); >> - >> - while (to_print->type != record_full_end) >> + for (auto entry : to_print->effects) >> { >> - switch (to_print->type) >> + switch (entry.type) >> { >> case record_full_reg: >> { >> - type *regtype = gdbarch_register_type (arch, to_print- >>> u.reg.num); >> + type *regtype = gdbarch_register_type (arch, entry.u.reg.num); >> value *val >> = value_from_contents (regtype, >> - record_full_get_loc (to_print)); >> + record_full_get_loc (&entry)); >> gdb_printf ("Register %s changed: ", >> - gdbarch_register_name (arch, to_print- >>> u.reg.num)); >> + gdbarch_register_name (arch, entry.u.reg.num)); >> struct value_print_options opts; >> get_user_print_options (&opts); >> opts.raw = true; >> @@ -2862,17 +2640,16 @@ maintenance_print_record_instruction (const >> char *args, int from_tty) >> } >> case record_full_mem: >> { >> - gdb_byte *b = record_full_get_loc (to_print); >> + gdb_byte *b = record_full_get_loc (&entry); >> gdb_printf ("%d bytes of memory at address %s changed from:", >> - to_print->u.mem.len, >> - print_core_address (arch, to_print->u.mem.addr)); >> - for (int i = 0; i < to_print->u.mem.len; i++) >> + entry.u.mem.len, >> + print_core_address (arch, entry.u.mem.addr)); >> + for (int i = 0; i < entry.u.mem.len; i++) >> gdb_printf (" %02x", b[i]); >> gdb_printf ("\n"); >> break; >> } >> } >> - to_print = to_print->next; >> } >> } >> >> @@ -2880,11 +2657,6 @@ INIT_GDB_FILE (record_full) >> { >> struct cmd_list_element *c; >> >> - /* Init record_full_first. */ >> - record_full_first.prev = NULL; >> - record_full_first.next = NULL; >> - record_full_first.type = record_full_end; >> - >> add_target (record_full_target_info, record_full_open); >> add_deprecated_target_alias (record_full_target_info, "record"); >> add_target (record_full_core_target_info, record_full_open); >> diff --git a/gdb/record-full.h b/gdb/record-full.h >> index 51effe74560..c327d879a8a 100644 >> --- a/gdb/record-full.h >> +++ b/gdb/record-full.h >> @@ -41,7 +41,6 @@ enum record_result >> >> extern int record_full_arch_list_add_reg (struct regcache *regcache, int >> num); >> extern int record_full_arch_list_add_mem (CORE_ADDR addr, int len); >> -extern int record_full_arch_list_add_end (void); >> >> /* Returns true if the process record target is open. */ >> extern int record_full_is_used (void); >> diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c >> index 6b5c9c02d46..f5b1f66844c 100644 >> --- a/gdb/riscv-tdep.c >> +++ b/gdb/riscv-tdep.c >> @@ -5466,8 +5466,5 @@ riscv_process_record (struct gdbarch *gdbarch, >> struct regcache *regcache, >> if (res != RECORD_SUCCESS) >> return res; >> >> - if (record_full_arch_list_add_end ()) >> - return -1; >> - >> return 0; >> } >> diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c >> index e135df94a40..f964889a49f 100644 >> --- a/gdb/rs6000-tdep.c >> +++ b/gdb/rs6000-tdep.c >> @@ -7138,8 +7138,6 @@ ppc_process_prefix_instruction (int insn_prefix, int >> insn_suffix, >> if (record_full_arch_list_add_reg (regcache, PPC_PC_REGNUM)) >> return -1; >> >> - if (record_full_arch_list_add_end ()) >> - return -1; >> return 0; >> } >> >> @@ -7447,8 +7445,6 @@ ppc_process_record (struct gdbarch *gdbarch, >> struct regcache *regcache, >> >> if (record_full_arch_list_add_reg (regcache, PPC_PC_REGNUM)) >> return -1; >> - if (record_full_arch_list_add_end ()) >> - return -1; >> return 0; >> } >> >> diff --git a/gdb/s390-linux-tdep.c b/gdb/s390-linux-tdep.c >> index 3cd6b359d13..6bea30f08dc 100644 >> --- a/gdb/s390-linux-tdep.c >> +++ b/gdb/s390-linux-tdep.c >> @@ -910,9 +910,6 @@ s390_linux_record_signal (struct gdbarch *gdbarch, >> struct regcache *regcache, >> if (record_full_arch_list_add_mem (sp, sizeof_rt_sigframe)) >> return -1; >> >> - if (record_full_arch_list_add_end ()) >> - return -1; >> - >> return 0; >> } >> >> diff --git a/gdb/s390-tdep.c b/gdb/s390-tdep.c >> index f9d7bdd04e4..36ce659ea15 100644 >> --- a/gdb/s390-tdep.c >> +++ b/gdb/s390-tdep.c >> @@ -7019,8 +7019,6 @@ s390_process_record (struct gdbarch *gdbarch, >> struct regcache *regcache, >> >> if (record_full_arch_list_add_reg (regcache, S390_PSWA_REGNUM)) >> return -1; >> - if (record_full_arch_list_add_end ()) >> - return -1; >> return 0; >> } >> >> -- >> 2.53.0 >> > Kind Regards, > 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 >