[08/14] gdb: pass program space to have_{full,partial}_symbols
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
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
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)
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
@@ -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);
@@ -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."),
@@ -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;
@@ -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."),
@@ -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
@@ -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... */
@@ -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."));
@@ -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;
@@ -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);
@@ -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
@@ -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);
@@ -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);
@@ -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'? "),
@@ -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;
@@ -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 ();