public inbox for insight@sourceware.org
 help / color / mirror / Atom feed
From: Andrew Burgess <aburgess@redhat.com>
To: insight@sourceware.org
Cc: Andrew Burgess <aburgess@redhat.com>
Subject: [PATCH] switch away from using specify_exec_file_hook
Date: Tue, 12 Dec 2023 11:15:22 +0000	[thread overview]
Message-ID: <6abeb45e97d9004ec331e94cf2089af00553de76.1702379379.git.aburgess@redhat.com> (raw)

I had a go at testing this patch using make check-gdb
TESTS="gdb.gdbtk/*.exp", but ran into numerous errors, I suspect that
the gdbtk testing has not been run in a while, and it certainly
doesn't appear to have kept up with changes to the GDB testsuite.

If there's any particular testing that I should be running then just
let me know.

Otherwise, if we could merge this patch, then we can remove a couple
of functions from GDB that are only used to support this one hook.

---

Stop using specify_exec_file_hook to spot when the executable GDB is
debugging changes, and instead use gdb::observers::executable_changed.

The specify_exec_file_hook would call a function with the name of the
executable as specified by the user, so long as the name of the
executable wasn't empty, e.g.:

  (gdb) file exec_name

would call the hook with 'exec_name', while:

  (gdb) file

would not call the hook at all.

The observer I'm proposing we switch too is slightly different.  The
observer is called with the program_space in which the executable
changed, even in the empty filename case.  The observer also receives
a flag which indicates if GDB is loading a new executable, or is
loading the same executable because the on-disk file changed.

To reduce the chance of breakage, within insight, if the new observer
function is called in the empty filename case I don't call into the
TCL code, this ensures the TCL code is only called in the same cases a
previously.

The TCL code will be called with the full path to the executable
rather than the partial string the user specified, however, the TCL
code would (in some cases at least) look up the full executable path
anyway, so I don't think the change should have a negative impact.

For now I'm ignoring the reload_p flag, that is, I call into the TCL
code both when the executable has completely changed, and when it's
the same executable filename, but the on-disk file changed, I think
this matches the behaviour before this patch.

And finally, while looking at the TCL code, I spotted that a comment
in interface.tcl relating to this code was slightly out of date, I've
updated the comment.
---
 gdbtk/generic/gdbtk-hooks.c | 16 +++++++++++-----
 gdbtk/library/interface.tcl |  2 +-
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/gdbtk/generic/gdbtk-hooks.c b/gdbtk/generic/gdbtk-hooks.c
index 75e89e7..fe42e7d 100644
--- a/gdbtk/generic/gdbtk-hooks.c
+++ b/gdbtk/generic/gdbtk-hooks.c
@@ -83,7 +83,7 @@ static void gdbtk_trace_start_stop (int, int);
 static void gdbtk_attach (void);
 static void gdbtk_detach (void);
 static void gdbtk_file_changed (const char *);
-static void gdbtk_exec_file_display (const char *);
+static void gdbtk_executable_changed (program_space *, bool);
 static void gdbtk_call_command (struct cmd_list_element *, const char *, int);
 static void gdbtk_target_pre_wait (ptid_t ptid);
 static void gdbtk_target_post_wait (ptid_t event_ptid);
@@ -140,6 +140,8 @@ gdbtk_add_hooks (void)
                                           "gdbtk-hooks");
   gdb::observers::target_post_wait.attach (gdbtk_target_post_wait,
                                            "gdbtk-hooks");
+  gdb::observers::executable_changed.attach (gdbtk_executable_changed,
+                                             "gdbtk-hooks");
 
   /* Hooks */
   deprecated_call_command_hook = gdbtk_call_command;
@@ -157,7 +159,6 @@ gdbtk_add_hooks (void)
   deprecated_pre_add_symbol_hook = gdbtk_pre_add_symbol;
   deprecated_post_add_symbol_hook = gdbtk_post_add_symbol;
   deprecated_file_changed_hook = gdbtk_file_changed;
-  specify_exec_file_hook (gdbtk_exec_file_display);
 
   deprecated_attach_hook            = gdbtk_attach;
   deprecated_detach_hook            = gdbtk_detach;
@@ -732,11 +733,16 @@ gdbtk_file_changed (const char *filename)
   gdbtk_two_elem_cmd ("gdbtk_tcl_file_changed", filename);
 }
 
-/* Called from exec_file_command */
+/* Called from exec_file_attach when the executable changes.  PSPACE is
+   the program space in which the executable was changed.  RELOAD_P is
+   true if the executable didn't actually change, but instead GDB detected
+   that the on-disk file change, so reloaded the current executable.  */
 static void
-gdbtk_exec_file_display (const char *filename)
+gdbtk_executable_changed (program_space *pspace, bool reload_p)
 {
-  gdbtk_two_elem_cmd ("gdbtk_tcl_exec_file_display", filename);
+  if (pspace->exec_filename.get () != nullptr)
+    gdbtk_two_elem_cmd ("gdbtk_tcl_exec_file_display",
+                        pspace->exec_filename.get ());
 }
 
 /* Called from error_begin, this hook is used to warn the gui
diff --git a/gdbtk/library/interface.tcl b/gdbtk/library/interface.tcl
index c69e2f0..da0bcae 100644
--- a/gdbtk/library/interface.tcl
+++ b/gdbtk/library/interface.tcl
@@ -798,7 +798,7 @@ proc gdbtk_tcl_file_changed {filename} {
 
 # ------------------------------------------------------------------
 #  PROCEDURE: gdbtk_tcl_exec_file_display
-#         This hook is called from exec_file_command. It's purpose
+#         This hook is called from exec_file_attach. It's purpose
 #         is to setup the gui for a new file. Note that we cannot
 #         look for main, since this hook is called BEFORE we
 #         read symbols. If the user used the "file" command,

base-commit: 0b35bf269b3625d05764ebaeec45b32a2ac03321
-- 
2.25.4


             reply	other threads:[~2023-12-12 11:15 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-12 11:15 Andrew Burgess [this message]
2024-01-02 11:43 ` Ping: " Andrew Burgess

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6abeb45e97d9004ec331e94cf2089af00553de76.1702379379.git.aburgess@redhat.com \
    --to=aburgess@redhat.com \
    --cc=insight@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).