From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id 2zRtJs12A2oHFjQAWB0awg (envelope-from ) for ; Tue, 12 May 2026 14:51:57 -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=gAjoExPT; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 89CBB1E067; Tue, 12 May 2026 14:51:57 -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 5D1BB1E067 for ; Tue, 12 May 2026 14:51:54 -0400 (EDT) Received: from vm01.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 55F474BA79A7 for ; Tue, 12 May 2026 18:51:53 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 55F474BA79A7 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=gAjoExPT 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 A11BE4BB1C3A for ; Tue, 12 May 2026 18:51:08 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org A11BE4BB1C3A 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 A11BE4BB1C3A 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=1778611868; cv=none; b=RZ6Wyk96Fm2cTwXCcvT7GE02mxPNvpMNuH5GyC1ThUs3ipSc49cnlEsHEllbZbLq7L1GaDdQQ0RLmtHShWW8yBvUWgE5kmW0qjZCDVLM9RAMxPXplINgfxE9/dxhKCDyDzd2RmGWiz6wWhOr2CoXQuzrJSswSy76cTf8mm8difA= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1778611868; c=relaxed/simple; bh=HYiNKewygoCg+YEMnUOYXI5bUZ3IgYNqsfxE1s+gp80=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=Fyjr5WqxA6LL3BskMPoOXEpOdgkgRAaiwgGu0CMliO63zKGeXMokSuPvywZ9rDFrzLMx4Im8K0MowPWnuS0JfrmD0bZCQQqsN+D/MKKFlo0uLxsi0/U3hPiyc0bBTIWs2EnTJjTnJXfWAEBZECGJEKNmsnW3HziXrVqDY1d70HU= 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=gAjoExPT DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org A11BE4BB1C3A DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1778611868; 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=J7zpli8IyeaHXyDn67hIVFHJeVgwXiMsN7xRBop2Lnc=; b=gAjoExPTa5JeQ62sNzDllThzw7xKSAuGtWrQsQwLkhRlVelm/nAY2x55kSj7GEmD+MhKlh ji7AubJrXHmnGTpJREg4cxyLTy/kPyJSIRKn+uX9rYFPAa3O7APLVMXC2XE/95PA7h4cYF kafvLdZXeOkSy5J6uWKIq3Qw75QM5h8= Received: from mail-dl1-f71.google.com (mail-dl1-f71.google.com [74.125.82.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-610-clwmbiaHOP-eDh_xDa08Dw-1; Tue, 12 May 2026 14:51:06 -0400 X-MC-Unique: clwmbiaHOP-eDh_xDa08Dw-1 X-Mimecast-MFC-AGG-ID: clwmbiaHOP-eDh_xDa08Dw_1778611866 Received: by mail-dl1-f71.google.com with SMTP id a92af1059eb24-1334825de51so789525c88.0 for ; Tue, 12 May 2026 11:51:06 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1778611865; x=1779216665; 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=J7zpli8IyeaHXyDn67hIVFHJeVgwXiMsN7xRBop2Lnc=; b=SAAo+3ek6Y9+300TtxMvPNJYJttJUN8F9LE3j4DftzDz2f/TXaCKwhD9JZc9HNl75I jCFcU/TUCr0rtNakSNiewQEoA6wY1qao62AgE3IUhTTK3oj/Lo+B8kw1kJKiL3rZ65jK QOQQTUkgilC/HkKhYEstmrRHcVwgotU0Ra+WE8wDwlozzM1gp2XO1LxEi7Aw8jjfTHEz epqxG2jRj6zjZqUTV8JkwN+h31QRxSR6mu13rRqs+juOsy0fBQd9GOXhtzyftyMeMafo c2QxUGY0dqzkBZrJiJqTmGyc8AVoC4VBKSPCmxA+GiYsMCdB03mX+Ka7h7uy0Ggs+tgR MLTw== X-Forwarded-Encrypted: i=1; AFNElJ9tQkfxTSvyNBFVAlKZac6XUthWKtdbmB2fiEebmA0YFftwCAcEVS2+ozJlxD7vXJgq3ZrpRYdxFjIItQ==@sourceware.org X-Gm-Message-State: AOJu0YxzaUvSGAgy/4XAFs41ydVDpMxBpV+bh/9XzQbcVrdWDA7dz7D5 KDNLj7oZzTZjHn4EtHB6HSbrWFBxDYLrQOULb97LARZPBEfisM5am7meODzPKapy3QqhkWgeCD4 tKztlBbDtF0t5Ka/8Yi6Ontq4mApL1O6hbbcWJT79ImTRNK5zWG76lHVNnJzGeF9jBeLhpV4= X-Gm-Gg: Acq92OGeUbsZ3SdkyRTpMzWBhwjtSziSpTmZIzTtlNkAUk0Doh5432c2OeGQB9e1lEx T6z/KU1RIgwhACKCog05KXmcVvseWgxos6iuns2AT88gMMMO5g6forBuvpPZdu+IFVuYr2uOFPz UgPs0GnMzrGA3h+3qCI5LQbe2lPqEBkPLoo4GLS/4D9AuhTq2CPAkTF+6YPJ3STU8MISpXbKuA/ 2t6g45BMx1/w9me8MngE1IHzod1b0hQWwQYu3quvpOvwNRecirRoy9KKMGKsDLwUQ/LNDhnwBpQ xo6E0r7yoQS1eGjXhup1VoWKdGtx23X2pB1JATLFFP15Az8BvGUoyDKHHzocqBhPmbipiwdeXtu kj3RIQpdmt89GP4fjfyAundUiqbfcVn0= X-Received: by 2002:a05:7022:1e04:b0:11b:9b98:aa4b with SMTP id a92af1059eb24-1342ef3d106mr128027c88.6.1778611864403; Tue, 12 May 2026 11:51:04 -0700 (PDT) X-Received: by 2002:a05:7022:1e04:b0:11b:9b98:aa4b with SMTP id a92af1059eb24-1342ef3d106mr127993c88.6.1778611863474; Tue, 12 May 2026 11:51:03 -0700 (PDT) Received: from ?IPV6:2804:14d:8084:993e::75d? ([2804:14d:8084:993e::75d]) by smtp.gmail.com with ESMTPSA id 5a478bee46e88-2f888e4016asm18823390eec.28.2026.05.12.11.51.01 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 12 May 2026 11:51:02 -0700 (PDT) Message-ID: <23236510-ef59-4396-91eb-5763725f01cc@redhat.com> Date: Tue, 12 May 2026 15:50:58 -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> <9de15785-0595-417b-b14d-fe5820738231@redhat.com> From: Guinevere Larsen In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: DrkYHoDouaaFwb-JRmaQ4ePkp5ws7w8WGr4aVkDMv60_1778611866 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/11/26 12:23 PM, Schimpe, Christina wrote: > Hi Guinevere, > > Please find some further comments below. Hi Christina! Thanks again for taking such a detailed look at this patch! I have some inlined comments, thanks for finding these issues! >> -----Original Message----- >> From: Guinevere Larsen >> Sent: Donnerstag, 7. Mai 2026 19:38 >> To: Schimpe, Christina ; gdb- >> patches@sourceware.org >> Subject: Re: [PATCH v2 1/6] gdb/record: Refactor record history >> >> 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; > It might be worth describing in the comment in which cases this does not > have a value, to point out the reason why this is now optional. > As I understand it before this patch we used GDB_SIGNAL_0 for this case, is that right? well, yeah, we used to use GDB_SIGNAL_0, but I think it is better to explicitly say "there was no signal" other than say "the signal was 0" which is treated as a nothing. It's the problem with NULL/nullptr, versus languages where nullptrs aren't allowed and instead accessing an unset pointer is just disallowed. I just want to add more mindful usage of it to make code safer. So I don't think it makes sense to add in a comment why it changed, when we instead needed a comment earlier about how to deal with "no signal was delivered" > >>>> + * 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; > Any particular reason why this is not static? no, I'll make that change. > >>>> /* 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 () > A function comment would be nice. > >>>> +{ >>>> + ++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); > I think this line will at some point call the default copy constructor of record_full_mem_entry > and record_full_reg_entry, which have those buffers: > > gdb_byte buf[sizeof (gdb_byte *)]; > > IIUC you'll only have a shallow copy. Should we use a std::move instead here? Might be worth, yeah... > > I further wonder if this problem might be related to the double free issues you > describe in patch #3. Would you mind checking? I did check it! When I was working on patch #3, I disabled copy construction, only allowed move constructor/assignment because I specifically thought that the vector push_back was the reason for the double frees, but it still happened. > And if you add the destructor, I think it would make sense to add a copy constructor etc., too: > https://en.cppreference.com/cpp/language/rule_of_three > >>>> + 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; > Similar comment as already in my earlier review: > Can record_full_next_insn be == 0 here or may record_full_list be empty ? Oh you're right. This is probably because I misunderstood the order of operations and was saving it to the current one, not the previous one, and when I fixed that I forgot to check the original > > I think it might be worth checking the patch if there are more similar issues. > The old patch had more guards to check this. yeah good point, I'll look through the changes and see if I notice it anywhere else. > > And I noticed in some other locations you cast it: > > if (sigval != GDB_SIGNAL_0) > record_full_incomplete_instruction.sigval = (gdb_signal) sigval; > > Not sure if this is necessary or not, but it might be for checking and making it consistent? I cast it in the other location because it is reading from the BFD and so sigval is declared as uint8_t, not as gdb_signal. > And is it intended that now we don't clear sigval anymore? > > Shouldn't we set it to std::nullopt in case signal == GDB_SIGNAL_0, to match the original > behaviour? yeah this is a mistake. The plan was that record_full_save_instruction clears the incomplete instruction, but I apparently forgot that. With that, it should always be clear so we won't need to explicitly set it to nullopt. I'll update the correct function. > >>>> 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 >>> ~~~ > I wonder, do we somewhere have a test for bookmarks? > I also find a specific description for this in the manual... what's the difference to checkpoints? Checkpoints fork the inferior and stop the fork immediately, allowing you to go back to that one point in time, but not executing backwards or anything, just changing forks and letting you see what things were like back then (that's why they won't work with multi-threaded programs). entirely unrelated to reverse debugging Bookmarks let you mark one instruction in the history and easily go back to it using the reverse mechanism... at least that's what I've been able to get from the code, this predates me and I have no idea how it is actually used.... I quickly grep'd for bookmark tests and couldn't find any. We should probably add them at some point, if anyone even uses that feature...... > > I am a bit confused about the bookmark feature (of course, this is nothing to fix for this series...). > >>>> 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 >>> > 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 > -- Cheers, Guinevere Larsen It/she