diff mbox series

[7/7] gdb: add program_space parameter to lookup_minimal_symbol_text

Message ID 20240717035307.2299961-8-simon.marchi@polymtl.ca
State New
Headers
Series Some more passing down program space |

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-arm success Test passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Test passed

Commit Message

Simon Marchi July 17, 2024, 3:52 a.m. UTC 
  Make the current program space reference bubble up one level.  Use a
program space from the context whenever that makes sense.

Change-Id: Id3b0bf4490178d71a9aecdbf404b9287c22b30f5
---
 gdb/breakpoint.c | 8 +++++---
 gdb/jit.c        | 2 +-
 gdb/minsyms.c    | 5 +++--
 gdb/minsyms.h    | 7 ++++---
 gdb/symtab.c     | 4 +++-
 5 files changed, 16 insertions(+), 10 deletions(-)

Comments

Tom Tromey Aug. 16, 2024, 5:30 p.m. UTC | #1 
>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:

Simon> Make the current program space reference bubble up one level.  Use a
Simon> program space from the context whenever that makes sense.

This patch causes a crash on Windows like so:

#0  0x00007ff6dcfca60a in find_pc_sect_line (pc=5368715072, section=0x0, notcurrent=0) at ../../src/gdb/symtab.c:3201 
#1  0x00007ff6dcfcba8d in find_function_start_sal_1 (func_addr=5368715072, section=0x0, funfirstline=true) at ../../src/gdb/symtab.c:3747 
#2  0x00007ff6dcfcbbf0 in find_function_start_sal (func_addr=5368715072, section=0x0, funfirstline=true) at ../../src/gdb/symtab.c:3786 
#3  0x00007ff6dce6b91e in minsym_found (self=0x5ff100, objfile=0x2cb2b90, msymbol=0x2cdebb0, result=0x5feff0) at ../../src/gdb/linespec.c:4142 

The basic issue is that here:

Simon> --- a/gdb/symtab.c
Simon> +++ b/gdb/symtab.c
Simon> @@ -3174,7 +3174,9 @@ find_pc_sect_line (CORE_ADDR pc, struct obj_section *section, int notcurrent)
Simon>      if (msymbol.minsym->type () == mst_solib_trampoline)
Simon>        {
Simon>  	bound_minimal_symbol mfunsym
Simon> -	  = lookup_minimal_symbol_text (msymbol.minsym->linkage_name (), NULL);
Simon> +	  = lookup_minimal_symbol_text (section->objfile->pspace (),
Simon> +					msymbol.minsym->linkage_name (),
Simon> +					nullptr);

... section==nullptr.  There seem to be a couple of spots, at least,
that do this.  I don't know why the crash isn't visible on Linux.

I'm going to test a patch to use current_program_space here instead, at
least when the section isn't provided.

Tom
Simon Marchi Aug. 16, 2024, 7:19 p.m. UTC | #2 
On 8/16/24 1:30 PM, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:
> 
> Simon> Make the current program space reference bubble up one level.  Use a
> Simon> program space from the context whenever that makes sense.
> 
> This patch causes a crash on Windows like so:
> 
> #0  0x00007ff6dcfca60a in find_pc_sect_line (pc=5368715072, section=0x0, notcurrent=0) at ../../src/gdb/symtab.c:3201 
> #1  0x00007ff6dcfcba8d in find_function_start_sal_1 (func_addr=5368715072, section=0x0, funfirstline=true) at ../../src/gdb/symtab.c:3747 
> #2  0x00007ff6dcfcbbf0 in find_function_start_sal (func_addr=5368715072, section=0x0, funfirstline=true) at ../../src/gdb/symtab.c:3786 
> #3  0x00007ff6dce6b91e in minsym_found (self=0x5ff100, objfile=0x2cb2b90, msymbol=0x2cdebb0, result=0x5feff0) at ../../src/gdb/linespec.c:4142 
> 
> The basic issue is that here:
> 
> Simon> --- a/gdb/symtab.c
> Simon> +++ b/gdb/symtab.c
> Simon> @@ -3174,7 +3174,9 @@ find_pc_sect_line (CORE_ADDR pc, struct obj_section *section, int notcurrent)
> Simon>      if (msymbol.minsym->type () == mst_solib_trampoline)
> Simon>        {
> Simon>  	bound_minimal_symbol mfunsym
> Simon> -	  = lookup_minimal_symbol_text (msymbol.minsym->linkage_name (), NULL);
> Simon> +	  = lookup_minimal_symbol_text (section->objfile->pspace (),
> Simon> +					msymbol.minsym->linkage_name (),
> Simon> +					nullptr);
> 
> ... section==nullptr.  There seem to be a couple of spots, at least,
> that do this.  I don't know why the crash isn't visible on Linux.
> 
> I'm going to test a patch to use current_program_space here instead, at
> least when the section isn't provided.
> 
> Tom

I think switching to current_program_space would be the right / safe
thing to do (it would have been the safe thing to do in the first
place).  The function uses current_program_space below anyway.

Thanks for dealing with this.

Simon
diff mbox series

Patch

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index cca17356b70d..3425bd76fbbd 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -3573,7 +3573,8 @@  create_overlay_event_breakpoint (void)
       if (bp_objfile_data->overlay_msym.minsym == NULL)
 	{
 	  bound_minimal_symbol m
-	    = lookup_minimal_symbol_text (func_name, objfile);
+	    = lookup_minimal_symbol_text (current_program_space, func_name,
+					  objfile);
 	  if (m.minsym == NULL)
 	    {
 	      /* Avoid future lookups in this objfile.  */
@@ -3677,7 +3678,8 @@  create_longjmp_master_breakpoint_names (objfile *objfile)
       if (bp_objfile_data->longjmp_msym[i].minsym == NULL)
 	{
 	  bound_minimal_symbol m
-	    = lookup_minimal_symbol_text (func_name, objfile);
+	    = lookup_minimal_symbol_text (objfile->pspace (), func_name,
+					  objfile);
 	  if (m.minsym == NULL)
 	    {
 	      /* Prevent future lookups in this objfile.  */
@@ -3847,7 +3849,7 @@  create_exception_master_breakpoint_hook (objfile *objfile)
   if (bp_objfile_data->exception_msym.minsym == NULL)
     {
       bound_minimal_symbol debug_hook
-	= lookup_minimal_symbol_text (func_name, objfile);
+	= lookup_minimal_symbol_text (objfile->pspace (), func_name, objfile);
       if (debug_hook.minsym == NULL)
 	{
 	  bp_objfile_data->exception_msym.minsym = &msym_not_found;
diff --git a/gdb/jit.c b/gdb/jit.c
index 2744d03f8f8d..d45133251906 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -880,7 +880,7 @@  jit_breakpoint_re_set_internal (struct gdbarch *gdbarch, program_space *pspace)
       /* Lookup the registration symbol.  If it is missing, then we
 	 assume we are not attached to a JIT.  */
       bound_minimal_symbol reg_symbol
-	= lookup_minimal_symbol_text (jit_break_name, the_objfile);
+	= lookup_minimal_symbol_text (pspace, jit_break_name, the_objfile);
       if (reg_symbol.minsym == NULL
 	  || reg_symbol.value_address () == 0)
 	{
diff --git a/gdb/minsyms.c b/gdb/minsyms.c
index 9c39749cbf76..59f0d5366eb6 100644
--- a/gdb/minsyms.c
+++ b/gdb/minsyms.c
@@ -607,7 +607,8 @@  lookup_minimal_symbol_linkage (program_space *pspace, const char *name,
 /* See minsyms.h.  */
 
 bound_minimal_symbol
-lookup_minimal_symbol_text (const char *name, struct objfile *objf)
+lookup_minimal_symbol_text (program_space *pspace, const char *name,
+			    objfile *objf)
 {
   struct minimal_symbol *msymbol;
   bound_minimal_symbol found_symbol;
@@ -643,7 +644,7 @@  lookup_minimal_symbol_text (const char *name, struct objfile *objf)
 
   if (objf == nullptr)
     {
-      for (objfile *objfile : current_program_space->objfiles ())
+      for (objfile *objfile : pspace->objfiles ())
 	{
 	  if (found_symbol.minsym != NULL)
 	    break;
diff --git a/gdb/minsyms.h b/gdb/minsyms.h
index f84d3e818480..9659f30dff8b 100644
--- a/gdb/minsyms.h
+++ b/gdb/minsyms.h
@@ -211,7 +211,7 @@  bound_minimal_symbol lookup_minimal_symbol (program_space *pspace,
 					    objfile *obj = nullptr,
 					    const char *sfile = nullptr);
 
-/* Look through all the current minimal symbol tables and find the
+/* Look through all the minimal symbol tables in PSPACE and find the
    first minimal symbol that matches NAME and has text type.  If OBJF
    is non-NULL, limit the search to that objfile.  Returns a bound
    minimal symbol that matches, or an "empty" bound minimal symbol
@@ -219,8 +219,9 @@  bound_minimal_symbol lookup_minimal_symbol (program_space *pspace,
 
    This function only searches the mangled (linkage) names.  */
 
-bound_minimal_symbol lookup_minimal_symbol_text (const char *,
-						 struct objfile *);
+bound_minimal_symbol lookup_minimal_symbol_text (program_space *pspace,
+						 const char *name,
+						 objfile *objf);
 
 /* Look through the minimal symbols in OBJF (and its separate debug
    objfiles) for a global (not file-local) minsym whose linkage name
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 9da148b3394d..9b3562eeb303 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -3174,7 +3174,9 @@  find_pc_sect_line (CORE_ADDR pc, struct obj_section *section, int notcurrent)
     if (msymbol.minsym->type () == mst_solib_trampoline)
       {
 	bound_minimal_symbol mfunsym
-	  = lookup_minimal_symbol_text (msymbol.minsym->linkage_name (), NULL);
+	  = lookup_minimal_symbol_text (section->objfile->pspace (),
+					msymbol.minsym->linkage_name (),
+					nullptr);
 
 	if (mfunsym.minsym == NULL)
 	  /* I eliminated this warning since it is coming out