* [PATCH v2 3/8] Back out earlier Ada exception change
2019-08-01 17:04 [PATCH v2 0/8] Handle copy relocations and add $_ada_exception Tom Tromey
@ 2019-08-01 17:04 ` Tom Tromey
2019-08-01 17:04 ` [PATCH v2 6/8] Make current_source_* per-program-space Tom Tromey
` (6 subsequent siblings)
7 siblings, 0 replies; 27+ messages in thread
From: Tom Tromey @ 2019-08-01 17:04 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
commit 2ff0a9473 (Fix "catch exception" with dynamic linking) changed
how ada-lang.c creates expressions to determine if an exception
catchpoint should stop.
That patch is no longer needed now that copy relocations are handled
more directly.
gdb/ChangeLog
2019-08-01 Tom Tromey <tromey@adacore.com>
* ada-lang.c (ada_lookup_simple_minsyms): Remove.
(create_excep_cond_exprs): Simplify exception string computation.
(ada_exception_catchpoint_cond_string): Likewise.
---
gdb/ChangeLog | 6 +++
gdb/ada-lang.c | 107 ++++++++++++-------------------------------------
2 files changed, 31 insertions(+), 82 deletions(-)
diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 4e1ee31887a..6851386b48b 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -4846,36 +4846,6 @@ ada_lookup_simple_minsym (const char *name)
return result;
}
-/* Return all the bound minimal symbols matching NAME according to Ada
- decoding rules. Returns an empty vector if there is no such
- minimal symbol. Names prefixed with "standard__" are handled
- specially: "standard__" is first stripped off, and only static and
- global symbols are searched. */
-
-static std::vector<struct bound_minimal_symbol>
-ada_lookup_simple_minsyms (const char *name)
-{
- std::vector<struct bound_minimal_symbol> result;
-
- symbol_name_match_type match_type = name_match_type_from_name (name);
- lookup_name_info lookup_name (name, match_type);
-
- symbol_name_matcher_ftype *match_name
- = ada_get_symbol_name_matcher (lookup_name);
-
- for (objfile *objfile : current_program_space->objfiles ())
- {
- for (minimal_symbol *msymbol : objfile->msymbols ())
- {
- if (match_name (MSYMBOL_LINKAGE_NAME (msymbol), lookup_name, NULL)
- && MSYMBOL_TYPE (msymbol) != mst_solib_trampoline)
- result.push_back ({msymbol, objfile});
- }
- }
-
- return result;
-}
-
/* For all subprograms that statically enclose the subprogram of the
selected frame, add symbols matching identifier NAME in DOMAIN
and their blocks to the list of data in OBSTACKP, as for
@@ -12316,6 +12286,8 @@ static void
create_excep_cond_exprs (struct ada_catchpoint *c,
enum ada_exception_catchpoint_kind ex)
{
+ struct bp_location *bl;
+
/* Nothing to do if there's no specific exception to catch. */
if (c->excep_string.empty ())
return;
@@ -12324,45 +12296,28 @@ create_excep_cond_exprs (struct ada_catchpoint *c,
if (c->loc == NULL)
return;
- /* We have to compute the expression once for each program space,
- because the expression may hold the addresses of multiple symbols
- in some cases. */
- std::multimap<program_space *, struct bp_location *> loc_map;
- for (bp_location *bl = c->loc; bl != NULL; bl = bl->next)
- loc_map.emplace (bl->pspace, bl);
-
- scoped_restore_current_program_space save_pspace;
+ /* Compute the condition expression in text form, from the specific
+ expection we want to catch. */
+ std::string cond_string
+ = ada_exception_catchpoint_cond_string (c->excep_string.c_str (), ex);
- std::string cond_string;
- program_space *last_ps = nullptr;
- for (auto iter : loc_map)
+ /* Iterate over all the catchpoint's locations, and parse an
+ expression for each. */
+ for (bl = c->loc; bl != NULL; bl = bl->next)
{
struct ada_catchpoint_location *ada_loc
- = (struct ada_catchpoint_location *) iter.second;
-
- if (ada_loc->pspace != last_ps)
- {
- last_ps = ada_loc->pspace;
- set_current_program_space (last_ps);
-
- /* Compute the condition expression in text form, from the
- specific expection we want to catch. */
- cond_string
- = ada_exception_catchpoint_cond_string (c->excep_string.c_str (),
- ex);
- }
-
+ = (struct ada_catchpoint_location *) bl;
expression_up exp;
- if (!ada_loc->shlib_disabled)
+ if (!bl->shlib_disabled)
{
const char *s;
s = cond_string.c_str ();
try
{
- exp = parse_exp_1 (&s, ada_loc->address,
- block_for_pc (ada_loc->address),
+ exp = parse_exp_1 (&s, bl->address,
+ block_for_pc (bl->address),
0);
}
catch (const gdb_exception_error &e)
@@ -13032,18 +12987,18 @@ ada_exception_catchpoint_cond_string (const char *excep_string,
enum ada_exception_catchpoint_kind ex)
{
int i;
+ bool is_standard_exc = false;
std::string result;
- const char *name;
if (ex == ada_catch_handlers)
{
/* For exception handlers catchpoints, the condition string does
not use the same parameter as for the other exceptions. */
- name = ("long_integer (GNAT_GCC_exception_Access"
- "(gcc_exception).all.occurrence.id)");
+ result = ("long_integer (GNAT_GCC_exception_Access"
+ "(gcc_exception).all.occurrence.id)");
}
else
- name = "long_integer (e)";
+ result = "long_integer (e)";
/* The standard exceptions are a special case. They are defined in
runtime units that have been compiled without debugging info; if
@@ -13062,35 +13017,23 @@ ada_exception_catchpoint_cond_string (const char *excep_string,
If an exception named contraint_error is defined in another package of
the inferior program, then the only way to specify this exception as a
breakpoint condition is to use its fully-qualified named:
- e.g. my_package.constraint_error.
-
- Furthermore, in some situations a standard exception's symbol may
- be present in more than one objfile, because the compiler may
- choose to emit copy relocations for them. So, we have to compare
- against all the possible addresses. */
+ e.g. my_package.constraint_error. */
- /* Storage for a rewritten symbol name. */
- std::string std_name;
for (i = 0; i < sizeof (standard_exc) / sizeof (char *); i++)
{
if (strcmp (standard_exc [i], excep_string) == 0)
{
- std_name = std::string ("standard.") + excep_string;
- excep_string = std_name.c_str ();
+ is_standard_exc = true;
break;
}
}
- excep_string = ada_encode (excep_string);
- std::vector<struct bound_minimal_symbol> symbols
- = ada_lookup_simple_minsyms (excep_string);
- for (const bound_minimal_symbol &msym : symbols)
- {
- if (!result.empty ())
- result += " or ";
- string_appendf (result, "%s = %s", name,
- pulongest (BMSYMBOL_VALUE_ADDRESS (msym)));
- }
+ result += " = ";
+
+ if (is_standard_exc)
+ string_appendf (result, "long_integer (&standard.%s)", excep_string);
+ else
+ string_appendf (result, "long_integer (&%s)", excep_string);
return result;
}
--
2.20.1
^ permalink raw reply [flat|nested] 27+ messages in thread* [PATCH v2 6/8] Make current_source_* per-program-space
2019-08-01 17:04 [PATCH v2 0/8] Handle copy relocations and add $_ada_exception Tom Tromey
2019-08-01 17:04 ` [PATCH v2 3/8] Back out earlier Ada exception change Tom Tromey
@ 2019-08-01 17:04 ` Tom Tromey
2019-08-20 15:57 ` Andrew Burgess
2019-08-01 17:04 ` [PATCH v2 7/8] Add $_ada_exception convenience variable Tom Tromey
` (5 subsequent siblings)
7 siblings, 1 reply; 27+ messages in thread
From: Tom Tromey @ 2019-08-01 17:04 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This changes current_source_symtab and current_source_line to be
per-program-space. This ensures that switching inferiors will
preserve the current "list" location for that inferior, and also
ensures that the default expression evaluation context always comes
with the current inferior.
No test case, because the latter problem crops up with an existing
gdb.multi test case once this entire series has been applied.
gdb/ChangeLog
2019-08-01 Tom Tromey <tromey@adacore.com>
* source.c (struct current_source_location): New.
(current_source_key): New global.
(current_source_symtab, current_source_line)
(current_source_pspace): Remove.
(get_source_location): New function.
(get_current_source_symtab_and_line)
(set_default_source_symtab_and_line)
(set_current_source_symtab_and_line)
(clear_current_source_symtab_and_line, select_source_symtab)
(info_source_command, print_source_lines_base)
(info_line_command, search_command_helper, _initialize_source):
Update.
---
gdb/ChangeLog | 15 ++++++
gdb/source.c | 128 ++++++++++++++++++++++++++++++--------------------
2 files changed, 93 insertions(+), 50 deletions(-)
diff --git a/gdb/source.c b/gdb/source.c
index 1aef019da44..d20cdc37c91 100644
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -66,15 +66,20 @@ struct substitute_path_rule
static struct substitute_path_rule *substitute_path_rules = NULL;
-/* Symtab of default file for listing lines of. */
+/* An instance of this is attached to each program space. */
-static struct symtab *current_source_symtab;
+struct current_source_location
+{
+ /* Symtab of default file for listing lines of. */
+
+ struct symtab *symtab = nullptr;
-/* Default next line to list. */
+ /* Default next line to list. */
-static int current_source_line;
+ int line = 0;
+};
-static struct program_space *current_source_pspace;
+static program_space_key<current_source_location> current_source_key;
/* Default number of lines to print with commands like "list".
This is based on guessing how many long (i.e. more than chars_per_line
@@ -162,6 +167,19 @@ get_lines_to_list (void)
return lines_to_list;
}
+/* A helper to return the current source location object for PSPACE,
+ creating it if it does not exist. */
+
+static current_source_location *
+get_source_location (program_space *pspace)
+{
+ current_source_location *loc
+ = current_source_key.get (pspace);
+ if (loc == nullptr)
+ loc = current_source_key.emplace (pspace);
+ return loc;
+}
+
/* Return the current source file for listing and next line to list.
NOTE: The returned sal pc and end fields are not valid. */
@@ -169,10 +187,11 @@ struct symtab_and_line
get_current_source_symtab_and_line (void)
{
symtab_and_line cursal;
+ current_source_location *loc = get_source_location (current_program_space);
- cursal.pspace = current_source_pspace;
- cursal.symtab = current_source_symtab;
- cursal.line = current_source_line;
+ cursal.pspace = current_program_space;
+ cursal.symtab = loc->symtab;
+ cursal.line = loc->line;
cursal.pc = 0;
cursal.end = 0;
@@ -194,7 +213,8 @@ set_default_source_symtab_and_line (void)
error (_("No symbol table is loaded. Use the \"file\" command."));
/* Pull in a current source symtab if necessary. */
- if (current_source_symtab == 0)
+ current_source_location *loc = get_source_location (current_program_space);
+ if (loc->symtab == 0)
select_source_symtab (0);
}
@@ -208,15 +228,16 @@ set_current_source_symtab_and_line (const symtab_and_line &sal)
{
symtab_and_line cursal;
- cursal.pspace = current_source_pspace;
- cursal.symtab = current_source_symtab;
- cursal.line = current_source_line;
+ current_source_location *loc = get_source_location (sal.pspace);
+
+ cursal.pspace = sal.pspace;
+ cursal.symtab = loc->symtab;
+ cursal.line = loc->line;
cursal.pc = 0;
cursal.end = 0;
- current_source_pspace = sal.pspace;
- current_source_symtab = sal.symtab;
- current_source_line = sal.line;
+ loc->symtab = sal.symtab;
+ loc->line = sal.line;
/* Force the next "list" to center around the current line. */
clear_lines_listed_range ();
@@ -229,8 +250,9 @@ set_current_source_symtab_and_line (const symtab_and_line &sal)
void
clear_current_source_symtab_and_line (void)
{
- current_source_symtab = 0;
- current_source_line = 0;
+ current_source_location *loc = get_source_location (current_program_space);
+ loc->symtab = 0;
+ loc->line = 0;
}
/* See source.h. */
@@ -240,13 +262,15 @@ select_source_symtab (struct symtab *s)
{
if (s)
{
- current_source_symtab = s;
- current_source_line = 1;
- current_source_pspace = SYMTAB_PSPACE (s);
+ current_source_location *loc
+ = get_source_location (SYMTAB_PSPACE (s));
+ loc->symtab = s;
+ loc->line = 1;
return;
}
- if (current_source_symtab)
+ current_source_location *loc = get_source_location (current_program_space);
+ if (loc->symtab)
return;
/* Make the default place to list be the function `main'
@@ -255,16 +279,15 @@ select_source_symtab (struct symtab *s)
if (bsym.symbol != nullptr && SYMBOL_CLASS (bsym.symbol) == LOC_BLOCK)
{
symtab_and_line sal = find_function_start_sal (bsym.symbol, true);
- current_source_pspace = sal.pspace;
- current_source_symtab = sal.symtab;
- current_source_line = std::max (sal.line - (lines_to_list - 1), 1);
+ loc->symtab = sal.symtab;
+ loc->line = std::max (sal.line - (lines_to_list - 1), 1);
return;
}
/* Alright; find the last file in the symtab list (ignoring .h's
and namespace symtabs). */
- current_source_line = 1;
+ loc->line = 1;
for (objfile *ofp : current_program_space->objfiles ())
{
@@ -277,15 +300,12 @@ select_source_symtab (struct symtab *s)
if (!(len > 2 && (strcmp (&name[len - 2], ".h") == 0
|| strcmp (name, "<<C++-namespaces>>") == 0)))
- {
- current_source_pspace = current_program_space;
- current_source_symtab = symtab;
- }
+ loc->symtab = symtab;
}
}
}
- if (current_source_symtab)
+ if (loc->symtab)
return;
for (objfile *objfile : current_program_space->objfiles ())
@@ -293,9 +313,9 @@ select_source_symtab (struct symtab *s)
if (objfile->sf)
s = objfile->sf->qf->find_last_source_symtab (objfile);
if (s)
- current_source_symtab = s;
+ loc->symtab = s;
}
- if (current_source_symtab)
+ if (loc->symtab)
return;
error (_("Can't find a default source file"));
@@ -624,7 +644,9 @@ add_path (const char *dirname, char **which_path, int parse_separators)
static void
info_source_command (const char *ignore, int from_tty)
{
- struct symtab *s = current_source_symtab;
+ current_source_location *loc
+ = get_source_location (current_program_space);
+ struct symtab *s = loc->symtab;
struct compunit_symtab *cust;
if (!s)
@@ -1222,8 +1244,11 @@ print_source_lines_base (struct symtab *s, int line, int stopline,
struct ui_out *uiout = current_uiout;
/* Regardless of whether we can open the file, set current_source_symtab. */
- current_source_symtab = s;
- current_source_line = line;
+ current_source_location *loc
+ = get_source_location (current_program_space);
+
+ loc->symtab = s;
+ loc->line = line;
first_line_listed = line;
/* If printing of source lines is disabled, just print file and line
@@ -1313,13 +1338,13 @@ print_source_lines_base (struct symtab *s, int line, int stopline,
{
char buf[20];
- last_line_listed = current_source_line;
+ last_line_listed = loc->line;
if (flags & PRINT_SOURCE_LINES_FILENAME)
{
uiout->text (symtab_to_filename_for_display (s));
uiout->text (":");
}
- xsnprintf (buf, sizeof (buf), "%d\t", current_source_line++);
+ xsnprintf (buf, sizeof (buf), "%d\t", loc->line++);
uiout->text (buf);
while (*iter != '\0')
@@ -1411,12 +1436,14 @@ info_line_command (const char *arg, int from_tty)
if (arg == 0)
{
- curr_sal.symtab = current_source_symtab;
+ current_source_location *loc
+ = get_source_location (current_program_space);
+ curr_sal.symtab = loc->symtab;
curr_sal.pspace = current_program_space;
if (last_line_listed != 0)
curr_sal.line = last_line_listed;
else
- curr_sal.line = current_source_line;
+ curr_sal.line = loc->line;
sals = curr_sal;
}
@@ -1518,23 +1545,25 @@ search_command_helper (const char *regex, int from_tty, bool forward)
if (msg)
error (("%s"), msg);
- if (current_source_symtab == 0)
+ current_source_location *loc
+ = get_source_location (current_program_space);
+ if (loc->symtab == 0)
select_source_symtab (0);
- scoped_fd desc (open_source_file_with_line_charpos (current_source_symtab));
+ scoped_fd desc (open_source_file_with_line_charpos (loc->symtab));
if (desc.get () < 0)
- perror_with_name (symtab_to_filename_for_display (current_source_symtab));
+ perror_with_name (symtab_to_filename_for_display (loc->symtab));
int line = (forward
? last_line_listed + 1
: last_line_listed - 1);
- if (line < 1 || line > current_source_symtab->nlines)
+ if (line < 1 || line > loc->symtab->nlines)
error (_("Expression not found"));
- if (lseek (desc.get (), current_source_symtab->line_charpos[line - 1], 0)
+ if (lseek (desc.get (), loc->symtab->line_charpos[line - 1], 0)
< 0)
- perror_with_name (symtab_to_filename_for_display (current_source_symtab));
+ perror_with_name (symtab_to_filename_for_display (loc->symtab));
gdb_file_up stream = desc.to_file (FDOPEN_MODE);
clearerr (stream.get ());
@@ -1569,9 +1598,9 @@ search_command_helper (const char *regex, int from_tty, bool forward)
if (re_exec (buf.data ()) > 0)
{
/* Match! */
- print_source_lines (current_source_symtab, line, line + 1, 0);
+ print_source_lines (loc->symtab, line, line + 1, 0);
set_internalvar_integer (lookup_internalvar ("_"), line);
- current_source_line = std::max (line - lines_to_list / 2, 1);
+ loc->line = std::max (line - lines_to_list / 2, 1);
return;
}
@@ -1583,10 +1612,10 @@ search_command_helper (const char *regex, int from_tty, bool forward)
if (line < 1)
break;
if (fseek (stream.get (),
- current_source_symtab->line_charpos[line - 1], 0) < 0)
+ loc->symtab->line_charpos[line - 1], 0) < 0)
{
const char *filename
- = symtab_to_filename_for_display (current_source_symtab);
+ = symtab_to_filename_for_display (loc->symtab);
perror_with_name (filename);
}
}
@@ -1850,7 +1879,6 @@ _initialize_source (void)
{
struct cmd_list_element *c;
- current_source_symtab = 0;
init_source_path ();
/* The intention is to use POSIX Basic Regular Expressions.
--
2.20.1
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v2 6/8] Make current_source_* per-program-space
2019-08-01 17:04 ` [PATCH v2 6/8] Make current_source_* per-program-space Tom Tromey
@ 2019-08-20 15:57 ` Andrew Burgess
2019-08-21 19:05 ` Tom Tromey
0 siblings, 1 reply; 27+ messages in thread
From: Andrew Burgess @ 2019-08-20 15:57 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
* Tom Tromey <tromey@adacore.com> [2019-08-01 11:04:10 -0600]:
> This changes current_source_symtab and current_source_line to be
> per-program-space. This ensures that switching inferiors will
> preserve the current "list" location for that inferior, and also
> ensures that the default expression evaluation context always comes
> with the current inferior.
>
> No test case, because the latter problem crops up with an existing
> gdb.multi test case once this entire series has been applied.
I can reproduce the failure with patches 1 -> 5 applied. I did try to
come up with a test that would trigger this on HEAD, but I guess I'm
missing something as everything seems fine.
I spotted a few minor coding standard things when looking through.
Thanks,
Andrew
>
> gdb/ChangeLog
> 2019-08-01 Tom Tromey <tromey@adacore.com>
>
> * source.c (struct current_source_location): New.
> (current_source_key): New global.
> (current_source_symtab, current_source_line)
> (current_source_pspace): Remove.
> (get_source_location): New function.
> (get_current_source_symtab_and_line)
> (set_default_source_symtab_and_line)
> (set_current_source_symtab_and_line)
> (clear_current_source_symtab_and_line, select_source_symtab)
> (info_source_command, print_source_lines_base)
> (info_line_command, search_command_helper, _initialize_source):
> Update.
> ---
> gdb/ChangeLog | 15 ++++++
> gdb/source.c | 128 ++++++++++++++++++++++++++++++--------------------
> 2 files changed, 93 insertions(+), 50 deletions(-)
>
> diff --git a/gdb/source.c b/gdb/source.c
> index 1aef019da44..d20cdc37c91 100644
> --- a/gdb/source.c
> +++ b/gdb/source.c
> @@ -66,15 +66,20 @@ struct substitute_path_rule
>
> static struct substitute_path_rule *substitute_path_rules = NULL;
>
> -/* Symtab of default file for listing lines of. */
> +/* An instance of this is attached to each program space. */
>
> -static struct symtab *current_source_symtab;
> +struct current_source_location
> +{
> + /* Symtab of default file for listing lines of. */
> +
> + struct symtab *symtab = nullptr;
>
> -/* Default next line to list. */
> + /* Default next line to list. */
>
> -static int current_source_line;
> + int line = 0;
> +};
>
> -static struct program_space *current_source_pspace;
> +static program_space_key<current_source_location> current_source_key;
>
> /* Default number of lines to print with commands like "list".
> This is based on guessing how many long (i.e. more than chars_per_line
> @@ -162,6 +167,19 @@ get_lines_to_list (void)
> return lines_to_list;
> }
>
> +/* A helper to return the current source location object for PSPACE,
> + creating it if it does not exist. */
> +
> +static current_source_location *
> +get_source_location (program_space *pspace)
> +{
> + current_source_location *loc
> + = current_source_key.get (pspace);
> + if (loc == nullptr)
> + loc = current_source_key.emplace (pspace);
> + return loc;
> +}
> +
> /* Return the current source file for listing and next line to list.
> NOTE: The returned sal pc and end fields are not valid. */
>
> @@ -169,10 +187,11 @@ struct symtab_and_line
> get_current_source_symtab_and_line (void)
> {
> symtab_and_line cursal;
> + current_source_location *loc = get_source_location (current_program_space);
>
> - cursal.pspace = current_source_pspace;
> - cursal.symtab = current_source_symtab;
> - cursal.line = current_source_line;
> + cursal.pspace = current_program_space;
> + cursal.symtab = loc->symtab;
> + cursal.line = loc->line;
> cursal.pc = 0;
> cursal.end = 0;
>
> @@ -194,7 +213,8 @@ set_default_source_symtab_and_line (void)
> error (_("No symbol table is loaded. Use the \"file\" command."));
>
> /* Pull in a current source symtab if necessary. */
> - if (current_source_symtab == 0)
> + current_source_location *loc = get_source_location (current_program_space);
> + if (loc->symtab == 0)
> select_source_symtab (0);
Could this become:
if (loc->symtab == nullptr)
select_source_symtab (nullptr);
> }
>
> @@ -208,15 +228,16 @@ set_current_source_symtab_and_line (const symtab_and_line &sal)
> {
> symtab_and_line cursal;
>
> - cursal.pspace = current_source_pspace;
> - cursal.symtab = current_source_symtab;
> - cursal.line = current_source_line;
> + current_source_location *loc = get_source_location (sal.pspace);
> +
> + cursal.pspace = sal.pspace;
> + cursal.symtab = loc->symtab;
> + cursal.line = loc->line;
> cursal.pc = 0;
> cursal.end = 0;
>
> - current_source_pspace = sal.pspace;
> - current_source_symtab = sal.symtab;
> - current_source_line = sal.line;
> + loc->symtab = sal.symtab;
> + loc->line = sal.line;
>
> /* Force the next "list" to center around the current line. */
> clear_lines_listed_range ();
> @@ -229,8 +250,9 @@ set_current_source_symtab_and_line (const symtab_and_line &sal)
> void
> clear_current_source_symtab_and_line (void)
> {
> - current_source_symtab = 0;
> - current_source_line = 0;
> + current_source_location *loc = get_source_location (current_program_space);
> + loc->symtab = 0;
nullptr again?
> + loc->line = 0;
> }
>
> /* See source.h. */
> @@ -240,13 +262,15 @@ select_source_symtab (struct symtab *s)
> {
> if (s)
> {
> - current_source_symtab = s;
> - current_source_line = 1;
> - current_source_pspace = SYMTAB_PSPACE (s);
> + current_source_location *loc
> + = get_source_location (SYMTAB_PSPACE (s));
> + loc->symtab = s;
> + loc->line = 1;
> return;
> }
>
> - if (current_source_symtab)
> + current_source_location *loc = get_source_location (current_program_space);
> + if (loc->symtab)
> return;
I think this is supposed to be:
if (loc->symtab != nullptr)
return;
>
> /* Make the default place to list be the function `main'
> @@ -255,16 +279,15 @@ select_source_symtab (struct symtab *s)
> if (bsym.symbol != nullptr && SYMBOL_CLASS (bsym.symbol) == LOC_BLOCK)
> {
> symtab_and_line sal = find_function_start_sal (bsym.symbol, true);
> - current_source_pspace = sal.pspace;
> - current_source_symtab = sal.symtab;
> - current_source_line = std::max (sal.line - (lines_to_list - 1), 1);
> + loc->symtab = sal.symtab;
> + loc->line = std::max (sal.line - (lines_to_list - 1), 1);
> return;
> }
>
> /* Alright; find the last file in the symtab list (ignoring .h's
> and namespace symtabs). */
>
> - current_source_line = 1;
> + loc->line = 1;
>
> for (objfile *ofp : current_program_space->objfiles ())
> {
> @@ -277,15 +300,12 @@ select_source_symtab (struct symtab *s)
>
> if (!(len > 2 && (strcmp (&name[len - 2], ".h") == 0
> || strcmp (name, "<<C++-namespaces>>") == 0)))
> - {
> - current_source_pspace = current_program_space;
> - current_source_symtab = symtab;
> - }
> + loc->symtab = symtab;
> }
> }
> }
>
> - if (current_source_symtab)
> + if (loc->symtab)
> return;
Same again.
>
> for (objfile *objfile : current_program_space->objfiles ())
> @@ -293,9 +313,9 @@ select_source_symtab (struct symtab *s)
> if (objfile->sf)
> s = objfile->sf->qf->find_last_source_symtab (objfile);
> if (s)
> - current_source_symtab = s;
> + loc->symtab = s;
> }
> - if (current_source_symtab)
> + if (loc->symtab)
> return;
And again.
>
> error (_("Can't find a default source file"));
> @@ -624,7 +644,9 @@ add_path (const char *dirname, char **which_path, int parse_separators)
> static void
> info_source_command (const char *ignore, int from_tty)
> {
> - struct symtab *s = current_source_symtab;
> + current_source_location *loc
> + = get_source_location (current_program_space);
> + struct symtab *s = loc->symtab;
> struct compunit_symtab *cust;
>
> if (!s)
> @@ -1222,8 +1244,11 @@ print_source_lines_base (struct symtab *s, int line, int stopline,
> struct ui_out *uiout = current_uiout;
>
> /* Regardless of whether we can open the file, set current_source_symtab. */
> - current_source_symtab = s;
> - current_source_line = line;
> + current_source_location *loc
> + = get_source_location (current_program_space);
> +
> + loc->symtab = s;
> + loc->line = line;
> first_line_listed = line;
>
> /* If printing of source lines is disabled, just print file and line
> @@ -1313,13 +1338,13 @@ print_source_lines_base (struct symtab *s, int line, int stopline,
> {
> char buf[20];
>
> - last_line_listed = current_source_line;
> + last_line_listed = loc->line;
> if (flags & PRINT_SOURCE_LINES_FILENAME)
> {
> uiout->text (symtab_to_filename_for_display (s));
> uiout->text (":");
> }
> - xsnprintf (buf, sizeof (buf), "%d\t", current_source_line++);
> + xsnprintf (buf, sizeof (buf), "%d\t", loc->line++);
> uiout->text (buf);
>
> while (*iter != '\0')
> @@ -1411,12 +1436,14 @@ info_line_command (const char *arg, int from_tty)
>
> if (arg == 0)
> {
> - curr_sal.symtab = current_source_symtab;
> + current_source_location *loc
> + = get_source_location (current_program_space);
> + curr_sal.symtab = loc->symtab;
> curr_sal.pspace = current_program_space;
> if (last_line_listed != 0)
> curr_sal.line = last_line_listed;
> else
> - curr_sal.line = current_source_line;
> + curr_sal.line = loc->line;
>
> sals = curr_sal;
> }
> @@ -1518,23 +1545,25 @@ search_command_helper (const char *regex, int from_tty, bool forward)
> if (msg)
> error (("%s"), msg);
>
> - if (current_source_symtab == 0)
> + current_source_location *loc
> + = get_source_location (current_program_space);
> + if (loc->symtab == 0)
> select_source_symtab (0);
Update 0 to nullptr.
>
> - scoped_fd desc (open_source_file_with_line_charpos (current_source_symtab));
> + scoped_fd desc (open_source_file_with_line_charpos (loc->symtab));
> if (desc.get () < 0)
> - perror_with_name (symtab_to_filename_for_display (current_source_symtab));
> + perror_with_name (symtab_to_filename_for_display (loc->symtab));
>
> int line = (forward
> ? last_line_listed + 1
> : last_line_listed - 1);
>
> - if (line < 1 || line > current_source_symtab->nlines)
> + if (line < 1 || line > loc->symtab->nlines)
> error (_("Expression not found"));
>
> - if (lseek (desc.get (), current_source_symtab->line_charpos[line - 1], 0)
> + if (lseek (desc.get (), loc->symtab->line_charpos[line - 1], 0)
> < 0)
> - perror_with_name (symtab_to_filename_for_display (current_source_symtab));
> + perror_with_name (symtab_to_filename_for_display (loc->symtab));
>
> gdb_file_up stream = desc.to_file (FDOPEN_MODE);
> clearerr (stream.get ());
> @@ -1569,9 +1598,9 @@ search_command_helper (const char *regex, int from_tty, bool forward)
> if (re_exec (buf.data ()) > 0)
> {
> /* Match! */
> - print_source_lines (current_source_symtab, line, line + 1, 0);
> + print_source_lines (loc->symtab, line, line + 1, 0);
> set_internalvar_integer (lookup_internalvar ("_"), line);
> - current_source_line = std::max (line - lines_to_list / 2, 1);
> + loc->line = std::max (line - lines_to_list / 2, 1);
> return;
> }
>
> @@ -1583,10 +1612,10 @@ search_command_helper (const char *regex, int from_tty, bool forward)
> if (line < 1)
> break;
> if (fseek (stream.get (),
> - current_source_symtab->line_charpos[line - 1], 0) < 0)
> + loc->symtab->line_charpos[line - 1], 0) < 0)
> {
> const char *filename
> - = symtab_to_filename_for_display (current_source_symtab);
> + = symtab_to_filename_for_display (loc->symtab);
> perror_with_name (filename);
> }
> }
> @@ -1850,7 +1879,6 @@ _initialize_source (void)
> {
> struct cmd_list_element *c;
>
> - current_source_symtab = 0;
> init_source_path ();
>
> /* The intention is to use POSIX Basic Regular Expressions.
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v2 7/8] Add $_ada_exception convenience variable
2019-08-01 17:04 [PATCH v2 0/8] Handle copy relocations and add $_ada_exception Tom Tromey
2019-08-01 17:04 ` [PATCH v2 3/8] Back out earlier Ada exception change Tom Tromey
2019-08-01 17:04 ` [PATCH v2 6/8] Make current_source_* per-program-space Tom Tromey
@ 2019-08-01 17:04 ` Tom Tromey
2019-08-01 17:56 ` Eli Zaretskii
2019-08-27 9:47 ` Andrew Burgess
2019-08-01 17:04 ` [PATCH v2 1/8] Change SYMBOL_VALUE_ADDRESS to be an rvalue Tom Tromey
` (4 subsequent siblings)
7 siblings, 2 replies; 27+ messages in thread
From: Tom Tromey @ 2019-08-01 17:04 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This adds the $_ada_exception convenience variable. It is set by the
Ada exception catchpoints, and holds the address of the exception
currently being thrown. This is useful because it allows more
fine-grained filtering of exceptions than is possible using the
existing "catch" syntax.
This also simplifies Ada catchpoints somewhat; because the catchpoint
must now carry the "kind", it's possible to remove many helper
functions.
2019-08-01 Tom Tromey <tromey@adacore.com>
* NEWS: Add $_ada_exception entry.
* ada-lang.c (struct ada_catchpoint): Add constructor.
<m_kind>: New member.
(allocate_location_exception, re_set_exception): Remove
"ex" parameter.
(should_stop_exception): Compute $_ada_exception.
(check_status_exception, print_it_exception)
(print_one_exception, print_mention_exception): Remove
"ex" parameter.
(allocate_location_catch_exception, re_set_catch_exception)
(check_status_exception, print_it_catch_exception)
(print_one_catch_exception, print_mention_catch_exception)
(print_recreate_catch_exception)
(allocate_location_catch_exception_unhandled)
(re_set_catch_exception_unhandled)
(check_status_exception, print_it_catch_exception_unhandled)
(print_one_catch_exception_unhandled)
(print_mention_catch_exception_unhandled)
(print_recreate_catch_exception_unhandled)
(allocate_location_catch_assert, re_set_catch_assert)
(check_status_assert, print_it_catch_assert)
(print_one_catch_assert, print_mention_catch_assert)
(print_recreate_catch_assert)
(allocate_location_catch_handlers, re_set_catch_handlers)
(check_status_handlers, print_it_catch_handlers)
(print_one_catch_handlers, print_mention_catch_handlers)
(print_recreate_catch_handlers): Remove.
(create_ada_exception_catchpoint): Update.
(initialize_ada_catchpoint_ops): Update.
gdb/doc/ChangeLog
2019-08-01 Tom Tromey <tromey@adacore.com>
* gdb.texinfo (Set Catchpoints, Convenience Vars): Document
$_ada_exception.
gdb/testsuite/ChangeLog
2019-08-01 Tom Tromey <tromey@adacore.com>
* gdb.ada/catch_ex_std.exp: Add $_ada_exception test.
---
gdb/ChangeLog | 32 +++
gdb/NEWS | 3 +
gdb/ada-lang.c | 307 +++++++------------------
gdb/doc/ChangeLog | 5 +
gdb/doc/gdb.texinfo | 19 +-
gdb/testsuite/ChangeLog | 4 +
gdb/testsuite/gdb.ada/catch_ex_std.exp | 3 +
7 files changed, 141 insertions(+), 232 deletions(-)
diff --git a/gdb/NEWS b/gdb/NEWS
index ac44399304c..6bee9b13a38 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -25,6 +25,9 @@
provide the exitcode or exit status of the shell commands launched by
GDB commands such as "shell", "pipe" and "make".
+* New convenience variable $_ada_exception holds the address of the
+ Ada exception being thrown. This is set by Ada-related catchpoints.
+
* Python API
** The gdb.Value type has a new method 'format_string' which returns a
diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 6851386b48b..286eca3bfa1 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -12275,8 +12275,16 @@ public:
struct ada_catchpoint : public breakpoint
{
+ explicit ada_catchpoint (enum ada_exception_catchpoint_kind kind)
+ : m_kind (kind)
+ {
+ }
+
/* The name of the specific exception the user specified. */
std::string excep_string;
+
+ /* What kind of catchpoint this is. */
+ enum ada_exception_catchpoint_kind m_kind;
};
/* Parse the exception condition string in the context of each of the
@@ -12336,8 +12344,7 @@ create_excep_cond_exprs (struct ada_catchpoint *c,
structure for all exception catchpoint kinds. */
static struct bp_location *
-allocate_location_exception (enum ada_exception_catchpoint_kind ex,
- struct breakpoint *self)
+allocate_location_exception (struct breakpoint *self)
{
return new ada_catchpoint_location (self);
}
@@ -12346,7 +12353,7 @@ allocate_location_exception (enum ada_exception_catchpoint_kind ex,
exception catchpoint kinds. */
static void
-re_set_exception (enum ada_exception_catchpoint_kind ex, struct breakpoint *b)
+re_set_exception (struct breakpoint *b)
{
struct ada_catchpoint *c = (struct ada_catchpoint *) b;
@@ -12356,7 +12363,7 @@ re_set_exception (enum ada_exception_catchpoint_kind ex, struct breakpoint *b)
/* Reparse the exception conditional expressions. One for each
location. */
- create_excep_cond_exprs (c, ex);
+ create_excep_cond_exprs (c, c->m_kind);
}
/* Returns true if we should stop for this breakpoint hit. If the
@@ -12371,6 +12378,30 @@ should_stop_exception (const struct bp_location *bl)
= (const struct ada_catchpoint_location *) bl;
int stop;
+ struct internalvar *var = lookup_internalvar ("_ada_exception");
+ if (c->m_kind == ada_catch_assert)
+ clear_internalvar (var);
+ else
+ {
+ try
+ {
+ const char *expr;
+
+ if (c->m_kind == ada_catch_handlers)
+ expr = ("GNAT_GCC_exception_Access(gcc_exception)"
+ ".all.occurrence.id");
+ else
+ expr = "e";
+
+ struct value *exc = parse_and_eval (expr);
+ set_internalvar (var, exc);
+ }
+ catch (const gdb_exception_error &ex)
+ {
+ clear_internalvar (var);
+ }
+ }
+
/* With no specific exception, should always stop. */
if (c->excep_string.empty ())
return 1;
@@ -12404,7 +12435,7 @@ should_stop_exception (const struct bp_location *bl)
for all exception catchpoint kinds. */
static void
-check_status_exception (enum ada_exception_catchpoint_kind ex, bpstat bs)
+check_status_exception (bpstat bs)
{
bs->stop = should_stop_exception (bs->bp_location_at);
}
@@ -12413,7 +12444,7 @@ check_status_exception (enum ada_exception_catchpoint_kind ex, bpstat bs)
for all exception catchpoint kinds. */
static enum print_stop_action
-print_it_exception (enum ada_exception_catchpoint_kind ex, bpstat bs)
+print_it_exception (bpstat bs)
{
struct ui_out *uiout = current_uiout;
struct breakpoint *b = bs->breakpoint_at;
@@ -12439,13 +12470,14 @@ print_it_exception (enum ada_exception_catchpoint_kind ex, bpstat bs)
ada_find_printable_frame). */
select_frame (get_current_frame ());
- switch (ex)
+ struct ada_catchpoint *c = (struct ada_catchpoint *) b;
+ switch (c->m_kind)
{
case ada_catch_exception:
case ada_catch_exception_unhandled:
case ada_catch_handlers:
{
- const CORE_ADDR addr = ada_exception_name_addr (ex, b);
+ const CORE_ADDR addr = ada_exception_name_addr (c->m_kind, b);
char exception_name[256];
if (addr != 0)
@@ -12469,7 +12501,7 @@ print_it_exception (enum ada_exception_catchpoint_kind ex, bpstat bs)
it clearer to the user which kind of catchpoint just got
hit. We used ui_out_text to make sure that this extra
info does not pollute the exception name in the MI case. */
- if (ex == ada_catch_exception_unhandled)
+ if (c->m_kind == ada_catch_exception_unhandled)
uiout->text ("unhandled ");
uiout->field_string ("exception-name", exception_name);
}
@@ -12502,8 +12534,7 @@ print_it_exception (enum ada_exception_catchpoint_kind ex, bpstat bs)
for all exception catchpoint kinds. */
static void
-print_one_exception (enum ada_exception_catchpoint_kind ex,
- struct breakpoint *b, struct bp_location **last_loc)
+print_one_exception (struct breakpoint *b, struct bp_location **last_loc)
{
struct ui_out *uiout = current_uiout;
struct ada_catchpoint *c = (struct ada_catchpoint *) b;
@@ -12515,7 +12546,7 @@ print_one_exception (enum ada_exception_catchpoint_kind ex,
uiout->field_skip ("addr");
annotate_field (5);
- switch (ex)
+ switch (c->m_kind)
{
case ada_catch_exception:
if (!c->excep_string.empty ())
@@ -12559,8 +12590,7 @@ print_one_exception (enum ada_exception_catchpoint_kind ex,
for all exception catchpoint kinds. */
static void
-print_mention_exception (enum ada_exception_catchpoint_kind ex,
- struct breakpoint *b)
+print_mention_exception (struct breakpoint *b)
{
struct ada_catchpoint *c = (struct ada_catchpoint *) b;
struct ui_out *uiout = current_uiout;
@@ -12570,7 +12600,7 @@ print_mention_exception (enum ada_exception_catchpoint_kind ex,
uiout->field_signed ("bkptno", b->number);
uiout->text (": ");
- switch (ex)
+ switch (c->m_kind)
{
case ada_catch_exception:
if (!c->excep_string.empty ())
@@ -12613,12 +12643,11 @@ print_mention_exception (enum ada_exception_catchpoint_kind ex,
for all exception catchpoint kinds. */
static void
-print_recreate_exception (enum ada_exception_catchpoint_kind ex,
- struct breakpoint *b, struct ui_file *fp)
+print_recreate_exception (struct breakpoint *b, struct ui_file *fp)
{
struct ada_catchpoint *c = (struct ada_catchpoint *) b;
- switch (ex)
+ switch (c->m_kind)
{
case ada_catch_exception:
fprintf_filtered (fp, "catch exception");
@@ -12644,192 +12673,10 @@ print_recreate_exception (enum ada_exception_catchpoint_kind ex,
print_recreate_thread (b, fp);
}
-/* Virtual table for "catch exception" breakpoints. */
-
-static struct bp_location *
-allocate_location_catch_exception (struct breakpoint *self)
-{
- return allocate_location_exception (ada_catch_exception, self);
-}
-
-static void
-re_set_catch_exception (struct breakpoint *b)
-{
- re_set_exception (ada_catch_exception, b);
-}
-
-static void
-check_status_catch_exception (bpstat bs)
-{
- check_status_exception (ada_catch_exception, bs);
-}
-
-static enum print_stop_action
-print_it_catch_exception (bpstat bs)
-{
- return print_it_exception (ada_catch_exception, bs);
-}
-
-static void
-print_one_catch_exception (struct breakpoint *b, struct bp_location **last_loc)
-{
- print_one_exception (ada_catch_exception, b, last_loc);
-}
-
-static void
-print_mention_catch_exception (struct breakpoint *b)
-{
- print_mention_exception (ada_catch_exception, b);
-}
-
-static void
-print_recreate_catch_exception (struct breakpoint *b, struct ui_file *fp)
-{
- print_recreate_exception (ada_catch_exception, b, fp);
-}
-
+/* Virtual tables for various breakpoint types. */
static struct breakpoint_ops catch_exception_breakpoint_ops;
-
-/* Virtual table for "catch exception unhandled" breakpoints. */
-
-static struct bp_location *
-allocate_location_catch_exception_unhandled (struct breakpoint *self)
-{
- return allocate_location_exception (ada_catch_exception_unhandled, self);
-}
-
-static void
-re_set_catch_exception_unhandled (struct breakpoint *b)
-{
- re_set_exception (ada_catch_exception_unhandled, b);
-}
-
-static void
-check_status_catch_exception_unhandled (bpstat bs)
-{
- check_status_exception (ada_catch_exception_unhandled, bs);
-}
-
-static enum print_stop_action
-print_it_catch_exception_unhandled (bpstat bs)
-{
- return print_it_exception (ada_catch_exception_unhandled, bs);
-}
-
-static void
-print_one_catch_exception_unhandled (struct breakpoint *b,
- struct bp_location **last_loc)
-{
- print_one_exception (ada_catch_exception_unhandled, b, last_loc);
-}
-
-static void
-print_mention_catch_exception_unhandled (struct breakpoint *b)
-{
- print_mention_exception (ada_catch_exception_unhandled, b);
-}
-
-static void
-print_recreate_catch_exception_unhandled (struct breakpoint *b,
- struct ui_file *fp)
-{
- print_recreate_exception (ada_catch_exception_unhandled, b, fp);
-}
-
static struct breakpoint_ops catch_exception_unhandled_breakpoint_ops;
-
-/* Virtual table for "catch assert" breakpoints. */
-
-static struct bp_location *
-allocate_location_catch_assert (struct breakpoint *self)
-{
- return allocate_location_exception (ada_catch_assert, self);
-}
-
-static void
-re_set_catch_assert (struct breakpoint *b)
-{
- re_set_exception (ada_catch_assert, b);
-}
-
-static void
-check_status_catch_assert (bpstat bs)
-{
- check_status_exception (ada_catch_assert, bs);
-}
-
-static enum print_stop_action
-print_it_catch_assert (bpstat bs)
-{
- return print_it_exception (ada_catch_assert, bs);
-}
-
-static void
-print_one_catch_assert (struct breakpoint *b, struct bp_location **last_loc)
-{
- print_one_exception (ada_catch_assert, b, last_loc);
-}
-
-static void
-print_mention_catch_assert (struct breakpoint *b)
-{
- print_mention_exception (ada_catch_assert, b);
-}
-
-static void
-print_recreate_catch_assert (struct breakpoint *b, struct ui_file *fp)
-{
- print_recreate_exception (ada_catch_assert, b, fp);
-}
-
static struct breakpoint_ops catch_assert_breakpoint_ops;
-
-/* Virtual table for "catch handlers" breakpoints. */
-
-static struct bp_location *
-allocate_location_catch_handlers (struct breakpoint *self)
-{
- return allocate_location_exception (ada_catch_handlers, self);
-}
-
-static void
-re_set_catch_handlers (struct breakpoint *b)
-{
- re_set_exception (ada_catch_handlers, b);
-}
-
-static void
-check_status_catch_handlers (bpstat bs)
-{
- check_status_exception (ada_catch_handlers, bs);
-}
-
-static enum print_stop_action
-print_it_catch_handlers (bpstat bs)
-{
- return print_it_exception (ada_catch_handlers, bs);
-}
-
-static void
-print_one_catch_handlers (struct breakpoint *b,
- struct bp_location **last_loc)
-{
- print_one_exception (ada_catch_handlers, b, last_loc);
-}
-
-static void
-print_mention_catch_handlers (struct breakpoint *b)
-{
- print_mention_exception (ada_catch_handlers, b);
-}
-
-static void
-print_recreate_catch_handlers (struct breakpoint *b,
- struct ui_file *fp)
-{
- print_recreate_exception (ada_catch_handlers, b, fp);
-}
-
static struct breakpoint_ops catch_handlers_breakpoint_ops;
/* See ada-lang.h. */
@@ -13103,7 +12950,7 @@ create_ada_exception_catchpoint (struct gdbarch *gdbarch,
const struct breakpoint_ops *ops = NULL;
struct symtab_and_line sal = ada_exception_sal (ex_kind, &addr_string, &ops);
- std::unique_ptr<ada_catchpoint> c (new ada_catchpoint ());
+ std::unique_ptr<ada_catchpoint> c (new ada_catchpoint (ex_kind));
init_ada_exception_breakpoint (c.get (), gdbarch, sal, addr_string.c_str (),
ops, tempflag, disabled, from_tty);
c->excep_string = excep_string;
@@ -14294,43 +14141,43 @@ initialize_ada_catchpoint_ops (void)
ops = &catch_exception_breakpoint_ops;
*ops = bkpt_breakpoint_ops;
- ops->allocate_location = allocate_location_catch_exception;
- ops->re_set = re_set_catch_exception;
- ops->check_status = check_status_catch_exception;
- ops->print_it = print_it_catch_exception;
- ops->print_one = print_one_catch_exception;
- ops->print_mention = print_mention_catch_exception;
- ops->print_recreate = print_recreate_catch_exception;
+ ops->allocate_location = allocate_location_exception;
+ ops->re_set = re_set_exception;
+ ops->check_status = check_status_exception;
+ ops->print_it = print_it_exception;
+ ops->print_one = print_one_exception;
+ ops->print_mention = print_mention_exception;
+ ops->print_recreate = print_recreate_exception;
ops = &catch_exception_unhandled_breakpoint_ops;
*ops = bkpt_breakpoint_ops;
- ops->allocate_location = allocate_location_catch_exception_unhandled;
- ops->re_set = re_set_catch_exception_unhandled;
- ops->check_status = check_status_catch_exception_unhandled;
- ops->print_it = print_it_catch_exception_unhandled;
- ops->print_one = print_one_catch_exception_unhandled;
- ops->print_mention = print_mention_catch_exception_unhandled;
- ops->print_recreate = print_recreate_catch_exception_unhandled;
+ ops->allocate_location = allocate_location_exception;
+ ops->re_set = re_set_exception;
+ ops->check_status = check_status_exception;
+ ops->print_it = print_it_exception;
+ ops->print_one = print_one_exception;
+ ops->print_mention = print_mention_exception;
+ ops->print_recreate = print_recreate_exception;
ops = &catch_assert_breakpoint_ops;
*ops = bkpt_breakpoint_ops;
- ops->allocate_location = allocate_location_catch_assert;
- ops->re_set = re_set_catch_assert;
- ops->check_status = check_status_catch_assert;
- ops->print_it = print_it_catch_assert;
- ops->print_one = print_one_catch_assert;
- ops->print_mention = print_mention_catch_assert;
- ops->print_recreate = print_recreate_catch_assert;
+ ops->allocate_location = allocate_location_exception;
+ ops->re_set = re_set_exception;
+ ops->check_status = check_status_exception;
+ ops->print_it = print_it_exception;
+ ops->print_one = print_one_exception;
+ ops->print_mention = print_mention_exception;
+ ops->print_recreate = print_recreate_exception;
ops = &catch_handlers_breakpoint_ops;
*ops = bkpt_breakpoint_ops;
- ops->allocate_location = allocate_location_catch_handlers;
- ops->re_set = re_set_catch_handlers;
- ops->check_status = check_status_catch_handlers;
- ops->print_it = print_it_catch_handlers;
- ops->print_one = print_one_catch_handlers;
- ops->print_mention = print_mention_catch_handlers;
- ops->print_recreate = print_recreate_catch_handlers;
+ ops->allocate_location = allocate_location_exception;
+ ops->re_set = re_set_exception;
+ ops->check_status = check_status_exception;
+ ops->print_it = print_it_exception;
+ ops->print_one = print_one_exception;
+ ops->print_mention = print_mention_exception;
+ ops->print_recreate = print_recreate_exception;
}
/* This module's 'new_objfile' observer. */
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 0fcd131f71e..ae266d4bfac 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -4788,9 +4788,15 @@ called @code{Constraint_Error} is defined in package @code{Pck}, then
the command to use to catch such exceptions is @kbd{catch exception
Pck.Constraint_Error}.
+The convenience variable @code{$_ada_exception} holds the address of
+the exception being thrown. This can be useful when setting a
+condition for such a catchpoint.
+
@item exception unhandled
@kindex catch exception unhandled
-An exception that was raised but is not handled by the program.
+An exception that was raised but is not handled by the program. The
+convenience variable @code{$_ada_exception} is set as for @code{catch
+exception}.
@item handlers @r{[}@var{name}@r{]}
@kindex catch handlers
@@ -4812,9 +4818,13 @@ user-defined one. For instance, assuming an exception called
command to use to catch such exceptions handling is
@kbd{catch handlers Pck.Constraint_Error}.
+The convenience variable @code{$_ada_exception} is set as for
+@code{catch exception}.
+
@item assert
@kindex catch assert
-A failed Ada assertion.
+A failed Ada assertion. Note that the convenience variable
+@code{$_ada_exception} is @emph{not} set by this catchpoint.
@item exec
@kindex catch exec
@@ -11742,6 +11752,11 @@ The program has exited
The variable @code{$_exception} is set to the exception object being
thrown at an exception-related catchpoint. @xref{Set Catchpoints}.
+@item $_ada_exception
+The variable @code{$_ada_exception} is set to the address of the
+exception being caught or thrown at an Ada exception-related
+catchpoint. @xref{Set Catchpoints}.
+
@item $_probe_argc
@itemx $_probe_arg0@dots{}$_probe_arg11
Arguments to a static probe. @xref{Static Probe Points}.
diff --git a/gdb/testsuite/gdb.ada/catch_ex_std.exp b/gdb/testsuite/gdb.ada/catch_ex_std.exp
index 63714a8aa81..839d0bb092f 100644
--- a/gdb/testsuite/gdb.ada/catch_ex_std.exp
+++ b/gdb/testsuite/gdb.ada/catch_ex_std.exp
@@ -101,3 +101,6 @@ gdb_test "catch exception some_kind_of_error" \
gdb_test "cont" \
"Catchpoint \[0-9\]+, .* at .*foo\.adb:\[0-9\]+.*" \
"caught the exception"
+
+gdb_test "print \$_ada_exception = some_package.some_kind_of_error'Address" \
+ " = true"
--
2.20.1
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v2 7/8] Add $_ada_exception convenience variable
2019-08-01 17:04 ` [PATCH v2 7/8] Add $_ada_exception convenience variable Tom Tromey
@ 2019-08-01 17:56 ` Eli Zaretskii
2019-09-20 19:16 ` Tom Tromey
2019-08-27 9:47 ` Andrew Burgess
1 sibling, 1 reply; 27+ messages in thread
From: Eli Zaretskii @ 2019-08-01 17:56 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
> From: Tom Tromey <tromey@adacore.com>
> Cc: Tom Tromey <tromey@adacore.com>
> Date: Thu, 1 Aug 2019 11:04:11 -0600
>
> This adds the $_ada_exception convenience variable. It is set by the
> Ada exception catchpoints, and holds the address of the exception
> currently being thrown. This is useful because it allows more
> fine-grained filtering of exceptions than is possible using the
> existing "catch" syntax.
>
> This also simplifies Ada catchpoints somewhat; because the catchpoint
> must now carry the "kind", it's possible to remove many helper
> functions.
>
> 2019-08-01 Tom Tromey <tromey@adacore.com>
>
> * NEWS: Add $_ada_exception entry.
> * ada-lang.c (struct ada_catchpoint): Add constructor.
> <m_kind>: New member.
> (allocate_location_exception, re_set_exception): Remove
> "ex" parameter.
> (should_stop_exception): Compute $_ada_exception.
> (check_status_exception, print_it_exception)
> (print_one_exception, print_mention_exception): Remove
> "ex" parameter.
> (allocate_location_catch_exception, re_set_catch_exception)
> (check_status_exception, print_it_catch_exception)
> (print_one_catch_exception, print_mention_catch_exception)
> (print_recreate_catch_exception)
> (allocate_location_catch_exception_unhandled)
> (re_set_catch_exception_unhandled)
> (check_status_exception, print_it_catch_exception_unhandled)
> (print_one_catch_exception_unhandled)
> (print_mention_catch_exception_unhandled)
> (print_recreate_catch_exception_unhandled)
> (allocate_location_catch_assert, re_set_catch_assert)
> (check_status_assert, print_it_catch_assert)
> (print_one_catch_assert, print_mention_catch_assert)
> (print_recreate_catch_assert)
> (allocate_location_catch_handlers, re_set_catch_handlers)
> (check_status_handlers, print_it_catch_handlers)
> (print_one_catch_handlers, print_mention_catch_handlers)
> (print_recreate_catch_handlers): Remove.
> (create_ada_exception_catchpoint): Update.
> (initialize_ada_catchpoint_ops): Update.
>
> gdb/doc/ChangeLog
> 2019-08-01 Tom Tromey <tromey@adacore.com>
>
> * gdb.texinfo (Set Catchpoints, Convenience Vars): Document
> $_ada_exception.
>
> gdb/testsuite/ChangeLog
> 2019-08-01 Tom Tromey <tromey@adacore.com>
>
> * gdb.ada/catch_ex_std.exp: Add $_ada_exception test.
Thanks. The documentation parts are approved, but I wonder whether we
should index this new variable, as others are indexed.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2 7/8] Add $_ada_exception convenience variable
2019-08-01 17:56 ` Eli Zaretskii
@ 2019-09-20 19:16 ` Tom Tromey
0 siblings, 0 replies; 27+ messages in thread
From: Tom Tromey @ 2019-09-20 19:16 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: Tom Tromey, gdb-patches
>> gdb/doc/ChangeLog
>> 2019-08-01 Tom Tromey <tromey@adacore.com>
>>
>> * gdb.texinfo (Set Catchpoints, Convenience Vars): Document
>> $_ada_exception.
Eli> Thanks. The documentation parts are approved, but I wonder whether we
Eli> should index this new variable, as others are indexed.
I've done this in the next version of the patch.
Tom
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2 7/8] Add $_ada_exception convenience variable
2019-08-01 17:04 ` [PATCH v2 7/8] Add $_ada_exception convenience variable Tom Tromey
2019-08-01 17:56 ` Eli Zaretskii
@ 2019-08-27 9:47 ` Andrew Burgess
2019-09-20 19:15 ` Tom Tromey
1 sibling, 1 reply; 27+ messages in thread
From: Andrew Burgess @ 2019-08-27 9:47 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
* Tom Tromey <tromey@adacore.com> [2019-08-01 11:04:11 -0600]:
> This adds the $_ada_exception convenience variable. It is set by the
> Ada exception catchpoints, and holds the address of the exception
> currently being thrown. This is useful because it allows more
> fine-grained filtering of exceptions than is possible using the
> existing "catch" syntax.
>
> This also simplifies Ada catchpoints somewhat; because the catchpoint
> must now carry the "kind", it's possible to remove many helper
> functions.
>
> 2019-08-01 Tom Tromey <tromey@adacore.com>
>
> * NEWS: Add $_ada_exception entry.
> * ada-lang.c (struct ada_catchpoint): Add constructor.
> <m_kind>: New member.
> (allocate_location_exception, re_set_exception): Remove
> "ex" parameter.
> (should_stop_exception): Compute $_ada_exception.
> (check_status_exception, print_it_exception)
> (print_one_exception, print_mention_exception): Remove
> "ex" parameter.
> (allocate_location_catch_exception, re_set_catch_exception)
> (check_status_exception, print_it_catch_exception)
> (print_one_catch_exception, print_mention_catch_exception)
> (print_recreate_catch_exception)
> (allocate_location_catch_exception_unhandled)
> (re_set_catch_exception_unhandled)
> (check_status_exception, print_it_catch_exception_unhandled)
> (print_one_catch_exception_unhandled)
> (print_mention_catch_exception_unhandled)
> (print_recreate_catch_exception_unhandled)
> (allocate_location_catch_assert, re_set_catch_assert)
> (check_status_assert, print_it_catch_assert)
> (print_one_catch_assert, print_mention_catch_assert)
> (print_recreate_catch_assert)
> (allocate_location_catch_handlers, re_set_catch_handlers)
> (check_status_handlers, print_it_catch_handlers)
> (print_one_catch_handlers, print_mention_catch_handlers)
> (print_recreate_catch_handlers): Remove.
> (create_ada_exception_catchpoint): Update.
> (initialize_ada_catchpoint_ops): Update.
>
> gdb/doc/ChangeLog
> 2019-08-01 Tom Tromey <tromey@adacore.com>
>
> * gdb.texinfo (Set Catchpoints, Convenience Vars): Document
> $_ada_exception.
>
> gdb/testsuite/ChangeLog
> 2019-08-01 Tom Tromey <tromey@adacore.com>
>
> * gdb.ada/catch_ex_std.exp: Add $_ada_exception test.
> ---
> gdb/ChangeLog | 32 +++
> gdb/NEWS | 3 +
> gdb/ada-lang.c | 307 +++++++------------------
> gdb/doc/ChangeLog | 5 +
> gdb/doc/gdb.texinfo | 19 +-
> gdb/testsuite/ChangeLog | 4 +
> gdb/testsuite/gdb.ada/catch_ex_std.exp | 3 +
> 7 files changed, 141 insertions(+), 232 deletions(-)
>
> diff --git a/gdb/NEWS b/gdb/NEWS
> index ac44399304c..6bee9b13a38 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -25,6 +25,9 @@
> provide the exitcode or exit status of the shell commands launched by
> GDB commands such as "shell", "pipe" and "make".
>
> +* New convenience variable $_ada_exception holds the address of the
> + Ada exception being thrown. This is set by Ada-related catchpoints.
> +
> * Python API
>
> ** The gdb.Value type has a new method 'format_string' which returns a
> diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
> index 6851386b48b..286eca3bfa1 100644
> --- a/gdb/ada-lang.c
> +++ b/gdb/ada-lang.c
> @@ -12275,8 +12275,16 @@ public:
>
> struct ada_catchpoint : public breakpoint
> {
> + explicit ada_catchpoint (enum ada_exception_catchpoint_kind kind)
> + : m_kind (kind)
> + {
> + }
> +
> /* The name of the specific exception the user specified. */
> std::string excep_string;
> +
> + /* What kind of catchpoint this is. */
> + enum ada_exception_catchpoint_kind m_kind;
> };
>
> /* Parse the exception condition string in the context of each of the
> @@ -12336,8 +12344,7 @@ create_excep_cond_exprs (struct ada_catchpoint *c,
> structure for all exception catchpoint kinds. */
>
> static struct bp_location *
> -allocate_location_exception (enum ada_exception_catchpoint_kind ex,
> - struct breakpoint *self)
> +allocate_location_exception (struct breakpoint *self)
> {
> return new ada_catchpoint_location (self);
> }
> @@ -12346,7 +12353,7 @@ allocate_location_exception (enum ada_exception_catchpoint_kind ex,
> exception catchpoint kinds. */
>
> static void
> -re_set_exception (enum ada_exception_catchpoint_kind ex, struct breakpoint *b)
> +re_set_exception (struct breakpoint *b)
> {
> struct ada_catchpoint *c = (struct ada_catchpoint *) b;
>
> @@ -12356,7 +12363,7 @@ re_set_exception (enum ada_exception_catchpoint_kind ex, struct breakpoint *b)
>
> /* Reparse the exception conditional expressions. One for each
> location. */
> - create_excep_cond_exprs (c, ex);
> + create_excep_cond_exprs (c, c->m_kind);
> }
>
> /* Returns true if we should stop for this breakpoint hit. If the
> @@ -12371,6 +12378,30 @@ should_stop_exception (const struct bp_location *bl)
> = (const struct ada_catchpoint_location *) bl;
> int stop;
>
> + struct internalvar *var = lookup_internalvar ("_ada_exception");
> + if (c->m_kind == ada_catch_assert)
> + clear_internalvar (var);
> + else
> + {
> + try
> + {
> + const char *expr;
> +
> + if (c->m_kind == ada_catch_handlers)
> + expr = ("GNAT_GCC_exception_Access(gcc_exception)"
> + ".all.occurrence.id");
> + else
> + expr = "e";
> +
> + struct value *exc = parse_and_eval (expr);
> + set_internalvar (var, exc);
> + }
> + catch (const gdb_exception_error &ex)
> + {
> + clear_internalvar (var);
> + }
> + }
> +
> /* With no specific exception, should always stop. */
> if (c->excep_string.empty ())
> return 1;
> @@ -12404,7 +12435,7 @@ should_stop_exception (const struct bp_location *bl)
> for all exception catchpoint kinds. */
>
> static void
> -check_status_exception (enum ada_exception_catchpoint_kind ex, bpstat bs)
> +check_status_exception (bpstat bs)
> {
> bs->stop = should_stop_exception (bs->bp_location_at);
> }
> @@ -12413,7 +12444,7 @@ check_status_exception (enum ada_exception_catchpoint_kind ex, bpstat bs)
> for all exception catchpoint kinds. */
>
> static enum print_stop_action
> -print_it_exception (enum ada_exception_catchpoint_kind ex, bpstat bs)
> +print_it_exception (bpstat bs)
> {
> struct ui_out *uiout = current_uiout;
> struct breakpoint *b = bs->breakpoint_at;
> @@ -12439,13 +12470,14 @@ print_it_exception (enum ada_exception_catchpoint_kind ex, bpstat bs)
> ada_find_printable_frame). */
> select_frame (get_current_frame ());
>
> - switch (ex)
> + struct ada_catchpoint *c = (struct ada_catchpoint *) b;
> + switch (c->m_kind)
> {
> case ada_catch_exception:
> case ada_catch_exception_unhandled:
> case ada_catch_handlers:
> {
> - const CORE_ADDR addr = ada_exception_name_addr (ex, b);
> + const CORE_ADDR addr = ada_exception_name_addr (c->m_kind, b);
> char exception_name[256];
>
> if (addr != 0)
> @@ -12469,7 +12501,7 @@ print_it_exception (enum ada_exception_catchpoint_kind ex, bpstat bs)
> it clearer to the user which kind of catchpoint just got
> hit. We used ui_out_text to make sure that this extra
> info does not pollute the exception name in the MI case. */
> - if (ex == ada_catch_exception_unhandled)
> + if (c->m_kind == ada_catch_exception_unhandled)
> uiout->text ("unhandled ");
> uiout->field_string ("exception-name", exception_name);
> }
> @@ -12502,8 +12534,7 @@ print_it_exception (enum ada_exception_catchpoint_kind ex, bpstat bs)
> for all exception catchpoint kinds. */
>
> static void
> -print_one_exception (enum ada_exception_catchpoint_kind ex,
> - struct breakpoint *b, struct bp_location **last_loc)
> +print_one_exception (struct breakpoint *b, struct bp_location **last_loc)
> {
> struct ui_out *uiout = current_uiout;
> struct ada_catchpoint *c = (struct ada_catchpoint *) b;
> @@ -12515,7 +12546,7 @@ print_one_exception (enum ada_exception_catchpoint_kind ex,
> uiout->field_skip ("addr");
>
> annotate_field (5);
> - switch (ex)
> + switch (c->m_kind)
> {
> case ada_catch_exception:
> if (!c->excep_string.empty ())
> @@ -12559,8 +12590,7 @@ print_one_exception (enum ada_exception_catchpoint_kind ex,
> for all exception catchpoint kinds. */
>
> static void
> -print_mention_exception (enum ada_exception_catchpoint_kind ex,
> - struct breakpoint *b)
> +print_mention_exception (struct breakpoint *b)
> {
> struct ada_catchpoint *c = (struct ada_catchpoint *) b;
> struct ui_out *uiout = current_uiout;
> @@ -12570,7 +12600,7 @@ print_mention_exception (enum ada_exception_catchpoint_kind ex,
> uiout->field_signed ("bkptno", b->number);
> uiout->text (": ");
>
> - switch (ex)
> + switch (c->m_kind)
> {
> case ada_catch_exception:
> if (!c->excep_string.empty ())
> @@ -12613,12 +12643,11 @@ print_mention_exception (enum ada_exception_catchpoint_kind ex,
> for all exception catchpoint kinds. */
>
> static void
> -print_recreate_exception (enum ada_exception_catchpoint_kind ex,
> - struct breakpoint *b, struct ui_file *fp)
> +print_recreate_exception (struct breakpoint *b, struct ui_file *fp)
> {
> struct ada_catchpoint *c = (struct ada_catchpoint *) b;
>
> - switch (ex)
> + switch (c->m_kind)
> {
> case ada_catch_exception:
> fprintf_filtered (fp, "catch exception");
> @@ -12644,192 +12673,10 @@ print_recreate_exception (enum ada_exception_catchpoint_kind ex,
> print_recreate_thread (b, fp);
> }
>
> -/* Virtual table for "catch exception" breakpoints. */
> -
> -static struct bp_location *
> -allocate_location_catch_exception (struct breakpoint *self)
> -{
> - return allocate_location_exception (ada_catch_exception, self);
> -}
> -
> -static void
> -re_set_catch_exception (struct breakpoint *b)
> -{
> - re_set_exception (ada_catch_exception, b);
> -}
> -
> -static void
> -check_status_catch_exception (bpstat bs)
> -{
> - check_status_exception (ada_catch_exception, bs);
> -}
> -
> -static enum print_stop_action
> -print_it_catch_exception (bpstat bs)
> -{
> - return print_it_exception (ada_catch_exception, bs);
> -}
> -
> -static void
> -print_one_catch_exception (struct breakpoint *b, struct bp_location **last_loc)
> -{
> - print_one_exception (ada_catch_exception, b, last_loc);
> -}
> -
> -static void
> -print_mention_catch_exception (struct breakpoint *b)
> -{
> - print_mention_exception (ada_catch_exception, b);
> -}
> -
> -static void
> -print_recreate_catch_exception (struct breakpoint *b, struct ui_file *fp)
> -{
> - print_recreate_exception (ada_catch_exception, b, fp);
> -}
> -
> +/* Virtual tables for various breakpoint types. */
> static struct breakpoint_ops catch_exception_breakpoint_ops;
> -
> -/* Virtual table for "catch exception unhandled" breakpoints. */
> -
> -static struct bp_location *
> -allocate_location_catch_exception_unhandled (struct breakpoint *self)
> -{
> - return allocate_location_exception (ada_catch_exception_unhandled, self);
> -}
> -
> -static void
> -re_set_catch_exception_unhandled (struct breakpoint *b)
> -{
> - re_set_exception (ada_catch_exception_unhandled, b);
> -}
> -
> -static void
> -check_status_catch_exception_unhandled (bpstat bs)
> -{
> - check_status_exception (ada_catch_exception_unhandled, bs);
> -}
> -
> -static enum print_stop_action
> -print_it_catch_exception_unhandled (bpstat bs)
> -{
> - return print_it_exception (ada_catch_exception_unhandled, bs);
> -}
> -
> -static void
> -print_one_catch_exception_unhandled (struct breakpoint *b,
> - struct bp_location **last_loc)
> -{
> - print_one_exception (ada_catch_exception_unhandled, b, last_loc);
> -}
> -
> -static void
> -print_mention_catch_exception_unhandled (struct breakpoint *b)
> -{
> - print_mention_exception (ada_catch_exception_unhandled, b);
> -}
> -
> -static void
> -print_recreate_catch_exception_unhandled (struct breakpoint *b,
> - struct ui_file *fp)
> -{
> - print_recreate_exception (ada_catch_exception_unhandled, b, fp);
> -}
> -
> static struct breakpoint_ops catch_exception_unhandled_breakpoint_ops;
> -
> -/* Virtual table for "catch assert" breakpoints. */
> -
> -static struct bp_location *
> -allocate_location_catch_assert (struct breakpoint *self)
> -{
> - return allocate_location_exception (ada_catch_assert, self);
> -}
> -
> -static void
> -re_set_catch_assert (struct breakpoint *b)
> -{
> - re_set_exception (ada_catch_assert, b);
> -}
> -
> -static void
> -check_status_catch_assert (bpstat bs)
> -{
> - check_status_exception (ada_catch_assert, bs);
> -}
> -
> -static enum print_stop_action
> -print_it_catch_assert (bpstat bs)
> -{
> - return print_it_exception (ada_catch_assert, bs);
> -}
> -
> -static void
> -print_one_catch_assert (struct breakpoint *b, struct bp_location **last_loc)
> -{
> - print_one_exception (ada_catch_assert, b, last_loc);
> -}
> -
> -static void
> -print_mention_catch_assert (struct breakpoint *b)
> -{
> - print_mention_exception (ada_catch_assert, b);
> -}
> -
> -static void
> -print_recreate_catch_assert (struct breakpoint *b, struct ui_file *fp)
> -{
> - print_recreate_exception (ada_catch_assert, b, fp);
> -}
> -
> static struct breakpoint_ops catch_assert_breakpoint_ops;
> -
> -/* Virtual table for "catch handlers" breakpoints. */
> -
> -static struct bp_location *
> -allocate_location_catch_handlers (struct breakpoint *self)
> -{
> - return allocate_location_exception (ada_catch_handlers, self);
> -}
> -
> -static void
> -re_set_catch_handlers (struct breakpoint *b)
> -{
> - re_set_exception (ada_catch_handlers, b);
> -}
> -
> -static void
> -check_status_catch_handlers (bpstat bs)
> -{
> - check_status_exception (ada_catch_handlers, bs);
> -}
> -
> -static enum print_stop_action
> -print_it_catch_handlers (bpstat bs)
> -{
> - return print_it_exception (ada_catch_handlers, bs);
> -}
> -
> -static void
> -print_one_catch_handlers (struct breakpoint *b,
> - struct bp_location **last_loc)
> -{
> - print_one_exception (ada_catch_handlers, b, last_loc);
> -}
> -
> -static void
> -print_mention_catch_handlers (struct breakpoint *b)
> -{
> - print_mention_exception (ada_catch_handlers, b);
> -}
> -
> -static void
> -print_recreate_catch_handlers (struct breakpoint *b,
> - struct ui_file *fp)
> -{
> - print_recreate_exception (ada_catch_handlers, b, fp);
> -}
> -
> static struct breakpoint_ops catch_handlers_breakpoint_ops;
>
> /* See ada-lang.h. */
> @@ -13103,7 +12950,7 @@ create_ada_exception_catchpoint (struct gdbarch *gdbarch,
> const struct breakpoint_ops *ops = NULL;
> struct symtab_and_line sal = ada_exception_sal (ex_kind, &addr_string, &ops);
>
> - std::unique_ptr<ada_catchpoint> c (new ada_catchpoint ());
> + std::unique_ptr<ada_catchpoint> c (new ada_catchpoint (ex_kind));
> init_ada_exception_breakpoint (c.get (), gdbarch, sal, addr_string.c_str (),
> ops, tempflag, disabled, from_tty);
> c->excep_string = excep_string;
> @@ -14294,43 +14141,43 @@ initialize_ada_catchpoint_ops (void)
>
> ops = &catch_exception_breakpoint_ops;
> *ops = bkpt_breakpoint_ops;
> - ops->allocate_location = allocate_location_catch_exception;
> - ops->re_set = re_set_catch_exception;
> - ops->check_status = check_status_catch_exception;
> - ops->print_it = print_it_catch_exception;
> - ops->print_one = print_one_catch_exception;
> - ops->print_mention = print_mention_catch_exception;
> - ops->print_recreate = print_recreate_catch_exception;
> + ops->allocate_location = allocate_location_exception;
> + ops->re_set = re_set_exception;
> + ops->check_status = check_status_exception;
> + ops->print_it = print_it_exception;
> + ops->print_one = print_one_exception;
> + ops->print_mention = print_mention_exception;
> + ops->print_recreate = print_recreate_exception;
>
> ops = &catch_exception_unhandled_breakpoint_ops;
> *ops = bkpt_breakpoint_ops;
> - ops->allocate_location = allocate_location_catch_exception_unhandled;
> - ops->re_set = re_set_catch_exception_unhandled;
> - ops->check_status = check_status_catch_exception_unhandled;
> - ops->print_it = print_it_catch_exception_unhandled;
> - ops->print_one = print_one_catch_exception_unhandled;
> - ops->print_mention = print_mention_catch_exception_unhandled;
> - ops->print_recreate = print_recreate_catch_exception_unhandled;
> + ops->allocate_location = allocate_location_exception;
> + ops->re_set = re_set_exception;
> + ops->check_status = check_status_exception;
> + ops->print_it = print_it_exception;
> + ops->print_one = print_one_exception;
> + ops->print_mention = print_mention_exception;
> + ops->print_recreate = print_recreate_exception;
>
> ops = &catch_assert_breakpoint_ops;
> *ops = bkpt_breakpoint_ops;
> - ops->allocate_location = allocate_location_catch_assert;
> - ops->re_set = re_set_catch_assert;
> - ops->check_status = check_status_catch_assert;
> - ops->print_it = print_it_catch_assert;
> - ops->print_one = print_one_catch_assert;
> - ops->print_mention = print_mention_catch_assert;
> - ops->print_recreate = print_recreate_catch_assert;
> + ops->allocate_location = allocate_location_exception;
> + ops->re_set = re_set_exception;
> + ops->check_status = check_status_exception;
> + ops->print_it = print_it_exception;
> + ops->print_one = print_one_exception;
> + ops->print_mention = print_mention_exception;
> + ops->print_recreate = print_recreate_exception;
>
> ops = &catch_handlers_breakpoint_ops;
> *ops = bkpt_breakpoint_ops;
> - ops->allocate_location = allocate_location_catch_handlers;
> - ops->re_set = re_set_catch_handlers;
> - ops->check_status = check_status_catch_handlers;
> - ops->print_it = print_it_catch_handlers;
> - ops->print_one = print_one_catch_handlers;
> - ops->print_mention = print_mention_catch_handlers;
> - ops->print_recreate = print_recreate_catch_handlers;
> + ops->allocate_location = allocate_location_exception;
> + ops->re_set = re_set_exception;
> + ops->check_status = check_status_exception;
> + ops->print_it = print_it_exception;
> + ops->print_one = print_one_exception;
> + ops->print_mention = print_mention_exception;
> + ops->print_recreate = print_recreate_exception;
> }
>
> /* This module's 'new_objfile' observer. */
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 0fcd131f71e..ae266d4bfac 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -4788,9 +4788,15 @@ called @code{Constraint_Error} is defined in package @code{Pck}, then
> the command to use to catch such exceptions is @kbd{catch exception
> Pck.Constraint_Error}.
>
> +The convenience variable @code{$_ada_exception} holds the address of
> +the exception being thrown. This can be useful when setting a
> +condition for such a catchpoint.
> +
> @item exception unhandled
> @kindex catch exception unhandled
> -An exception that was raised but is not handled by the program.
> +An exception that was raised but is not handled by the program. The
> +convenience variable @code{$_ada_exception} is set as for @code{catch
> +exception}.
>
> @item handlers @r{[}@var{name}@r{]}
> @kindex catch handlers
> @@ -4812,9 +4818,13 @@ user-defined one. For instance, assuming an exception called
> command to use to catch such exceptions handling is
> @kbd{catch handlers Pck.Constraint_Error}.
>
> +The convenience variable @code{$_ada_exception} is set as for
> +@code{catch exception}.
> +
> @item assert
> @kindex catch assert
> -A failed Ada assertion.
> +A failed Ada assertion. Note that the convenience variable
> +@code{$_ada_exception} is @emph{not} set by this catchpoint.
>
> @item exec
> @kindex catch exec
> @@ -11742,6 +11752,11 @@ The program has exited
> The variable @code{$_exception} is set to the exception object being
> thrown at an exception-related catchpoint. @xref{Set Catchpoints}.
>
> +@item $_ada_exception
> +The variable @code{$_ada_exception} is set to the address of the
> +exception being caught or thrown at an Ada exception-related
> +catchpoint. @xref{Set Catchpoints}.
> +
> @item $_probe_argc
> @itemx $_probe_arg0@dots{}$_probe_arg11
> Arguments to a static probe. @xref{Static Probe Points}.
> diff --git a/gdb/testsuite/gdb.ada/catch_ex_std.exp b/gdb/testsuite/gdb.ada/catch_ex_std.exp
> index 63714a8aa81..839d0bb092f 100644
> --- a/gdb/testsuite/gdb.ada/catch_ex_std.exp
> +++ b/gdb/testsuite/gdb.ada/catch_ex_std.exp
> @@ -101,3 +101,6 @@ gdb_test "catch exception some_kind_of_error" \
> gdb_test "cont" \
> "Catchpoint \[0-9\]+, .* at .*foo\.adb:\[0-9\]+.*" \
> "caught the exception"
> +
> +gdb_test "print \$_ada_exception = some_package.some_kind_of_error'Address" \
> + " = true"
When I ran these tests I noticed that this new test was failing for
me. It turns out that a previous test in this file already fails for
me, like this:
(gdb) catch exception some_kind_of_error
Your Ada runtime appears to be missing some debugging information.
Cannot insert Ada exception catchpoint in this configuration.
(gdb) FAIL: gdb.ada/catch_ex_std.exp: catch exception some_kind_of_error
And as a result, the new test will also fail.
I suspect there's probably a package I should be installing to resolve
this issue, but I put together the patch below that spots the above
warning and reports UNSUPPORTED rather than FAIL. I don't know if
this is something you feel is worth including or not?
Obviously I've only confirmed that the UNSUPPORTED path through the
code works, so you'd need to test the PASS path for me.
Thanks,
Andrew
---
diff --git a/gdb/testsuite/gdb.ada/catch_ex_std.exp b/gdb/testsuite/gdb.ada/catch_ex_std.exp
index 839d0bb092f..97ea307c08f 100644
--- a/gdb/testsuite/gdb.ada/catch_ex_std.exp
+++ b/gdb/testsuite/gdb.ada/catch_ex_std.exp
@@ -95,12 +95,24 @@ if {![runto_main]} then {
return 0
}
-gdb_test "catch exception some_kind_of_error" \
- "Catchpoint \[0-9\]+: `some_kind_of_error' Ada exception"
+set can_catch_exceptions 0
+set test "catch exception some_kind_of_error"
+gdb_test_multiple "$test" "$test" {
+ -re "Catchpoint \[0-9\]+: `some_kind_of_error' Ada exception\r\n$gdb_prompt $" {
+ pass $test
+ set can_catch_exceptions 1
+ }
-gdb_test "cont" \
- "Catchpoint \[0-9\]+, .* at .*foo\.adb:\[0-9\]+.*" \
- "caught the exception"
+ -re "Your Ada runtime appears to be missing some debugging information.\r\nCannot insert Ada exception catchpoint in this configuration.\r\n$gdb_prompt $" {
+ unsupported $test
+ }
+}
-gdb_test "print \$_ada_exception = some_package.some_kind_of_error'Address" \
- " = true"
+if { $can_catch_exceptions } {
+ gdb_test "cont" \
+ "Catchpoint \[0-9\]+, .* at .*foo\.adb:\[0-9\]+.*" \
+ "caught the exception"
+
+ gdb_test "print \$_ada_exception = some_package.some_kind_of_error'Address" \
+ " = true"
+}
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v2 7/8] Add $_ada_exception convenience variable
2019-08-27 9:47 ` Andrew Burgess
@ 2019-09-20 19:15 ` Tom Tromey
0 siblings, 0 replies; 27+ messages in thread
From: Tom Tromey @ 2019-09-20 19:15 UTC (permalink / raw)
To: Andrew Burgess; +Cc: Tom Tromey, gdb-patches
>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:
>> +gdb_test "print \$_ada_exception = some_package.some_kind_of_error'Address" \
>> + " = true"
Andrew> When I ran these tests I noticed that this new test was failing for
Andrew> me. It turns out that a previous test in this file already fails for
Andrew> me, like this:
Andrew> (gdb) catch exception some_kind_of_error
Andrew> Your Ada runtime appears to be missing some debugging information.
Andrew> Cannot insert Ada exception catchpoint in this configuration.
Andrew> (gdb) FAIL: gdb.ada/catch_ex_std.exp: catch exception some_kind_of_error
Andrew> And as a result, the new test will also fail.
Andrew> I suspect there's probably a package I should be installing to resolve
Andrew> this issue, but I put together the patch below that spots the above
Andrew> warning and reports UNSUPPORTED rather than FAIL. I don't know if
Andrew> this is something you feel is worth including or not?
Andrew> Obviously I've only confirmed that the UNSUPPORTED path through the
Andrew> code works, so you'd need to test the PASS path for me.
Thanks. This does still work for me. I think you should check it in.
Maybe it's also needed by catch_ex.exp?
Tom
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v2 1/8] Change SYMBOL_VALUE_ADDRESS to be an rvalue
2019-08-01 17:04 [PATCH v2 0/8] Handle copy relocations and add $_ada_exception Tom Tromey
` (2 preceding siblings ...)
2019-08-01 17:04 ` [PATCH v2 7/8] Add $_ada_exception convenience variable Tom Tromey
@ 2019-08-01 17:04 ` Tom Tromey
2019-08-01 17:04 ` [PATCH v2 8/8] Make print-file-var.exp test attribute visibility hidden, dlopen, and main symbol Tom Tromey
` (3 subsequent siblings)
7 siblings, 0 replies; 27+ messages in thread
From: Tom Tromey @ 2019-08-01 17:04 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This changes SYMBOL_VALUE_ADDRESS to be an rvalue. The symbol readers
generally assign using this, so this also introduces
SET_SYMBOL_VALUE_ADDRESS and updates the readers. Making this change
is useful in a subsequent patch, which redefined SYMBOL_VALUE_ADDRESS.
gdb/ChangeLog
2019-08-01 Tom Tromey <tromey@adacore.com>
* coffread.c (process_coff_symbol): Update.
* dwarf2read.c (var_decode_location, new_symbol): Update.
* mdebugread.c (parse_symbol): Update.
* objfiles.c (relocate_one_symbol): Update.
* stabsread.c (define_symbol, fix_common_block)
(scan_file_globals): Update.
* symtab.h (SYMBOL_VALUE_ADDRESS): Expand to an rvalue.
(SET_SYMBOL_VALUE_ADDRESS): New macro.
* xcoffread.c (process_xcoff_symbol): Update.
---
gdb/ChangeLog | 12 ++++++++++++
gdb/coffread.c | 14 ++++++++------
gdb/dwarf2read.c | 19 ++++++++++++-------
gdb/mdebugread.c | 6 +++---
gdb/objfiles.c | 4 +++-
gdb/stabsread.c | 22 +++++++++++++---------
gdb/symtab.h | 4 +++-
gdb/xcoffread.c | 6 ++++--
8 files changed, 58 insertions(+), 29 deletions(-)
diff --git a/gdb/coffread.c b/gdb/coffread.c
index 0c7c4b58b6f..5234a84aa2e 100644
--- a/gdb/coffread.c
+++ b/gdb/coffread.c
@@ -1609,9 +1609,10 @@ process_coff_symbol (struct coff_symbol *cs,
case C_THUMBEXTFUNC:
case C_EXT:
SYMBOL_ACLASS_INDEX (sym) = LOC_STATIC;
- SYMBOL_VALUE_ADDRESS (sym) = (CORE_ADDR) cs->c_value;
- SYMBOL_VALUE_ADDRESS (sym) += ANOFFSET (objfile->section_offsets,
- SECT_OFF_TEXT (objfile));
+ SET_SYMBOL_VALUE_ADDRESS (sym,
+ (CORE_ADDR) cs->c_value
+ + ANOFFSET (objfile->section_offsets,
+ SECT_OFF_TEXT (objfile)));
add_symbol_to_list (sym, get_global_symbols ());
break;
@@ -1619,9 +1620,10 @@ process_coff_symbol (struct coff_symbol *cs,
case C_THUMBSTATFUNC:
case C_STAT:
SYMBOL_ACLASS_INDEX (sym) = LOC_STATIC;
- SYMBOL_VALUE_ADDRESS (sym) = (CORE_ADDR) cs->c_value;
- SYMBOL_VALUE_ADDRESS (sym) += ANOFFSET (objfile->section_offsets,
- SECT_OFF_TEXT (objfile));
+ SET_SYMBOL_VALUE_ADDRESS (sym,
+ (CORE_ADDR) cs->c_value
+ + ANOFFSET (objfile->section_offsets,
+ SECT_OFF_TEXT (objfile)));
if (within_function)
{
/* Static symbol of local scope. */
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 3d90d632891..d3b9d64f342 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -21426,15 +21426,20 @@ var_decode_location (struct attribute *attr, struct symbol *sym,
unsigned int dummy;
if (DW_BLOCK (attr)->data[0] == DW_OP_addr)
- SYMBOL_VALUE_ADDRESS (sym) =
- read_address (objfile->obfd, DW_BLOCK (attr)->data + 1, cu, &dummy);
+ SET_SYMBOL_VALUE_ADDRESS (sym,
+ read_address (objfile->obfd,
+ DW_BLOCK (attr)->data + 1,
+ cu, &dummy));
else
- SYMBOL_VALUE_ADDRESS (sym) =
- read_addr_index_from_leb128 (cu, DW_BLOCK (attr)->data + 1, &dummy);
+ SET_SYMBOL_VALUE_ADDRESS
+ (sym, read_addr_index_from_leb128 (cu, DW_BLOCK (attr)->data + 1,
+ &dummy));
SYMBOL_ACLASS_INDEX (sym) = LOC_STATIC;
fixup_symbol_section (sym, objfile);
- SYMBOL_VALUE_ADDRESS (sym) += ANOFFSET (objfile->section_offsets,
- SYMBOL_SECTION (sym));
+ SET_SYMBOL_VALUE_ADDRESS (sym,
+ SYMBOL_VALUE_ADDRESS (sym)
+ + ANOFFSET (objfile->section_offsets,
+ SYMBOL_SECTION (sym)));
return;
}
@@ -21548,7 +21553,7 @@ new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
addr = attr_value_as_address (attr);
addr = gdbarch_adjust_dwarf2_addr (gdbarch, addr + baseaddr);
- SYMBOL_VALUE_ADDRESS (sym) = addr;
+ SET_SYMBOL_VALUE_ADDRESS (sym, addr);
}
SYMBOL_TYPE (sym) = objfile_type (objfile)->builtin_core_addr;
SYMBOL_DOMAIN (sym) = LABEL_DOMAIN;
diff --git a/gdb/mdebugread.c b/gdb/mdebugread.c
index 7d0cbb71a91..b81d7c41d67 100644
--- a/gdb/mdebugread.c
+++ b/gdb/mdebugread.c
@@ -632,7 +632,7 @@ parse_symbol (SYMR *sh, union aux_ext *ax, char *ext_sh, int bigend,
b = BLOCKVECTOR_BLOCK (SYMTAB_BLOCKVECTOR (top_stack->cur_st),
GLOBAL_BLOCK);
s = new_symbol (name);
- SYMBOL_VALUE_ADDRESS (s) = (CORE_ADDR) sh->value;
+ SET_SYMBOL_VALUE_ADDRESS (s, (CORE_ADDR) sh->value);
add_data_symbol (sh, ax, bigend, s, LOC_STATIC, b, objfile, name);
break;
@@ -649,7 +649,7 @@ parse_symbol (SYMR *sh, union aux_ext *ax, char *ext_sh, int bigend,
global_sym_chain[bucket] = s;
}
else
- SYMBOL_VALUE_ADDRESS (s) = (CORE_ADDR) sh->value;
+ SET_SYMBOL_VALUE_ADDRESS (s, (CORE_ADDR) sh->value);
add_data_symbol (sh, ax, bigend, s, LOC_STATIC, b, objfile, name);
break;
@@ -706,7 +706,7 @@ parse_symbol (SYMR *sh, union aux_ext *ax, char *ext_sh, int bigend,
s = new_symbol (name);
SYMBOL_DOMAIN (s) = VAR_DOMAIN; /* So that it can be used */
SYMBOL_ACLASS_INDEX (s) = LOC_LABEL; /* but not misused. */
- SYMBOL_VALUE_ADDRESS (s) = (CORE_ADDR) sh->value;
+ SET_SYMBOL_VALUE_ADDRESS (s, (CORE_ADDR) sh->value);
SYMBOL_TYPE (s) = objfile_type (objfile)->builtin_int;
add_symbol (s, top_stack->cur_st, top_stack->cur_block);
break;
diff --git a/gdb/objfiles.c b/gdb/objfiles.c
index 84d9681bf4e..cf482cdeac3 100644
--- a/gdb/objfiles.c
+++ b/gdb/objfiles.c
@@ -711,7 +711,9 @@ relocate_one_symbol (struct symbol *sym, struct objfile *objfile,
|| SYMBOL_CLASS (sym) == LOC_STATIC)
&& SYMBOL_SECTION (sym) >= 0)
{
- SYMBOL_VALUE_ADDRESS (sym) += ANOFFSET (delta, SYMBOL_SECTION (sym));
+ SET_SYMBOL_VALUE_ADDRESS (sym,
+ SYMBOL_VALUE_ADDRESS (sym)
+ + ANOFFSET (delta, SYMBOL_SECTION (sym)));
}
}
diff --git a/gdb/stabsread.c b/gdb/stabsread.c
index a3fe6c99ca3..36b20f2694e 100644
--- a/gdb/stabsread.c
+++ b/gdb/stabsread.c
@@ -942,7 +942,7 @@ define_symbol (CORE_ADDR valu, const char *string, int desc, int type,
SYMBOL_TYPE (sym) = read_type (&p, objfile);
SYMBOL_ACLASS_INDEX (sym) = LOC_LABEL;
SYMBOL_DOMAIN (sym) = VAR_DOMAIN;
- SYMBOL_VALUE_ADDRESS (sym) = valu;
+ SET_SYMBOL_VALUE_ADDRESS (sym, valu);
add_symbol_to_list (sym, get_local_symbols ());
break;
@@ -1188,7 +1188,7 @@ define_symbol (CORE_ADDR valu, const char *string, int desc, int type,
/* Static symbol at top level of file. */
SYMBOL_TYPE (sym) = read_type (&p, objfile);
SYMBOL_ACLASS_INDEX (sym) = LOC_STATIC;
- SYMBOL_VALUE_ADDRESS (sym) = valu;
+ SET_SYMBOL_VALUE_ADDRESS (sym, valu);
if (gdbarch_static_transform_name_p (gdbarch)
&& gdbarch_static_transform_name (gdbarch,
SYMBOL_LINKAGE_NAME (sym))
@@ -1204,7 +1204,8 @@ define_symbol (CORE_ADDR valu, const char *string, int desc, int type,
(gdbarch, SYMBOL_LINKAGE_NAME (sym));
SYMBOL_SET_LINKAGE_NAME (sym, new_name);
- SYMBOL_VALUE_ADDRESS (sym) = BMSYMBOL_VALUE_ADDRESS (msym);
+ SET_SYMBOL_VALUE_ADDRESS (sym,
+ BMSYMBOL_VALUE_ADDRESS (msym));
}
}
SYMBOL_DOMAIN (sym) = VAR_DOMAIN;
@@ -1380,7 +1381,7 @@ define_symbol (CORE_ADDR valu, const char *string, int desc, int type,
/* Static symbol of local scope. */
SYMBOL_TYPE (sym) = read_type (&p, objfile);
SYMBOL_ACLASS_INDEX (sym) = LOC_STATIC;
- SYMBOL_VALUE_ADDRESS (sym) = valu;
+ SET_SYMBOL_VALUE_ADDRESS (sym, valu);
if (gdbarch_static_transform_name_p (gdbarch)
&& gdbarch_static_transform_name (gdbarch,
SYMBOL_LINKAGE_NAME (sym))
@@ -1396,7 +1397,7 @@ define_symbol (CORE_ADDR valu, const char *string, int desc, int type,
(gdbarch, SYMBOL_LINKAGE_NAME (sym));
SYMBOL_SET_LINKAGE_NAME (sym, new_name);
- SYMBOL_VALUE_ADDRESS (sym) = BMSYMBOL_VALUE_ADDRESS (msym);
+ SET_SYMBOL_VALUE_ADDRESS (sym, BMSYMBOL_VALUE_ADDRESS (msym));
}
}
SYMBOL_DOMAIN (sym) = VAR_DOMAIN;
@@ -4368,7 +4369,9 @@ fix_common_block (struct symbol *sym, CORE_ADDR valu)
int j;
for (j = next->nsyms - 1; j >= 0; j--)
- SYMBOL_VALUE_ADDRESS (next->symbol[j]) += valu;
+ SET_SYMBOL_VALUE_ADDRESS (next->symbol[j],
+ SYMBOL_VALUE_ADDRESS (next->symbol[j])
+ + valu);
}
}
\f
@@ -4646,8 +4649,9 @@ scan_file_globals (struct objfile *objfile)
}
else
{
- SYMBOL_VALUE_ADDRESS (sym)
- = MSYMBOL_VALUE_ADDRESS (resolve_objfile, msymbol);
+ SET_SYMBOL_VALUE_ADDRESS
+ (sym, MSYMBOL_VALUE_ADDRESS (resolve_objfile,
+ msymbol));
}
SYMBOL_SECTION (sym) = MSYMBOL_SECTION (msymbol);
}
@@ -4685,7 +4689,7 @@ scan_file_globals (struct objfile *objfile)
/* Change the symbol address from the misleading chain value
to address zero. */
- SYMBOL_VALUE_ADDRESS (prev) = 0;
+ SET_SYMBOL_VALUE_ADDRESS (prev, 0);
/* Complain about unresolved common block symbols. */
if (SYMBOL_CLASS (prev) == LOC_STATIC)
diff --git a/gdb/symtab.h b/gdb/symtab.h
index 9880ecc4c53..9f1791ba63c 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -463,7 +463,9 @@ extern CORE_ADDR symbol_overlayed_address (CORE_ADDR, struct obj_section *);
field only, instead of the SYMBOL parameter. */
#define SYMBOL_VALUE(symbol) (symbol)->ginfo.value.ivalue
-#define SYMBOL_VALUE_ADDRESS(symbol) (symbol)->ginfo.value.address
+#define SYMBOL_VALUE_ADDRESS(symbol) ((symbol)->ginfo.value.address + 0)
+#define SET_SYMBOL_VALUE_ADDRESS(symbol, new_value) \
+ ((symbol)->ginfo.value.address = (new_value))
#define SYMBOL_VALUE_BYTES(symbol) (symbol)->ginfo.value.bytes
#define SYMBOL_VALUE_COMMON_BLOCK(symbol) (symbol)->ginfo.value.common_block
#define SYMBOL_BLOCK_VALUE(symbol) (symbol)->ginfo.value.block
diff --git a/gdb/xcoffread.c b/gdb/xcoffread.c
index f4892a8054f..028af9a0955 100644
--- a/gdb/xcoffread.c
+++ b/gdb/xcoffread.c
@@ -1576,7 +1576,7 @@ process_xcoff_symbol (struct coff_symbol *cs, struct objfile *objfile)
initialize_objfile_symbol (sym);
/* default assumptions */
- SYMBOL_VALUE_ADDRESS (sym) = cs->c_value + off;
+ SET_SYMBOL_VALUE_ADDRESS (sym, cs->c_value + off);
SYMBOL_DOMAIN (sym) = VAR_DOMAIN;
SYMBOL_SECTION (sym) = secnum_to_section (cs->c_secnum, objfile);
@@ -1675,7 +1675,9 @@ process_xcoff_symbol (struct coff_symbol *cs, struct objfile *objfile)
cs->c_name, 0, 0, objfile);
if (sym != NULL)
{
- SYMBOL_VALUE_ADDRESS (sym) += static_block_base;
+ SET_SYMBOL_VALUE_ADDRESS (sym,
+ SYMBOL_VALUE_ADDRESS (sym)
+ + static_block_base);
SYMBOL_SECTION (sym) = static_block_section;
}
return sym;
--
2.20.1
^ permalink raw reply [flat|nested] 27+ messages in thread* [PATCH v2 8/8] Make print-file-var.exp test attribute visibility hidden, dlopen, and main symbol
2019-08-01 17:04 [PATCH v2 0/8] Handle copy relocations and add $_ada_exception Tom Tromey
` (3 preceding siblings ...)
2019-08-01 17:04 ` [PATCH v2 1/8] Change SYMBOL_VALUE_ADDRESS to be an rvalue Tom Tromey
@ 2019-08-01 17:04 ` Tom Tromey
2019-08-20 13:41 ` Andrew Burgess
2019-08-01 17:04 ` [PATCH v2 2/8] Handle copy relocations Tom Tromey
` (2 subsequent siblings)
7 siblings, 1 reply; 27+ messages in thread
From: Tom Tromey @ 2019-08-01 17:04 UTC (permalink / raw)
To: gdb-patches; +Cc: Pedro Alves
From: Pedro Alves <palves@redhat.com>
Make gdb.base/print-file-var.exp test all combinations of:
- attribute hidden in the this_version_id symbols or not
- dlopen or not
- this_version_id symbol in main file or not
2019-08-01 Pedro Alves <palves@redhat.com>
* gdb.base/print-file-var-lib1.c: Include <stdio.h> and
"print-file-var.h".
(this_version_id) Use ATTRIBUTE_VISIBILITY.
(get_version_1): Print this_version_id and its address.
* gdb.base/print-file-var-lib2.c: Include <stdio.h> and
"print-file-var.h".
(this_version_id) Use ATTRIBUTE_VISIBILITY.
(get_version_2): Print this_version_id and its address.
* gdb.base/print-file-var-main.c: Include <dlfcn.h>, <assert.h>,
<stddef.h> and "print-file-var.h".
[VERSION_ID_MAIN] (this_version_id): Define.
(main): Define v0. Use dlopen if SHLIB_NAME is defined.
* gdb.base/print-file-var.exp (test): New, factored out from top
level.
(top level): Test all combinations of attribute hidden or not,
dlopen or not, and this_version_id symbol in main file or not.
---
gdb/testsuite/ChangeLog | 19 ++
gdb/testsuite/gdb.base/print-file-var-lib1.c | 7 +-
gdb/testsuite/gdb.base/print-file-var-lib2.c | 6 +-
gdb/testsuite/gdb.base/print-file-var-main.c | 38 +++-
gdb/testsuite/gdb.base/print-file-var.exp | 181 ++++++++++++-------
gdb/testsuite/gdb.base/print-file-var.h | 26 +++
6 files changed, 199 insertions(+), 78 deletions(-)
create mode 100644 gdb/testsuite/gdb.base/print-file-var.h
diff --git a/gdb/testsuite/gdb.base/print-file-var-lib1.c b/gdb/testsuite/gdb.base/print-file-var-lib1.c
index b5f4fb90b39..aec04a9b02b 100644
--- a/gdb/testsuite/gdb.base/print-file-var-lib1.c
+++ b/gdb/testsuite/gdb.base/print-file-var-lib1.c
@@ -14,10 +14,15 @@
You should have received a copy of the GNU General Public License
along with this program. If not, see <http://www.gnu.org/licenses/>. */
-int this_version_id = 104;
+#include <stdio.h>
+#include "print-file-var.h"
+
+ATTRIBUTE_VISIBILITY int this_version_id = 104;
int
get_version_1 (void)
{
+ printf ("get_version_1: &this_version_id=%p, this_version_id=%d\n", &this_version_id, this_version_id);
+
return this_version_id;
}
diff --git a/gdb/testsuite/gdb.base/print-file-var-lib2.c b/gdb/testsuite/gdb.base/print-file-var-lib2.c
index 28bd1acb17f..4dfdfa04c99 100644
--- a/gdb/testsuite/gdb.base/print-file-var-lib2.c
+++ b/gdb/testsuite/gdb.base/print-file-var-lib2.c
@@ -14,10 +14,14 @@
You should have received a copy of the GNU General Public License
along with this program. If not, see <http://www.gnu.org/licenses/>. */
-int this_version_id = 203;
+#include <stdio.h>
+#include "print-file-var.h"
+
+ATTRIBUTE_VISIBILITY int this_version_id = 203;
int
get_version_2 (void)
{
+ printf ("get_version_2: &this_version_id=%p, this_version_id=%d\n", &this_version_id, this_version_id);
return this_version_id;
}
diff --git a/gdb/testsuite/gdb.base/print-file-var-main.c b/gdb/testsuite/gdb.base/print-file-var-main.c
index ddc54f14d98..29d4fed22d1 100644
--- a/gdb/testsuite/gdb.base/print-file-var-main.c
+++ b/gdb/testsuite/gdb.base/print-file-var-main.c
@@ -14,21 +14,45 @@
You should have received a copy of the GNU General Public License
along with this program. If not, see <http://www.gnu.org/licenses/>. */
+#ifdef SHLIB_NAME
+# include <dlfcn.h>
+#endif
+
+#include <assert.h>
+#include <stddef.h>
+
+#include "print-file-var.h"
+
extern int get_version_1 (void);
extern int get_version_2 (void);
+#if VERSION_ID_MAIN
+ATTRIBUTE_VISIBILITY int this_version_id = 55;
+#endif
+
int
main (void)
{
+#if VERSION_ID_MAIN
+ int vm = this_version_id;
+#endif
int v1 = get_version_1 ();
- int v2 = get_version_2 ();
+ int v2;
- if (v1 != 104)
- return 1;
- /* The value returned by get_version_2 depends on the target system. */
- if (v2 != 104 && v2 != 203)
- return 2;
+#ifdef SHLIB_NAME
+ {
+ void *handle = dlopen (SHLIB_NAME, RTLD_LAZY);
+ int (*getver2) (void);
+
+ assert (handle != NULL);
+
+ getver2 = (int (*)(void)) dlsym (handle, "get_version_2");
+
+ v2 = getver2 ();
+ }
+#else
+ v2 = get_version_2 ();
+#endif
return 0; /* STOP */
}
-
diff --git a/gdb/testsuite/gdb.base/print-file-var.exp b/gdb/testsuite/gdb.base/print-file-var.exp
index 1f733fb4dee..a37cca70de6 100644
--- a/gdb/testsuite/gdb.base/print-file-var.exp
+++ b/gdb/testsuite/gdb.base/print-file-var.exp
@@ -17,76 +17,119 @@ if {[skip_shlib_tests]} {
return -1
}
-set executable print-file-var-main
+proc test {hidden dlopen version_id_main} {
+ global srcdir subdir
+
+ set main "print-file-var-main"
+
+ set suffix "-hidden$hidden-dlopen$dlopen-version_id_main$version_id_main"
+
+ set executable $main$suffix
+
+ set lib1 "print-file-var-lib1"
+ set lib2 "print-file-var-lib2"
+
+ set libobj1 [standard_output_file ${lib1}$suffix.so]
+ set libobj2 [standard_output_file ${lib2}$suffix.so]
+
+ set lib_opts { debug additional_flags=-fPIC }
+ lappend lib_opts "additional_flags=-DHIDDEN=$hidden"
+
+ if { [gdb_compile_shlib ${srcdir}/${subdir}/${lib1}.c \
+ ${libobj1} \
+ ${lib_opts} ] != "" } {
+ return -1
+ }
+ if { [gdb_compile_shlib ${srcdir}/${subdir}/${lib2}.c \
+ ${libobj2} \
+ ${lib_opts} ] != "" } {
+ return -1
+ }
+
+ set main_opts [list debug shlib=${libobj1}]
+
+ if {$dlopen} {
+ lappend main_opts "shlib_load" \
+ "additional_flags=-DSHLIB_NAME=\"$libobj2\""
+ } else {
+ lappend main_opts "shlib=${libobj2}"
+ }
+
+ lappend main_opts "additional_flags=-DVERSION_ID_MAIN=$version_id_main"
+
+ if { [gdb_compile "${srcdir}/${subdir}/${main}.c" \
+ [standard_output_file ${executable}] \
+ executable \
+ $main_opts]
+ != ""} {
+ return -1
+ }
+
+ clean_restart $executable
+ gdb_load_shlib $libobj1
+ gdb_load_shlib $libobj2
+
+ if ![runto_main] {
+ untested "could not run to main"
+ return -1
+ }
+
+ # Try printing "this_version_num" qualified with the name of the file
+ # where the variables are defined. There are three global variables
+ # with that name, and some systems such as GNU/Linux merge them
+ # into one single entity, while some other systems such as Windows
+ # keep them separate. In the first situation, we have to verify
+ # that GDB does not randomly select the wrong instance, even when
+ # a specific filename is used to qualified the lookup. And in the
+ # second case, we have to verify that GDB does select the instance
+ # defined in the given filename.
+ #
+ # To avoid adding target-specific code in this testcase, the program
+ # sets three local variables named 'vm', 'v1' and 'v2' with the value of
+ # our global variables. This allows us to compare the value that
+ # GDB returns for each query against the actual value seen by
+ # the program itself.
+
+ # Get past the initialization of the v* variables.
+
+ set bp_location \
+ [gdb_get_line_number "STOP" "${main}.c"]
+ gdb_test "break $main.c:$bp_location" \
+ "Breakpoint \[0-9\]+ at 0x\[0-9a-fA-F\]+: .*" \
+ "breapoint at STOP marker"
+
+ gdb_test "continue" \
+ "Breakpoint \[0-9\]+, main \\(\\) at.*STOP.*" \
+ "continue to STOP marker"
+
+ # Now check the value of this_version_id in all of
+ # print-file-var-main.c, print-file-var-lib1.c and
+ # print-file-var-lib2.c.
+
+ # Compare the values of $sym1 and $sym2.
+ proc compare {sym1 sym2} {
+ # Done this way instead of comparing the symbols with "print $sym1
+ # == sym2" in GDB directly so that the values of the symbols end
+ # up visible in the logs, for debug purposes.
+ set vsym1 [get_integer_valueof $sym1 -1]
+ set vsym2 [get_integer_valueof $sym2 -1]
+ gdb_assert {$vsym1 == $vsym2} "$sym1 == $sym2"
+ }
+
+ if $version_id_main {
+ compare "'print-file-var-main.c'::this_version_id" "vm"
+ compare "this_version_id" "vm"
+ }
+
+ compare "'print-file-var-lib1.c'::this_version_id" "v1"
+ compare "'print-file-var-lib2.c'::this_version_id" "v2"
-set lib1 "print-file-var-lib1"
-set lib2 "print-file-var-lib2"
-
-set libobj1 [standard_output_file ${lib1}.so]
-set libobj2 [standard_output_file ${lib2}.so]
-
-set lib_opts { debug additional_flags=-fPIC }
-
-if { [gdb_compile_shlib ${srcdir}/${subdir}/${lib1}.c \
- ${libobj1} \
- ${lib_opts} ] != "" } {
- return -1
}
-if { [gdb_compile_shlib ${srcdir}/${subdir}/${lib2}.c \
- ${libobj2} \
- ${lib_opts} ] != "" } {
- return -1
-}
-if { [gdb_compile "${srcdir}/${subdir}/${executable}.c" \
- [standard_output_file ${executable}] \
- executable \
- [list debug shlib=${libobj1} shlib=${libobj2}]]
- != ""} {
- return -1
-}
-
-clean_restart $executable
-gdb_load_shlib $libobj1
-gdb_load_shlib $libobj2
-if ![runto_main] {
- untested "could not run to main"
- return -1
+foreach_with_prefix hidden {0 1} {
+ foreach_with_prefix dlopen {0 1} {
+ foreach_with_prefix version_id_main {0 1} {
+ test $hidden $dlopen $version_id_main
+ }
+ }
}
-
-# Try printing "this_version_num" qualified with the name of the file
-# where the variables are defined. There are two global variables
-# with that name, and some systems such as GNU/Linux merge them
-# into one single entity, while some other systems such as Windows
-# keep them separate. In the first situation, we have to verify
-# that GDB does not randomly select the wrong instance, even when
-# a specific filename is used to qualified the lookup. And in the
-# second case, we have to verify that GDB does select the instance
-# defined in the given filename.
-#
-# To avoid adding target-specific code in this testcase, the program
-# sets two local variable named 'v1' and 'v2' with the value of
-# our global variables. This allows us to compare the value that
-# GDB returns for each query against the actual value seen by
-# the program itself.
-
-# Get past the initialization of variables 'v1' and 'v2'.
-
-set bp_location \
- [gdb_get_line_number "STOP" "${executable}.c"]
-gdb_test "break $executable.c:$bp_location" \
- "Breakpoint \[0-9\]+ at 0x\[0-9a-fA-F\]+: .*" \
- "breapoint past v1 & v2 initialization"
-
-gdb_test "continue" \
- "Breakpoint \[0-9\]+, main \\(\\) at.*STOP.*" \
- "continue to STOP marker"
-
-# Now check the value of this_version_id in both print-file-var-lib1.c
-# and print-file-var-lib2.c.
-
-gdb_test "print 'print-file-var-lib1.c'::this_version_id == v1" \
- " = 1"
-
-gdb_test "print 'print-file-var-lib2.c'::this_version_id == v2" \
- " = 1"
diff --git a/gdb/testsuite/gdb.base/print-file-var.h b/gdb/testsuite/gdb.base/print-file-var.h
new file mode 100644
index 00000000000..fe7a3460edb
--- /dev/null
+++ b/gdb/testsuite/gdb.base/print-file-var.h
@@ -0,0 +1,26 @@
+/* This testcase is part of GDB, the GNU debugger.
+ Copyright 2019 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#ifndef PRINT_FILE_VAR_H
+#define PRINT_FILE_VAR_H
+
+#if HIDDEN
+# define ATTRIBUTE_VISIBILITY __attribute__((visibility ("hidden")))
+#else
+# define ATTRIBUTE_VISIBILITY
+#endif
+
+#endif /* PRINT_FILE_VAR_H */
--
2.20.1
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v2 8/8] Make print-file-var.exp test attribute visibility hidden, dlopen, and main symbol
2019-08-01 17:04 ` [PATCH v2 8/8] Make print-file-var.exp test attribute visibility hidden, dlopen, and main symbol Tom Tromey
@ 2019-08-20 13:41 ` Andrew Burgess
2019-08-21 14:35 ` Andrew Burgess
0 siblings, 1 reply; 27+ messages in thread
From: Andrew Burgess @ 2019-08-20 13:41 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches, Pedro Alves
* Tom Tromey <tromey@adacore.com> [2019-08-01 11:04:12 -0600]:
> From: Pedro Alves <palves@redhat.com>
>
> Make gdb.base/print-file-var.exp test all combinations of:
>
> - attribute hidden in the this_version_id symbols or not
> - dlopen or not
> - this_version_id symbol in main file or not
>
> 2019-08-01 Pedro Alves <palves@redhat.com>
>
> * gdb.base/print-file-var-lib1.c: Include <stdio.h> and
> "print-file-var.h".
> (this_version_id) Use ATTRIBUTE_VISIBILITY.
> (get_version_1): Print this_version_id and its address.
> * gdb.base/print-file-var-lib2.c: Include <stdio.h> and
> "print-file-var.h".
> (this_version_id) Use ATTRIBUTE_VISIBILITY.
> (get_version_2): Print this_version_id and its address.
> * gdb.base/print-file-var-main.c: Include <dlfcn.h>, <assert.h>,
> <stddef.h> and "print-file-var.h".
> [VERSION_ID_MAIN] (this_version_id): Define.
> (main): Define v0. Use dlopen if SHLIB_NAME is defined.
> * gdb.base/print-file-var.exp (test): New, factored out from top
> level.
> (top level): Test all combinations of attribute hidden or not,
> dlopen or not, and this_version_id symbol in main file or not.
With this patch series applied to current(ish) HEAD (ac533243bea) I
see the following failures:
FAIL: gdb.base/print-file-var.exp: hidden=0: dlopen=0: version_id_main=0: 'print-file-var-lib2.c'::this_version_id == v2
FAIL: gdb.base/print-file-var.exp: hidden=0: dlopen=1: version_id_main=0: 'print-file-var-lib2.c'::this_version_id == v2
FAIL: gdb.base/print-file-var.exp: hidden=1: dlopen=0: version_id_main=1: 'print-file-var-lib1.c'::this_version_id == v1
FAIL: gdb.base/print-file-var.exp: hidden=1: dlopen=0: version_id_main=1: 'print-file-var-lib2.c'::this_version_id == v2
FAIL: gdb.base/print-file-var.exp: hidden=1: dlopen=1: version_id_main=1: 'print-file-var-lib1.c'::this_version_id == v1
FAIL: gdb.base/print-file-var.exp: hidden=1: dlopen=1: version_id_main=1: 'print-file-var-lib2.c'::this_version_id == v2
This is using GCC versions:
* gcc (GCC) 7.3.1 20180712 (Red Hat 7.3.1-6), and
* gcc (GCC) 9.2.0
Thanks,
Andrew
> ---
> gdb/testsuite/ChangeLog | 19 ++
> gdb/testsuite/gdb.base/print-file-var-lib1.c | 7 +-
> gdb/testsuite/gdb.base/print-file-var-lib2.c | 6 +-
> gdb/testsuite/gdb.base/print-file-var-main.c | 38 +++-
> gdb/testsuite/gdb.base/print-file-var.exp | 181 ++++++++++++-------
> gdb/testsuite/gdb.base/print-file-var.h | 26 +++
> 6 files changed, 199 insertions(+), 78 deletions(-)
> create mode 100644 gdb/testsuite/gdb.base/print-file-var.h
>
> diff --git a/gdb/testsuite/gdb.base/print-file-var-lib1.c b/gdb/testsuite/gdb.base/print-file-var-lib1.c
> index b5f4fb90b39..aec04a9b02b 100644
> --- a/gdb/testsuite/gdb.base/print-file-var-lib1.c
> +++ b/gdb/testsuite/gdb.base/print-file-var-lib1.c
> @@ -14,10 +14,15 @@
> You should have received a copy of the GNU General Public License
> along with this program. If not, see <http://www.gnu.org/licenses/>. */
>
> -int this_version_id = 104;
> +#include <stdio.h>
> +#include "print-file-var.h"
> +
> +ATTRIBUTE_VISIBILITY int this_version_id = 104;
>
> int
> get_version_1 (void)
> {
> + printf ("get_version_1: &this_version_id=%p, this_version_id=%d\n", &this_version_id, this_version_id);
> +
> return this_version_id;
> }
> diff --git a/gdb/testsuite/gdb.base/print-file-var-lib2.c b/gdb/testsuite/gdb.base/print-file-var-lib2.c
> index 28bd1acb17f..4dfdfa04c99 100644
> --- a/gdb/testsuite/gdb.base/print-file-var-lib2.c
> +++ b/gdb/testsuite/gdb.base/print-file-var-lib2.c
> @@ -14,10 +14,14 @@
> You should have received a copy of the GNU General Public License
> along with this program. If not, see <http://www.gnu.org/licenses/>. */
>
> -int this_version_id = 203;
> +#include <stdio.h>
> +#include "print-file-var.h"
> +
> +ATTRIBUTE_VISIBILITY int this_version_id = 203;
>
> int
> get_version_2 (void)
> {
> + printf ("get_version_2: &this_version_id=%p, this_version_id=%d\n", &this_version_id, this_version_id);
> return this_version_id;
> }
> diff --git a/gdb/testsuite/gdb.base/print-file-var-main.c b/gdb/testsuite/gdb.base/print-file-var-main.c
> index ddc54f14d98..29d4fed22d1 100644
> --- a/gdb/testsuite/gdb.base/print-file-var-main.c
> +++ b/gdb/testsuite/gdb.base/print-file-var-main.c
> @@ -14,21 +14,45 @@
> You should have received a copy of the GNU General Public License
> along with this program. If not, see <http://www.gnu.org/licenses/>. */
>
> +#ifdef SHLIB_NAME
> +# include <dlfcn.h>
> +#endif
> +
> +#include <assert.h>
> +#include <stddef.h>
> +
> +#include "print-file-var.h"
> +
> extern int get_version_1 (void);
> extern int get_version_2 (void);
>
> +#if VERSION_ID_MAIN
> +ATTRIBUTE_VISIBILITY int this_version_id = 55;
> +#endif
> +
> int
> main (void)
> {
> +#if VERSION_ID_MAIN
> + int vm = this_version_id;
> +#endif
> int v1 = get_version_1 ();
> - int v2 = get_version_2 ();
> + int v2;
>
> - if (v1 != 104)
> - return 1;
> - /* The value returned by get_version_2 depends on the target system. */
> - if (v2 != 104 && v2 != 203)
> - return 2;
> +#ifdef SHLIB_NAME
> + {
> + void *handle = dlopen (SHLIB_NAME, RTLD_LAZY);
> + int (*getver2) (void);
> +
> + assert (handle != NULL);
> +
> + getver2 = (int (*)(void)) dlsym (handle, "get_version_2");
> +
> + v2 = getver2 ();
> + }
> +#else
> + v2 = get_version_2 ();
> +#endif
>
> return 0; /* STOP */
> }
> -
> diff --git a/gdb/testsuite/gdb.base/print-file-var.exp b/gdb/testsuite/gdb.base/print-file-var.exp
> index 1f733fb4dee..a37cca70de6 100644
> --- a/gdb/testsuite/gdb.base/print-file-var.exp
> +++ b/gdb/testsuite/gdb.base/print-file-var.exp
> @@ -17,76 +17,119 @@ if {[skip_shlib_tests]} {
> return -1
> }
>
> -set executable print-file-var-main
> +proc test {hidden dlopen version_id_main} {
> + global srcdir subdir
> +
> + set main "print-file-var-main"
> +
> + set suffix "-hidden$hidden-dlopen$dlopen-version_id_main$version_id_main"
> +
> + set executable $main$suffix
> +
> + set lib1 "print-file-var-lib1"
> + set lib2 "print-file-var-lib2"
> +
> + set libobj1 [standard_output_file ${lib1}$suffix.so]
> + set libobj2 [standard_output_file ${lib2}$suffix.so]
> +
> + set lib_opts { debug additional_flags=-fPIC }
> + lappend lib_opts "additional_flags=-DHIDDEN=$hidden"
> +
> + if { [gdb_compile_shlib ${srcdir}/${subdir}/${lib1}.c \
> + ${libobj1} \
> + ${lib_opts} ] != "" } {
> + return -1
> + }
> + if { [gdb_compile_shlib ${srcdir}/${subdir}/${lib2}.c \
> + ${libobj2} \
> + ${lib_opts} ] != "" } {
> + return -1
> + }
> +
> + set main_opts [list debug shlib=${libobj1}]
> +
> + if {$dlopen} {
> + lappend main_opts "shlib_load" \
> + "additional_flags=-DSHLIB_NAME=\"$libobj2\""
> + } else {
> + lappend main_opts "shlib=${libobj2}"
> + }
> +
> + lappend main_opts "additional_flags=-DVERSION_ID_MAIN=$version_id_main"
> +
> + if { [gdb_compile "${srcdir}/${subdir}/${main}.c" \
> + [standard_output_file ${executable}] \
> + executable \
> + $main_opts]
> + != ""} {
> + return -1
> + }
> +
> + clean_restart $executable
> + gdb_load_shlib $libobj1
> + gdb_load_shlib $libobj2
> +
> + if ![runto_main] {
> + untested "could not run to main"
> + return -1
> + }
> +
> + # Try printing "this_version_num" qualified with the name of the file
> + # where the variables are defined. There are three global variables
> + # with that name, and some systems such as GNU/Linux merge them
> + # into one single entity, while some other systems such as Windows
> + # keep them separate. In the first situation, we have to verify
> + # that GDB does not randomly select the wrong instance, even when
> + # a specific filename is used to qualified the lookup. And in the
> + # second case, we have to verify that GDB does select the instance
> + # defined in the given filename.
> + #
> + # To avoid adding target-specific code in this testcase, the program
> + # sets three local variables named 'vm', 'v1' and 'v2' with the value of
> + # our global variables. This allows us to compare the value that
> + # GDB returns for each query against the actual value seen by
> + # the program itself.
> +
> + # Get past the initialization of the v* variables.
> +
> + set bp_location \
> + [gdb_get_line_number "STOP" "${main}.c"]
> + gdb_test "break $main.c:$bp_location" \
> + "Breakpoint \[0-9\]+ at 0x\[0-9a-fA-F\]+: .*" \
> + "breapoint at STOP marker"
> +
> + gdb_test "continue" \
> + "Breakpoint \[0-9\]+, main \\(\\) at.*STOP.*" \
> + "continue to STOP marker"
> +
> + # Now check the value of this_version_id in all of
> + # print-file-var-main.c, print-file-var-lib1.c and
> + # print-file-var-lib2.c.
> +
> + # Compare the values of $sym1 and $sym2.
> + proc compare {sym1 sym2} {
> + # Done this way instead of comparing the symbols with "print $sym1
> + # == sym2" in GDB directly so that the values of the symbols end
> + # up visible in the logs, for debug purposes.
> + set vsym1 [get_integer_valueof $sym1 -1]
> + set vsym2 [get_integer_valueof $sym2 -1]
> + gdb_assert {$vsym1 == $vsym2} "$sym1 == $sym2"
> + }
> +
> + if $version_id_main {
> + compare "'print-file-var-main.c'::this_version_id" "vm"
> + compare "this_version_id" "vm"
> + }
> +
> + compare "'print-file-var-lib1.c'::this_version_id" "v1"
> + compare "'print-file-var-lib2.c'::this_version_id" "v2"
>
> -set lib1 "print-file-var-lib1"
> -set lib2 "print-file-var-lib2"
> -
> -set libobj1 [standard_output_file ${lib1}.so]
> -set libobj2 [standard_output_file ${lib2}.so]
> -
> -set lib_opts { debug additional_flags=-fPIC }
> -
> -if { [gdb_compile_shlib ${srcdir}/${subdir}/${lib1}.c \
> - ${libobj1} \
> - ${lib_opts} ] != "" } {
> - return -1
> }
> -if { [gdb_compile_shlib ${srcdir}/${subdir}/${lib2}.c \
> - ${libobj2} \
> - ${lib_opts} ] != "" } {
> - return -1
> -}
> -if { [gdb_compile "${srcdir}/${subdir}/${executable}.c" \
> - [standard_output_file ${executable}] \
> - executable \
> - [list debug shlib=${libobj1} shlib=${libobj2}]]
> - != ""} {
> - return -1
> -}
> -
> -clean_restart $executable
> -gdb_load_shlib $libobj1
> -gdb_load_shlib $libobj2
>
> -if ![runto_main] {
> - untested "could not run to main"
> - return -1
> +foreach_with_prefix hidden {0 1} {
> + foreach_with_prefix dlopen {0 1} {
> + foreach_with_prefix version_id_main {0 1} {
> + test $hidden $dlopen $version_id_main
> + }
> + }
> }
> -
> -# Try printing "this_version_num" qualified with the name of the file
> -# where the variables are defined. There are two global variables
> -# with that name, and some systems such as GNU/Linux merge them
> -# into one single entity, while some other systems such as Windows
> -# keep them separate. In the first situation, we have to verify
> -# that GDB does not randomly select the wrong instance, even when
> -# a specific filename is used to qualified the lookup. And in the
> -# second case, we have to verify that GDB does select the instance
> -# defined in the given filename.
> -#
> -# To avoid adding target-specific code in this testcase, the program
> -# sets two local variable named 'v1' and 'v2' with the value of
> -# our global variables. This allows us to compare the value that
> -# GDB returns for each query against the actual value seen by
> -# the program itself.
> -
> -# Get past the initialization of variables 'v1' and 'v2'.
> -
> -set bp_location \
> - [gdb_get_line_number "STOP" "${executable}.c"]
> -gdb_test "break $executable.c:$bp_location" \
> - "Breakpoint \[0-9\]+ at 0x\[0-9a-fA-F\]+: .*" \
> - "breapoint past v1 & v2 initialization"
> -
> -gdb_test "continue" \
> - "Breakpoint \[0-9\]+, main \\(\\) at.*STOP.*" \
> - "continue to STOP marker"
> -
> -# Now check the value of this_version_id in both print-file-var-lib1.c
> -# and print-file-var-lib2.c.
> -
> -gdb_test "print 'print-file-var-lib1.c'::this_version_id == v1" \
> - " = 1"
> -
> -gdb_test "print 'print-file-var-lib2.c'::this_version_id == v2" \
> - " = 1"
> diff --git a/gdb/testsuite/gdb.base/print-file-var.h b/gdb/testsuite/gdb.base/print-file-var.h
> new file mode 100644
> index 00000000000..fe7a3460edb
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/print-file-var.h
> @@ -0,0 +1,26 @@
> +/* This testcase is part of GDB, the GNU debugger.
> + Copyright 2019 Free Software Foundation, Inc.
> +
> + This program is free software; you can redistribute it and/or modify
> + it under the terms of the GNU General Public License as published by
> + the Free Software Foundation; either version 3 of the License, or
> + (at your option) any later version.
> +
> + This program is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + GNU General Public License for more details.
> +
> + You should have received a copy of the GNU General Public License
> + along with this program. If not, see <http://www.gnu.org/licenses/>. */
> +
> +#ifndef PRINT_FILE_VAR_H
> +#define PRINT_FILE_VAR_H
> +
> +#if HIDDEN
> +# define ATTRIBUTE_VISIBILITY __attribute__((visibility ("hidden")))
> +#else
> +# define ATTRIBUTE_VISIBILITY
> +#endif
> +
> +#endif /* PRINT_FILE_VAR_H */
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v2 8/8] Make print-file-var.exp test attribute visibility hidden, dlopen, and main symbol
2019-08-20 13:41 ` Andrew Burgess
@ 2019-08-21 14:35 ` Andrew Burgess
0 siblings, 0 replies; 27+ messages in thread
From: Andrew Burgess @ 2019-08-21 14:35 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches, Pedro Alves
* Andrew Burgess <andrew.burgess@embecosm.com> [2019-08-20 14:41:18 +0100]:
> * Tom Tromey <tromey@adacore.com> [2019-08-01 11:04:12 -0600]:
>
> > From: Pedro Alves <palves@redhat.com>
> >
> > Make gdb.base/print-file-var.exp test all combinations of:
> >
> > - attribute hidden in the this_version_id symbols or not
> > - dlopen or not
> > - this_version_id symbol in main file or not
> >
> > 2019-08-01 Pedro Alves <palves@redhat.com>
> >
> > * gdb.base/print-file-var-lib1.c: Include <stdio.h> and
> > "print-file-var.h".
> > (this_version_id) Use ATTRIBUTE_VISIBILITY.
> > (get_version_1): Print this_version_id and its address.
> > * gdb.base/print-file-var-lib2.c: Include <stdio.h> and
> > "print-file-var.h".
> > (this_version_id) Use ATTRIBUTE_VISIBILITY.
> > (get_version_2): Print this_version_id and its address.
> > * gdb.base/print-file-var-main.c: Include <dlfcn.h>, <assert.h>,
> > <stddef.h> and "print-file-var.h".
> > [VERSION_ID_MAIN] (this_version_id): Define.
> > (main): Define v0. Use dlopen if SHLIB_NAME is defined.
> > * gdb.base/print-file-var.exp (test): New, factored out from top
> > level.
> > (top level): Test all combinations of attribute hidden or not,
> > dlopen or not, and this_version_id symbol in main file or not.
>
> With this patch series applied to current(ish) HEAD (ac533243bea) I
> see the following failures:
>
> FAIL: gdb.base/print-file-var.exp: hidden=0: dlopen=0: version_id_main=0: 'print-file-var-lib2.c'::this_version_id == v2
> FAIL: gdb.base/print-file-var.exp: hidden=0: dlopen=1: version_id_main=0: 'print-file-var-lib2.c'::this_version_id == v2
> FAIL: gdb.base/print-file-var.exp: hidden=1: dlopen=0: version_id_main=1: 'print-file-var-lib1.c'::this_version_id == v1
> FAIL: gdb.base/print-file-var.exp: hidden=1: dlopen=0: version_id_main=1: 'print-file-var-lib2.c'::this_version_id == v2
> FAIL: gdb.base/print-file-var.exp: hidden=1: dlopen=1: version_id_main=1: 'print-file-var-lib1.c'::this_version_id == v1
> FAIL: gdb.base/print-file-var.exp: hidden=1: dlopen=1: version_id_main=1: 'print-file-var-lib2.c'::this_version_id == v2
>
> This is using GCC versions:
>
> * gcc (GCC) 7.3.1 20180712 (Red Hat 7.3.1-6), and
> * gcc (GCC) 9.2.0
Turns out this is operator error - if turns out I have to apply the
patches _correctly_ in order to not see any errors - who'd have
thought it!
Sorry for the noise.
Andrew
>
> Thanks,
>
> Andrew
>
>
>
> > ---
> > gdb/testsuite/ChangeLog | 19 ++
> > gdb/testsuite/gdb.base/print-file-var-lib1.c | 7 +-
> > gdb/testsuite/gdb.base/print-file-var-lib2.c | 6 +-
> > gdb/testsuite/gdb.base/print-file-var-main.c | 38 +++-
> > gdb/testsuite/gdb.base/print-file-var.exp | 181 ++++++++++++-------
> > gdb/testsuite/gdb.base/print-file-var.h | 26 +++
> > 6 files changed, 199 insertions(+), 78 deletions(-)
> > create mode 100644 gdb/testsuite/gdb.base/print-file-var.h
> >
> > diff --git a/gdb/testsuite/gdb.base/print-file-var-lib1.c b/gdb/testsuite/gdb.base/print-file-var-lib1.c
> > index b5f4fb90b39..aec04a9b02b 100644
> > --- a/gdb/testsuite/gdb.base/print-file-var-lib1.c
> > +++ b/gdb/testsuite/gdb.base/print-file-var-lib1.c
> > @@ -14,10 +14,15 @@
> > You should have received a copy of the GNU General Public License
> > along with this program. If not, see <http://www.gnu.org/licenses/>. */
> >
> > -int this_version_id = 104;
> > +#include <stdio.h>
> > +#include "print-file-var.h"
> > +
> > +ATTRIBUTE_VISIBILITY int this_version_id = 104;
> >
> > int
> > get_version_1 (void)
> > {
> > + printf ("get_version_1: &this_version_id=%p, this_version_id=%d\n", &this_version_id, this_version_id);
> > +
> > return this_version_id;
> > }
> > diff --git a/gdb/testsuite/gdb.base/print-file-var-lib2.c b/gdb/testsuite/gdb.base/print-file-var-lib2.c
> > index 28bd1acb17f..4dfdfa04c99 100644
> > --- a/gdb/testsuite/gdb.base/print-file-var-lib2.c
> > +++ b/gdb/testsuite/gdb.base/print-file-var-lib2.c
> > @@ -14,10 +14,14 @@
> > You should have received a copy of the GNU General Public License
> > along with this program. If not, see <http://www.gnu.org/licenses/>. */
> >
> > -int this_version_id = 203;
> > +#include <stdio.h>
> > +#include "print-file-var.h"
> > +
> > +ATTRIBUTE_VISIBILITY int this_version_id = 203;
> >
> > int
> > get_version_2 (void)
> > {
> > + printf ("get_version_2: &this_version_id=%p, this_version_id=%d\n", &this_version_id, this_version_id);
> > return this_version_id;
> > }
> > diff --git a/gdb/testsuite/gdb.base/print-file-var-main.c b/gdb/testsuite/gdb.base/print-file-var-main.c
> > index ddc54f14d98..29d4fed22d1 100644
> > --- a/gdb/testsuite/gdb.base/print-file-var-main.c
> > +++ b/gdb/testsuite/gdb.base/print-file-var-main.c
> > @@ -14,21 +14,45 @@
> > You should have received a copy of the GNU General Public License
> > along with this program. If not, see <http://www.gnu.org/licenses/>. */
> >
> > +#ifdef SHLIB_NAME
> > +# include <dlfcn.h>
> > +#endif
> > +
> > +#include <assert.h>
> > +#include <stddef.h>
> > +
> > +#include "print-file-var.h"
> > +
> > extern int get_version_1 (void);
> > extern int get_version_2 (void);
> >
> > +#if VERSION_ID_MAIN
> > +ATTRIBUTE_VISIBILITY int this_version_id = 55;
> > +#endif
> > +
> > int
> > main (void)
> > {
> > +#if VERSION_ID_MAIN
> > + int vm = this_version_id;
> > +#endif
> > int v1 = get_version_1 ();
> > - int v2 = get_version_2 ();
> > + int v2;
> >
> > - if (v1 != 104)
> > - return 1;
> > - /* The value returned by get_version_2 depends on the target system. */
> > - if (v2 != 104 && v2 != 203)
> > - return 2;
> > +#ifdef SHLIB_NAME
> > + {
> > + void *handle = dlopen (SHLIB_NAME, RTLD_LAZY);
> > + int (*getver2) (void);
> > +
> > + assert (handle != NULL);
> > +
> > + getver2 = (int (*)(void)) dlsym (handle, "get_version_2");
> > +
> > + v2 = getver2 ();
> > + }
> > +#else
> > + v2 = get_version_2 ();
> > +#endif
> >
> > return 0; /* STOP */
> > }
> > -
> > diff --git a/gdb/testsuite/gdb.base/print-file-var.exp b/gdb/testsuite/gdb.base/print-file-var.exp
> > index 1f733fb4dee..a37cca70de6 100644
> > --- a/gdb/testsuite/gdb.base/print-file-var.exp
> > +++ b/gdb/testsuite/gdb.base/print-file-var.exp
> > @@ -17,76 +17,119 @@ if {[skip_shlib_tests]} {
> > return -1
> > }
> >
> > -set executable print-file-var-main
> > +proc test {hidden dlopen version_id_main} {
> > + global srcdir subdir
> > +
> > + set main "print-file-var-main"
> > +
> > + set suffix "-hidden$hidden-dlopen$dlopen-version_id_main$version_id_main"
> > +
> > + set executable $main$suffix
> > +
> > + set lib1 "print-file-var-lib1"
> > + set lib2 "print-file-var-lib2"
> > +
> > + set libobj1 [standard_output_file ${lib1}$suffix.so]
> > + set libobj2 [standard_output_file ${lib2}$suffix.so]
> > +
> > + set lib_opts { debug additional_flags=-fPIC }
> > + lappend lib_opts "additional_flags=-DHIDDEN=$hidden"
> > +
> > + if { [gdb_compile_shlib ${srcdir}/${subdir}/${lib1}.c \
> > + ${libobj1} \
> > + ${lib_opts} ] != "" } {
> > + return -1
> > + }
> > + if { [gdb_compile_shlib ${srcdir}/${subdir}/${lib2}.c \
> > + ${libobj2} \
> > + ${lib_opts} ] != "" } {
> > + return -1
> > + }
> > +
> > + set main_opts [list debug shlib=${libobj1}]
> > +
> > + if {$dlopen} {
> > + lappend main_opts "shlib_load" \
> > + "additional_flags=-DSHLIB_NAME=\"$libobj2\""
> > + } else {
> > + lappend main_opts "shlib=${libobj2}"
> > + }
> > +
> > + lappend main_opts "additional_flags=-DVERSION_ID_MAIN=$version_id_main"
> > +
> > + if { [gdb_compile "${srcdir}/${subdir}/${main}.c" \
> > + [standard_output_file ${executable}] \
> > + executable \
> > + $main_opts]
> > + != ""} {
> > + return -1
> > + }
> > +
> > + clean_restart $executable
> > + gdb_load_shlib $libobj1
> > + gdb_load_shlib $libobj2
> > +
> > + if ![runto_main] {
> > + untested "could not run to main"
> > + return -1
> > + }
> > +
> > + # Try printing "this_version_num" qualified with the name of the file
> > + # where the variables are defined. There are three global variables
> > + # with that name, and some systems such as GNU/Linux merge them
> > + # into one single entity, while some other systems such as Windows
> > + # keep them separate. In the first situation, we have to verify
> > + # that GDB does not randomly select the wrong instance, even when
> > + # a specific filename is used to qualified the lookup. And in the
> > + # second case, we have to verify that GDB does select the instance
> > + # defined in the given filename.
> > + #
> > + # To avoid adding target-specific code in this testcase, the program
> > + # sets three local variables named 'vm', 'v1' and 'v2' with the value of
> > + # our global variables. This allows us to compare the value that
> > + # GDB returns for each query against the actual value seen by
> > + # the program itself.
> > +
> > + # Get past the initialization of the v* variables.
> > +
> > + set bp_location \
> > + [gdb_get_line_number "STOP" "${main}.c"]
> > + gdb_test "break $main.c:$bp_location" \
> > + "Breakpoint \[0-9\]+ at 0x\[0-9a-fA-F\]+: .*" \
> > + "breapoint at STOP marker"
> > +
> > + gdb_test "continue" \
> > + "Breakpoint \[0-9\]+, main \\(\\) at.*STOP.*" \
> > + "continue to STOP marker"
> > +
> > + # Now check the value of this_version_id in all of
> > + # print-file-var-main.c, print-file-var-lib1.c and
> > + # print-file-var-lib2.c.
> > +
> > + # Compare the values of $sym1 and $sym2.
> > + proc compare {sym1 sym2} {
> > + # Done this way instead of comparing the symbols with "print $sym1
> > + # == sym2" in GDB directly so that the values of the symbols end
> > + # up visible in the logs, for debug purposes.
> > + set vsym1 [get_integer_valueof $sym1 -1]
> > + set vsym2 [get_integer_valueof $sym2 -1]
> > + gdb_assert {$vsym1 == $vsym2} "$sym1 == $sym2"
> > + }
> > +
> > + if $version_id_main {
> > + compare "'print-file-var-main.c'::this_version_id" "vm"
> > + compare "this_version_id" "vm"
> > + }
> > +
> > + compare "'print-file-var-lib1.c'::this_version_id" "v1"
> > + compare "'print-file-var-lib2.c'::this_version_id" "v2"
> >
> > -set lib1 "print-file-var-lib1"
> > -set lib2 "print-file-var-lib2"
> > -
> > -set libobj1 [standard_output_file ${lib1}.so]
> > -set libobj2 [standard_output_file ${lib2}.so]
> > -
> > -set lib_opts { debug additional_flags=-fPIC }
> > -
> > -if { [gdb_compile_shlib ${srcdir}/${subdir}/${lib1}.c \
> > - ${libobj1} \
> > - ${lib_opts} ] != "" } {
> > - return -1
> > }
> > -if { [gdb_compile_shlib ${srcdir}/${subdir}/${lib2}.c \
> > - ${libobj2} \
> > - ${lib_opts} ] != "" } {
> > - return -1
> > -}
> > -if { [gdb_compile "${srcdir}/${subdir}/${executable}.c" \
> > - [standard_output_file ${executable}] \
> > - executable \
> > - [list debug shlib=${libobj1} shlib=${libobj2}]]
> > - != ""} {
> > - return -1
> > -}
> > -
> > -clean_restart $executable
> > -gdb_load_shlib $libobj1
> > -gdb_load_shlib $libobj2
> >
> > -if ![runto_main] {
> > - untested "could not run to main"
> > - return -1
> > +foreach_with_prefix hidden {0 1} {
> > + foreach_with_prefix dlopen {0 1} {
> > + foreach_with_prefix version_id_main {0 1} {
> > + test $hidden $dlopen $version_id_main
> > + }
> > + }
> > }
> > -
> > -# Try printing "this_version_num" qualified with the name of the file
> > -# where the variables are defined. There are two global variables
> > -# with that name, and some systems such as GNU/Linux merge them
> > -# into one single entity, while some other systems such as Windows
> > -# keep them separate. In the first situation, we have to verify
> > -# that GDB does not randomly select the wrong instance, even when
> > -# a specific filename is used to qualified the lookup. And in the
> > -# second case, we have to verify that GDB does select the instance
> > -# defined in the given filename.
> > -#
> > -# To avoid adding target-specific code in this testcase, the program
> > -# sets two local variable named 'v1' and 'v2' with the value of
> > -# our global variables. This allows us to compare the value that
> > -# GDB returns for each query against the actual value seen by
> > -# the program itself.
> > -
> > -# Get past the initialization of variables 'v1' and 'v2'.
> > -
> > -set bp_location \
> > - [gdb_get_line_number "STOP" "${executable}.c"]
> > -gdb_test "break $executable.c:$bp_location" \
> > - "Breakpoint \[0-9\]+ at 0x\[0-9a-fA-F\]+: .*" \
> > - "breapoint past v1 & v2 initialization"
> > -
> > -gdb_test "continue" \
> > - "Breakpoint \[0-9\]+, main \\(\\) at.*STOP.*" \
> > - "continue to STOP marker"
> > -
> > -# Now check the value of this_version_id in both print-file-var-lib1.c
> > -# and print-file-var-lib2.c.
> > -
> > -gdb_test "print 'print-file-var-lib1.c'::this_version_id == v1" \
> > - " = 1"
> > -
> > -gdb_test "print 'print-file-var-lib2.c'::this_version_id == v2" \
> > - " = 1"
> > diff --git a/gdb/testsuite/gdb.base/print-file-var.h b/gdb/testsuite/gdb.base/print-file-var.h
> > new file mode 100644
> > index 00000000000..fe7a3460edb
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.base/print-file-var.h
> > @@ -0,0 +1,26 @@
> > +/* This testcase is part of GDB, the GNU debugger.
> > + Copyright 2019 Free Software Foundation, Inc.
> > +
> > + This program is free software; you can redistribute it and/or modify
> > + it under the terms of the GNU General Public License as published by
> > + the Free Software Foundation; either version 3 of the License, or
> > + (at your option) any later version.
> > +
> > + This program is distributed in the hope that it will be useful,
> > + but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + GNU General Public License for more details.
> > +
> > + You should have received a copy of the GNU General Public License
> > + along with this program. If not, see <http://www.gnu.org/licenses/>. */
> > +
> > +#ifndef PRINT_FILE_VAR_H
> > +#define PRINT_FILE_VAR_H
> > +
> > +#if HIDDEN
> > +# define ATTRIBUTE_VISIBILITY __attribute__((visibility ("hidden")))
> > +#else
> > +# define ATTRIBUTE_VISIBILITY
> > +#endif
> > +
> > +#endif /* PRINT_FILE_VAR_H */
> > --
> > 2.20.1
> >
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v2 2/8] Handle copy relocations
2019-08-01 17:04 [PATCH v2 0/8] Handle copy relocations and add $_ada_exception Tom Tromey
` (4 preceding siblings ...)
2019-08-01 17:04 ` [PATCH v2 8/8] Make print-file-var.exp test attribute visibility hidden, dlopen, and main symbol Tom Tromey
@ 2019-08-01 17:04 ` Tom Tromey
2019-08-21 15:35 ` Andrew Burgess
2019-08-01 17:12 ` [PATCH v2 5/8] Don't call decode_line_with_current_source from select_source_symtab Tom Tromey
2019-08-01 17:12 ` [PATCH v2 4/8] Search global block from basic_lookup_symbol_nonlocal Tom Tromey
7 siblings, 1 reply; 27+ messages in thread
From: Tom Tromey @ 2019-08-01 17:04 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
In ELF, if a data symbol is defined in a shared library and used by
the main program, it will be subject to a "copy relocation". In this
scenario, the main program has a copy of the symbol in question, and a
relocation that tells ld.so to copy the data from the shared library.
Then the symbol in the main program is used to satisfy all references.
This patch changes gdb to handle this scenario. Data symbols coming
from ELF shared libraries get a special flag that indicates that the
symbol's address may be subject to copy relocation.
I looked briefly into handling copy relocations by looking at the
actual relocations in the main program, but this seemed difficult to
do with BFD.
Note that no caching is done here. Perhaps this could be changed if
need be; I wanted to avoid possible problems with either objfile
lifetimes and changes, or conflicts with the long-term (vapor-ware)
objfile splitting project.
gdb/ChangeLog
2019-08-01 Tom Tromey <tromey@adacore.com>
* symmisc.c (dump_msymbols): Don't use MSYMBOL_VALUE_ADDRESS.
* ada-lang.c (lesseq_defined_than): Handle
LOC_STATIC.
* dwarf2read.c (dwarf2_per_objfile): Add can_copy
parameter.
(dwarf2_has_info): Likewise.
(new_symbol): Set maybe_copied on symbol when
appropriate.
* dwarf2read.h (dwarf2_per_objfile): Add can_copy
parameter.
<can_copy>: New member.
* elfread.c (record_minimal_symbol): Set maybe_copied
on symbol when appropriate.
(elf_symfile_read): Update call to dwarf2_has_info.
* minsyms.c (lookup_minimal_symbol_linkage): New
function.
* minsyms.h (lookup_minimal_symbol_linkage): Declare.
* symtab.c (get_symbol_address, get_msymbol_address):
New functions.
* symtab.h (get_symbol_address, get_msymbol_address):
Declare.
(SYMBOL_VALUE_ADDRESS, MSYMBOL_VALUE_ADDRESS): Handle
maybe_copied.
(struct symbol, struct minimal_symbol) <maybe_copied>:
New member.
---
gdb/ChangeLog | 28 ++++++++++++++++++++++++++++
gdb/ada-lang.c | 9 +++++++++
gdb/dwarf2read.c | 44 ++++++++++++++++++++++++++------------------
gdb/dwarf2read.h | 10 ++++++++--
gdb/elfread.c | 16 +++++++++++-----
gdb/minsyms.c | 20 ++++++++++++++++++++
gdb/minsyms.h | 7 +++++++
gdb/symfile.h | 3 ++-
gdb/symmisc.c | 10 +++++++---
gdb/symtab.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
gdb/symtab.h | 42 +++++++++++++++++++++++++++++++++++++++---
11 files changed, 201 insertions(+), 32 deletions(-)
diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 7a5cc4272c6..4e1ee31887a 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -4734,6 +4734,15 @@ lesseq_defined_than (struct symbol *sym0, struct symbol *sym1)
case LOC_CONST:
return SYMBOL_VALUE (sym0) == SYMBOL_VALUE (sym1)
&& equiv_types (SYMBOL_TYPE (sym0), SYMBOL_TYPE (sym1));
+
+ case LOC_STATIC:
+ {
+ const char *name0 = SYMBOL_LINKAGE_NAME (sym0);
+ const char *name1 = SYMBOL_LINKAGE_NAME (sym1);
+ return (strcmp (name0, name1) == 0
+ && SYMBOL_VALUE_ADDRESS (sym0) == SYMBOL_VALUE_ADDRESS (sym1));
+ }
+
default:
return 0;
}
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index d3b9d64f342..90dfde0f1e9 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -2130,8 +2130,10 @@ attr_value_as_address (struct attribute *attr)
/* See declaration. */
dwarf2_per_objfile::dwarf2_per_objfile (struct objfile *objfile_,
- const dwarf2_debug_sections *names)
- : objfile (objfile_)
+ const dwarf2_debug_sections *names,
+ bool can_copy_)
+ : objfile (objfile_),
+ can_copy (can_copy_)
{
if (names == NULL)
names = &dwarf2_elf_names;
@@ -2206,11 +2208,14 @@ private:
/* Try to locate the sections we need for DWARF 2 debugging
information and return true if we have enough to do something.
NAMES points to the dwarf2 section names, or is NULL if the standard
- ELF names are used. */
+ ELF names are used. CAN_COPY is true for formats where symbol
+ interposition is possible and so symbol values must follow copy
+ relocation rules. */
int
dwarf2_has_info (struct objfile *objfile,
- const struct dwarf2_debug_sections *names)
+ const struct dwarf2_debug_sections *names,
+ bool can_copy)
{
if (objfile->flags & OBJF_READNEVER)
return 0;
@@ -2220,7 +2225,8 @@ dwarf2_has_info (struct objfile *objfile,
if (dwarf2_per_objfile == NULL)
dwarf2_per_objfile = dwarf2_objfile_data_key.emplace (objfile, objfile,
- names);
+ names,
+ can_copy);
return (!dwarf2_per_objfile->info.is_virtual
&& dwarf2_per_objfile->info.s.section != NULL
@@ -21646,19 +21652,21 @@ new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
}
else if (attr2 && (DW_UNSND (attr2) != 0))
{
- /* Workaround gfortran PR debug/40040 - it uses
- DW_AT_location for variables in -fPIC libraries which may
- get overriden by other libraries/executable and get
- a different address. Resolve it by the minimal symbol
- which may come from inferior's executable using copy
- relocation. Make this workaround only for gfortran as for
- other compilers GDB cannot guess the minimal symbol
- Fortran mangling kind. */
- if (cu->language == language_fortran && die->parent
- && die->parent->tag == DW_TAG_module
- && cu->producer
- && startswith (cu->producer, "GNU Fortran"))
- SYMBOL_ACLASS_INDEX (sym) = LOC_UNRESOLVED;
+ if (SYMBOL_CLASS (sym) == LOC_STATIC
+ && (objfile->flags & OBJF_MAINLINE) == 0
+ && dwarf2_per_objfile->can_copy)
+ {
+ /* A global static variable might be subject to
+ copy relocation. We first check for a local
+ minsym, though, because maybe the symbol was
+ marked hidden, in which case this would not
+ apply. */
+ minimal_symbol *found
+ = (lookup_minimal_symbol_linkage
+ (SYMBOL_LINKAGE_NAME (sym), objfile));
+ if (found != nullptr)
+ sym->maybe_copied = 1;
+ }
/* A variable with DW_AT_external is never static,
but it may be block-scoped. */
diff --git a/gdb/dwarf2read.h b/gdb/dwarf2read.h
index 8939f97af53..94cfc652e3e 100644
--- a/gdb/dwarf2read.h
+++ b/gdb/dwarf2read.h
@@ -104,9 +104,12 @@ struct dwarf2_per_objfile
{
/* Construct a dwarf2_per_objfile for OBJFILE. NAMES points to the
dwarf2 section names, or is NULL if the standard ELF names are
- used. */
+ used. CAN_COPY is true for formats where symbol
+ interposition is possible and so symbol values must follow copy
+ relocation rules. */
dwarf2_per_objfile (struct objfile *objfile,
- const dwarf2_debug_sections *names);
+ const dwarf2_debug_sections *names,
+ bool can_copy);
~dwarf2_per_objfile ();
@@ -206,6 +209,9 @@ public:
original data was compressed using 'dwz -m'. */
std::unique_ptr<struct dwz_file> dwz_file;
+ /* Whether copy relocations are supported by this object format. */
+ bool can_copy;
+
/* A flag indicating whether this objfile has a section loaded at a
VMA of 0. */
bool has_section_at_zero = false;
diff --git a/gdb/elfread.c b/gdb/elfread.c
index 630550b80dc..b2fade4124b 100644
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -203,10 +203,16 @@ record_minimal_symbol (minimal_symbol_reader &reader,
|| ms_type == mst_text_gnu_ifunc)
address = gdbarch_addr_bits_remove (gdbarch, address);
- return reader.record_full (name, name_len, copy_name, address,
- ms_type,
- gdb_bfd_section_index (objfile->obfd,
- bfd_section));
+ struct minimal_symbol *result
+ = reader.record_full (name, name_len, copy_name, address,
+ ms_type,
+ gdb_bfd_section_index (objfile->obfd,
+ bfd_section));
+ if ((objfile->flags & OBJF_MAINLINE) == 0
+ && (ms_type == mst_data || ms_type == mst_bss))
+ result->maybe_copied = 1;
+
+ return result;
}
/* Read the symbol table of an ELF file.
@@ -1239,7 +1245,7 @@ elf_symfile_read (struct objfile *objfile, symfile_add_flags symfile_flags)
bfd_section_size (abfd, str_sect));
}
- if (dwarf2_has_info (objfile, NULL))
+ if (dwarf2_has_info (objfile, NULL, true))
{
dw_index_kind index_kind;
diff --git a/gdb/minsyms.c b/gdb/minsyms.c
index 0f734ebc761..6f4afa979f9 100644
--- a/gdb/minsyms.c
+++ b/gdb/minsyms.c
@@ -519,6 +519,26 @@ iterate_over_minimal_symbols
/* See minsyms.h. */
+struct minimal_symbol *
+lookup_minimal_symbol_linkage (const char *name, struct objfile *objf)
+{
+ unsigned int hash = msymbol_hash (name) % MINIMAL_SYMBOL_HASH_SIZE;
+
+ for (minimal_symbol *msymbol = objf->per_bfd->msymbol_hash[hash];
+ msymbol != NULL;
+ msymbol = msymbol->hash_next)
+ {
+ if (strcmp (MSYMBOL_LINKAGE_NAME (msymbol), name) == 0
+ && (MSYMBOL_TYPE (msymbol) == mst_data
+ || MSYMBOL_TYPE (msymbol) == mst_bss))
+ return msymbol;
+ }
+
+ return nullptr;
+}
+
+/* See minsyms.h. */
+
struct bound_minimal_symbol
lookup_minimal_symbol_text (const char *name, struct objfile *objf)
{
diff --git a/gdb/minsyms.h b/gdb/minsyms.h
index bb43165620d..fae6cd25496 100644
--- a/gdb/minsyms.h
+++ b/gdb/minsyms.h
@@ -193,6 +193,13 @@ struct bound_minimal_symbol lookup_minimal_symbol (const char *,
struct bound_minimal_symbol lookup_bound_minimal_symbol (const char *);
+/* Look through the minimal symbols in OBJF for a global (not
+ file-local) minsym whose linkage name is NAME. Returns a minimal
+ symbol that matches, or nullptr otherwise. */
+
+struct minimal_symbol *lookup_minimal_symbol_linkage
+ (const char *name, struct objfile *objf);
+
/* Look through all the current minimal symbol tables 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
diff --git a/gdb/symfile.h b/gdb/symfile.h
index 741b085e0c4..170308f10d5 100644
--- a/gdb/symfile.h
+++ b/gdb/symfile.h
@@ -585,7 +585,8 @@ struct dwarf2_debug_sections {
};
extern int dwarf2_has_info (struct objfile *,
- const struct dwarf2_debug_sections *);
+ const struct dwarf2_debug_sections *,
+ bool = false);
/* Dwarf2 sections that can be accessed by dwarf2_get_section_info. */
enum dwarf2_section_enum {
diff --git a/gdb/symmisc.c b/gdb/symmisc.c
index 7666de390cd..db93d716704 100644
--- a/gdb/symmisc.c
+++ b/gdb/symmisc.c
@@ -236,9 +236,13 @@ dump_msymbols (struct objfile *objfile, struct ui_file *outfile)
break;
}
fprintf_filtered (outfile, "[%2d] %c ", index, ms_type);
- fputs_filtered (paddress (gdbarch, MSYMBOL_VALUE_ADDRESS (objfile,
- msymbol)),
- outfile);
+
+ /* Use the relocated address as shown in the symbol here -- do
+ not try to respect copy relocations. */
+ CORE_ADDR addr = (msymbol->value.address
+ + ANOFFSET (objfile->section_offsets,
+ msymbol->section));
+ fputs_filtered (paddress (gdbarch, addr), outfile);
fprintf_filtered (outfile, " %s", MSYMBOL_LINKAGE_NAME (msymbol));
if (section)
{
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 87a0c8e4da6..0ff212e0d97 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -6068,6 +6068,50 @@ symbol_set_symtab (struct symbol *symbol, struct symtab *symtab)
symbol->owner.symtab = symtab;
}
+/* See symtab.h. */
+
+CORE_ADDR
+get_symbol_address (const struct symbol *sym)
+{
+ gdb_assert (sym->maybe_copied);
+ gdb_assert (SYMBOL_CLASS (sym) == LOC_STATIC);
+
+ const char *linkage_name = SYMBOL_LINKAGE_NAME (sym);
+
+ for (objfile *objfile : current_program_space->objfiles ())
+ {
+ minimal_symbol *minsym
+ = lookup_minimal_symbol_linkage (linkage_name, objfile);
+ if (minsym != nullptr)
+ return MSYMBOL_VALUE_ADDRESS (objfile, minsym);
+ }
+ return sym->ginfo.value.address;
+}
+
+/* See symtab.h. */
+
+CORE_ADDR
+get_msymbol_address (struct objfile *objf, const struct minimal_symbol *minsym)
+{
+ gdb_assert (minsym->maybe_copied);
+ gdb_assert ((objf->flags & OBJF_MAINLINE) == 0);
+
+ const char *linkage_name = MSYMBOL_LINKAGE_NAME (minsym);
+
+ for (objfile *objfile : current_program_space->objfiles ())
+ {
+ if ((objfile->flags & OBJF_MAINLINE) != 0)
+ {
+ minimal_symbol *found
+ = lookup_minimal_symbol_linkage (linkage_name, objfile);
+ if (found != nullptr)
+ return MSYMBOL_VALUE_ADDRESS (objfile, found);
+ }
+ }
+ return (minsym->value.address
+ + ANOFFSET (objf->section_offsets, minsym->section));
+}
+
\f
void
diff --git a/gdb/symtab.h b/gdb/symtab.h
index 9f1791ba63c..dd5a1299b9b 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -454,6 +454,14 @@ extern const char *symbol_get_demangled_name
extern CORE_ADDR symbol_overlayed_address (CORE_ADDR, struct obj_section *);
+/* Return the address of SYM. The MAYBE_COPIED flag must be set on
+ SYM. If SYM appears in the main program's minimal symbols, then
+ that minsym's address is returned; otherwise, SYM's address is
+ returned. This should generally only be used via the
+ SYMBOL_VALUE_ADDRESS macro. */
+
+extern CORE_ADDR get_symbol_address (const struct symbol *sym);
+
/* Note that all the following SYMBOL_* macros are used with the
SYMBOL argument being either a partial symbol or
a full symbol. Both types have a ginfo field. In particular
@@ -463,7 +471,9 @@ extern CORE_ADDR symbol_overlayed_address (CORE_ADDR, struct obj_section *);
field only, instead of the SYMBOL parameter. */
#define SYMBOL_VALUE(symbol) (symbol)->ginfo.value.ivalue
-#define SYMBOL_VALUE_ADDRESS(symbol) ((symbol)->ginfo.value.address + 0)
+#define SYMBOL_VALUE_ADDRESS(symbol) \
+ (((symbol)->maybe_copied) ? get_symbol_address (symbol) \
+ : ((symbol)->ginfo.value.address))
#define SET_SYMBOL_VALUE_ADDRESS(symbol, new_value) \
((symbol)->ginfo.value.address = (new_value))
#define SYMBOL_VALUE_BYTES(symbol) (symbol)->ginfo.value.bytes
@@ -671,6 +681,14 @@ struct minimal_symbol : public general_symbol_info
the object file format may not carry that piece of information. */
unsigned int has_size : 1;
+ /* For data symbols only, if this is set, then the symbol might be
+ subject to copy relocation. In this case, a minimal symbol
+ matching the symbol's linkage name is first looked for in the
+ main objfile. If found, then that address is used; otherwise the
+ address in this symbol is used. */
+
+ unsigned maybe_copied : 1;
+
/* Minimal symbols with the same hash key are kept on a linked
list. This is the link. */
@@ -690,6 +708,15 @@ struct minimal_symbol : public general_symbol_info
bool text_p () const;
};
+/* Return the address of MINSYM, which comes from OBJF. The
+ MAYBE_COPIED flag must be set on MINSYM. If MINSYM appears in the
+ main program's minimal symbols, then that minsym's address is
+ returned; otherwise, MINSYM's address is returned. This should
+ generally only be used via the MSYMBOL_VALUE_ADDRESS macro. */
+
+extern CORE_ADDR get_msymbol_address (struct objfile *objf,
+ const struct minimal_symbol *minsym);
+
#define MSYMBOL_TARGET_FLAG_1(msymbol) (msymbol)->target_flag_1
#define MSYMBOL_TARGET_FLAG_2(msymbol) (msymbol)->target_flag_2
#define MSYMBOL_SIZE(msymbol) ((msymbol)->size + 0)
@@ -708,8 +735,9 @@ struct minimal_symbol : public general_symbol_info
/* The relocated address of the minimal symbol, using the section
offsets from OBJFILE. */
#define MSYMBOL_VALUE_ADDRESS(objfile, symbol) \
- ((symbol)->value.address \
- + ANOFFSET ((objfile)->section_offsets, ((symbol)->section)))
+ (((symbol)->maybe_copied) ? get_msymbol_address (objfile, symbol) \
+ : ((symbol)->value.address \
+ + ANOFFSET ((objfile)->section_offsets, ((symbol)->section))))
/* For a bound minsym, we can easily compute the address directly. */
#define BMSYMBOL_VALUE_ADDRESS(symbol) \
MSYMBOL_VALUE_ADDRESS ((symbol).objfile, (symbol).minsym)
@@ -1112,6 +1140,14 @@ struct symbol
/* Whether this is an inlined function (class LOC_BLOCK only). */
unsigned is_inlined : 1;
+ /* For LOC_STATIC only, if this is set, then the symbol might be
+ subject to copy relocation. In this case, a minimal symbol
+ matching the symbol's linkage name is first looked for in the
+ main objfile. If found, then that address is used; otherwise the
+ address in this symbol is used. */
+
+ unsigned maybe_copied : 1;
+
/* The concrete type of this symbol. */
ENUM_BITFIELD (symbol_subclass_kind) subclass : 2;
--
2.20.1
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v2 2/8] Handle copy relocations
2019-08-01 17:04 ` [PATCH v2 2/8] Handle copy relocations Tom Tromey
@ 2019-08-21 15:35 ` Andrew Burgess
2019-08-21 19:11 ` Tom Tromey
0 siblings, 1 reply; 27+ messages in thread
From: Andrew Burgess @ 2019-08-21 15:35 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
* Tom Tromey <tromey@adacore.com> [2019-08-01 11:04:06 -0600]:
> In ELF, if a data symbol is defined in a shared library and used by
> the main program, it will be subject to a "copy relocation". In this
> scenario, the main program has a copy of the symbol in question, and a
> relocation that tells ld.so to copy the data from the shared library.
> Then the symbol in the main program is used to satisfy all references.
>
> This patch changes gdb to handle this scenario. Data symbols coming
> from ELF shared libraries get a special flag that indicates that the
> symbol's address may be subject to copy relocation.
>
> I looked briefly into handling copy relocations by looking at the
> actual relocations in the main program, but this seemed difficult to
> do with BFD.
>
> Note that no caching is done here. Perhaps this could be changed if
> need be; I wanted to avoid possible problems with either objfile
> lifetimes and changes, or conflicts with the long-term (vapor-ware)
> objfile splitting project.
>
> gdb/ChangeLog
> 2019-08-01 Tom Tromey <tromey@adacore.com>
>
> * symmisc.c (dump_msymbols): Don't use MSYMBOL_VALUE_ADDRESS.
> * ada-lang.c (lesseq_defined_than): Handle
> LOC_STATIC.
> * dwarf2read.c (dwarf2_per_objfile): Add can_copy
> parameter.
> (dwarf2_has_info): Likewise.
> (new_symbol): Set maybe_copied on symbol when
> appropriate.
> * dwarf2read.h (dwarf2_per_objfile): Add can_copy
> parameter.
> <can_copy>: New member.
> * elfread.c (record_minimal_symbol): Set maybe_copied
> on symbol when appropriate.
> (elf_symfile_read): Update call to dwarf2_has_info.
> * minsyms.c (lookup_minimal_symbol_linkage): New
> function.
> * minsyms.h (lookup_minimal_symbol_linkage): Declare.
> * symtab.c (get_symbol_address, get_msymbol_address):
> New functions.
> * symtab.h (get_symbol_address, get_msymbol_address):
> Declare.
> (SYMBOL_VALUE_ADDRESS, MSYMBOL_VALUE_ADDRESS): Handle
> maybe_copied.
> (struct symbol, struct minimal_symbol) <maybe_copied>:
> New member.
> ---
> gdb/ChangeLog | 28 ++++++++++++++++++++++++++++
> gdb/ada-lang.c | 9 +++++++++
> gdb/dwarf2read.c | 44 ++++++++++++++++++++++++++------------------
> gdb/dwarf2read.h | 10 ++++++++--
> gdb/elfread.c | 16 +++++++++++-----
> gdb/minsyms.c | 20 ++++++++++++++++++++
> gdb/minsyms.h | 7 +++++++
> gdb/symfile.h | 3 ++-
> gdb/symmisc.c | 10 +++++++---
> gdb/symtab.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
> gdb/symtab.h | 42 +++++++++++++++++++++++++++++++++++++++---
> 11 files changed, 201 insertions(+), 32 deletions(-)
>
> diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
> index 7a5cc4272c6..4e1ee31887a 100644
> --- a/gdb/ada-lang.c
> +++ b/gdb/ada-lang.c
> @@ -4734,6 +4734,15 @@ lesseq_defined_than (struct symbol *sym0, struct symbol *sym1)
> case LOC_CONST:
> return SYMBOL_VALUE (sym0) == SYMBOL_VALUE (sym1)
> && equiv_types (SYMBOL_TYPE (sym0), SYMBOL_TYPE (sym1));
> +
> + case LOC_STATIC:
> + {
> + const char *name0 = SYMBOL_LINKAGE_NAME (sym0);
> + const char *name1 = SYMBOL_LINKAGE_NAME (sym1);
> + return (strcmp (name0, name1) == 0
> + && SYMBOL_VALUE_ADDRESS (sym0) == SYMBOL_VALUE_ADDRESS (sym1));
> + }
> +
> default:
> return 0;
> }
> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> index d3b9d64f342..90dfde0f1e9 100644
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
> @@ -2130,8 +2130,10 @@ attr_value_as_address (struct attribute *attr)
> /* See declaration. */
>
> dwarf2_per_objfile::dwarf2_per_objfile (struct objfile *objfile_,
> - const dwarf2_debug_sections *names)
> - : objfile (objfile_)
> + const dwarf2_debug_sections *names,
> + bool can_copy_)
> + : objfile (objfile_),
> + can_copy (can_copy_)
> {
> if (names == NULL)
> names = &dwarf2_elf_names;
> @@ -2206,11 +2208,14 @@ private:
> /* Try to locate the sections we need for DWARF 2 debugging
> information and return true if we have enough to do something.
> NAMES points to the dwarf2 section names, or is NULL if the standard
> - ELF names are used. */
> + ELF names are used. CAN_COPY is true for formats where symbol
> + interposition is possible and so symbol values must follow copy
> + relocation rules. */
>
> int
> dwarf2_has_info (struct objfile *objfile,
> - const struct dwarf2_debug_sections *names)
> + const struct dwarf2_debug_sections *names,
> + bool can_copy)
> {
> if (objfile->flags & OBJF_READNEVER)
> return 0;
> @@ -2220,7 +2225,8 @@ dwarf2_has_info (struct objfile *objfile,
>
> if (dwarf2_per_objfile == NULL)
> dwarf2_per_objfile = dwarf2_objfile_data_key.emplace (objfile, objfile,
> - names);
> + names,
> + can_copy);
>
> return (!dwarf2_per_objfile->info.is_virtual
> && dwarf2_per_objfile->info.s.section != NULL
> @@ -21646,19 +21652,21 @@ new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
> }
> else if (attr2 && (DW_UNSND (attr2) != 0))
> {
> - /* Workaround gfortran PR debug/40040 - it uses
> - DW_AT_location for variables in -fPIC libraries which may
> - get overriden by other libraries/executable and get
> - a different address. Resolve it by the minimal symbol
> - which may come from inferior's executable using copy
> - relocation. Make this workaround only for gfortran as for
> - other compilers GDB cannot guess the minimal symbol
> - Fortran mangling kind. */
> - if (cu->language == language_fortran && die->parent
> - && die->parent->tag == DW_TAG_module
> - && cu->producer
> - && startswith (cu->producer, "GNU Fortran"))
> - SYMBOL_ACLASS_INDEX (sym) = LOC_UNRESOLVED;
> + if (SYMBOL_CLASS (sym) == LOC_STATIC
> + && (objfile->flags & OBJF_MAINLINE) == 0
> + && dwarf2_per_objfile->can_copy)
> + {
> + /* A global static variable might be subject to
> + copy relocation. We first check for a local
> + minsym, though, because maybe the symbol was
> + marked hidden, in which case this would not
> + apply. */
> + minimal_symbol *found
> + = (lookup_minimal_symbol_linkage
> + (SYMBOL_LINKAGE_NAME (sym), objfile));
> + if (found != nullptr)
> + sym->maybe_copied = 1;
> + }
>
> /* A variable with DW_AT_external is never static,
> but it may be block-scoped. */
> diff --git a/gdb/dwarf2read.h b/gdb/dwarf2read.h
> index 8939f97af53..94cfc652e3e 100644
> --- a/gdb/dwarf2read.h
> +++ b/gdb/dwarf2read.h
> @@ -104,9 +104,12 @@ struct dwarf2_per_objfile
> {
> /* Construct a dwarf2_per_objfile for OBJFILE. NAMES points to the
> dwarf2 section names, or is NULL if the standard ELF names are
> - used. */
> + used. CAN_COPY is true for formats where symbol
> + interposition is possible and so symbol values must follow copy
> + relocation rules. */
> dwarf2_per_objfile (struct objfile *objfile,
> - const dwarf2_debug_sections *names);
> + const dwarf2_debug_sections *names,
> + bool can_copy);
>
> ~dwarf2_per_objfile ();
>
> @@ -206,6 +209,9 @@ public:
> original data was compressed using 'dwz -m'. */
> std::unique_ptr<struct dwz_file> dwz_file;
>
> + /* Whether copy relocations are supported by this object format. */
> + bool can_copy;
> +
> /* A flag indicating whether this objfile has a section loaded at a
> VMA of 0. */
> bool has_section_at_zero = false;
> diff --git a/gdb/elfread.c b/gdb/elfread.c
> index 630550b80dc..b2fade4124b 100644
> --- a/gdb/elfread.c
> +++ b/gdb/elfread.c
> @@ -203,10 +203,16 @@ record_minimal_symbol (minimal_symbol_reader &reader,
> || ms_type == mst_text_gnu_ifunc)
> address = gdbarch_addr_bits_remove (gdbarch, address);
>
> - return reader.record_full (name, name_len, copy_name, address,
> - ms_type,
> - gdb_bfd_section_index (objfile->obfd,
> - bfd_section));
> + struct minimal_symbol *result
> + = reader.record_full (name, name_len, copy_name, address,
> + ms_type,
> + gdb_bfd_section_index (objfile->obfd,
> + bfd_section));
> + if ((objfile->flags & OBJF_MAINLINE) == 0
> + && (ms_type == mst_data || ms_type == mst_bss))
> + result->maybe_copied = 1;
> +
> + return result;
> }
>
> /* Read the symbol table of an ELF file.
> @@ -1239,7 +1245,7 @@ elf_symfile_read (struct objfile *objfile, symfile_add_flags symfile_flags)
> bfd_section_size (abfd, str_sect));
> }
>
> - if (dwarf2_has_info (objfile, NULL))
> + if (dwarf2_has_info (objfile, NULL, true))
> {
> dw_index_kind index_kind;
>
> diff --git a/gdb/minsyms.c b/gdb/minsyms.c
> index 0f734ebc761..6f4afa979f9 100644
> --- a/gdb/minsyms.c
> +++ b/gdb/minsyms.c
> @@ -519,6 +519,26 @@ iterate_over_minimal_symbols
>
> /* See minsyms.h. */
>
> +struct minimal_symbol *
> +lookup_minimal_symbol_linkage (const char *name, struct objfile *objf)
> +{
> + unsigned int hash = msymbol_hash (name) % MINIMAL_SYMBOL_HASH_SIZE;
> +
> + for (minimal_symbol *msymbol = objf->per_bfd->msymbol_hash[hash];
> + msymbol != NULL;
> + msymbol = msymbol->hash_next)
> + {
> + if (strcmp (MSYMBOL_LINKAGE_NAME (msymbol), name) == 0
> + && (MSYMBOL_TYPE (msymbol) == mst_data
> + || MSYMBOL_TYPE (msymbol) == mst_bss))
> + return msymbol;
> + }
> +
> + return nullptr;
> +}
I found the name chosen for this function rather confusing. When
comparing to say 'lookup_minimal_symbol_text' which does
sort-of-almost the same thing but for text type symbols, I might have
been tempted to go with 'lookup_minimal_symbol_data'. Am I missing
something behind the choice of function name?
Thanks,
Andrew
> +
> +/* See minsyms.h. */
> +
> struct bound_minimal_symbol
> lookup_minimal_symbol_text (const char *name, struct objfile *objf)
> {
> diff --git a/gdb/minsyms.h b/gdb/minsyms.h
> index bb43165620d..fae6cd25496 100644
> --- a/gdb/minsyms.h
> +++ b/gdb/minsyms.h
> @@ -193,6 +193,13 @@ struct bound_minimal_symbol lookup_minimal_symbol (const char *,
>
> struct bound_minimal_symbol lookup_bound_minimal_symbol (const char *);
>
> +/* Look through the minimal symbols in OBJF for a global (not
> + file-local) minsym whose linkage name is NAME. Returns a minimal
> + symbol that matches, or nullptr otherwise. */
> +
> +struct minimal_symbol *lookup_minimal_symbol_linkage
> + (const char *name, struct objfile *objf);
> +
> /* Look through all the current minimal symbol tables 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
> diff --git a/gdb/symfile.h b/gdb/symfile.h
> index 741b085e0c4..170308f10d5 100644
> --- a/gdb/symfile.h
> +++ b/gdb/symfile.h
> @@ -585,7 +585,8 @@ struct dwarf2_debug_sections {
> };
>
> extern int dwarf2_has_info (struct objfile *,
> - const struct dwarf2_debug_sections *);
> + const struct dwarf2_debug_sections *,
> + bool = false);
>
> /* Dwarf2 sections that can be accessed by dwarf2_get_section_info. */
> enum dwarf2_section_enum {
> diff --git a/gdb/symmisc.c b/gdb/symmisc.c
> index 7666de390cd..db93d716704 100644
> --- a/gdb/symmisc.c
> +++ b/gdb/symmisc.c
> @@ -236,9 +236,13 @@ dump_msymbols (struct objfile *objfile, struct ui_file *outfile)
> break;
> }
> fprintf_filtered (outfile, "[%2d] %c ", index, ms_type);
> - fputs_filtered (paddress (gdbarch, MSYMBOL_VALUE_ADDRESS (objfile,
> - msymbol)),
> - outfile);
> +
> + /* Use the relocated address as shown in the symbol here -- do
> + not try to respect copy relocations. */
> + CORE_ADDR addr = (msymbol->value.address
> + + ANOFFSET (objfile->section_offsets,
> + msymbol->section));
> + fputs_filtered (paddress (gdbarch, addr), outfile);
> fprintf_filtered (outfile, " %s", MSYMBOL_LINKAGE_NAME (msymbol));
> if (section)
> {
> diff --git a/gdb/symtab.c b/gdb/symtab.c
> index 87a0c8e4da6..0ff212e0d97 100644
> --- a/gdb/symtab.c
> +++ b/gdb/symtab.c
> @@ -6068,6 +6068,50 @@ symbol_set_symtab (struct symbol *symbol, struct symtab *symtab)
> symbol->owner.symtab = symtab;
> }
>
> +/* See symtab.h. */
> +
> +CORE_ADDR
> +get_symbol_address (const struct symbol *sym)
> +{
> + gdb_assert (sym->maybe_copied);
> + gdb_assert (SYMBOL_CLASS (sym) == LOC_STATIC);
> +
> + const char *linkage_name = SYMBOL_LINKAGE_NAME (sym);
> +
> + for (objfile *objfile : current_program_space->objfiles ())
> + {
> + minimal_symbol *minsym
> + = lookup_minimal_symbol_linkage (linkage_name, objfile);
> + if (minsym != nullptr)
> + return MSYMBOL_VALUE_ADDRESS (objfile, minsym);
> + }
> + return sym->ginfo.value.address;
> +}
> +
> +/* See symtab.h. */
> +
> +CORE_ADDR
> +get_msymbol_address (struct objfile *objf, const struct minimal_symbol *minsym)
> +{
> + gdb_assert (minsym->maybe_copied);
> + gdb_assert ((objf->flags & OBJF_MAINLINE) == 0);
> +
> + const char *linkage_name = MSYMBOL_LINKAGE_NAME (minsym);
> +
> + for (objfile *objfile : current_program_space->objfiles ())
> + {
> + if ((objfile->flags & OBJF_MAINLINE) != 0)
> + {
> + minimal_symbol *found
> + = lookup_minimal_symbol_linkage (linkage_name, objfile);
> + if (found != nullptr)
> + return MSYMBOL_VALUE_ADDRESS (objfile, found);
> + }
> + }
> + return (minsym->value.address
> + + ANOFFSET (objf->section_offsets, minsym->section));
> +}
> +
> \f
>
> void
> diff --git a/gdb/symtab.h b/gdb/symtab.h
> index 9f1791ba63c..dd5a1299b9b 100644
> --- a/gdb/symtab.h
> +++ b/gdb/symtab.h
> @@ -454,6 +454,14 @@ extern const char *symbol_get_demangled_name
>
> extern CORE_ADDR symbol_overlayed_address (CORE_ADDR, struct obj_section *);
>
> +/* Return the address of SYM. The MAYBE_COPIED flag must be set on
> + SYM. If SYM appears in the main program's minimal symbols, then
> + that minsym's address is returned; otherwise, SYM's address is
> + returned. This should generally only be used via the
> + SYMBOL_VALUE_ADDRESS macro. */
> +
> +extern CORE_ADDR get_symbol_address (const struct symbol *sym);
> +
> /* Note that all the following SYMBOL_* macros are used with the
> SYMBOL argument being either a partial symbol or
> a full symbol. Both types have a ginfo field. In particular
> @@ -463,7 +471,9 @@ extern CORE_ADDR symbol_overlayed_address (CORE_ADDR, struct obj_section *);
> field only, instead of the SYMBOL parameter. */
>
> #define SYMBOL_VALUE(symbol) (symbol)->ginfo.value.ivalue
> -#define SYMBOL_VALUE_ADDRESS(symbol) ((symbol)->ginfo.value.address + 0)
> +#define SYMBOL_VALUE_ADDRESS(symbol) \
> + (((symbol)->maybe_copied) ? get_symbol_address (symbol) \
> + : ((symbol)->ginfo.value.address))
> #define SET_SYMBOL_VALUE_ADDRESS(symbol, new_value) \
> ((symbol)->ginfo.value.address = (new_value))
> #define SYMBOL_VALUE_BYTES(symbol) (symbol)->ginfo.value.bytes
> @@ -671,6 +681,14 @@ struct minimal_symbol : public general_symbol_info
> the object file format may not carry that piece of information. */
> unsigned int has_size : 1;
>
> + /* For data symbols only, if this is set, then the symbol might be
> + subject to copy relocation. In this case, a minimal symbol
> + matching the symbol's linkage name is first looked for in the
> + main objfile. If found, then that address is used; otherwise the
> + address in this symbol is used. */
> +
> + unsigned maybe_copied : 1;
> +
> /* Minimal symbols with the same hash key are kept on a linked
> list. This is the link. */
>
> @@ -690,6 +708,15 @@ struct minimal_symbol : public general_symbol_info
> bool text_p () const;
> };
>
> +/* Return the address of MINSYM, which comes from OBJF. The
> + MAYBE_COPIED flag must be set on MINSYM. If MINSYM appears in the
> + main program's minimal symbols, then that minsym's address is
> + returned; otherwise, MINSYM's address is returned. This should
> + generally only be used via the MSYMBOL_VALUE_ADDRESS macro. */
> +
> +extern CORE_ADDR get_msymbol_address (struct objfile *objf,
> + const struct minimal_symbol *minsym);
> +
> #define MSYMBOL_TARGET_FLAG_1(msymbol) (msymbol)->target_flag_1
> #define MSYMBOL_TARGET_FLAG_2(msymbol) (msymbol)->target_flag_2
> #define MSYMBOL_SIZE(msymbol) ((msymbol)->size + 0)
> @@ -708,8 +735,9 @@ struct minimal_symbol : public general_symbol_info
> /* The relocated address of the minimal symbol, using the section
> offsets from OBJFILE. */
> #define MSYMBOL_VALUE_ADDRESS(objfile, symbol) \
> - ((symbol)->value.address \
> - + ANOFFSET ((objfile)->section_offsets, ((symbol)->section)))
> + (((symbol)->maybe_copied) ? get_msymbol_address (objfile, symbol) \
> + : ((symbol)->value.address \
> + + ANOFFSET ((objfile)->section_offsets, ((symbol)->section))))
> /* For a bound minsym, we can easily compute the address directly. */
> #define BMSYMBOL_VALUE_ADDRESS(symbol) \
> MSYMBOL_VALUE_ADDRESS ((symbol).objfile, (symbol).minsym)
> @@ -1112,6 +1140,14 @@ struct symbol
> /* Whether this is an inlined function (class LOC_BLOCK only). */
> unsigned is_inlined : 1;
>
> + /* For LOC_STATIC only, if this is set, then the symbol might be
> + subject to copy relocation. In this case, a minimal symbol
> + matching the symbol's linkage name is first looked for in the
> + main objfile. If found, then that address is used; otherwise the
> + address in this symbol is used. */
> +
> + unsigned maybe_copied : 1;
> +
> /* The concrete type of this symbol. */
>
> ENUM_BITFIELD (symbol_subclass_kind) subclass : 2;
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v2 2/8] Handle copy relocations
2019-08-21 15:35 ` Andrew Burgess
@ 2019-08-21 19:11 ` Tom Tromey
2019-08-22 8:41 ` Andrew Burgess
0 siblings, 1 reply; 27+ messages in thread
From: Tom Tromey @ 2019-08-21 19:11 UTC (permalink / raw)
To: Andrew Burgess; +Cc: Tom Tromey, gdb-patches
>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:
>> +struct minimal_symbol *
>> +lookup_minimal_symbol_linkage (const char *name, struct objfile *objf)
Andrew> I found the name chosen for this function rather confusing. When
Andrew> comparing to say 'lookup_minimal_symbol_text' which does
Andrew> sort-of-almost the same thing but for text type symbols, I might have
Andrew> been tempted to go with 'lookup_minimal_symbol_data'. Am I missing
Andrew> something behind the choice of function name?
You aren't missing anything.
lookup_minimal_symbol_data seems maybe confusing too, because
lookup_minimal_symbol_text loops over all objfiles, but this one does not.
I guess I don't care all that much (as evidenced by the original choice
of name ;) so if you still like that better, I'll make the change.
Tom
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2 2/8] Handle copy relocations
2019-08-21 19:11 ` Tom Tromey
@ 2019-08-22 8:41 ` Andrew Burgess
2019-09-19 17:45 ` Tom Tromey
0 siblings, 1 reply; 27+ messages in thread
From: Andrew Burgess @ 2019-08-22 8:41 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
* Tom Tromey <tromey@adacore.com> [2019-08-21 13:11:54 -0600]:
> >>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:
>
> >> +struct minimal_symbol *
> >> +lookup_minimal_symbol_linkage (const char *name, struct objfile *objf)
>
> Andrew> I found the name chosen for this function rather confusing. When
> Andrew> comparing to say 'lookup_minimal_symbol_text' which does
> Andrew> sort-of-almost the same thing but for text type symbols, I might have
> Andrew> been tempted to go with 'lookup_minimal_symbol_data'. Am I missing
> Andrew> something behind the choice of function name?
>
> You aren't missing anything.
>
> lookup_minimal_symbol_data seems maybe confusing too, because
> lookup_minimal_symbol_text loops over all objfiles, but this one
> does not.
Indeed, and the return type is different. I also notice that
lookup_minimal_symbol_text can be restricted to a single objfile,
however in that case we _still_ iterate over all object files.
So then I thought, surely we can do better, maybe add a single object
iterator and have lookup_minimal_symbol_text not walk through all
object files.... but then I spotted this condition:
if (objf == NULL || objf == objfile
|| objf == objfile->separate_debug_objfile_backlink)
{
/* Process object file. */
}
So, in this case we have to scan all objfiles in case we have split
debug information. Wouldn't this apply to your new function too?
I guess I'm thinking that we could merge the core of
lookup_minimal_symbol_text and lookup_minimal_symbol_linkage (or
_data), and just pass in a comparison function to check for either
text or data symbols.
>
> I guess I don't care all that much (as evidenced by the original choice
> of name ;) so if you still like that better, I'll make the change.
Meh! I guess my only request would be can the comment at least explain
that it's similar to *_text but for data as currently neither the
function name nor the comment mention data at all.
Thanks,
Andrew
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v2 2/8] Handle copy relocations
2019-08-22 8:41 ` Andrew Burgess
@ 2019-09-19 17:45 ` Tom Tromey
2019-09-19 18:28 ` Tom Tromey
0 siblings, 1 reply; 27+ messages in thread
From: Tom Tromey @ 2019-09-19 17:45 UTC (permalink / raw)
To: Andrew Burgess; +Cc: Tom Tromey, gdb-patches
>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:
Andrew> So then I thought, surely we can do better, maybe add a single object
Andrew> iterator and have lookup_minimal_symbol_text not walk through all
Andrew> object files.... but then I spotted this condition:
Andrew> if (objf == NULL || objf == objfile
Andrew> || objf == objfile->separate_debug_objfile_backlink)
Andrew> {
Andrew> /* Process object file. */
Andrew> }
Andrew> So, in this case we have to scan all objfiles in case we have split
Andrew> debug information. Wouldn't this apply to your new function too?
TBH I was not sure. Can we really have a minimal symbol that appears in
the separate debug info but not in the main objfile? I guess maybe it's
possible if one strips the main objfile.
I'll change my patch.
Andrew> I guess I'm thinking that we could merge the core of
Andrew> lookup_minimal_symbol_text and lookup_minimal_symbol_linkage (or
Andrew> _data), and just pass in a comparison function to check for either
Andrew> text or data symbols.
Sounds good, I'll try that. There is already a separate debug iterator,
so maybe that can be used as well.
>> I guess I don't care all that much (as evidenced by the original choice
>> of name ;) so if you still like that better, I'll make the change.
Andrew> Meh! I guess my only request would be can the comment at least explain
Andrew> that it's similar to *_text but for data as currently neither the
Andrew> function name nor the comment mention data at all.
I'll fix that up too.
Tom
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v2 2/8] Handle copy relocations
2019-09-19 17:45 ` Tom Tromey
@ 2019-09-19 18:28 ` Tom Tromey
2019-09-19 18:40 ` Andrew Burgess
0 siblings, 1 reply; 27+ messages in thread
From: Tom Tromey @ 2019-09-19 18:28 UTC (permalink / raw)
To: Tom Tromey; +Cc: Andrew Burgess, Tom Tromey, gdb-patches
>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:
Andrew> I guess I'm thinking that we could merge the core of
Andrew> lookup_minimal_symbol_text and lookup_minimal_symbol_linkage (or
Andrew> _data), and just pass in a comparison function to check for either
Andrew> text or data symbols.
Tom> Sounds good, I'll try that. There is already a separate debug iterator,
Tom> so maybe that can be used as well.
I looked at this, but there isn't really that much to share.
IMO callbacks tend to obfuscate the code. So I think I'd rather not do
that, if that's ok with you.
Maybe introducing an iterator over minimal symbols, given a hash, would
simplify things a bit. But, that's not really much of a savings.
Tom
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2 2/8] Handle copy relocations
2019-09-19 18:28 ` Tom Tromey
@ 2019-09-19 18:40 ` Andrew Burgess
0 siblings, 0 replies; 27+ messages in thread
From: Andrew Burgess @ 2019-09-19 18:40 UTC (permalink / raw)
To: Tom Tromey; +Cc: Tom Tromey, gdb-patches
* Tom Tromey <tromey@adacore.com> [2019-09-19 12:27:58 -0600]:
> >>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:
>
> Andrew> I guess I'm thinking that we could merge the core of
> Andrew> lookup_minimal_symbol_text and lookup_minimal_symbol_linkage (or
> Andrew> _data), and just pass in a comparison function to check for either
> Andrew> text or data symbols.
>
> Tom> Sounds good, I'll try that. There is already a separate debug iterator,
> Tom> so maybe that can be used as well.
>
> I looked at this, but there isn't really that much to share.
>
> IMO callbacks tend to obfuscate the code. So I think I'd rather not do
> that, if that's ok with you.
That's fine it was only an idea.
Thanks,
Andrew
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v2 5/8] Don't call decode_line_with_current_source from select_source_symtab
2019-08-01 17:04 [PATCH v2 0/8] Handle copy relocations and add $_ada_exception Tom Tromey
` (5 preceding siblings ...)
2019-08-01 17:04 ` [PATCH v2 2/8] Handle copy relocations Tom Tromey
@ 2019-08-01 17:12 ` Tom Tromey
2019-08-01 17:12 ` [PATCH v2 4/8] Search global block from basic_lookup_symbol_nonlocal Tom Tromey
7 siblings, 0 replies; 27+ messages in thread
From: Tom Tromey @ 2019-08-01 17:12 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
select_source_symtab currently calls decode_line_with_current_source.
However, this function iterates over all program spaces, and so it is
possible that it will return a "main" from some other program space.
This patch changes select_source_symtab to simply use the symbol it
already found in the current program space.
gdb/ChangeLog
2019-08-01 Tom Tromey <tromey@adacore.com>
* source.c (select_source_symtab): Don't call
decode_line_with_current_source.
---
gdb/ChangeLog | 5 +++++
gdb/source.c | 11 ++++-------
2 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/gdb/source.c b/gdb/source.c
index a83e55e5699..1aef019da44 100644
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -251,17 +251,14 @@ select_source_symtab (struct symtab *s)
/* Make the default place to list be the function `main'
if one exists. */
- if (lookup_symbol (main_name (), 0, VAR_DOMAIN, 0).symbol)
+ block_symbol bsym = lookup_symbol (main_name (), 0, VAR_DOMAIN, 0);
+ if (bsym.symbol != nullptr && SYMBOL_CLASS (bsym.symbol) == LOC_BLOCK)
{
- std::vector<symtab_and_line> sals
- = decode_line_with_current_source (main_name (),
- DECODE_LINE_FUNFIRSTLINE);
- const symtab_and_line &sal = sals[0];
+ symtab_and_line sal = find_function_start_sal (bsym.symbol, true);
current_source_pspace = sal.pspace;
current_source_symtab = sal.symtab;
current_source_line = std::max (sal.line - (lines_to_list - 1), 1);
- if (current_source_symtab)
- return;
+ return;
}
/* Alright; find the last file in the symtab list (ignoring .h's
--
2.20.1
^ permalink raw reply [flat|nested] 27+ messages in thread* [PATCH v2 4/8] Search global block from basic_lookup_symbol_nonlocal
2019-08-01 17:04 [PATCH v2 0/8] Handle copy relocations and add $_ada_exception Tom Tromey
` (6 preceding siblings ...)
2019-08-01 17:12 ` [PATCH v2 5/8] Don't call decode_line_with_current_source from select_source_symtab Tom Tromey
@ 2019-08-01 17:12 ` Tom Tromey
2019-08-21 10:32 ` Andrew Burgess
7 siblings, 1 reply; 27+ messages in thread
From: Tom Tromey @ 2019-08-01 17:12 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This changes basic_lookup_symbol_nonlocal to look in the global block
of the passed-in block. If no block was passed in, it reverts to the
previous behavior.
This change is needed to ensure that 'FILENAME'::NAME lookups work
properly. As debugging Pedro's test case showed, this was not working
properly in the case where multiple identical names could be found
(the one situation where this feature is truly needed :-).
This also removes some old comments from basic_lookup_symbol_nonlocal
that no longer apply once this patch goes in.
gdb/ChangeLog
2019-08-01 Tom Tromey <tromey@adacore.com>
* symtab.c (basic_lookup_symbol_nonlocal): Search global block.
Remove old comments.
---
gdb/ChangeLog | 5 +++++
gdb/symtab.c | 44 ++++++++++++++++----------------------------
2 files changed, 21 insertions(+), 28 deletions(-)
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 0ff212e0d97..b8f33509c09 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -2417,34 +2417,6 @@ basic_lookup_symbol_nonlocal (const struct language_defn *langdef,
{
struct block_symbol result;
- /* NOTE: carlton/2003-05-19: The comments below were written when
- this (or what turned into this) was part of lookup_symbol_aux;
- I'm much less worried about these questions now, since these
- decisions have turned out well, but I leave these comments here
- for posterity. */
-
- /* NOTE: carlton/2002-12-05: There is a question as to whether or
- not it would be appropriate to search the current global block
- here as well. (That's what this code used to do before the
- is_a_field_of_this check was moved up.) On the one hand, it's
- redundant with the lookup in all objfiles search that happens
- next. On the other hand, if decode_line_1 is passed an argument
- like filename:var, then the user presumably wants 'var' to be
- searched for in filename. On the third hand, there shouldn't be
- multiple global variables all of which are named 'var', and it's
- not like decode_line_1 has ever restricted its search to only
- global variables in a single filename. All in all, only
- searching the static block here seems best: it's correct and it's
- cleanest. */
-
- /* NOTE: carlton/2002-12-05: There's also a possible performance
- issue here: if you usually search for global symbols in the
- current file, then it would be slightly better to search the
- current global block before searching all the symtabs. But there
- are other factors that have a much greater effect on performance
- than that one, so I don't think we should worry about that for
- now. */
-
/* NOTE: dje/2014-10-26: The lookup in all objfiles search could skip
the current objfile. Searching the current objfile first is useful
for both matching user expectations as well as performance. */
@@ -2453,6 +2425,22 @@ basic_lookup_symbol_nonlocal (const struct language_defn *langdef,
if (result.symbol != NULL)
return result;
+ /* If a block was passed in, we want to search the corresponding
+ global block now. This yields "more expected" behavior, and is
+ needed to support 'FILENAME'::VARIABLE lookups. */
+ const struct block *global_block = block_global_block (block);
+ if (global_block != nullptr)
+ {
+ result.symbol = lookup_symbol_in_block (name,
+ symbol_name_match_type::FULL,
+ global_block, domain);
+ if (result.symbol != nullptr)
+ {
+ result.block = global_block;
+ return result;
+ }
+ }
+
/* If we didn't find a definition for a builtin type in the static block,
search for it now. This is actually the right thing to do and can be
a massive performance win. E.g., when debugging a program with lots of
--
2.20.1
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v2 4/8] Search global block from basic_lookup_symbol_nonlocal
2019-08-01 17:12 ` [PATCH v2 4/8] Search global block from basic_lookup_symbol_nonlocal Tom Tromey
@ 2019-08-21 10:32 ` Andrew Burgess
2019-08-27 9:54 ` Andrew Burgess
0 siblings, 1 reply; 27+ messages in thread
From: Andrew Burgess @ 2019-08-21 10:32 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
* Tom Tromey <tromey@adacore.com> [2019-08-01 11:04:08 -0600]:
> This changes basic_lookup_symbol_nonlocal to look in the global block
> of the passed-in block. If no block was passed in, it reverts to the
> previous behavior.
>
> This change is needed to ensure that 'FILENAME'::NAME lookups work
> properly. As debugging Pedro's test case showed, this was not working
> properly in the case where multiple identical names could be found
> (the one situation where this feature is truly needed :-).
>
> This also removes some old comments from basic_lookup_symbol_nonlocal
> that no longer apply once this patch goes in.
So I guess the tests for this are going to be in the
gdb.base/print-file-var.exp changes that are part of patch #8. It
would be great if the commit message could mention this - it just
makes life easier later on.
I wonder if we need to update other *_lookup_symbol_nonlocal functions
in a similar way? For example can the C tests be compiled as C++,
which should cause GDB to use cp_lookup_symbol_nonlocal.
Looking at both basic_lookup_symbol_nonlocal and
cp_lookup_symbol_nonlocal, I wonder if your fix could be moved into
lookup_global_symbol? And just have 'block_global_block (block)'
checked before the search of all global blocks?
Some other languages provide their own *_lookup_symbol_nonlocal, I
don't know if these would also need fixing.
Thanks,
Andrew
>
> gdb/ChangeLog
> 2019-08-01 Tom Tromey <tromey@adacore.com>
>
> * symtab.c (basic_lookup_symbol_nonlocal): Search global block.
> Remove old comments.
> ---
> gdb/ChangeLog | 5 +++++
> gdb/symtab.c | 44 ++++++++++++++++----------------------------
> 2 files changed, 21 insertions(+), 28 deletions(-)
>
> diff --git a/gdb/symtab.c b/gdb/symtab.c
> index 0ff212e0d97..b8f33509c09 100644
> --- a/gdb/symtab.c
> +++ b/gdb/symtab.c
> @@ -2417,34 +2417,6 @@ basic_lookup_symbol_nonlocal (const struct language_defn *langdef,
> {
> struct block_symbol result;
>
> - /* NOTE: carlton/2003-05-19: The comments below were written when
> - this (or what turned into this) was part of lookup_symbol_aux;
> - I'm much less worried about these questions now, since these
> - decisions have turned out well, but I leave these comments here
> - for posterity. */
> -
> - /* NOTE: carlton/2002-12-05: There is a question as to whether or
> - not it would be appropriate to search the current global block
> - here as well. (That's what this code used to do before the
> - is_a_field_of_this check was moved up.) On the one hand, it's
> - redundant with the lookup in all objfiles search that happens
> - next. On the other hand, if decode_line_1 is passed an argument
> - like filename:var, then the user presumably wants 'var' to be
> - searched for in filename. On the third hand, there shouldn't be
> - multiple global variables all of which are named 'var', and it's
> - not like decode_line_1 has ever restricted its search to only
> - global variables in a single filename. All in all, only
> - searching the static block here seems best: it's correct and it's
> - cleanest. */
> -
> - /* NOTE: carlton/2002-12-05: There's also a possible performance
> - issue here: if you usually search for global symbols in the
> - current file, then it would be slightly better to search the
> - current global block before searching all the symtabs. But there
> - are other factors that have a much greater effect on performance
> - than that one, so I don't think we should worry about that for
> - now. */
> -
> /* NOTE: dje/2014-10-26: The lookup in all objfiles search could skip
> the current objfile. Searching the current objfile first is useful
> for both matching user expectations as well as performance. */
> @@ -2453,6 +2425,22 @@ basic_lookup_symbol_nonlocal (const struct language_defn *langdef,
> if (result.symbol != NULL)
> return result;
>
> + /* If a block was passed in, we want to search the corresponding
> + global block now. This yields "more expected" behavior, and is
> + needed to support 'FILENAME'::VARIABLE lookups. */
> + const struct block *global_block = block_global_block (block);
> + if (global_block != nullptr)
> + {
> + result.symbol = lookup_symbol_in_block (name,
> + symbol_name_match_type::FULL,
> + global_block, domain);
> + if (result.symbol != nullptr)
> + {
> + result.block = global_block;
> + return result;
> + }
> + }
> +
> /* If we didn't find a definition for a builtin type in the static block,
> search for it now. This is actually the right thing to do and can be
> a massive performance win. E.g., when debugging a program with lots of
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v2 4/8] Search global block from basic_lookup_symbol_nonlocal
2019-08-21 10:32 ` Andrew Burgess
@ 2019-08-27 9:54 ` Andrew Burgess
2019-08-27 18:17 ` Christian Biesinger via gdb-patches
0 siblings, 1 reply; 27+ messages in thread
From: Andrew Burgess @ 2019-08-27 9:54 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
* Andrew Burgess <andrew.burgess@embecosm.com> [2019-08-21 11:32:25 +0100]:
> * Tom Tromey <tromey@adacore.com> [2019-08-01 11:04:08 -0600]:
>
> > This changes basic_lookup_symbol_nonlocal to look in the global block
> > of the passed-in block. If no block was passed in, it reverts to the
> > previous behavior.
> >
> > This change is needed to ensure that 'FILENAME'::NAME lookups work
> > properly. As debugging Pedro's test case showed, this was not working
> > properly in the case where multiple identical names could be found
> > (the one situation where this feature is truly needed :-).
> >
> > This also removes some old comments from basic_lookup_symbol_nonlocal
> > that no longer apply once this patch goes in.
>
> So I guess the tests for this are going to be in the
> gdb.base/print-file-var.exp changes that are part of patch #8. It
> would be great if the commit message could mention this - it just
> makes life easier later on.
>
> I wonder if we need to update other *_lookup_symbol_nonlocal functions
> in a similar way? For example can the C tests be compiled as C++,
> which should cause GDB to use cp_lookup_symbol_nonlocal.
>
> Looking at both basic_lookup_symbol_nonlocal and
> cp_lookup_symbol_nonlocal, I wonder if your fix could be moved into
> lookup_global_symbol? And just have 'block_global_block (block)'
> checked before the search of all global blocks?
The patch below applies on top of this series and extends
gdb.base/print-file-var.exp to compile as both C++ and C. The C++
will fail initially.
The patch also includes a proposed fix to move the searching of the
"current" global block into lookup_global_symbol, after this both the
C++ and C versions of the test pass, and there are no other test
regressions.
This patch is going to clash with Christian Biesinger's patch to
refactor lookup_global_symbol, but hopefully merging these two
shouldn't be that hard.
I also tweaked the test to remove some duplicate test names.
What do you think?
Thanks,
Andrew
---
diff --git a/gdb/symtab.c b/gdb/symtab.c
index bd6fa35db6a..63a39e2996a 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -2426,22 +2426,6 @@ basic_lookup_symbol_nonlocal (const struct language_defn *langdef,
if (result.symbol != NULL)
return result;
- /* If a block was passed in, we want to search the corresponding
- global block now. This yields "more expected" behavior, and is
- needed to support 'FILENAME'::VARIABLE lookups. */
- const struct block *global_block = block_global_block (block);
- if (global_block != nullptr)
- {
- result.symbol = lookup_symbol_in_block (name,
- symbol_name_match_type::FULL,
- global_block, domain);
- if (result.symbol != nullptr)
- {
- result.block = global_block;
- return result;
- }
- }
-
/* If we didn't find a definition for a builtin type in the static block,
search for it now. This is actually the right thing to do and can be
a massive performance win. E.g., when debugging a program with lots of
@@ -2662,6 +2646,22 @@ lookup_global_symbol (const char *name,
if (objfile != NULL)
result = solib_global_lookup (objfile, name, domain);
+ /* If a block was passed in, we want to search the corresponding
+ global block first. This yields "more expected" behavior, and is
+ needed to support 'FILENAME'::VARIABLE lookups. */
+ const struct block *global_block = block_global_block (block);
+ if (global_block != nullptr)
+ {
+ result.symbol = lookup_symbol_in_block (name,
+ symbol_name_match_type::FULL,
+ global_block, domain);
+ if (result.symbol != nullptr)
+ {
+ result.block = global_block;
+ return result;
+ }
+ }
+
/* If that didn't work go a global search (of global blocks, heh). */
if (result.symbol == NULL)
{
diff --git a/gdb/testsuite/gdb.base/print-file-var-lib1.c b/gdb/testsuite/gdb.base/print-file-var-lib1.c
index aec04a9b02b..d172c15bc7d 100644
--- a/gdb/testsuite/gdb.base/print-file-var-lib1.c
+++ b/gdb/testsuite/gdb.base/print-file-var-lib1.c
@@ -19,6 +19,8 @@
ATTRIBUTE_VISIBILITY int this_version_id = 104;
+START_EXTERN_C
+
int
get_version_1 (void)
{
@@ -26,3 +28,5 @@ get_version_1 (void)
return this_version_id;
}
+
+END_EXTERN_C
diff --git a/gdb/testsuite/gdb.base/print-file-var-lib2.c b/gdb/testsuite/gdb.base/print-file-var-lib2.c
index 4dfdfa04c99..b392aff9f3d 100644
--- a/gdb/testsuite/gdb.base/print-file-var-lib2.c
+++ b/gdb/testsuite/gdb.base/print-file-var-lib2.c
@@ -19,9 +19,13 @@
ATTRIBUTE_VISIBILITY int this_version_id = 203;
+START_EXTERN_C
+
int
get_version_2 (void)
{
printf ("get_version_2: &this_version_id=%p, this_version_id=%d\n", &this_version_id, this_version_id);
return this_version_id;
}
+
+END_EXTERN_C
diff --git a/gdb/testsuite/gdb.base/print-file-var-main.c b/gdb/testsuite/gdb.base/print-file-var-main.c
index 29d4fed22d1..1472bd44883 100644
--- a/gdb/testsuite/gdb.base/print-file-var-main.c
+++ b/gdb/testsuite/gdb.base/print-file-var-main.c
@@ -23,9 +23,13 @@
#include "print-file-var.h"
+START_EXTERN_C
+
extern int get_version_1 (void);
extern int get_version_2 (void);
+END_EXTERN_C
+
#if VERSION_ID_MAIN
ATTRIBUTE_VISIBILITY int this_version_id = 55;
#endif
diff --git a/gdb/testsuite/gdb.base/print-file-var.exp b/gdb/testsuite/gdb.base/print-file-var.exp
index a37cca70de6..1a065cf568b 100644
--- a/gdb/testsuite/gdb.base/print-file-var.exp
+++ b/gdb/testsuite/gdb.base/print-file-var.exp
@@ -17,7 +17,7 @@ if {[skip_shlib_tests]} {
return -1
}
-proc test {hidden dlopen version_id_main} {
+proc test {hidden dlopen version_id_main lang} {
global srcdir subdir
set main "print-file-var-main"
@@ -32,7 +32,7 @@ proc test {hidden dlopen version_id_main} {
set libobj1 [standard_output_file ${lib1}$suffix.so]
set libobj2 [standard_output_file ${lib2}$suffix.so]
- set lib_opts { debug additional_flags=-fPIC }
+ set lib_opts { debug additional_flags=-fPIC $lang }
lappend lib_opts "additional_flags=-DHIDDEN=$hidden"
if { [gdb_compile_shlib ${srcdir}/${subdir}/${lib1}.c \
@@ -46,7 +46,7 @@ proc test {hidden dlopen version_id_main} {
return -1
}
- set main_opts [list debug shlib=${libobj1}]
+ set main_opts [list debug shlib=${libobj1} $lang]
if {$dlopen} {
lappend main_opts "shlib_load" \
@@ -108,12 +108,14 @@ proc test {hidden dlopen version_id_main} {
# Compare the values of $sym1 and $sym2.
proc compare {sym1 sym2} {
- # Done this way instead of comparing the symbols with "print $sym1
- # == sym2" in GDB directly so that the values of the symbols end
- # up visible in the logs, for debug purposes.
- set vsym1 [get_integer_valueof $sym1 -1]
- set vsym2 [get_integer_valueof $sym2 -1]
- gdb_assert {$vsym1 == $vsym2} "$sym1 == $sym2"
+ with_test_prefix "sym1=$sym1,sym2=$sym2" {
+ # Done this way instead of comparing the symbols with "print $sym1
+ # == sym2" in GDB directly so that the values of the symbols end
+ # up visible in the logs, for debug purposes.
+ set vsym1 [get_integer_valueof $sym1 -1]
+ set vsym2 [get_integer_valueof $sym2 -1]
+ gdb_assert {$vsym1 == $vsym2} "$sym1 == $sym2"
+ }
}
if $version_id_main {
@@ -123,13 +125,14 @@ proc test {hidden dlopen version_id_main} {
compare "'print-file-var-lib1.c'::this_version_id" "v1"
compare "'print-file-var-lib2.c'::this_version_id" "v2"
-
}
-foreach_with_prefix hidden {0 1} {
- foreach_with_prefix dlopen {0 1} {
- foreach_with_prefix version_id_main {0 1} {
- test $hidden $dlopen $version_id_main
+foreach_with_prefix lang { c c++ } {
+ foreach_with_prefix hidden {0 1} {
+ foreach_with_prefix dlopen {0 1} {
+ foreach_with_prefix version_id_main {0 1} {
+ test $hidden $dlopen $version_id_main $lang
+ }
}
}
}
diff --git a/gdb/testsuite/gdb.base/print-file-var.h b/gdb/testsuite/gdb.base/print-file-var.h
index fe7a3460edb..c44e4848b4a 100644
--- a/gdb/testsuite/gdb.base/print-file-var.h
+++ b/gdb/testsuite/gdb.base/print-file-var.h
@@ -23,4 +23,12 @@
# define ATTRIBUTE_VISIBILITY
#endif
+#ifdef __cplusplus
+# define START_EXTERN_C extern "C" {
+# define END_EXTERN_C }
+#else
+# define START_EXTERN_C
+# define END_EXTERN_C
+#endif
+
#endif /* PRINT_FILE_VAR_H */
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v2 4/8] Search global block from basic_lookup_symbol_nonlocal
2019-08-27 9:54 ` Andrew Burgess
@ 2019-08-27 18:17 ` Christian Biesinger via gdb-patches
2019-08-28 12:34 ` Andrew Burgess
0 siblings, 1 reply; 27+ messages in thread
From: Christian Biesinger via gdb-patches @ 2019-08-27 18:17 UTC (permalink / raw)
To: Andrew Burgess; +Cc: Tom Tromey, gdb-patches
On Tue, Aug 27, 2019 at 4:54 AM Andrew Burgess
<andrew.burgess@embecosm.com> wrote:
> @@ -2662,6 +2646,22 @@ lookup_global_symbol (const char *name,
> if (objfile != NULL)
> result = solib_global_lookup (objfile, name, domain);
>
> + /* If a block was passed in, we want to search the corresponding
> + global block first. This yields "more expected" behavior, and is
> + needed to support 'FILENAME'::VARIABLE lookups. */
> + const struct block *global_block = block_global_block (block);
> + if (global_block != nullptr)
> + {
> + result.symbol = lookup_symbol_in_block (name,
> + symbol_name_match_type::FULL,
> + global_block, domain);
> + if (result.symbol != nullptr)
> + {
> + result.block = global_block;
> + return result;
> + }
> + }
> +
> /* If that didn't work go a global search (of global blocks, heh). */
> if (result.symbol == NULL)
> {
I like this change but shouldn't this call happen before solib_global_lookup?
(That said, I would like to remove that function...
https://patches-gcc.linaro.org/patch/21673/)
Christian
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v2 4/8] Search global block from basic_lookup_symbol_nonlocal
2019-08-27 18:17 ` Christian Biesinger via gdb-patches
@ 2019-08-28 12:34 ` Andrew Burgess
0 siblings, 0 replies; 27+ messages in thread
From: Andrew Burgess @ 2019-08-28 12:34 UTC (permalink / raw)
To: Christian Biesinger; +Cc: Tom Tromey, gdb-patches
* Christian Biesinger <cbiesinger@google.com> [2019-08-27 13:16:37 -0500]:
> On Tue, Aug 27, 2019 at 4:54 AM Andrew Burgess
> <andrew.burgess@embecosm.com> wrote:
> > @@ -2662,6 +2646,22 @@ lookup_global_symbol (const char *name,
> > if (objfile != NULL)
> > result = solib_global_lookup (objfile, name, domain);
> >
> > + /* If a block was passed in, we want to search the corresponding
> > + global block first. This yields "more expected" behavior, and is
> > + needed to support 'FILENAME'::VARIABLE lookups. */
> > + const struct block *global_block = block_global_block (block);
> > + if (global_block != nullptr)
> > + {
> > + result.symbol = lookup_symbol_in_block (name,
> > + symbol_name_match_type::FULL,
> > + global_block, domain);
> > + if (result.symbol != nullptr)
> > + {
> > + result.block = global_block;
> > + return result;
> > + }
> > + }
> > +
> > /* If that didn't work go a global search (of global blocks, heh). */
> > if (result.symbol == NULL)
> > {
>
> I like this change but shouldn't this call happen before solib_global_lookup?
>
> (That said, I would like to remove that function...
> https://patches-gcc.linaro.org/patch/21673/)
Here is a version of my patch that applies when Tom's series is
rebased onto current upstream HEAD. As Christian suggests, this moves
the check before the call to solib_global_lookup.
I regression tested this again and see no failures.
---
commit ba3a8d0bf2863ec9fede2901b119bfdeb67c9434
Author: Andrew Burgess <andrew.burgess@embecosm.com>
Date: Tue Aug 27 10:47:19 2019 +0100
gdb: Fix FILE::VAR symbol lookup for C++
Extend the recently added test case to compile as C++ and C, then
check that we can find all of the symbols. Move a recently added
check of a global block from basic_lookup_symbol_nonlocal into
lookup_global_symbol, fixing the C++ case.
I've also made test names in gdb.base/print-file-var.exp unique.
gdb/ChangeLog:
* symtab.c (basic_lookup_symbol_nonlocal): Move check of global
block from here, to...
(lookup_global_symbol): ...here.
gdb/testsuite/ChangeLog:
* gdb.base/print-file-var-lib1.c: Add extern "C" wrappers around
interface functions.
* gdb.base/print-file-var-lib2.c: Likewise.
* gdb.base/print-file-var-main.c: Likewise around declarations.
* gdb.base/print-file-var.exp: Compile tests as both C++ and C,
make test names unique.
* gdb.base/print-file-var.h: Add some #defines to simplify setting
up extern "C" blocks.
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index c6351403819..3eb9d487e98 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2019-08-27 Andrew Burgess <andrew.burgess@embecosm.com>
+
+ * symtab.c (basic_lookup_symbol_nonlocal): Move check of global
+ block from here, to...
+ (lookup_global_symbol): ...here.
+
2019-08-21 Tom Tromey <tromey@adacore.com>
* NEWS: Add $_ada_exception entry.
diff --git a/gdb/symtab.c b/gdb/symtab.c
index f05bebfbcbb..49bcef14305 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -2425,22 +2425,6 @@ basic_lookup_symbol_nonlocal (const struct language_defn *langdef,
if (result.symbol != NULL)
return result;
- /* If a block was passed in, we want to search the corresponding
- global block now. This yields "more expected" behavior, and is
- needed to support 'FILENAME'::VARIABLE lookups. */
- const struct block *global_block = block_global_block (block);
- if (global_block != nullptr)
- {
- result.symbol = lookup_symbol_in_block (name,
- symbol_name_match_type::FULL,
- global_block, domain);
- if (result.symbol != nullptr)
- {
- result.block = global_block;
- return result;
- }
- }
-
/* If we didn't find a definition for a builtin type in the static block,
search for it now. This is actually the right thing to do and can be
a massive performance win. E.g., when debugging a program with lots of
@@ -2665,6 +2649,19 @@ lookup_global_symbol (const char *name,
const struct block *block,
const domain_enum domain)
{
+ /* If a block was passed in, we want to search the corresponding
+ global block first. This yields "more expected" behavior, and is
+ needed to support 'FILENAME'::VARIABLE lookups. */
+ const struct block *global_block = block_global_block (block);
+ if (global_block != nullptr)
+ {
+ symbol *sym = lookup_symbol_in_block (name,
+ symbol_name_match_type::FULL,
+ global_block, domain);
+ if (sym != nullptr)
+ return { sym, global_block };
+ }
+
struct objfile *objfile = lookup_objfile_from_block (block);
return lookup_global_or_static_symbol (name, GLOBAL_BLOCK, objfile, domain);
}
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index e0b0efc6cfd..69effa30f0f 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,14 @@
+2019-08-27 Andrew Burgess <andrew.burgess@embecosm.com>
+
+ * gdb.base/print-file-var-lib1.c: Add extern "C" wrappers around
+ interface functions.
+ * gdb.base/print-file-var-lib2.c: Likewise.
+ * gdb.base/print-file-var-main.c: Likewise around declarations.
+ * gdb.base/print-file-var.exp: Compile tests as both C++ and C,
+ make test names unique.
+ * gdb.base/print-file-var.h: Add some #defines to simplify setting
+ up extern "C" blocks.
+
2019-08-27 Andrew Burgess <andrew.burgess@embecosm.com>
* gdb.ada/catch_ex_std.exp: Handle case where exceptions can't be
diff --git a/gdb/testsuite/gdb.base/print-file-var-lib1.c b/gdb/testsuite/gdb.base/print-file-var-lib1.c
index aec04a9b02b..d172c15bc7d 100644
--- a/gdb/testsuite/gdb.base/print-file-var-lib1.c
+++ b/gdb/testsuite/gdb.base/print-file-var-lib1.c
@@ -19,6 +19,8 @@
ATTRIBUTE_VISIBILITY int this_version_id = 104;
+START_EXTERN_C
+
int
get_version_1 (void)
{
@@ -26,3 +28,5 @@ get_version_1 (void)
return this_version_id;
}
+
+END_EXTERN_C
diff --git a/gdb/testsuite/gdb.base/print-file-var-lib2.c b/gdb/testsuite/gdb.base/print-file-var-lib2.c
index 4dfdfa04c99..b392aff9f3d 100644
--- a/gdb/testsuite/gdb.base/print-file-var-lib2.c
+++ b/gdb/testsuite/gdb.base/print-file-var-lib2.c
@@ -19,9 +19,13 @@
ATTRIBUTE_VISIBILITY int this_version_id = 203;
+START_EXTERN_C
+
int
get_version_2 (void)
{
printf ("get_version_2: &this_version_id=%p, this_version_id=%d\n", &this_version_id, this_version_id);
return this_version_id;
}
+
+END_EXTERN_C
diff --git a/gdb/testsuite/gdb.base/print-file-var-main.c b/gdb/testsuite/gdb.base/print-file-var-main.c
index 29d4fed22d1..1472bd44883 100644
--- a/gdb/testsuite/gdb.base/print-file-var-main.c
+++ b/gdb/testsuite/gdb.base/print-file-var-main.c
@@ -23,9 +23,13 @@
#include "print-file-var.h"
+START_EXTERN_C
+
extern int get_version_1 (void);
extern int get_version_2 (void);
+END_EXTERN_C
+
#if VERSION_ID_MAIN
ATTRIBUTE_VISIBILITY int this_version_id = 55;
#endif
diff --git a/gdb/testsuite/gdb.base/print-file-var.exp b/gdb/testsuite/gdb.base/print-file-var.exp
index a37cca70de6..1a065cf568b 100644
--- a/gdb/testsuite/gdb.base/print-file-var.exp
+++ b/gdb/testsuite/gdb.base/print-file-var.exp
@@ -17,7 +17,7 @@ if {[skip_shlib_tests]} {
return -1
}
-proc test {hidden dlopen version_id_main} {
+proc test {hidden dlopen version_id_main lang} {
global srcdir subdir
set main "print-file-var-main"
@@ -32,7 +32,7 @@ proc test {hidden dlopen version_id_main} {
set libobj1 [standard_output_file ${lib1}$suffix.so]
set libobj2 [standard_output_file ${lib2}$suffix.so]
- set lib_opts { debug additional_flags=-fPIC }
+ set lib_opts { debug additional_flags=-fPIC $lang }
lappend lib_opts "additional_flags=-DHIDDEN=$hidden"
if { [gdb_compile_shlib ${srcdir}/${subdir}/${lib1}.c \
@@ -46,7 +46,7 @@ proc test {hidden dlopen version_id_main} {
return -1
}
- set main_opts [list debug shlib=${libobj1}]
+ set main_opts [list debug shlib=${libobj1} $lang]
if {$dlopen} {
lappend main_opts "shlib_load" \
@@ -108,12 +108,14 @@ proc test {hidden dlopen version_id_main} {
# Compare the values of $sym1 and $sym2.
proc compare {sym1 sym2} {
- # Done this way instead of comparing the symbols with "print $sym1
- # == sym2" in GDB directly so that the values of the symbols end
- # up visible in the logs, for debug purposes.
- set vsym1 [get_integer_valueof $sym1 -1]
- set vsym2 [get_integer_valueof $sym2 -1]
- gdb_assert {$vsym1 == $vsym2} "$sym1 == $sym2"
+ with_test_prefix "sym1=$sym1,sym2=$sym2" {
+ # Done this way instead of comparing the symbols with "print $sym1
+ # == sym2" in GDB directly so that the values of the symbols end
+ # up visible in the logs, for debug purposes.
+ set vsym1 [get_integer_valueof $sym1 -1]
+ set vsym2 [get_integer_valueof $sym2 -1]
+ gdb_assert {$vsym1 == $vsym2} "$sym1 == $sym2"
+ }
}
if $version_id_main {
@@ -123,13 +125,14 @@ proc test {hidden dlopen version_id_main} {
compare "'print-file-var-lib1.c'::this_version_id" "v1"
compare "'print-file-var-lib2.c'::this_version_id" "v2"
-
}
-foreach_with_prefix hidden {0 1} {
- foreach_with_prefix dlopen {0 1} {
- foreach_with_prefix version_id_main {0 1} {
- test $hidden $dlopen $version_id_main
+foreach_with_prefix lang { c c++ } {
+ foreach_with_prefix hidden {0 1} {
+ foreach_with_prefix dlopen {0 1} {
+ foreach_with_prefix version_id_main {0 1} {
+ test $hidden $dlopen $version_id_main $lang
+ }
}
}
}
diff --git a/gdb/testsuite/gdb.base/print-file-var.h b/gdb/testsuite/gdb.base/print-file-var.h
index fe7a3460edb..c44e4848b4a 100644
--- a/gdb/testsuite/gdb.base/print-file-var.h
+++ b/gdb/testsuite/gdb.base/print-file-var.h
@@ -23,4 +23,12 @@
# define ATTRIBUTE_VISIBILITY
#endif
+#ifdef __cplusplus
+# define START_EXTERN_C extern "C" {
+# define END_EXTERN_C }
+#else
+# define START_EXTERN_C
+# define END_EXTERN_C
+#endif
+
#endif /* PRINT_FILE_VAR_H */
^ permalink raw reply [flat|nested] 27+ messages in thread