[PATCH] gdb: add debug_file_directory changed observer

[Andrew Burgess <aburgess@redhat.com>]

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