From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id U/JhIXJb2WjibBcAWB0awg (envelope-from ) for ; Sun, 28 Sep 2025 11:59:46 -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=BHxINRzn; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 776971E0BA; Sun, 28 Sep 2025 11:59:46 -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 3A3751E047 for ; Sun, 28 Sep 2025 11:59:45 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 68DD43858C56 for ; Sun, 28 Sep 2025 15:59:44 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 68DD43858C56 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=BHxINRzn 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 BDE6E3858D33 for ; Sun, 28 Sep 2025 15:59:08 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org BDE6E3858D33 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 BDE6E3858D33 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1759075148; cv=none; b=rm2v7Imgv8r04f+wB6oHkDMntCSNN93oc8zJjBYzoPYbW3Qf38VGkNHRfqNc/NS4nZ8gvkAunhCzAHtZcX1NXuW+Sl4D+VclH22w/KWMgFqd4FRb+3eX+ZMTd4EAr08squaXhgRCl5kxgT0Hiyn506lDF8xld1sevAjWh0YKUV0= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1759075148; c=relaxed/simple; bh=kP676X6SREb3jmSA978b3hj129Xp9iHXvmDxMG3sAEg=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=gRbIB/yHObv2rDneYgem1HiKQTvjvZI+P/6dpUDWrK7tegRK0c31gqHFqxbr96dpfMviBlmejC7bHIrbduu8meSoM5LKIoJrDXrw8vbRegVKSzYNb5lSOkShCyyNLLHdsDNTGTDjH2LPaMZeY+p742L1x54jT9D+OcM3OK8LYik= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org BDE6E3858D33 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1759075148; 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: in-reply-to:in-reply-to:references:references; bh=15lXMl81iW9VnBew3fxGX0CN0tsGOVLoH7v6ibSsZck=; b=BHxINRzn5KAHY3B4QGn8RKMfUtqzzvUXjxfgLn4yYV4hA66zkTcRWuGaLHrSy+dFmNyqGz 3XlRD1vlAFoHACZ/vTL8GlmtuyHdjyKPegpw2ARRjQXVzMtgNN76b0XOzOlQVH+W6dG0vW +6nsP2/E6ZteV/h56ebsmDeSUWBNGzE= Received: from mail-wm1-f71.google.com (mail-wm1-f71.google.com [209.85.128.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-364-U7r8urf4OY6ELdsztPgFUQ-1; Sun, 28 Sep 2025 11:59:06 -0400 X-MC-Unique: U7r8urf4OY6ELdsztPgFUQ-1 X-Mimecast-MFC-AGG-ID: U7r8urf4OY6ELdsztPgFUQ_1759075145 Received: by mail-wm1-f71.google.com with SMTP id 5b1f17b1804b1-46e502a37cdso2165885e9.0 for ; Sun, 28 Sep 2025 08:59:06 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1759075145; x=1759679945; h=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=15lXMl81iW9VnBew3fxGX0CN0tsGOVLoH7v6ibSsZck=; b=NDfX55qQ+wBgGL+A3hgBPwQDcw+MVp7VK0cngpkixelfaNh/KzOoEmrYhaNnaNn34g QoI3bhrkG00dgE93Y7uO+X1dS3oSn1d0X2MaO9e3JDqDrqYA2aNidGZLxWyMxHno803A dwzO9ccj0ZqQOeQcMyAjHow07DvYQnDNCYP9qGcT9IUvmrxPNNolTRB0tYX9UUe6PMvQ +BW/FL3JdGZJRMZXKass45jV6q5EgommiLl/hpJa+l5JM7+j2/oAZOjprYY7ZSldO21N pDES6xq12Ciwh9cDCk8OGk0tGPrbwyJc9sJI7QL92cRCyjcJwcI8VSCL78KPAdwN7CFf mlTQ== X-Forwarded-Encrypted: i=1; AJvYcCU5ERO+JMFGkZ5UYvmOfxjEGlN2DBVCHb9RxT/7yqPm8RWkocfdf7xocXG1nKXG3UWuhoEzZc+hda2Jew==@sourceware.org X-Gm-Message-State: AOJu0Yyxk0g5HZY1Oa4CTjxkRk3qqN+VI3HWUicBkL+tODcco4yVNwI4 pz54K6fZS+q4oZdhxWdG9inVE4jbURPWParNZWVrZxgJHaSt9P+szurRFkTLCm4lS6OO9hy3g0N nhZPQedLv0S1aJroj2NblMvLXvm/EEJMjHcv1Km0cYBbiubOKoSSJLW8pQUQdIxZtlnre5M0= X-Gm-Gg: ASbGncsc4cS1IWG2cyotX8C6rYY/7leo1A7AL0bwWK3nyvZ0pG61kf8YMDqA3TshhdF 1nMIdX9UioB0f2cuRUhW9aQMEs7AFinn2wnzQLazsnZkh0GW/j04OerqbSpXjiLvbOb38S7Tra5 5SXg+KFEGo34oL+uZBZ6J6nNgzd02Km2e/cPUjoQwo8ZYDPb1G5jh24haPp8lp3hQirEzEyrZEe kUo4++Ton2hTBAlVsPkdVB23BDSjOfwyArkF43Uga5Nogk05SouPQX41F6WaQf6xFIHGFTW/6Ia DC+IdeodsOgTzd8EvMMFHGFwBPz3dEJpTCM= X-Received: by 2002:a05:600c:8b16:b0:45d:f7f9:9ac7 with SMTP id 5b1f17b1804b1-46e3299b8cbmr121848835e9.6.1759075145079; Sun, 28 Sep 2025 08:59:05 -0700 (PDT) X-Google-Smtp-Source: AGHT+IGo24R+Mc9qLEZBoSsp2ykBTFLa/nuUWOQ6WJMaevmCFNiLu6+LmGWaoShd9BKSy1jhMQsHqw== X-Received: by 2002:a05:600c:8b16:b0:45d:f7f9:9ac7 with SMTP id 5b1f17b1804b1-46e3299b8cbmr121848715e9.6.1759075144600; Sun, 28 Sep 2025 08:59:04 -0700 (PDT) Received: from localhost ([31.111.84.207]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-46e354e07f2sm70749465e9.9.2025.09.28.08.59.03 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 28 Sep 2025 08:59:04 -0700 (PDT) From: Andrew Burgess To: guoshengyuan1 , gdb-patches@sourceware.org Cc: guoshengyuan1 Subject: Re: [PATCH] [gdb/python]: add the orginal_value field in RegisterChangedEvent In-Reply-To: <20250928111808.271065-1-guoshengyuan1@xiaomi.com> References: <20250928111808.271065-1-guoshengyuan1@xiaomi.com> Date: Sun, 28 Sep 2025 16:59:02 +0100 Message-ID: <87ikh2efg9.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: 54VDFahkDkoAe3bdEKlgxa9VBdfliNcqD_pMmzE85gQ_1759075145 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~public-inbox=simark.ca@sourceware.org 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_Assignment 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 which 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_value); > 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 *inferior, 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 = create_event_object (®ister_changed_event_object_type); > if (event == NULL) > @@ -82,6 +84,15 @@ create_register_changed_event_object (const frame_info_ptr &frame, > if (evpy_add_attribute (event.get (), "regnum", regnum_obj.get ()) < 0) > return NULL; > > + gdbpy_ref<> original_value_obj = gdb_py_object_from_longest (original_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 == NULL) > + return NULL; > + > + auto original_value_status = 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 = create_register_changed_event_object (frame, regnum); > + gdbpy_ref<> event = create_register_changed_event_object > + (frame, regnum, original_value); > if (event != NULL) > return evpy_emit_event (event.get (), gdb_py_events.register_changed); > 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 = 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