[08/14] gdb: pass program space to have_{full,partial}_symbols

Message ID 20240711195307.1544451-9-simon.marchi@polymtl.ca
State New
Headers
Series Some random program space cleanups |

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

Commit Message

Simon Marchi July 11, 2024, 7:52 p.m. UTC
  Make the current program space reference bubble up one level.

Change-Id: I19c4fc2ca955f9c828ef426a077b43983865697b
---
 gdb/ada-exp.y        |  4 +++-
 gdb/c-exp.y          |  3 ++-
 gdb/cli/cli-cmds.c   |  3 ++-
 gdb/d-exp.y          |  3 ++-
 gdb/go-exp.y         |  4 ++--
 gdb/infcmd.c         |  2 +-
 gdb/linespec.c       |  9 +++++----
 gdb/objfiles.c       | 12 ++++++------
 gdb/objfiles.h       | 14 ++++++--------
 gdb/p-exp.y          |  4 ++--
 gdb/parse.c          |  3 ++-
 gdb/source.c         |  5 +++--
 gdb/symfile.c        |  8 +++++---
 gdb/symtab.c         |  8 +++++---
 gdb/tui/tui-disasm.c |  3 ++-
 15 files changed, 48 insertions(+), 37 deletions(-)
  

Comments

Thiago Jung Bauermann July 13, 2024, 5:09 a.m. UTC | #1
Simon Marchi <simon.marchi@polymtl.ca> writes:

> diff --git a/gdb/ada-exp.y b/gdb/ada-exp.y
> index 0126db5f2bee..dfcbb2bebd98 100644
> --- a/gdb/ada-exp.y
> +++ b/gdb/ada-exp.y
> @@ -1857,7 +1857,9 @@ write_var_or_type (struct parser_state *par_state,
>  	    }
>  	}
>  
> -      if (!have_full_symbols () && !have_partial_symbols () && block == NULL)
> +      if (!have_full_symbols (current_program_space)
> +	  && !have_partial_symbols (current_program_space)
> +	  && block == NULL)

If block != nullptr, does it make sense to use block->objfile ()->pspace
here?

>  	error (_("No symbol table is loaded.  Use the \"file\" command."));
>        if (block == par_state->expression_context_block)
>  	error (_("No definition of \"%s\" in current context."), name0.ptr);
> diff --git a/gdb/linespec.c b/gdb/linespec.c
> index 7a1fbc2ebec5..7a6b21e1b741 100644
> --- a/gdb/linespec.c
> +++ b/gdb/linespec.c
> @@ -3716,7 +3716,8 @@ symtabs_from_filename (const char *filename,
>  
>    if (result.empty ())
>      {
> -      if (!have_full_symbols () && !have_partial_symbols ())
> +      if (!have_full_symbols (current_program_space)
> +	  && !have_partial_symbols (current_program_space))

If search_pspace != nullptr, does it make sense to use it here?

>  	throw_error (NOT_FOUND_ERROR,
>  		     _("No symbol table is loaded.  "
>  		       "Use the \"file\" command."));
> diff --git a/gdb/symfile.c b/gdb/symfile.c
> index 0bdec9af1987..b9a576f24c9c 100644
> --- a/gdb/symfile.c
> +++ b/gdb/symfile.c
> @@ -1053,9 +1053,10 @@ symbol_file_add_with_addrs (const gdb_bfd_ref_ptr &abfd, const char *name,
>  
>    if (from_tty
>        && (always_confirm
> -	  || ((have_full_symbols () || have_partial_symbols ())
> +	  || ((have_full_symbols (current_program_space)
> +	       || have_partial_symbols (current_program_space))

If parent != nullptr, does it make sense to use parent->pspace here?

>  	      && mainline))
> -      && !query (_("Load new symbol table from \"%s\"? "), name))
> +      && !query (_ ("Load new symbol table from \"%s\"? "), name))
>      error (_("Not confirmed."));
>  
>    if (mainline)
  
Simon Marchi July 15, 2024, 2:22 p.m. UTC | #2
On 7/13/24 1:09 AM, Thiago Jung Bauermann wrote:
> Simon Marchi <simon.marchi@polymtl.ca> writes:
> 
>> diff --git a/gdb/ada-exp.y b/gdb/ada-exp.y
>> index 0126db5f2bee..dfcbb2bebd98 100644
>> --- a/gdb/ada-exp.y
>> +++ b/gdb/ada-exp.y
>> @@ -1857,7 +1857,9 @@ write_var_or_type (struct parser_state *par_state,
>>  	    }
>>  	}
>>  
>> -      if (!have_full_symbols () && !have_partial_symbols () && block == NULL)
>> +      if (!have_full_symbols (current_program_space)
>> +	  && !have_partial_symbols (current_program_space)
>> +	  && block == NULL)
> 
> If block != nullptr, does it make sense to use block->objfile ()->pspace
> here?

Yeah, but we also have to think about what to use when block is nullptr.
And what we use when block is nullptr would also probably be fine to use
when block isn't nullptr.  In other words, I don't think we need to
introduce some `if (block != nullptr)` just for the sake of getting a
program space.

If we go up the stack enough, I think we will find that the expression
is parsed in some context (e.g. the current inferior), and the pspace
should be passed down from there, probably in the parser_state
structure.

I'll keep that as future work, since it's non-trivial.


>>  	error (_("No symbol table is loaded.  Use the \"file\" command."));
>>        if (block == par_state->expression_context_block)
>>  	error (_("No definition of \"%s\" in current context."), name0.ptr);
>> diff --git a/gdb/linespec.c b/gdb/linespec.c
>> index 7a1fbc2ebec5..7a6b21e1b741 100644
>> --- a/gdb/linespec.c
>> +++ b/gdb/linespec.c
>> @@ -3716,7 +3716,8 @@ symtabs_from_filename (const char *filename,
>>  
>>    if (result.empty ())
>>      {
>> -      if (!have_full_symbols () && !have_partial_symbols ())
>> +      if (!have_full_symbols (current_program_space)
>> +	  && !have_partial_symbols (current_program_space))
> 
> If search_pspace != nullptr, does it make sense to use it here?

I think more thought / changes would be needed.

When search_pspace != nullptr, it means "search in this pspace".  When
it's nullptr, it means "search in all pspaces" (see
collect_symtabs_from_filename).

So, when we search in all pspaces and the result vector is empty, the
code uses have_full_symbols and have_partial_symbols on the current
pspace, to know if it should throw an error (my patch doesn't change the
behavior, it just makes it more obvious).  This probably doesn't make
sense?

What if you have two inferiors / pspaces:

 inf 1, pspace 1, with debug symbols
 inf 2, pspace 2, no debug symbols, current

Then, symtabs_from_filename is called (I guess when the user tries to
set a breakpoint by filename?) and the result vector is empty.  The
have_full_symbols/have_partial_symbols checks on the current pspace will
just see that pspace 2 doesn't have symbols defined, so we will error
out:

      if (!have_full_symbols (current_program_space)
	  && !have_partial_symbols (current_program_space))
	throw_error (NOT_FOUND_ERROR,
		     _("No symbol table is loaded.  "
		       "Use the \"file\" command."));

Is it what we want, given that we also searched pspace 1, which does
have symbols?

If we searched in all pspaces, perhaps we need to check if all pspaces
have no symbols.  Since it's non-trivial, I'll keep the patch as-is.

> 
>>  	throw_error (NOT_FOUND_ERROR,
>>  		     _("No symbol table is loaded.  "
>>  		       "Use the \"file\" command."));
>> diff --git a/gdb/symfile.c b/gdb/symfile.c
>> index 0bdec9af1987..b9a576f24c9c 100644
>> --- a/gdb/symfile.c
>> +++ b/gdb/symfile.c
>> @@ -1053,9 +1053,10 @@ symbol_file_add_with_addrs (const gdb_bfd_ref_ptr &abfd, const char *name,
>>  
>>    if (from_tty
>>        && (always_confirm
>> -	  || ((have_full_symbols () || have_partial_symbols ())
>> +	  || ((have_full_symbols (current_program_space)
>> +	       || have_partial_symbols (current_program_space))
> 
> If parent != nullptr, does it make sense to use parent->pspace here?

I don't think so.  Like the first case above, ultimately, the chosen
program space shouldn't depend on `parent`, it should probably be passed
down from above (when calling the function, we ask "please add an
objfile to this program space").  What we could add (but not in this
patch) is a gdb_assert to check that if parent != nullptr, the pspace we
are adding an objfile to is equal to parent->pspace, since it wouldn't
make sense to have child/parent objfiles belonging to different pspaces.

Thanks,

Simon
  

Patch

diff --git a/gdb/ada-exp.y b/gdb/ada-exp.y
index 0126db5f2bee..dfcbb2bebd98 100644
--- a/gdb/ada-exp.y
+++ b/gdb/ada-exp.y
@@ -1857,7 +1857,9 @@  write_var_or_type (struct parser_state *par_state,
 	    }
 	}
 
-      if (!have_full_symbols () && !have_partial_symbols () && block == NULL)
+      if (!have_full_symbols (current_program_space)
+	  && !have_partial_symbols (current_program_space)
+	  && block == NULL)
 	error (_("No symbol table is loaded.  Use the \"file\" command."));
       if (block == par_state->expression_context_block)
 	error (_("No definition of \"%s\" in current context."), name0.ptr);
diff --git a/gdb/c-exp.y b/gdb/c-exp.y
index 60223172a23c..a1a74a985a6f 100644
--- a/gdb/c-exp.y
+++ b/gdb/c-exp.y
@@ -1213,7 +1213,8 @@  variable:	name_not_typename
 				= lookup_bound_minimal_symbol (arg.c_str ());
 			      if (msymbol.minsym == NULL)
 				{
-				  if (!have_full_symbols () && !have_partial_symbols ())
+				  if (!have_full_symbols (current_program_space)
+				      && !have_partial_symbols (current_program_space))
 				    error (_("No symbol table is loaded.  Use the \"file\" command."));
 				  else
 				    error (_("No symbol \"%s\" in current context."),
diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index 450009156d4b..9d3a90b057d2 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -1327,7 +1327,8 @@  list_command (const char *arg, int from_tty)
      and clear NO_END; however, if one of the arguments is blank,
      set DUMMY_BEG or DUMMY_END to record that fact.  */
 
-  if (!have_full_symbols () && !have_partial_symbols ())
+  if (!have_full_symbols (current_program_space)
+      && !have_partial_symbols (current_program_space))
     error (_("No symbol table is loaded.  Use the \"file\" command."));
 
   std::vector<symtab_and_line> sals;
diff --git a/gdb/d-exp.y b/gdb/d-exp.y
index 13d2cfa44d88..6feacd8e3648 100644
--- a/gdb/d-exp.y
+++ b/gdb/d-exp.y
@@ -466,7 +466,8 @@  PrimaryExpression:
 		      msymbol = lookup_bound_minimal_symbol (copy.c_str ());
 		      if (msymbol.minsym != NULL)
 			pstate->push_new<var_msym_value_operation> (msymbol);
-		      else if (!have_full_symbols () && !have_partial_symbols ())
+		      else if (!have_full_symbols (current_program_space)
+			       && !have_partial_symbols (current_program_space))
 			error (_("No symbol table is loaded.  Use the \"file\" command"));
 		      else
 			error (_("No symbol \"%s\" in current context."),
diff --git a/gdb/go-exp.y b/gdb/go-exp.y
index 1a6ebbe135be..115c71ba8ea9 100644
--- a/gdb/go-exp.y
+++ b/gdb/go-exp.y
@@ -577,8 +577,8 @@  variable:	name_not_typename
 			      if (msymbol.minsym != NULL)
 				pstate->push_new<var_msym_value_operation>
 				  (msymbol);
-			      else if (!have_full_symbols ()
-				       && !have_partial_symbols ())
+			      else if (!have_full_symbols (current_program_space)
+				       && !have_partial_symbols (current_program_space))
 				error (_("No symbol table is loaded.  "
 				       "Use the \"file\" command."));
 			      else
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index f021d3f8a62d..e34ce80dbf0c 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -519,7 +519,7 @@  start_command (const char *args, int from_tty)
   /* Some languages such as Ada need to search inside the program
      minimal symbols for the location where to put the temporary
      breakpoint before starting.  */
-  if (!have_minimal_symbols ())
+  if (!have_minimal_symbols (current_program_space))
     error (_("No symbol table loaded.  Use the \"file\" command."));
 
   /* Run the program until reaching the main procedure...  */
diff --git a/gdb/linespec.c b/gdb/linespec.c
index 7a1fbc2ebec5..7a6b21e1b741 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -1550,9 +1550,9 @@  symbol_not_found_error (const char *symbol, const char *filename)
   if (symbol == NULL)
     symbol = "";
 
-  if (!have_full_symbols ()
-      && !have_partial_symbols ()
-      && !have_minimal_symbols ())
+  if (!have_full_symbols (current_program_space)
+      && !have_partial_symbols (current_program_space)
+      && !have_minimal_symbols (current_program_space))
     throw_error (NOT_FOUND_ERROR,
 		 _("No symbol table is loaded.  Use the \"file\" command."));
 
@@ -3716,7 +3716,8 @@  symtabs_from_filename (const char *filename,
 
   if (result.empty ())
     {
-      if (!have_full_symbols () && !have_partial_symbols ())
+      if (!have_full_symbols (current_program_space)
+	  && !have_partial_symbols (current_program_space))
 	throw_error (NOT_FOUND_ERROR,
 		     _("No symbol table is loaded.  "
 		       "Use the \"file\" command."));
diff --git a/gdb/objfiles.c b/gdb/objfiles.c
index e4ab73775a22..77d759b3aa59 100644
--- a/gdb/objfiles.c
+++ b/gdb/objfiles.c
@@ -755,9 +755,9 @@  objfile_has_symbols (objfile *objfile)
 /* See objfiles.h.  */
 
 bool
-have_partial_symbols ()
+have_partial_symbols (program_space *pspace)
 {
-  for (objfile *ofp : current_program_space->objfiles ())
+  for (objfile *ofp : pspace->objfiles ())
     if (ofp->has_partial_symbols ())
       return true;
 
@@ -767,9 +767,9 @@  have_partial_symbols ()
 /* See objfiles.h.  */
 
 bool
-have_full_symbols ()
+have_full_symbols (program_space *pspace)
 {
-  for (objfile *ofp : current_program_space->objfiles ())
+  for (objfile *ofp : pspace->objfiles ())
     if (objfile_has_full_symbols (ofp))
       return true;
 
@@ -795,9 +795,9 @@  objfile_purge_solibs (program_space *pspace)
 /* See objfiles.h.  */
 
 bool
-have_minimal_symbols ()
+have_minimal_symbols (program_space *pspace)
 {
-  for (objfile *ofp : current_program_space->objfiles ())
+  for (objfile *ofp : pspace->objfiles ())
     if (ofp->per_bfd->minimal_symbol_count > 0)
       return true;
 
diff --git a/gdb/objfiles.h b/gdb/objfiles.h
index e81cfe68c2de..2fb786592b64 100644
--- a/gdb/objfiles.h
+++ b/gdb/objfiles.h
@@ -927,14 +927,13 @@  extern bool objfile_has_full_symbols (objfile *objfile);
 
 extern bool objfile_has_symbols (objfile *objfile);
 
-/* Return true if any objfile of the current program space has partial
-   symbols.  */
+/* Return true if any objfile of PSPACE has partial symbols.  */
 
-extern bool have_partial_symbols ();
+extern bool have_partial_symbols (program_space *pspace);
 
-/* Return true if any objfile of the current program space has full symbols.  */
+/* Return true if any objfile of PSPACE has full symbols.  */
 
-extern bool have_full_symbols ();
+extern bool have_full_symbols (program_space *pspace);
 
 extern void objfile_set_sym_fns (struct objfile *objfile,
 				 const struct sym_fns *sf);
@@ -961,10 +960,9 @@  extern void objfile_purge_solibs (program_space *pspace);
 /* Functions for dealing with the minimal symbol table, really a misc
    address<->symbol mapping for things we don't have debug symbols for.  */
 
-/* Return true if any objfile of the current program space has minimal
-   symbols.  */
+/* Return true if any objfile of PSPACE has minimal symbols.  */
 
-extern bool have_minimal_symbols ();
+extern bool have_minimal_symbols (program_space *pspace);
 
 extern struct obj_section *find_pc_section (CORE_ADDR pc);
 
diff --git a/gdb/p-exp.y b/gdb/p-exp.y
index f334db6b5237..0a0fa95e8688 100644
--- a/gdb/p-exp.y
+++ b/gdb/p-exp.y
@@ -725,8 +725,8 @@  variable:	name_not_typename
 			      if (msymbol.minsym != NULL)
 				pstate->push_new<var_msym_value_operation>
 				  (msymbol);
-			      else if (!have_full_symbols ()
-				       && !have_partial_symbols ())
+			      else if (!have_full_symbols (current_program_space)
+				       && !have_partial_symbols (current_program_space))
 				error (_("No symbol table is loaded.  "
 				       "Use the \"file\" command."));
 			      else
diff --git a/gdb/parse.c b/gdb/parse.c
index 0a2b2a5b2abf..c10b09af485e 100644
--- a/gdb/parse.c
+++ b/gdb/parse.c
@@ -148,7 +148,8 @@  parser_state::push_symbol (const char *name, block_symbol sym)
       struct bound_minimal_symbol msymbol = lookup_bound_minimal_symbol (name);
       if (msymbol.minsym != NULL)
 	push_new<expr::var_msym_value_operation> (msymbol);
-      else if (!have_full_symbols () && !have_partial_symbols ())
+      else if (!have_full_symbols (current_program_space)
+	       && !have_partial_symbols (current_program_space))
 	error (_("No symbol table is loaded.  Use the \"file\" command."));
       else
 	error (_("No symbol \"%s\" in current context."), name);
diff --git a/gdb/source.c b/gdb/source.c
index b80eeae68a7d..854ba0f90f31 100644
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -260,8 +260,9 @@  get_current_source_symtab_and_line (void)
 void
 set_default_source_symtab_and_line (void)
 {
-  if (!have_full_symbols () && !have_partial_symbols ())
-    error (_("No symbol table is loaded.  Use the \"file\" command."));
+  if (!have_full_symbols (current_program_space)
+      && !have_partial_symbols (current_program_space))
+    error (_ ("No symbol table is loaded.  Use the \"file\" command."));
 
   /* Pull in a current source symtab if necessary.  */
   current_source_location *loc = get_source_location (current_program_space);
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 0bdec9af1987..b9a576f24c9c 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -1053,9 +1053,10 @@  symbol_file_add_with_addrs (const gdb_bfd_ref_ptr &abfd, const char *name,
 
   if (from_tty
       && (always_confirm
-	  || ((have_full_symbols () || have_partial_symbols ())
+	  || ((have_full_symbols (current_program_space)
+	       || have_partial_symbols (current_program_space))
 	      && mainline))
-      && !query (_("Load new symbol table from \"%s\"? "), name))
+      && !query (_ ("Load new symbol table from \"%s\"? "), name))
     error (_("Not confirmed."));
 
   if (mainline)
@@ -1199,7 +1200,8 @@  symbol_file_add_main_1 (const char *args, symfile_add_flags add_flags,
 void
 symbol_file_clear (int from_tty)
 {
-  if ((have_full_symbols () || have_partial_symbols ())
+  if ((have_full_symbols (current_program_space)
+       || have_partial_symbols (current_program_space))
       && from_tty
       && (current_program_space->symfile_object_file
 	  ? !query (_("Discard symbol table from `%s'? "),
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 41d71beec2ac..e075c17a0fd0 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -4764,8 +4764,9 @@  info_sources_worker (struct ui_out *uiout,
 static void
 info_sources_command (const char *args, int from_tty)
 {
-  if (!have_full_symbols () && !have_partial_symbols ())
-    error (_("No symbol table is loaded.  Use the \"file\" command."));
+  if (!have_full_symbols (current_program_space)
+      && !have_partial_symbols (current_program_space))
+    error (_ ("No symbol table is loaded.  Use the \"file\" command."));
 
   filename_partial_match_opts match_opts;
   auto group = make_info_sources_options_def_group (&match_opts);
@@ -6381,7 +6382,8 @@  make_source_files_completion_list (const char *text, const char *word)
   const char *base_name;
   struct add_partial_filename_data datum;
 
-  if (!have_full_symbols () && !have_partial_symbols ())
+  if (!have_full_symbols (current_program_space)
+      && !have_partial_symbols (current_program_space))
     return list;
 
   filename_seen_cache filenames_seen;
diff --git a/gdb/tui/tui-disasm.c b/gdb/tui/tui-disasm.c
index 0727d3a90a82..2f25fedbcab3 100644
--- a/gdb/tui/tui-disasm.c
+++ b/gdb/tui/tui-disasm.c
@@ -389,7 +389,8 @@  tui_get_begin_asm_address (struct gdbarch **gdbarch_p, CORE_ADDR *addr_p)
 
   if (tui_location.addr () == 0)
     {
-      if (have_full_symbols () || have_partial_symbols ())
+      if (have_full_symbols (current_program_space)
+	  || have_partial_symbols (current_program_space))
 	{
 	  set_default_source_symtab_and_line ();
 	  struct symtab_and_line sal = get_current_source_symtab_and_line ();