Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [PATCH] [gdb/python]: add the orginal_value field in RegisterChangedEvent
@ 2025-09-28 11:18 guoshengyuan1
  2025-09-28 12:09 ` Eli Zaretskii
  2025-09-28 15:59 ` Andrew Burgess
  0 siblings, 2 replies; 7+ messages in thread
From: guoshengyuan1 @ 2025-09-28 11:18 UTC (permalink / raw)
  To: gdb-patches; +Cc: guoshengyuan1

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.

Signed-off-by: guoshengyuan1 <guoshengyuan1@xiaomi.com>
---
 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.
+@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<ptid_t /* thread */, CORE_ADDR /* address */>
     inferior_call_post;

 /* A register in the inferior has been modified by the gdb user.  */
-extern observable<const frame_info_ptr &/* frame */, int /* regnum */>
+extern observable<const frame_info_ptr &/* frame */, int /* regnum */,
+                 LONGEST /* original_value */>
     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 <http://www.gnu.org/licenses/>.  */

+#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 (&register_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 (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))


 def memory_changed_handler(event):
diff --git a/gdb/tui/tui-hooks.c b/gdb/tui/tui-hooks.c
index 024fedd9955..e6bc56a26d4 100644
--- a/gdb/tui/tui-hooks.c
+++ b/gdb/tui/tui-hooks.c
@@ -17,6 +17,7 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */

+#include "gdbsupport/common-types.h"
 #include "symtab.h"
 #include "inferior.h"
 #include "symfile.h"
@@ -47,7 +48,8 @@ tui_new_objfile_hook (struct objfile* objfile)
 /* Observer for the register_changed notification.  */

 static void
-tui_register_changed (const frame_info_ptr &frame, int regno)
+tui_register_changed (const frame_info_ptr &frame, int regno,
+                     LONGEST original_value)
 {
   frame_info_ptr fi;

diff --git a/gdb/valops.c b/gdb/valops.c
index fa87546770a..df8582336d2 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -1194,6 +1194,8 @@ value_assign (struct value *toval, struct value *fromval)
       {
        frame_info_ptr next_frame = frame_find_by_id (toval->next_frame_id ());
        int value_reg = toval->regnum ();
+       const auto original_value_reg = value_as_long
+         (get_frame_register_value(frame_find_by_id(old_frame), value_reg));

        if (next_frame == nullptr)
          error (_("Value being assigned to is no longer active."));
@@ -1255,7 +1257,7 @@ value_assign (struct value *toval, struct value *fromval)
          }

        gdb::observers::register_changed.notify
-         (get_prev_frame_always (next_frame), value_reg);
+         (get_prev_frame_always (next_frame), value_reg, original_value_reg);
        break;
       }

--
2.47.3

#/******±¾Óʼþ¼°Æä¸½¼þº¬ÓÐСÃ×¹«Ë¾µÄ±£ÃÜÐÅÏ¢£¬½öÏÞÓÚ·¢Ë͸øÉÏÃæµØÖ·ÖÐÁгöµÄ¸öÈË»òȺ×é¡£½ûÖ¹ÈÎºÎÆäËûÈËÒÔÈκÎÐÎʽʹÓ㨰üÀ¨µ«²»ÏÞÓÚÈ«²¿»ò²¿·ÖµØÐ¹Â¶¡¢¸´ÖÆ¡¢»òÉ¢·¢£©±¾ÓʼþÖеÄÐÅÏ¢¡£Èç¹ûÄú´íÊÕÁ˱¾Óʼþ£¬ÇëÄúÁ¢¼´µç»°»òÓʼþ֪ͨ·¢¼þÈ˲¢É¾³ý±¾Óʼþ£¡ This e-mail and its attachments contain confidential information from XIAOMI, which is intended only for the person or entity whose address is listed above. Any use of the information contained herein in any way (including, but not limited to, total or partial disclosure, reproduction, or dissemination) by persons other than the intended recipient(s) is prohibited. If you receive this e-mail in error, please notify the sender by phone or email immediately and delete it!******/#

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] [gdb/python]: add the orginal_value field in RegisterChangedEvent
  2025-09-28 11:18 [PATCH] [gdb/python]: add the orginal_value field in RegisterChangedEvent guoshengyuan1
@ 2025-09-28 12:09 ` Eli Zaretskii
  2025-09-29  4:23   ` [External Mail]Re: " 郭胜元
  2025-09-28 15:59 ` Andrew Burgess
  1 sibling, 1 reply; 7+ messages in thread
From: Eli Zaretskii @ 2025-09-28 12:09 UTC (permalink / raw)
  To: guoshengyuan1; +Cc: gdb-patches

> From: guoshengyuan1 <guoshengyuan1@xiaomi.com>
> CC: guoshengyuan1 <guoshengyuan1@xiaomi.com>
> Date: Sun, 28 Sep 2025 19:18:08 +0800
> 
> 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.
> 
> Signed-off-by: guoshengyuan1 <guoshengyuan1@xiaomi.com>
> ---
>  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(-)

Thanks, the python.texi part is okay.

Should this be called out in NEWS as well?

Reviewed-By: Eli Zaretskii <eliz@gnu.org>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] [gdb/python]: add the orginal_value field in RegisterChangedEvent
  2025-09-28 11:18 [PATCH] [gdb/python]: add the orginal_value field in RegisterChangedEvent guoshengyuan1
  2025-09-28 12:09 ` Eli Zaretskii
@ 2025-09-28 15:59 ` Andrew Burgess
  2025-09-29  7:47   ` [External Mail]Re: " 郭胜元
  1 sibling, 1 reply; 7+ messages in thread
From: Andrew Burgess @ 2025-09-28 15:59 UTC (permalink / raw)
  To: guoshengyuan1, gdb-patches; +Cc: guoshengyuan1

guoshengyuan1 <guoshengyuan1@xiaomi.com> 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 <guoshengyuan1@xiaomi.com>

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<ptid_t /* thread */, CORE_ADDR /* address */>
>      inferior_call_post;
>
>  /* A register in the inferior has been modified by the gdb user.  */
> -extern observable<const frame_info_ptr &/* frame */, int /* regnum */>
> +extern observable<const frame_info_ptr &/* frame */, int /* regnum */,
> +                 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 <http://www.gnu.org/licenses/>.  */
>
> +#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 (&register_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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [External Mail]Re: [PATCH] [gdb/python]: add the orginal_value field in RegisterChangedEvent
  2025-09-28 12:09 ` Eli Zaretskii
@ 2025-09-29  4:23   ` 郭胜元
  2025-09-29 11:09     ` Eli Zaretskii
  0 siblings, 1 reply; 7+ messages in thread
From: 郭胜元 @ 2025-09-29  4:23 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 2108 bytes --]

在 2025/9/28 20:09, Eli Zaretskii 写道:

From: guoshengyuan1 <guoshengyuan1@xiaomi.com><mailto:guoshengyuan1@xiaomi.com>
CC: guoshengyuan1 <guoshengyuan1@xiaomi.com><mailto:guoshengyuan1@xiaomi.com>
Date: Sun, 28 Sep 2025 19:18:08 +0800

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.

Signed-off-by: guoshengyuan1 <guoshengyuan1@xiaomi.com><mailto:guoshengyuan1@xiaomi.com>
---
 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(-)



Thanks, the python.texi part is okay.

Should this be called out in NEWS as well?

Reviewed-By: Eli Zaretskii <eliz@gnu.org><mailto:eliz@gnu.org>


What you said makes sense. I think this API change should be mentioned in the NEWS. Do I need to write the NEWS?

#/******本邮件及其附件含有小米公司的保密信息,仅限于发送给上面地址中列出的个人或群组。禁止任何其他人以任何形式使用(包括但不限于全部或部分地泄露、复制、或散发)本邮件中的信息。如果您错收了本邮件,请您立即电话或邮件通知发件人并删除本邮件! This e-mail and its attachments contain confidential information from XIAOMI, which is intended only for the person or entity whose address is listed above. Any use of the information contained herein in any way (including, but not limited to, total or partial disclosure, reproduction, or dissemination) by persons other than the intended recipient(s) is prohibited. If you receive this e-mail in error, please notify the sender by phone or email immediately and delete it!******/#

[-- Attachment #2: Type: text/html, Size: 2872 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [External Mail]Re: [PATCH] [gdb/python]: add the orginal_value field in RegisterChangedEvent
  2025-09-28 15:59 ` Andrew Burgess
@ 2025-09-29  7:47   ` 郭胜元
  2025-10-06 16:01     ` Andrew Burgess
  0 siblings, 1 reply; 7+ messages in thread
From: 郭胜元 @ 2025-09-29  7:47 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: gdb-patches

在 2025/9/28 23:59, Andrew Burgess 写道:
> guoshengyuan1 <guoshengyuan1@xiaomi.com> 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 <guoshengyuan1@xiaomi.com>
> 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<ptid_t /* thread */, CORE_ADDR /* address */>
>>       inferior_call_post;
>>
>>   /* A register in the inferior has been modified by the gdb user.  */
>> -extern observable<const frame_info_ptr &/* frame */, int /* regnum */>
>> +extern observable<const frame_info_ptr &/* frame */, int /* regnum */,
>> +                 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 <http://www.gnu.org/licenses/>.  */
>>
>> +#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 (&register_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
>

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 patch


 From 3b0b5ac6c4679aae5adc166e6ad76a12a6f183d9 Mon Sep 17 00:00:00 2001
From: guoshengyuan1 <guoshengyuan1@xiaomi.com>
Date: Sun, 28 Sep 2025 17:35:10 +0800
Subject: [PATCH] [gdb/python]: add the orginal_value field in
  RegisterChangedEvent

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.
---
  gdb/doc/python.texi                    |  3 +++
  gdb/observable.h                       |  3 ++-
  gdb/python/py-event.h                  |  2 +-
  gdb/python/py-inferior.c               |  5 +++--
  gdb/python/py-infevents.c              | 18 +++++++++++++++---
  gdb/testsuite/gdb.python/py-events.exp |  3 +++
  gdb/testsuite/gdb.python/py-events.py  |  1 +
  gdb/tui/tui-hooks.c                    |  3 ++-
  gdb/valops.c                           |  4 +++-
  9 files changed, 33 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.
+@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..c67377fbaab 100644
--- a/gdb/observable.h
+++ b/gdb/observable.h
@@ -222,7 +222,8 @@ extern observable<ptid_t /* thread */, CORE_ADDR /*
address */>
      inferior_call_post;

  /* A register in the inferior has been modified by the gdb user. */
-extern observable<const frame_info_ptr &/* frame */, int /* regnum */>
+extern observable<const frame_info_ptr &/* frame */, int /* regnum */,
+         struct value * /* original_value */>
      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..b751bc3f00f 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, struct value *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..db9a11b3f2c 100644
--- a/gdb/python/py-inferior.c
+++ b/gdb/python/py-inferior.c
@@ -158,11 +158,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,
+              struct value *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..8f062b485a7 100644
--- a/gdb/python/py-infevents.c
+++ b/gdb/python/py-infevents.c
@@ -62,7 +62,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,
+                     struct value *original_value)
  {
    gdbpy_ref<> event = create_event_object
(&register_changed_event_object_type);
    if (event == NULL)
@@ -82,6 +83,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 {value_to_value_object (original_value)};
+  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 +160,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,
+                struct value *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.exp
b/gdb/testsuite/gdb.python/py-events.exp
index b440ac4db6a..f70e6c1fa0b 100644
--- a/gdb/testsuite/gdb.python/py-events.exp
+++ b/gdb/testsuite/gdb.python/py-events.exp
@@ -110,16 +110,19 @@ gdb_test_sequence {set $sp = 0} "" {
      "event type: register-changed"
      "frame: "
      "num: "
+    "original_value: "
  }
  gdb_test_sequence {set $sp = 1} "" {
      "event type: register-changed"
      "frame: "
      "num: "
+    "original_value: "
  }
  gdb_test_sequence {set $sp = $old_sp} "" {
      "event type: register-changed"
      "frame: "
      "num: "
+    "original_value: "
  }

  # Test that no register_changed event is generated on "non-user"
diff --git a/gdb/testsuite/gdb.python/py-events.py
b/gdb/testsuite/gdb.python/py-events.py
index 0573b16659b..1207aa4e959 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("original_value: %s" % (event.original_value))


  def memory_changed_handler(event):
diff --git a/gdb/tui/tui-hooks.c b/gdb/tui/tui-hooks.c
index 024fedd9955..841de5a0b38 100644
--- a/gdb/tui/tui-hooks.c
+++ b/gdb/tui/tui-hooks.c
@@ -47,7 +47,8 @@ tui_new_objfile_hook (struct objfile* objfile)
  /* Observer for the register_changed notification.  */

  static void
-tui_register_changed (const frame_info_ptr &frame, int regno)
+tui_register_changed (const frame_info_ptr &frame, int regno,
+             struct value *original_value)
  {
    frame_info_ptr fi;

diff --git a/gdb/valops.c b/gdb/valops.c
index fa87546770a..02bed498be9 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -1194,6 +1194,8 @@ value_assign (struct value *toval, struct value
*fromval)
        {
      frame_info_ptr next_frame = frame_find_by_id (toval->next_frame_id
());
      int value_reg = toval->regnum ();
+    const auto frame_id = frame_find_by_id(old_frame);
+    const auto original_value_reg = get_frame_register_value(frame_id,
value_reg);

      if (next_frame == nullptr)
       error (_("Value being assigned to is no longer active."));
@@ -1255,7 +1257,7 @@ value_assign (struct value *toval, struct value
*fromval)
       }

      gdb::observers::register_changed.notify
-     (get_prev_frame_always (next_frame), value_reg);
+     (get_prev_frame_always (next_frame), value_reg, original_value_reg);
      break;
        }

--
2.47.3

#/******本邮件及其附件含有小米公司的保密信息,仅限于发送给上面地址中列出的个人或群组。禁止任何其他人以任何形式使用(包括但不限于全部或部分地泄露、复制、或散发)本邮件中的信息。如果您错收了本邮件,请您立即电话或邮件通知发件人并删除本邮件! This e-mail and its attachments contain confidential information from XIAOMI, which is intended only for the person or entity whose address is listed above. Any use of the information contained herein in any way (including, but not limited to, total or partial disclosure, reproduction, or dissemination) by persons other than the intended recipient(s) is prohibited. If you receive this e-mail in error, please notify the sender by phone or email immediately and delete it!******/#

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [External Mail]Re: [PATCH] [gdb/python]: add the orginal_value field in RegisterChangedEvent
  2025-09-29  4:23   ` [External Mail]Re: " 郭胜元
@ 2025-09-29 11:09     ` Eli Zaretskii
  0 siblings, 0 replies; 7+ messages in thread
From: Eli Zaretskii @ 2025-09-29 11:09 UTC (permalink / raw)
  To: 郭胜元; +Cc: gdb-patches

> From: 郭胜元 <guoshengyuan1@xiaomi.com>
> CC: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
> Date: Mon, 29 Sep 2025 04:23:43 +0000
> 
> 在 2025/9/28 20:09, Eli Zaretskii 写道: 
> 
>  From: guoshengyuan1 <guoshengyuan1@xiaomi.com>
> CC: guoshengyuan1 <guoshengyuan1@xiaomi.com>
> Date: Sun, 28 Sep 2025 19:18:08 +0800
> 
> 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.
> 
> Signed-off-by: guoshengyuan1 <guoshengyuan1@xiaomi.com>
> ---
>  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(-)
> 
> 
> Thanks, the python.texi part is okay.
> 
> Should this be called out in NEWS as well?
> 
> Reviewed-By: Eli Zaretskii <eliz@gnu.org>
> 
> What you said makes sense. I think this API change should be mentioned in the NEWS. Do I need to write
> the NEWS?

Yes, please.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [External Mail]Re: [PATCH] [gdb/python]: add the orginal_value field in RegisterChangedEvent
  2025-09-29  7:47   ` [External Mail]Re: " 郭胜元
@ 2025-10-06 16:01     ` Andrew Burgess
  0 siblings, 0 replies; 7+ messages in thread
From: Andrew Burgess @ 2025-10-06 16:01 UTC (permalink / raw)
  To: 郭胜元; +Cc: gdb-patches

郭胜元 <guoshengyuan1@xiaomi.com> writes:

> 在 2025/9/28 23:59, Andrew Burgess 写道:
>> guoshengyuan1 <guoshengyuan1@xiaomi.com> 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 <guoshengyuan1@xiaomi.com>
>> 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<ptid_t /* thread */, CORE_ADDR /* address */>
>>>       inferior_call_post;
>>>
>>>   /* A register in the inferior has been modified by the gdb user.  */
>>> -extern observable<const frame_info_ptr &/* frame */, int /* regnum */>
>>> +extern observable<const frame_info_ptr &/* frame */, int /* regnum */,
>>> +                 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 <http://www.gnu.org/licenses/>.  */
>>>
>>> +#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 (&register_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
>>
>
> 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 patch

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_Assignment

Or you can email "assign@gnu.org" and say that you have some gdb
patches; they will get you started.

Thanks,
Andrew


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2025-10-06 16:01 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-09-28 11:18 [PATCH] [gdb/python]: add the orginal_value field in RegisterChangedEvent guoshengyuan1
2025-09-28 12:09 ` Eli Zaretskii
2025-09-29  4:23   ` [External Mail]Re: " 郭胜元
2025-09-29 11:09     ` Eli Zaretskii
2025-09-28 15:59 ` Andrew Burgess
2025-09-29  7:47   ` [External Mail]Re: " 郭胜元
2025-10-06 16:01     ` Andrew Burgess

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox