Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [PATCH] gdb: add debug_file_directory changed observer
@ 2026-10-21 21:47 Andrew Burgess
  2026-10-23 18:40 ` Tom Tromey
  0 siblings, 1 reply; 5+ messages in thread
From: Andrew Burgess @ 2026-10-21 21:47 UTC (permalink / raw)
  To: gdb-patches; +Cc: Andrew Burgess

Add a new observer gdb::observers::gdb_debugdir_changed, this is
triggered whenever the debug_file_directory is updated.

This new observer is analogous to the existing gdb_datadir_changed
observer.

I was motivated to add this when trying to understand some code in
auto-load.c, in file_is_auto_load_safe we call
filename_is_in_auto_load_safe_path_vec, and if this fails we call
auto_load_safe_path_vec_update, then
filename_is_in_auto_load_safe_path_vec again.  This seemed weird to
me, why would we need to update the safe path vector again at this
point?

I think there are two reasons why this might be needed (before this
patch):

 1. If the definition of $debugdir changes then the vector needs to be
    rebuilt based on the new value, and

 2. If any symlinks on the filesystem change the gdb_realpath might
    return new directory or filenames, which would then cause
    different things to match.

The first case here is similar to $datadir, but for $datadir we have
an observer which triggers an update of the safe path vector as soon
as the variable changes.  I figure that it makes sense to add an
observer for the $debugdir case too.

This leaves just case (2) as being the motivation for rebuilding the
vector in the auto-load code.  I've also added a comment about this
too the code.
---
 gdb/auto-load.c  | 18 +++++++++++++++---
 gdb/observable.c |  1 +
 gdb/observable.h |  4 ++++
 gdb/symfile.c    | 12 +++++++++++-
 4 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/gdb/auto-load.c b/gdb/auto-load.c
index ccbd091e1c2..6dff57ec38b 100644
--- a/gdb/auto-load.c
+++ b/gdb/auto-load.c
@@ -298,10 +298,11 @@ auto_load_safe_path_vec_update (void)
     }
 }
 
-/* Variable gdb_datadir has been set.  Update content depending on $datadir.  */
+/* One of the variables gdb_datadir or debug_file_directory might have
+   changed.  Update content depending on $datadir and $debugdir.  */
 
 static void
-auto_load_gdb_datadir_changed (void)
+auto_load_gdb_vars_changed ()
 {
   auto_load_safe_path_vec_update ();
 }
@@ -503,6 +504,15 @@ file_is_auto_load_safe (const char *filename)
   if (filename_is_in_auto_load_safe_path_vec (filename, &filename_real))
     return true;
 
+  /* The reason for rebuilding the vector, and retrying the match at
+     this point is in case the user has updated some symlinks on their
+     file system.  Rebuilding the safe path vector calls gdb_realpath
+     on each entry and adds the result to the vector.  As a result, if
+     any symlinks change then this update will change the set of paths
+     we consider safe.
+
+     This does mean that a stale entry might trigger a match in the
+     above check, for now though we just accept this risk.  */
   auto_load_safe_path_vec_update ();
   if (filename_is_in_auto_load_safe_path_vec (filename, &filename_real))
     return true;
@@ -1677,7 +1687,9 @@ This option has security implications for untrusted inferiors."),
 				     show_auto_load_safe_path,
 				     auto_load_set_cmdlist_get (),
 				     auto_load_show_cmdlist_get ());
-  gdb::observers::gdb_datadir_changed.attach (auto_load_gdb_datadir_changed,
+  gdb::observers::gdb_datadir_changed.attach (auto_load_gdb_vars_changed,
+					      "auto-load");
+  gdb::observers::gdb_debugdir_changed.attach (auto_load_gdb_vars_changed,
 					      "auto-load");
 
   cmd = add_cmd ("add-auto-load-safe-path", class_support,
diff --git a/gdb/observable.c b/gdb/observable.c
index 151014590a8..6a658303ee9 100644
--- a/gdb/observable.c
+++ b/gdb/observable.c
@@ -63,6 +63,7 @@ DEFINE_OBSERVABLE (inferior_cloned);
 DEFINE_OBSERVABLE (memory_changed);
 DEFINE_OBSERVABLE (before_prompt);
 DEFINE_OBSERVABLE (gdb_datadir_changed);
+DEFINE_OBSERVABLE (gdb_debugdir_changed);
 DEFINE_OBSERVABLE (inferior_call_pre);
 DEFINE_OBSERVABLE (inferior_call_post);
 DEFINE_OBSERVABLE (register_changed);
diff --git a/gdb/observable.h b/gdb/observable.h
index 979e9d1c3ac..9fe5cd44c4d 100644
--- a/gdb/observable.h
+++ b/gdb/observable.h
@@ -213,6 +213,10 @@ extern observable<const char */* current_prompt */> before_prompt;
    change.  */
 extern observable<> gdb_datadir_changed;
 
+/* Variable debug_file_directory has been updated.  The value may not
+   necessarily have changed.  */
+extern observable<> gdb_debugdir_changed;
+
 /* An inferior function at ADDRESS is about to be called in thread
    THREAD.  */
 extern observable<ptid_t /* thread */, CORE_ADDR /* address */>
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 017f7a49d8d..f8a7378120f 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -1340,6 +1340,16 @@ show_debug_file_directory (struct ui_file *file, int from_tty,
 	      value);
 }
 
+/* Implement 'set debug-file-directory' command.  */
+
+static void
+set_debug_file_directory (const char *args, int from_tty,
+			  struct cmd_list_element *e)
+{
+  /* The new value has already been installed by this point.  */
+  gdb::observers::gdb_debugdir_changed.notify ();
+}
+
 #if ! defined (DEBUG_SUBDIRECTORY)
 #define DEBUG_SUBDIRECTORY ".debug"
 #endif
@@ -3885,7 +3895,7 @@ Separate debug symbols are first searched for in the same\n\
 directory as the binary, then in the `" DEBUG_SUBDIRECTORY "' subdirectory,\n\
 and lastly at the path of the directory of the binary with\n\
 each global debug-file-directory component prepended."),
-				     NULL,
+				     set_debug_file_directory,
 				     show_debug_file_directory,
 				     &setlist, &showlist);
 

base-commit: 13db356ae83f72de0828e11826a256a4a28d17b1
-- 
2.25.4


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

* Re: [PATCH] gdb: add debug_file_directory changed observer
  2026-10-21 21:47 [PATCH] gdb: add debug_file_directory changed observer Andrew Burgess
@ 2026-10-23 18:40 ` Tom Tromey
  2026-10-28 11:40   ` [PATCHv2 0/2] Address issues with auto-load safe path checking Andrew Burgess
  0 siblings, 1 reply; 5+ messages in thread
From: Tom Tromey @ 2026-10-23 18:40 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: gdb-patches

>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:

Andrew> Add a new observer gdb::observers::gdb_debugdir_changed, this is
Andrew> triggered whenever the debug_file_directory is updated.

Andrew> This new observer is analogous to the existing gdb_datadir_changed
Andrew> observer.

The code looks good to me but I wonder if a test is possible and/or
appropriate; at least without undue effort.

Tom

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

* [PATCHv2 0/2] Address issues with auto-load safe path checking
  2026-10-23 18:40 ` Tom Tromey
@ 2026-10-28 11:40   ` Andrew Burgess
  2026-10-28 11:40     ` [PATCHv2 1/2] gdb: check auto-load safe path after refreshing cached vector Andrew Burgess
  2026-10-28 11:40     ` [PATCHv2 2/2] gdb: add debug_file_directory changed observer Andrew Burgess
  0 siblings, 2 replies; 5+ messages in thread
From: Andrew Burgess @ 2026-10-28 11:40 UTC (permalink / raw)
  To: gdb-patches; +Cc: Andrew Burgess, Tom Tromey

Tom,

You were right to ask for tests.  As is too often the case, as I wrote
the tests I ran into another bug in the same area, so now there are
two patches.

The first patch is new and addresses an existing bug which would cause
legitimate files to be rejected in some edge cases.

The second patch is the same code as you saw before (with a tweak to
the comment in observable.h), with the test from patch #1 extended to
cover this case too.

Thanks,
Andrew

---

Andrew Burgess (2):
  gdb: check auto-load safe path after refreshing cached vector
  gdb: add debug_file_directory changed observer

 gdb/auto-load.c                               |  20 ++-
 gdb/observable.c                              |   1 +
 gdb/observable.h                              |   4 +
 gdb/symfile.c                                 |  12 +-
 .../auto-load-after-debug-dir-change-1.gdb    |  17 +++
 .../auto-load-after-debug-dir-change-2.gdb    |  17 +++
 .../auto-load-after-debug-dir-change.exp      | 124 ++++++++++++++++++
 7 files changed, 190 insertions(+), 5 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/auto-load-after-debug-dir-change-1.gdb
 create mode 100644 gdb/testsuite/gdb.base/auto-load-after-debug-dir-change-2.gdb
 create mode 100644 gdb/testsuite/gdb.base/auto-load-after-debug-dir-change.exp


base-commit: b2fe3a449cb5d130ea2fec65bb214601826cdeff
-- 
2.25.4


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

* [PATCHv2 1/2] gdb: check auto-load safe path after refreshing cached vector
  2026-10-28 11:40   ` [PATCHv2 0/2] Address issues with auto-load safe path checking Andrew Burgess
@ 2026-10-28 11:40     ` Andrew Burgess
  2026-10-28 11:40     ` [PATCHv2 2/2] gdb: add debug_file_directory changed observer Andrew Burgess
  1 sibling, 0 replies; 5+ messages in thread
From: Andrew Burgess @ 2026-10-28 11:40 UTC (permalink / raw)
  To: gdb-patches; +Cc: Andrew Burgess, Tom Tromey

This commit fixes an issue that exists in the interaction between
file_is_auto_load_safe and filename_is_in_auto_load_safe_path_vec.  I
believe that this issue has existed since the code was originally
added in commit bccbefd2aab863e2.

The file_is_auto_load_safe function takes a FILENAME and checks if
that file is in the auto-load safe path.  To avoid the expense of
expanding the auto-load safe path on every check GDB pre-computes the
expanded safe path in the global AUTO_LOAD_SAFE_PATH_VEC.  The
filename_is_in_auto_load_safe_path_vec checks if FILENAME is within
the pre-computed vector or not.

However, FILENAME might also be a symlink to another file.  In this
case, we also want to check if the real file is in the pre-computed
vector.  To do this we can call gdb_realpath on FILENAME and check
this against the vector too.  This work is also done in the
filename_is_in_auto_load_safe_path_vec function.

If filename_is_in_auto_load_safe_path_vec doesn't find FILENAME or the
gdb_realpath equivalent in the auto-load safe path vector, then we
need to consider that the vector might be out of date and needs
recomputing.

This recomputing is done in file_is_auto_load_safe with a call to the
function auto_load_safe_path_vec_update.  After this we can call
filename_is_in_auto_load_safe_path_vec again to check against the
updated vector.

As both filename_is_in_auto_load_safe_path_vec calls might need to
call gdb_realpath, and calling gdb_realpath is not cheap, the
filename_is_in_auto_load_safe_path_vec allows the answer from the
first call to be saved and reused in the second call.  This also means
that should the filesystem change under GDB's feet then the same real
filename will be used by both filename_is_in_auto_load_safe_path_vec
calls.  This is the FILENAME_REALP argument that is passed to
filename_is_in_auto_load_safe_path_vec; this is initially NULL, and
might be set by the first call if FILENAME is not found in the
auto-load safe path vector.

There are some optimisations in place to try and avoid duplicate
work.  In filename_is_in_auto_load_safe_path_vec, if the computed
FILENAME_REALP value is the same as the original FILENAME, then there
is no need to check for FILENAME_REALP in AUTO_LOAD_SAFE_PATH_VEC.
This will be a very common case and occurs when FILENAME is the actual
file, not a symlink to some other file.

However, there is another piece of code which I think is a similar
idea, but is actually a mistake.  The first check in
filename_is_in_auto_load_safe_path_vec looks like this:

  for (const gdb::unique_xmalloc_ptr<char> &p : auto_load_safe_path_vec)
    if (*filename_realp == NULL && filename_is_in_pattern (filename, p.get ()))
      {
	pattern = p.get ();
	break;
      }

This is where we check if FILENAME is in AUTO_LOAD_SAFE_PATH_VEC.  The
first time that filename_is_in_auto_load_safe_path_vec is called the
*FILENAME_REALP will always be NULL (see file_is_auto_load_safe to
confirm this).

But if the first filename_is_in_auto_load_safe_path_vec call returns
false then gdb_realpath will have been called and *FILENAME_REALP will
no longer be NULL.  Only when the first call to
filename_is_in_auto_load_safe_path_vec returns false do we make a
second call to filename_is_in_auto_load_safe_path_vec.

This means that for the second filename_is_in_auto_load_safe_path_vec
call *FILENAME_REALP will never be NULL, and so based on the above
code, which checks '*filename_realp == NULL', we will not check if
FILENAME is in AUTO_LOAD_SAFE_PATH_VEC.

But we already discussed the optimisation that avoids checking for
FILENAME_REALP in AUTO_LOAD_SAFE_PATH_VEC when FILENAME and
FILENAME_REALP are the same (which is often the case).  So this means
that, when FILENAME is a real file, one for which gdb_realpath returns
the same filename unchanged, the second call to
filename_is_in_auto_load_safe_path_vec will actually not perform any
checks at all.

This feels like it has to be a mistake.

I think that this first '*filename_realp == NULL' check should be
removed.  Then, after we update the auto-load safe path vector, and
make a second call to filename_is_in_auto_load_safe_path_vec, we will
correctly check if FILENAME is in the now updated
AUTO_LOAD_SAFE_PATH_VEC.
---
 gdb/auto-load.c                               |   2 +-
 .../auto-load-after-debug-dir-change-1.gdb    |  17 +++
 .../auto-load-after-debug-dir-change-2.gdb    |  17 +++
 .../auto-load-after-debug-dir-change.exp      | 101 ++++++++++++++++++
 4 files changed, 136 insertions(+), 1 deletion(-)
 create mode 100644 gdb/testsuite/gdb.base/auto-load-after-debug-dir-change-1.gdb
 create mode 100644 gdb/testsuite/gdb.base/auto-load-after-debug-dir-change-2.gdb
 create mode 100644 gdb/testsuite/gdb.base/auto-load-after-debug-dir-change.exp

diff --git a/gdb/auto-load.c b/gdb/auto-load.c
index ccbd091e1c2..371d24c2b37 100644
--- a/gdb/auto-load.c
+++ b/gdb/auto-load.c
@@ -457,7 +457,7 @@ filename_is_in_auto_load_safe_path_vec (const char *filename,
   const char *pattern = NULL;
 
   for (const gdb::unique_xmalloc_ptr<char> &p : auto_load_safe_path_vec)
-    if (*filename_realp == NULL && filename_is_in_pattern (filename, p.get ()))
+    if (filename_is_in_pattern (filename, p.get ()))
       {
 	pattern = p.get ();
 	break;
diff --git a/gdb/testsuite/gdb.base/auto-load-after-debug-dir-change-1.gdb b/gdb/testsuite/gdb.base/auto-load-after-debug-dir-change-1.gdb
new file mode 100644
index 00000000000..05505085e7f
--- /dev/null
+++ b/gdb/testsuite/gdb.base/auto-load-after-debug-dir-change-1.gdb
@@ -0,0 +1,17 @@
+# Copyright 2026 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+echo script_1_loaded\n
+set $script_loaded=100
diff --git a/gdb/testsuite/gdb.base/auto-load-after-debug-dir-change-2.gdb b/gdb/testsuite/gdb.base/auto-load-after-debug-dir-change-2.gdb
new file mode 100644
index 00000000000..92bfe0f6b18
--- /dev/null
+++ b/gdb/testsuite/gdb.base/auto-load-after-debug-dir-change-2.gdb
@@ -0,0 +1,17 @@
+# Copyright 2026 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+echo script_2_loaded\n
+set $script_loaded=200
diff --git a/gdb/testsuite/gdb.base/auto-load-after-debug-dir-change.exp b/gdb/testsuite/gdb.base/auto-load-after-debug-dir-change.exp
new file mode 100644
index 00000000000..6cb7a0a8432
--- /dev/null
+++ b/gdb/testsuite/gdb.base/auto-load-after-debug-dir-change.exp
@@ -0,0 +1,101 @@
+# Copyright 2026 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test GDB's checking of the auto-load safe path via the $debugdir
+# variable.
+#
+# The test changes the auto-load search-path and the
+# debug-file-directory, and checks that GDB can keep up with these
+# changes as the executable is repeatedly reloaded.
+
+require {!is_remote host}
+
+standard_testfile main.c -1.gdb -2.gdb
+
+if { [build_executable "build executable" ${testfile} ${srcfile}] } {
+    return
+}
+
+set binfile_dir [file dirname $binfile]
+
+array set debug_dir {}
+foreach i { 1 2 } {
+    set debug_dir($i) [file normalize [standard_output_file "debug_dir_$i"]]
+    remote_exec host "mkdir -p $debug_dir($i)"
+    remote_exec host "mkdir -p $debug_dir($i)/$binfile_dir"
+}
+
+remote_exec host "cp $srcdir/$subdir/$srcfile2 $debug_dir(1)/$binfile_dir/$testfile-gdb.gdb"
+remote_exec host "cp $srcdir/$subdir/$srcfile3 $debug_dir(2)/$binfile_dir/$testfile-gdb.gdb"
+
+clean_restart
+
+# Only load scripts from the debug file directory.
+gdb_test_no_output "set auto-load safe-path \$debugdir"
+
+# Set the auto-load search path to be exactly the SEARCH_NUM entry in
+# the global debug_dir array.
+#
+# Then set the debug file directory to the DEBUG_NUM entry in the
+# global debug_dir array.  As GDB's default auto-load search path
+# includes '$debugdir', this means GDB will accept scripts found in
+# this directory.
+#
+# Finally, load the executable, and check to see if the expected
+# auto-load script was loaded.
+#
+# Every call to run_test_1 is done in the same GDB session so that any
+# caching of the auto-load safe path will carry over between calls to
+# this proc.
+proc run_test_1 { search_num debug_num } {
+    global debug_dir
+
+    gdb_test_no_output "set variable \$script_loaded = 0" \
+	"reset script_loaded variable to a known value"
+
+    # Where GDB should look for auto-load scripts.
+    gdb_test_no_output "set auto-load scripts-directory $debug_dir($search_num)" \
+	"setup scripts directory"
+
+    # The auto-load safe path is based on the debug file directory,
+    # which we setup here.
+    gdb_test_no_output "set debug-file-directory $debug_dir($debug_num)" \
+	"set debug-file-directory"
+
+    gdb_load ${::binfile}
+
+    gdb_test {print $script_loaded} " = ${search_num}00"
+
+    # Check that the expected script shows as auto-loaded in GDB's output.
+    gdb_assert {[regexp "script_${search_num}_loaded" $::gdb_file_cmd_msg]} \
+	"expected script was loaded"
+
+    # The '3 - SEARCH_NUM' converts: 2 => 1 and 1 => 2.  This checks
+    # that the other script was not auto-loaded.
+    gdb_assert {![regexp "script_[expr {3 - $search_num}]_loaded" $::gdb_file_cmd_msg]} \
+	"other script was not loaded"
+}
+
+# Wrapper around run_test_1 which adds a test prefix.  See run_test_1
+# for details.
+proc run_test { search_num debug_num } {
+    with_test_prefix "search=$search_num, debug=$debug_num" {
+	run_test_1 $search_num $debug_num
+    }
+}
+
+# Run the tests.
+run_test 1 1
+run_test 2 2
-- 
2.25.4


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

* [PATCHv2 2/2] gdb: add debug_file_directory changed observer
  2026-10-28 11:40   ` [PATCHv2 0/2] Address issues with auto-load safe path checking Andrew Burgess
  2026-10-28 11:40     ` [PATCHv2 1/2] gdb: check auto-load safe path after refreshing cached vector Andrew Burgess
@ 2026-10-28 11:40     ` Andrew Burgess
  1 sibling, 0 replies; 5+ messages in thread
From: Andrew Burgess @ 2026-10-28 11:40 UTC (permalink / raw)
  To: gdb-patches; +Cc: Andrew Burgess, Tom Tromey

Add a new observer gdb::observers::gdb_debugdir_changed; this is
triggered whenever the debug_file_directory is updated.

This new observer is analogous to the existing gdb_datadir_changed
observer.

I was motivated to add this when trying to understand some code in
auto-load.c.  In file_is_auto_load_safe we call
filename_is_in_auto_load_safe_path_vec, and if this fails we call
auto_load_safe_path_vec_update, then
filename_is_in_auto_load_safe_path_vec again.  This seemed weird to
me, why would we need to update the safe path vector again at this
point?

I think there are two reasons why this might be needed (before this
patch):

 1. If the definition of $debugdir changes then the vector needs to be
    rebuilt based on the new value, and

 2. If any symlinks on the filesystem change the gdb_realpath might
    return new directory or filenames, which would then cause
    different things to match.

The first case here is similar to $datadir, but for $datadir we have
an observer which triggers an update of the safe path vector as soon
as the variable changes.  I figure that it makes sense to add an
observer for the $debugdir case too.

This leaves just case (2) as being the motivation for rebuilding the
vector in the auto-load code.  I've also added a comment about this
to the code.

I extended the existing auto-load-after-debug-dir-change.exp test to
cover this case.

The setup for the new test is that we set the auto-load safe path to
'$debugdir', then set $debugdir to a directory D1 and cause GDB to
compute the auto-load safe path vector.  We can then set the auto load
script path to the same D1 directory.

At this point GDB will look for auto load script in D1, find that
these match the auto-load safe path, and load them.

Now if we change debug file directory (aka $debugdir) to a different
directory D2 and try loading an executable, GDB will use the cached
auto-load safe path which still contains D1, GDB will incorrectly load
the script when the script should be rejected.

With this patch the script will be rejected as expected.
---
 gdb/auto-load.c                               | 18 +++++++--
 gdb/observable.c                              |  1 +
 gdb/observable.h                              |  4 ++
 gdb/symfile.c                                 | 12 +++++-
 .../auto-load-after-debug-dir-change.exp      | 39 +++++++++++++++----
 5 files changed, 62 insertions(+), 12 deletions(-)

diff --git a/gdb/auto-load.c b/gdb/auto-load.c
index 371d24c2b37..8b597c8070b 100644
--- a/gdb/auto-load.c
+++ b/gdb/auto-load.c
@@ -298,10 +298,11 @@ auto_load_safe_path_vec_update (void)
     }
 }
 
-/* Variable gdb_datadir has been set.  Update content depending on $datadir.  */
+/* One of the variables gdb_datadir or debug_file_directory might have
+   changed.  Update content depending on $datadir and $debugdir.  */
 
 static void
-auto_load_gdb_datadir_changed (void)
+auto_load_gdb_vars_changed ()
 {
   auto_load_safe_path_vec_update ();
 }
@@ -503,6 +504,15 @@ file_is_auto_load_safe (const char *filename)
   if (filename_is_in_auto_load_safe_path_vec (filename, &filename_real))
     return true;
 
+  /* The reason for rebuilding the vector, and retrying the match at
+     this point is in case the user has updated some symlinks on their
+     file system.  Rebuilding the safe path vector calls gdb_realpath
+     on each entry and adds the result to the vector.  As a result, if
+     any symlinks change then this update will change the set of paths
+     we consider safe.
+
+     This does mean that a stale entry might trigger a match in the
+     above check, for now though we just accept this risk.  */
   auto_load_safe_path_vec_update ();
   if (filename_is_in_auto_load_safe_path_vec (filename, &filename_real))
     return true;
@@ -1677,7 +1687,9 @@ This option has security implications for untrusted inferiors."),
 				     show_auto_load_safe_path,
 				     auto_load_set_cmdlist_get (),
 				     auto_load_show_cmdlist_get ());
-  gdb::observers::gdb_datadir_changed.attach (auto_load_gdb_datadir_changed,
+  gdb::observers::gdb_datadir_changed.attach (auto_load_gdb_vars_changed,
+					      "auto-load");
+  gdb::observers::gdb_debugdir_changed.attach (auto_load_gdb_vars_changed,
 					      "auto-load");
 
   cmd = add_cmd ("add-auto-load-safe-path", class_support,
diff --git a/gdb/observable.c b/gdb/observable.c
index 151014590a8..6a658303ee9 100644
--- a/gdb/observable.c
+++ b/gdb/observable.c
@@ -63,6 +63,7 @@ DEFINE_OBSERVABLE (inferior_cloned);
 DEFINE_OBSERVABLE (memory_changed);
 DEFINE_OBSERVABLE (before_prompt);
 DEFINE_OBSERVABLE (gdb_datadir_changed);
+DEFINE_OBSERVABLE (gdb_debugdir_changed);
 DEFINE_OBSERVABLE (inferior_call_pre);
 DEFINE_OBSERVABLE (inferior_call_post);
 DEFINE_OBSERVABLE (register_changed);
diff --git a/gdb/observable.h b/gdb/observable.h
index 979e9d1c3ac..0581b48b711 100644
--- a/gdb/observable.h
+++ b/gdb/observable.h
@@ -213,6 +213,10 @@ extern observable<const char */* current_prompt */> before_prompt;
    change.  */
 extern observable<> gdb_datadir_changed;
 
+/* Variable debug_file_directory has been set.  The value may not
+   necessarily change.  */
+extern observable<> gdb_debugdir_changed;
+
 /* An inferior function at ADDRESS is about to be called in thread
    THREAD.  */
 extern observable<ptid_t /* thread */, CORE_ADDR /* address */>
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 017f7a49d8d..f8a7378120f 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -1340,6 +1340,16 @@ show_debug_file_directory (struct ui_file *file, int from_tty,
 	      value);
 }
 
+/* Implement 'set debug-file-directory' command.  */
+
+static void
+set_debug_file_directory (const char *args, int from_tty,
+			  struct cmd_list_element *e)
+{
+  /* The new value has already been installed by this point.  */
+  gdb::observers::gdb_debugdir_changed.notify ();
+}
+
 #if ! defined (DEBUG_SUBDIRECTORY)
 #define DEBUG_SUBDIRECTORY ".debug"
 #endif
@@ -3885,7 +3895,7 @@ Separate debug symbols are first searched for in the same\n\
 directory as the binary, then in the `" DEBUG_SUBDIRECTORY "' subdirectory,\n\
 and lastly at the path of the directory of the binary with\n\
 each global debug-file-directory component prepended."),
-				     NULL,
+				     set_debug_file_directory,
 				     show_debug_file_directory,
 				     &setlist, &showlist);
 
diff --git a/gdb/testsuite/gdb.base/auto-load-after-debug-dir-change.exp b/gdb/testsuite/gdb.base/auto-load-after-debug-dir-change.exp
index 6cb7a0a8432..4bcdb5670a2 100644
--- a/gdb/testsuite/gdb.base/auto-load-after-debug-dir-change.exp
+++ b/gdb/testsuite/gdb.base/auto-load-after-debug-dir-change.exp
@@ -76,16 +76,37 @@ proc run_test_1 { search_num debug_num } {
 
     gdb_load ${::binfile}
 
-    gdb_test {print $script_loaded} " = ${search_num}00"
+    if { $search_num == $debug_num } {
+	set expected_num "${search_num}00"
+    } else {
+	set expected_num "0"
+    }
 
-    # Check that the expected script shows as auto-loaded in GDB's output.
-    gdb_assert {[regexp "script_${search_num}_loaded" $::gdb_file_cmd_msg]} \
-	"expected script was loaded"
+    gdb_test {print $script_loaded} " = ${expected_num}"
 
-    # The '3 - SEARCH_NUM' converts: 2 => 1 and 1 => 2.  This checks
-    # that the other script was not auto-loaded.
-    gdb_assert {![regexp "script_[expr {3 - $search_num}]_loaded" $::gdb_file_cmd_msg]} \
-	"other script was not loaded"
+    if { $search_num == $debug_num } {
+	# Check that the expected script shows as auto-loaded in GDB's output.
+	gdb_assert {[regexp "script_${search_num}_loaded" \
+			 $::gdb_file_cmd_msg]} \
+	    "expected script was loaded"
+
+	# The '3 - SEARCH_NUM' converts: 2 => 1 and 1 => 2.  This
+	# checks that the other script was not auto-loaded.
+	gdb_assert {![regexp "script_[expr {3 - $search_num}]_loaded" \
+			  $::gdb_file_cmd_msg]} \
+	    "other script was not loaded"
+    } else {
+	set re "warning: File \[^\r\n\]+/debug_dir_${search_num}/\[^\r\n\]+ auto-loading has been declined"
+	gdb_assert {[regexp $re $::gdb_file_cmd_msg]} \
+	    "saw script rejected message"
+
+	gdb_assert {![regexp "script_1_loaded" \
+			  $::gdb_file_cmd_msg]} \
+	    "script 1 was not loaded"
+	gdb_assert {![regexp "script_2_loaded" \
+			  $::gdb_file_cmd_msg]} \
+	    "script 2 was not loaded"
+    }
 }
 
 # Wrapper around run_test_1 which adds a test prefix.  See run_test_1
@@ -99,3 +120,5 @@ proc run_test { search_num debug_num } {
 # Run the tests.
 run_test 1 1
 run_test 2 2
+run_test 2 1
+run_test 1 2
-- 
2.25.4


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

end of thread, other threads:[~2026-10-28 11:43 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-21 21:47 [PATCH] gdb: add debug_file_directory changed observer Andrew Burgess
2026-10-23 18:40 ` Tom Tromey
2026-10-28 11:40   ` [PATCHv2 0/2] Address issues with auto-load safe path checking Andrew Burgess
2026-10-28 11:40     ` [PATCHv2 1/2] gdb: check auto-load safe path after refreshing cached vector Andrew Burgess
2026-10-28 11:40     ` [PATCHv2 2/2] gdb: add debug_file_directory changed observer Andrew Burgess

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