[4/5] gdb: add program_space parameter to clear_solib

Message ID 20240206171514.119244-5-simon.marchi@efficios.com
State New
Headers
Series Random cleanup patches |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm fail Testing failed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 fail Testing failed

Commit Message

Simon Marchi Feb. 6, 2024, 5:14 p.m. UTC
  Make the current_program_space reference bubble up one level.

Remove one unnecessary declaration of clear_solib.

Change-Id: I234e2c8c0b71713364fc7b76cee2bee2b026bd6d
---
 gdb/corelow.c |  2 +-
 gdb/solib.c   | 21 ++++++++++++---------
 gdb/solib.h   |  6 +++---
 gdb/symtab.h  |  4 ----
 4 files changed, 16 insertions(+), 17 deletions(-)
  

Comments

Simon Marchi Feb. 7, 2024, 3:37 a.m. UTC | #1
On 2/6/24 12:14, Simon Marchi wrote:
> Make the current_program_space reference bubble up one level.
> 
> Remove one unnecessary declaration of clear_solib.
> 
> Change-Id: I234e2c8c0b71713364fc7b76cee2bee2b026bd6d
> ---
>  gdb/corelow.c |  2 +-
>  gdb/solib.c   | 21 ++++++++++++---------
>  gdb/solib.h   |  6 +++---
>  gdb/symtab.h  |  4 ----
>  4 files changed, 16 insertions(+), 17 deletions(-)
> 
> diff --git a/gdb/corelow.c b/gdb/corelow.c
> index 43c1f69b1317..f291b2aba191 100644
> --- a/gdb/corelow.c
> +++ b/gdb/corelow.c
> @@ -335,7 +335,7 @@ core_target::clear_core ()
>  
>        /* Clear out solib state while the bfd is still open.  See
>  	 comments in clear_solib in solib.c.  */
> -      clear_solib ();
> +      clear_solib (current_program_space);
>  
>        current_program_space->cbfd.reset (nullptr);
>      }
> diff --git a/gdb/solib.c b/gdb/solib.c
> index 98cda039a833..bd69c549b8e8 100644
> --- a/gdb/solib.c
> +++ b/gdb/solib.c
> @@ -1180,23 +1180,26 @@ solib_keep_data_in_core (CORE_ADDR vaddr, unsigned long size)
>      return false;
>  }
>  
> -/* Called by free_all_symtabs */
> +/* See solib.h.  */
>  
>  void
> -clear_solib (void)
> +clear_solib (program_space *pspace)
>  {
> -  const solib_ops *ops = gdbarch_so_ops (current_inferior ()->arch ());
> +  inferior *inf = find_inferior_for_program_space (pspace);
> +  gdb_assert (inf != nullptr);

Thanks to the Linaro CI for pointing it out, this particular change
causes some regressions, for instance in gdb.base/catch-fork-kill.exp.
The assertion fails because inf is nullptr.

We're here:

    #0  internal_error_loc (file=0x55adc73f51c0 "/home/smarchi/src/binutils-gdb/gdb/solib.c", line=1189,
        fmt=0x55adc73f4b00 "%s: Assertion `%s' failed.") at /home/smarchi/src/binutils-gdb/gdbsupport/errors.cc:57
    #1  0x000055adcb297e6d in clear_solib (pspace=0x613000011d80) at /home/smarchi/src/binutils-gdb/gdb/solib.c:1189
    #2  0x000055adcb29822e in no_shared_libraries (ignored=0x0, from_tty=0) at /home/smarchi/src/binutils-gdb/gdb/solib.c:1250
    #3  0x000055adca9f26d4 in program_space::~program_space (this=0x613000011d80, __in_chrg=<optimized out>)
        at /home/smarchi/src/binutils-gdb/gdb/progspace.c:120
    #4  0x000055adca0c570c in delete_inferior (inf=0x618000225480) at /home/smarchi/src/binutils-gdb/gdb/inferior.c:302
    #5  0x000055adca0c801d in prune_inferiors () at /home/smarchi/src/binutils-gdb/gdb/inferior.c:492

What I observe is that the current inferior is #1, bound to pspace #1,
and we're deleting inferior #2, bound to pspace #2.  My change doesn't
work, because at this point inferior #2 has been removed from the
inferior list, so find_inferior_for_program_space returns nullptr.

However, the current code seems wrong too, because we're using inferior
#1's arch to get hold of an solib_ops to call ops->clear_solib on,
despite deleting program space #2, which was bound to inferior #2.  It
works here, because both inferiors have the same gdbarch with the same
solib_ops.  But it would be possible for the two inferiors to have
different gdbarches and solib_ops.

My upcoming solib series should improve the situation, by having a
backlink from program_space to solib_ops.  So we won't need an inferior
here in order to get the solib_ops.  In the mean time, I'll try to
revert using current_inferior to get the arch and solib_ops, even though
I know it's wrong.

Simon
  

Patch

diff --git a/gdb/corelow.c b/gdb/corelow.c
index 43c1f69b1317..f291b2aba191 100644
--- a/gdb/corelow.c
+++ b/gdb/corelow.c
@@ -335,7 +335,7 @@  core_target::clear_core ()
 
       /* Clear out solib state while the bfd is still open.  See
 	 comments in clear_solib in solib.c.  */
-      clear_solib ();
+      clear_solib (current_program_space);
 
       current_program_space->cbfd.reset (nullptr);
     }
diff --git a/gdb/solib.c b/gdb/solib.c
index 98cda039a833..bd69c549b8e8 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -1180,23 +1180,26 @@  solib_keep_data_in_core (CORE_ADDR vaddr, unsigned long size)
     return false;
 }
 
-/* Called by free_all_symtabs */
+/* See solib.h.  */
 
 void
-clear_solib (void)
+clear_solib (program_space *pspace)
 {
-  const solib_ops *ops = gdbarch_so_ops (current_inferior ()->arch ());
+  inferior *inf = find_inferior_for_program_space (pspace);
+  gdb_assert (inf != nullptr);
+
+  const solib_ops *ops = gdbarch_so_ops (inf->arch ());
 
-  disable_breakpoints_in_shlibs (current_program_space);
+  disable_breakpoints_in_shlibs (pspace);
 
-  current_program_space->so_list.clear_and_dispose ([] (solib *so) {
-    notify_solib_unloaded (current_program_space, *so);
-    current_program_space->remove_target_sections (so);
+  pspace->so_list.clear_and_dispose ([pspace] (solib *so) {
+    notify_solib_unloaded (pspace, *so);
+    pspace->remove_target_sections (so);
     delete so;
   });
 
   if (ops->clear_solib != nullptr)
-    ops->clear_solib (current_program_space);
+    ops->clear_solib (pspace);
 }
 
 /* Shared library startup support.  When GDB starts up the inferior,
@@ -1244,7 +1247,7 @@  no_shared_libraries (const char *ignored, int from_tty)
      access to their associated objfiles.  Therefore, we can not purge the
      solibs' objfiles before clear_solib has been called.  */
 
-  clear_solib ();
+  clear_solib (current_program_space);
   objfile_purge_solibs ();
 }
 
diff --git a/gdb/solib.h b/gdb/solib.h
index 69183278318b..f7a93c0718f0 100644
--- a/gdb/solib.h
+++ b/gdb/solib.h
@@ -42,10 +42,10 @@  extern bool debug_solib;
 #define SOLIB_SCOPED_DEBUG_START_END(fmt, ...) \
   scoped_debug_start_end (debug_solib, "solib", fmt, ##__VA_ARGS__)
 
-/* Called when we free all symtabs, to free the shared library information
-   as well.  */
+/* Called when we free all symtabs of PSPACE, to free the shared library
+   information as well.  */
 
-extern void clear_solib (void);
+extern void clear_solib (program_space *pspace);
 
 /* Called to add symbols from a shared library to gdb's symbol table.  */
 
diff --git a/gdb/symtab.h b/gdb/symtab.h
index 3b067a77b3c8..ca5a5b0f7fde 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -2423,10 +2423,6 @@  extern bool find_line_pc_range (struct symtab_and_line, CORE_ADDR *,
 
 extern void resolve_sal_pc (struct symtab_and_line *);
 
-/* solib.c */
-
-extern void clear_solib (void);
-
 /* The reason we're calling into a completion match list collector
    function.  */
 enum class complete_symbol_mode