diff mbox series

gdb: new_objfile observer, update some comments, add some asserts

Message ID b47a1d4d614125f817320b6f1374fa1a5e988039.1773830615.git.aburgess@redhat.com
State New
Headers
Series gdb: new_objfile observer, update some comments, add some asserts |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 fail Patch failed to apply
linaro-tcwg-bot/tcwg_gdb_check--master-arm fail Patch failed to apply

Commit Message

Andrew Burgess March 18, 2026, 10:44 a.m. UTC 
  I was looking at the new_objfile observer and noticed a comment in
reread_symbols (symfile.c) that seemed out of date, it talked about
calling the new objfile observer with a NULL objfile argument.

A little digging indicates that the comment is indeed out of date, and
that we never call the new_objfile observer with a NULL argument any
more.

So in this commit I have:

  1. Updated the out of date comment in reread_symbols.

  2. Updated the comment in observable.h to indicate that the objfile
     argument should never be NULL.

  3. Gone through the existing new_objfile observers, and, where the
     objfile argument is used, added a gdb_assert that the argument is
     not NULL.  These observers are not called that often, so adding
     these asserts will have minimal performance impact.

There should be no user visible changes after this commit.
---
 gdb/ada-lang.c           | 1 +
 gdb/ada-tasks.c          | 1 +
 gdb/agent.c              | 2 ++
 gdb/arm-tdep.c           | 2 ++
 gdb/auto-load.c          | 2 ++
 gdb/linux-thread-db.c    | 2 ++
 gdb/observable.h         | 3 ++-
 gdb/python/py-inferior.c | 6 +++---
 gdb/remote.c             | 1 +
 gdb/symfile.c            | 4 ++--
 gdb/symtab.c             | 1 +
 11 files changed, 19 insertions(+), 6 deletions(-)


base-commit: e4fe38115fea0a9f357527959893f18ca8d51a03

Comments

Simon Marchi March 18, 2026, 2:39 p.m. UTC | #1 
On 2026-03-18 06:44, Andrew Burgess wrote:
> I was looking at the new_objfile observer and noticed a comment in
> reread_symbols (symfile.c) that seemed out of date, it talked about
> calling the new objfile observer with a NULL objfile argument.
> 
> A little digging indicates that the comment is indeed out of date, and
> that we never call the new_objfile observer with a NULL argument any
> more.

Indeed.
> 
> So in this commit I have:
> 
>   1. Updated the out of date comment in reread_symbols.
> 
>   2. Updated the comment in observable.h to indicate that the objfile
>      argument should never be NULL.
> 
>   3. Gone through the existing new_objfile observers, and, where the
>      objfile argument is used, added a gdb_assert that the argument is
>      not NULL.  These observers are not called that often, so adding
>      these asserts will have minimal performance impact.

You could also change the parameter to be a reference, this way it's
intrinsically never NULL.  It would seem better to me than changing all
observers to have an assert.

Simon
diff mbox series

Patch

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index eb4cd6d2f60..309d0210071 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -14061,6 +14061,7 @@  static struct cmd_list_element *show_ada_list;
 static void
 ada_new_objfile_observer (struct objfile *objfile)
 {
+  gdb_assert (objfile != nullptr);
   ada_clear_symbol_cache (objfile->pspace ());
 }
 
diff --git a/gdb/ada-tasks.c b/gdb/ada-tasks.c
index 0e124e1a2a2..ecf08bc144c 100644
--- a/gdb/ada-tasks.c
+++ b/gdb/ada-tasks.c
@@ -1468,6 +1468,7 @@  ada_tasks_clear_pspace_data (program_space *pspace)
 static void
 ada_tasks_new_objfile_observer (objfile *objfile)
 {
+  gdb_assert (objfile != nullptr);
   ada_tasks_clear_pspace_data (objfile->pspace ());
 }
 
diff --git a/gdb/agent.c b/gdb/agent.c
index beeaf62b552..533f9aaf657 100644
--- a/gdb/agent.c
+++ b/gdb/agent.c
@@ -70,6 +70,8 @@  agent_new_objfile (struct objfile *objfile)
   if (can_use_agent == can_use_agent_off)
     return;
 
+  gdb_assert (objfile != nullptr);
+
   agent_look_up_symbols (objfile);
 }
 
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index d7a9b469ef8..7fd512a8ca8 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -2537,6 +2537,8 @@  arm_exidx_new_objfile (struct objfile *objfile)
   bfd_vma exidx_vma = 0, extab_vma = 0;
   LONGEST i;
 
+  gdb_assert (objfile != nullptr);
+
   /* If we've already touched this file, do nothing.  */
   if (arm_exidx_data_key.get (objfile->obfd.get ()) != nullptr)
     return;
diff --git a/gdb/auto-load.c b/gdb/auto-load.c
index b5fec13743c..8e0812f1c50 100644
--- a/gdb/auto-load.c
+++ b/gdb/auto-load.c
@@ -1201,6 +1201,8 @@  auto_load_section_scripts (struct objfile *objfile, const char *section_name)
 void
 load_auto_scripts_for_objfile (struct objfile *objfile)
 {
+  gdb_assert (objfile != nullptr);
+
   /* Return immediately if auto-loading has been globally disabled.
      This is to handle sequencing of operations during gdb startup.
      Also return immediately if OBJFILE was not created from a file
diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c
index bd45a2f2978..654263081e6 100644
--- a/gdb/linux-thread-db.c
+++ b/gdb/linux-thread-db.c
@@ -1274,6 +1274,8 @@  thread_db_new_objfile (struct objfile *objfile)
   /* This observer must always be called with inferior_ptid set
      correctly.  */
 
+  gdb_assert (objfile != nullptr);
+
   if (/* libpthread with separate debug info has its debug info file already
 	 loaded (and notified without successful thread_db initialization)
 	 the time gdb::observers::new_objfile.notify is called for the library itself.
diff --git a/gdb/observable.h b/gdb/observable.h
index 89877950a3c..ebd3766a988 100644
--- a/gdb/observable.h
+++ b/gdb/observable.h
@@ -111,7 +111,8 @@  extern observable<solib &/* solib */> solib_loaded;
 extern observable<program_space *, const solib &/* solib */,
 		  bool /* still_in_use */, bool /* silent */> solib_unloaded;
 
-/* The symbol file specified by OBJFILE has been loaded.  */
+/* The symbol file specified by OBJFILE has been loaded.  OBJFILE will not
+   be NULL.  */
 extern observable<struct objfile */* objfile */> new_objfile;
 
 /*  All objfiles from PSPACE were removed.  */
diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
index 76e3da9f620..f9deba82d4b 100644
--- a/gdb/python/py-inferior.c
+++ b/gdb/python/py-inferior.c
@@ -163,9 +163,8 @@  python_inferior_exit (struct inferior *inf)
     gdbpy_print_stack ();
 }
 
-/* Callback used to notify Python listeners about new objfiles loaded in the
-   inferior.  OBJFILE may be NULL which means that the objfile list has been
-   cleared (emptied).  */
+/* Callback used to notify Python listeners that OBJFILE has been loaded in
+   to the inferior, OBJFILE will not be NULL.  */
 
 static void
 python_new_objfile (struct objfile *objfile)
@@ -173,6 +172,7 @@  python_new_objfile (struct objfile *objfile)
   if (!gdb_python_initialized)
     return;
 
+  gdb_assert (objfile != nullptr);
   gdbpy_enter enter_py (objfile->arch ());
 
   if (emit_new_objfile_event (objfile) < 0)
diff --git a/gdb/remote.c b/gdb/remote.c
index 88b06e688bc..e76f1ca0c08 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -16228,6 +16228,7 @@  remote_objfile_changed_check_symbols (program_space *pspace)
 static void
 remote_new_objfile (struct objfile *objfile)
 {
+  gdb_assert (objfile != nullptr);
   remote_objfile_changed_check_symbols (objfile->pspace ());
 }
 
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 44a9480d9d9..5eb137f3b39 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -2662,8 +2662,8 @@  reread_symbols (int from_tty)
     {
       clear_symtab_users (0);
 
-      /* The registry for each objfile was cleared and
-	 gdb::observers::new_objfile.notify (NULL) has been called by
+      /* The registry for each objfile was cleared and the
+	 all_objfiles_removed observer was notified by the call to
 	 clear_symtab_users above.  Notify the new files now.  */
       for (auto iter : new_objfiles)
 	gdb::observers::new_objfile.notify (iter);
diff --git a/gdb/symtab.c b/gdb/symtab.c
index cd3bf876551..444cf7dcb20 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -1700,6 +1700,7 @@  maintenance_print_symbol_cache_statistics (const char *args, int from_tty)
 static void
 symtab_new_objfile_observer (struct objfile *objfile)
 {
+  gdb_assert (objfile != nullptr);
   symbol_cache_flush (objfile->pspace ());
 }