From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id Sj3GG+nn42guxyEAWB0awg (envelope-from ) for ; Mon, 06 Oct 2025 12:01:45 -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=hOn/7ul1; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 5FBEF1E047; Mon, 06 Oct 2025 12:01:45 -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 server2.sourceware.org (server2.sourceware.org [8.43.85.97]) (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 7F65C1E047 for ; Mon, 06 Oct 2025 12:01:44 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 0383D3858D1E for ; Mon, 6 Oct 2025 16:01:44 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 0383D3858D1E 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=hOn/7ul1 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTP id 3E6A13858D1E for ; Mon, 6 Oct 2025 16:01:10 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 3E6A13858D1E 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 3E6A13858D1E Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1759766470; cv=none; b=GH1+SsVr5hZlM2qwrQIMKnh6AnD/SP2g3fF9p+qrxBVRSEHy1w1ouGKP2GqYu3eWRG9y51vw1pzs+m7nI3E53ZyJxffGSRMRl1ltwa4IN98pfTgotq6XzkKUnb4SwaQR3haQgI6pzR7qFvZpHVwXeyCo1RtIJTlGI/DKY/tennk= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1759766470; c=relaxed/simple; bh=coK+gv02RiMerL5Te4etB+/JvIpDJJ0yOwTKphQ9hH8=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=Lt9dVdsPNBcui1y1mos957zj5ak4x4NsEDl7/sYT3D448PWFVWQmE6j7tP1rEfC9q/LcAi8JIErG9dGQF17z5evlOfLQEe40+ubQH+ARFi9M50xSxNso5UEqeF+Ecfz3+zbBKpD1QJboLZm1UCemsAkrubAVieOC3yR1iCiyxII= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 3E6A13858D1E DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1759766470; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=GiAimJ92Iq1z4S6Z7rt6muQv1jV4WIFAwzY5jLJ3JEY=; b=hOn/7ul1bB/DwgkISxiBEDPRnRBA8AvMuGhLfMbUdh1q8SO0Xu5otO5kw3/gQTg0JBb3TJ 3c+3UBAexgq9BjCMlvD915nGqg3Rl1JKcE2nj1QjU1WRrpyPjug3DYVyV5EmErUAGtQEYI qwq6mTKzkPAJIiludBWUwMSLaSEYD1k= Received: from mail-wr1-f70.google.com (mail-wr1-f70.google.com [209.85.221.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-369-_fhW6GsOPcGLsaDYYct5AQ-1; Mon, 06 Oct 2025 12:01:08 -0400 X-MC-Unique: _fhW6GsOPcGLsaDYYct5AQ-1 X-Mimecast-MFC-AGG-ID: _fhW6GsOPcGLsaDYYct5AQ_1759766467 Received: by mail-wr1-f70.google.com with SMTP id ffacd0b85a97d-3f7b5c27d41so2805773f8f.0 for ; Mon, 06 Oct 2025 09:01:08 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1759766467; x=1760371267; h=content-transfer-encoding:mime-version:message-id:date:references :in-reply-to:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=GiAimJ92Iq1z4S6Z7rt6muQv1jV4WIFAwzY5jLJ3JEY=; b=tsGR5czha1g9Be01Xs+49Z17syTdmKbo6wSeundi3nIbsoQAc2u2WbOXSMHBa+jtaf sohmMF7fR72SEf+oV6yFghl0Q3RchwZhGz69NVVdMQGM6KebJfTBnX5rW2HQ19OwRzt5 Zhp8i3O/ZFWrqYaww+O0iodwUSdy2DginLC8DoAcoHajfxJEwm+/Nc6L0thx22RjGiyV 96Nzz/BXLSNPFR+EsAFOY7DeS1noiyxQSK6CBn+D5LSYEFUXGJ07CGRwPrJLVGkznwkw r1ub/H8Mt5UVt6wmJebMVfgrF/5VFezXe8V82dMBTYG7xvXZj7TdQyRsKbEcqz9xeAiJ fHrg== X-Gm-Message-State: AOJu0YwAB0eDDnTyohy2VaSVbC2pmaaIFDbdMsjZhWehsvyC8hsqXfOZ t14vSTuurYkDg7iBxniw+nwNcfM5xsgYltgiEPKrOtPxzU5oMK7SF6DC8XRNAvkOfVh2gpr9LZc OGkzytrKSvCfpsYDsY878mSg4ue7NwfTTznXurcbOIxhFCkqBJMnt/g3LBq+eqPPhvZV1TEk= X-Gm-Gg: ASbGncuxxaMKdF4jwbw5+wUEsUTQOLHPy320IerjAJ13G4MNk1eS7ppVGmzJdy0Mkli NNtr4MvkfINE1vNF3EKtjSNsGJ5bWaXnAgh/9EUxADywdoO5Got6boNDnuQ623A/h8+1GzupeBe ilmbTqyEYl+R9kSlliDaMNak22MwjHbpeN6hHGM9pfYizRmHORVaweQ5ct2pNe0sHmK/0J5AtO8 vn+n14R4g3otVls909H7cRnUBA1nNMSFYnFJAPtRKIECBPS9lMRvXNMHgjcf77o34KT8zU75CxE CorsKMZHbKY47Kezfa+8DtcsrxcCUsNxQlBJA2nH X-Received: by 2002:a05:6000:4202:b0:3d8:3eca:a97d with SMTP id ffacd0b85a97d-425829be09emr91177f8f.11.1759766466801; Mon, 06 Oct 2025 09:01:06 -0700 (PDT) X-Google-Smtp-Source: AGHT+IH0V19kHnKYN5P6GgAwV5493TYkTummmYCDiReBbvRsKRAwT28O4DC3Uahnh10I7kOlcT+SJQ== X-Received: by 2002:a05:6000:4202:b0:3d8:3eca:a97d with SMTP id ffacd0b85a97d-425829be09emr91136f8f.11.1759766466136; Mon, 06 Oct 2025 09:01:06 -0700 (PDT) Received: from localhost ([31.111.84.207]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-4255d8ab909sm21046592f8f.19.2025.10.06.09.01.05 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 06 Oct 2025 09:01:05 -0700 (PDT) From: Andrew Burgess To: =?utf-8?B?6YOt6IOc5YWD?= Cc: "gdb-patches@sourceware.org" Subject: Re: [External Mail]Re: [PATCH] [gdb/python]: add the orginal_value field in RegisterChangedEvent In-Reply-To: References: <20250928111808.271065-1-guoshengyuan1@xiaomi.com> <87ikh2efg9.fsf@redhat.com> Date: Mon, 06 Oct 2025 17:01:05 +0100 Message-ID: <87bjmk6mv2.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: uKMkrg4nmVxMiAirV7R0T4yCx2-Yt_3Rxn-6p3QP7Co_1759766467 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable 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 =E9=83=AD=E8=83=9C=E5=85=83 writes: > =E5=9C=A8 2025/9/28 23:59, Andrew Burgess =E5=86=99=E9=81=93: >> guoshengyuan1 writes: >> >>> When the register_changed event is triggered, the user should be told >>> what the previous value was so they can make a decision, rather >>> than just being told what the modified value is. >> Thanks for working on this. >> >>> Signed-off-by: guoshengyuan1 >> We don't currently use Signed-off-by headers in GDB. >> >> Do you currently have a copyright assignment in place? If not then take >> a look at: >> https://sourceware.org/gdb/wiki/ContributionChecklist#FSF_copyright_Assi= gnment >> and feel free to ask if you have any questions. >> >>> --- >>> gdb/doc/python.texi | 3 +++ >>> gdb/observable.h | 4 +++- >>> gdb/python/py-event.h | 2 +- >>> gdb/python/py-inferior.c | 6 ++++-- >>> gdb/python/py-infevents.c | 19 ++++++++++++++++--- >>> gdb/testsuite/gdb.python/py-events.py | 1 + >>> gdb/tui/tui-hooks.c | 4 +++- >>> gdb/valops.c | 4 +++- >>> 8 files changed, 34 insertions(+), 9 deletions(-) >>> >>> diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi >>> index 3763eee9d63..1d74910f400 100644 >>> --- a/gdb/doc/python.texi >>> +++ b/gdb/doc/python.texi >>> @@ -3979,6 +3979,9 @@ A gdb.Frame object representing the frame in whic= h the register was modified. >>> @defvar RegisterChangedEvent.regnum >>> Denotes which register was modified. >>> @end defvar >>> +@defvar RegisterChangedEvent.original_value >>> +Denotes the value of the register before it is modified. >> This should describe the type of original_value. More on this below... >> >>> +@end defvar >>> >>> @item events.breakpoint_created >>> This is emitted when a new breakpoint has been created. The argument >>> diff --git a/gdb/observable.h b/gdb/observable.h >>> index 4d913010c56..f65d42a2b00 100644 >>> --- a/gdb/observable.h >>> +++ b/gdb/observable.h >>> @@ -20,6 +20,7 @@ >>> #ifndef GDB_OBSERVABLE_H >>> #define GDB_OBSERVABLE_H >>> >>> +#include "gdbsupport/common-types.h" >>> #include "gdbsupport/observable.h" >>> #include "target/waitstatus.h" >>> >>> @@ -222,7 +223,8 @@ extern observable >>> inferior_call_post; >>> >>> /* A register in the inferior has been modified by the gdb user. */ >>> -extern observable >>> +extern observable>> + LONGEST /* original_value */> >> Not every register value can fit into a LONGEST. For example many >> architectures these days have vector registers which are longer than a >> LONGEST. >> >> You would be better passing a 'value *' around. >> >>> register_changed; >>> >>> /* The user-selected inferior, thread and/or frame has changed. The >>> diff --git a/gdb/python/py-event.h b/gdb/python/py-event.h >>> index 3938368c9ca..2b696e23ff8 100644 >>> --- a/gdb/python/py-event.h >>> +++ b/gdb/python/py-event.h >>> @@ -56,7 +56,7 @@ enum inferior_call_kind >>> extern int emit_inferior_call_event (inferior_call_kind kind, >>> ptid_t thread, CORE_ADDR addr); >>> extern int emit_register_changed_event (const frame_info_ptr &frame, >>> - int regnum); >>> + int regnum, LONGEST original_va= lue); >>> extern int emit_memory_changed_event (CORE_ADDR addr, ssize_t len); >>> extern int evpy_emit_event (PyObject *event, >>> eventregistry_object *registry); >>> diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c >>> index 2aa11d3160d..91f4bd862e5 100644 >>> --- a/gdb/python/py-inferior.c >>> +++ b/gdb/python/py-inferior.c >>> @@ -19,6 +19,7 @@ >>> >>> #include "auto-load.h" >>> #include "gdbcore.h" >>> +#include "gdbsupport/common-types.h" >>> #include "gdbthread.h" >>> #include "inferior.h" >>> #include "objfiles.h" >>> @@ -158,11 +159,12 @@ python_on_memory_change (struct inferior *inferio= r, CORE_ADDR addr, ssize_t len, >>> command). */ >>> >>> static void >>> -python_on_register_change (const frame_info_ptr &frame, int regnum) >>> +python_on_register_change (const frame_info_ptr &frame, int regnum, >>> + LONGEST orginal_value) >>> { >>> gdbpy_enter enter_py (current_inferior ()->arch ()); >>> >>> - if (emit_register_changed_event (frame, regnum) < 0) >>> + if (emit_register_changed_event (frame, regnum, orginal_value) < 0) >>> gdbpy_print_stack (); >>> } >>> >>> diff --git a/gdb/python/py-infevents.c b/gdb/python/py-infevents.c >>> index f74fb017edc..424abffcd1c 100644 >>> --- a/gdb/python/py-infevents.c >>> +++ b/gdb/python/py-infevents.c >>> @@ -17,6 +17,7 @@ >>> You should have received a copy of the GNU General Public License >>> along with this program. If not, see . */ >>> >>> +#include "gdbsupport/common-types.h" >>> #include "py-event.h" >>> >>> /* Construct either a gdb.InferiorCallPreEvent or a >>> @@ -62,7 +63,8 @@ create_inferior_call_event_object (inferior_call_kind= flag, ptid_t ptid, >>> >>> static gdbpy_ref<> >>> create_register_changed_event_object (const frame_info_ptr &frame, >>> - int regnum) >>> + int regnum, >>> + LONGEST original_value) >>> { >>> gdbpy_ref<> event =3D create_event_object (®ister_changed_event_= object_type); >>> if (event =3D=3D NULL) >>> @@ -82,6 +84,15 @@ create_register_changed_event_object (const frame_in= fo_ptr &frame, >>> if (evpy_add_attribute (event.get (), "regnum", regnum_obj.get ()) = < 0) >>> return NULL; >>> >>> + gdbpy_ref<> original_value_obj =3D gdb_py_object_from_longest (origi= nal_value); >> If the original_value argument becomes 'value *' then you should switch >> to value_to_value_object here which will give you a gdb.Value object, >> which I think would be the right thing to pass to the user. >> >>> + if (original_value_obj =3D=3D NULL) >>> + return NULL; >>> + >>> + auto original_value_status =3D evpy_add_attribute >>> + (event.get (), "original_value", original_value_obj.get ()); >>> + if (original_value_status < 0) >>> + return NULL; >>> + >>> return event; >>> } >>> >>> @@ -150,12 +161,14 @@ emit_memory_changed_event (CORE_ADDR addr, ssize_= t len) >>> will create a new Python register changed event object. */ >>> >>> int >>> -emit_register_changed_event (const frame_info_ptr &frame, int regnum) >>> +emit_register_changed_event (const frame_info_ptr &frame, int regnum, >>> + LONGEST original_value) >>> { >>> if (evregpy_no_listeners_p (gdb_py_events.register_changed)) >>> return 0; >>> >>> - gdbpy_ref<> event =3D create_register_changed_event_object (frame, r= egnum); >>> + gdbpy_ref<> event =3D create_register_changed_event_object >>> + (frame, regnum, original_value); >>> if (event !=3D NULL) >>> return evpy_emit_event (event.get (), gdb_py_events.register_chan= ged); >>> return -1; >>> diff --git a/gdb/testsuite/gdb.python/py-events.py b/gdb/testsuite/gdb.= python/py-events.py >>> index 0573b16659b..663b9fb2119 100644 >>> --- a/gdb/testsuite/gdb.python/py-events.py >>> +++ b/gdb/testsuite/gdb.python/py-events.py >>> @@ -90,6 +90,7 @@ def register_changed_handler(event): >>> assert isinstance(event.frame, gdb.Frame) >>> print("frame: %s" % (event.frame)) >>> print("num: %s" % (event.regnum)) >>> + print("num: %s" % (event.original_value)) >> I don't think using 'num:' here is correct. >> >> It's a shame that the event testing isn't very extensive in >> py-events.exp, but I think at a minimum you should extend these blocks: >> >> gdb_test_sequence {set $sp =3D 0} "" { >> "event type: register-changed" >> "frame: " >> "num: " >> } >> >> to look for whatever new tag you add. >> >> Ideally we'd be checking for the actual previous value, but given how >> limited the existing tests are this might be an unfair request. Still, >> if you were feeling adventurous... >> >> Thanks, >> Andrew >> > > Thank you for your review. I changed the value of original_value to > struct value and modified the testsuite part. The following is my new pat= ch Hi! Do you have a copyright assignment in place? If not then there's the web resource here: https://sourceware.org/gdb/wiki/ContributionChecklist#FSF_copyright_Assig= nment Or you can email "assign@gnu.org" and say that you have some gdb patches; they will get you started. Thanks, Andrew