* [PATCH 2/4] Handle copy relocations
2019-06-27 14:52 [PATCH 0/4] Handle copy relocations and add $_ada_exception Tom Tromey
@ 2019-06-27 14:52 ` Tom Tromey
2019-07-20 1:11 ` Pedro Alves
2019-06-27 14:52 ` [PATCH 1/4] Change SYMBOL_VALUE_ADDRESS to be an lvalue Tom Tromey
` (2 subsequent siblings)
3 siblings, 1 reply; 14+ messages in thread
From: Tom Tromey @ 2019-06-27 14:52 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-06-27 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 | 36 ++++++++++++++++++------------------
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 | 47 +++++++++++++++++++++++++++++++++++++++++++++++
gdb/symtab.h | 42 +++++++++++++++++++++++++++++++++++++++---
11 files changed, 196 insertions(+), 32 deletions(-)
diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 1e996557b6e..5a055fced7b 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 2945a2d090d..eec6eea9085 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -2126,8 +2126,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;
@@ -2202,11 +2204,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;
@@ -2216,7 +2221,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
@@ -21610,19 +21616,13 @@ 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)
+ {
+ /* A global static variable might be subject to
+ copy relocation. */
+ sym->maybe_copied = dwarf2_per_objfile->can_copy;
+ }
/* 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 9251bde1259..9c56a57b6bb 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 e64e5df1a18..d9abce851e4 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 daddd2e21ab..35c7f9e4d87 100644
--- a/gdb/symfile.h
+++ b/gdb/symfile.h
@@ -579,7 +579,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 4920d94a247..4906e9cfb9b 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -6003,6 +6003,53 @@ 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 ())
+ {
+ if ((objfile->flags & OBJF_MAINLINE) != 0)
+ {
+ 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 7b3bb5d3775..4455bb09b83 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] 14+ messages in thread* Re: [PATCH 2/4] Handle copy relocations
2019-06-27 14:52 ` [PATCH 2/4] Handle copy relocations Tom Tromey
@ 2019-07-20 1:11 ` Pedro Alves
2019-07-30 21:00 ` Tom Tromey
0 siblings, 1 reply; 14+ messages in thread
From: Pedro Alves @ 2019-07-20 1:11 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 6/27/19 3:52 PM, Tom Tromey wrote:
> 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".
To be more correct, this depends on ABI, or how you compile your
binary. Compiling the main binary as PIC/PIE may or may not use
copy relocations and use GOT accesses instead, for example.
> 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.
I'm surprised to find no tests in this patch?
I'm not sure whether this is the right approach, since it seems to assume
that interposition/preemption happens, while it may not, given
possibility of attribute visibility hidden/protected. Ideally whatever
design we take considers how no-preemption would be addressed.
This came up earlier here:
https://sourceware.org/ml/gdb-patches/2017-10/msg00710.html
I've taken the examples from that email and converted them to
a proper testcase today. See below.
With current master, we get:
FAIL: gdb.base/print-file-var.exp: hidden=1: dlopen=0: 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=0: '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
Your patch curiously doesn't make a difference here. Could we add
something here to show what your patch improves?
But I'm wondering whether fixing the hidden=1 cases would fit
with the design of your patch, or does that scenario require
thinking about all this differently?
From ffe918c15e9dc10540ed1590d9d37fee3fcea844 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Sat, 20 Jul 2019 01:20:35 +0100
Subject: [PATCH] Make print-file-var.exp test attribute visibility hidden,
dlopen, and main symbol
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
I did not reindent the body of "test" in order to keep the patch
readable. To be reindended before pushing.
gdb/ChangeLog:
yyyy-mm-dd 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/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 | 76 +++++++++++++++++++++-------
gdb/testsuite/gdb.base/print-file-var.h | 26 ++++++++++
5 files changed, 127 insertions(+), 26 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..4f4afc161e2 100644
--- a/gdb/testsuite/gdb.base/print-file-var.exp
+++ b/gdb/testsuite/gdb.base/print-file-var.exp
@@ -17,15 +17,23 @@ 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}.so]
-set libobj2 [standard_output_file ${lib2}.so]
+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} \
@@ -37,10 +45,21 @@ if { [gdb_compile_shlib ${srcdir}/${subdir}/${lib2}.c \
${lib_opts} ] != "" } {
return -1
}
-if { [gdb_compile "${srcdir}/${subdir}/${executable}.c" \
+
+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 \
- [list debug shlib=${libobj1} shlib=${libobj2}]]
+ $main_opts]
!= ""} {
return -1
}
@@ -55,7 +74,7 @@ if ![runto_main] {
}
# Try printing "this_version_num" qualified with the name of the file
-# where the variables are defined. There are two global variables
+# 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
@@ -65,28 +84,51 @@ if ![runto_main] {
# 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
+# 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 variables 'v1' and 'v2'.
+# Get past the initialization of the v* variables.
set bp_location \
- [gdb_get_line_number "STOP" "${executable}.c"]
-gdb_test "break $executable.c:$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 past v1 & v2 initialization"
+ "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 both print-file-var-lib1.c
-# and print-file-var-lib2.c.
+# 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"
+}
-gdb_test "print 'print-file-var-lib1.c'::this_version_id == v1" \
- " = 1"
+compare "'print-file-var-lib1.c'::this_version_id" "v1"
+compare "'print-file-var-lib2.c'::this_version_id" "v2"
-gdb_test "print 'print-file-var-lib2.c'::this_version_id == v2" \
- " = 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
+ }
+ }
+}
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.14.5
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 2/4] Handle copy relocations
2019-07-20 1:11 ` Pedro Alves
@ 2019-07-30 21:00 ` Tom Tromey
2019-07-30 21:28 ` Tom Tromey
0 siblings, 1 reply; 14+ messages in thread
From: Tom Tromey @ 2019-07-30 21:00 UTC (permalink / raw)
To: Pedro Alves; +Cc: Tom Tromey, gdb-patches
Pedro> To be more correct, this depends on ABI, or how you compile your
Pedro> binary. Compiling the main binary as PIC/PIE may or may not use
Pedro> copy relocations and use GOT accesses instead, for example.
Yes, thanks for mentioning that.
Pedro> I'm surprised to find no tests in this patch?
The existing Ada exception-catching tests cover this, so I didn't add a
new test.
Pedro> FAIL: gdb.base/print-file-var.exp: hidden=1: dlopen=0: version_id_main=0: 'print-file-var-lib2.c'::this_version_id == v2
I haven't investigated the other failures yet, but I think this one in
particular is an existing bug in c-exp.y. What happens is that this
does not actually look up the symbol from that source file! Instead it
finds the symbol in print-file-var-lib1.c.
Pedro> But I'm wondering whether fixing the hidden=1 cases would fit
Pedro> with the design of your patch, or does that scenario require
Pedro> thinking about all this differently?
I think applying the appended probably makes sense. This rules out
symbols where the corresponding minsym is hidden.
Tom
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index f81efb4f433..90dfde0f1e9 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -21653,11 +21653,19 @@ new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
else if (attr2 && (DW_UNSND (attr2) != 0))
{
if (SYMBOL_CLASS (sym) == LOC_STATIC
- && (objfile->flags & OBJF_MAINLINE) == 0)
+ && (objfile->flags & OBJF_MAINLINE) == 0
+ && dwarf2_per_objfile->can_copy)
{
/* A global static variable might be subject to
- copy relocation. */
- sym->maybe_copied = dwarf2_per_objfile->can_copy;
+ 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,
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 2/4] Handle copy relocations
2019-07-30 21:00 ` Tom Tromey
@ 2019-07-30 21:28 ` Tom Tromey
2019-07-31 19:21 ` Tom Tromey
0 siblings, 1 reply; 14+ messages in thread
From: Tom Tromey @ 2019-07-30 21:28 UTC (permalink / raw)
To: Tom Tromey; +Cc: Pedro Alves, gdb-patches
Tom> I haven't investigated the other failures yet, but I think this one in
Tom> particular is an existing bug in c-exp.y. What happens is that this
Tom> does not actually look up the symbol from that source file! Instead it
Tom> finds the symbol in print-file-var-lib1.c.
Digging a bit deeper, the culprit seems to be lookup_global_symbol.
I think this function should respect a block that's passed in.
However, this code also calls
gdbarch_iterate_over_objfiles_in_search_order and solib_global_lookup,
and now I wonder if one or both of these needs to be changed
as well.
For example, if stopped in the library, "print this_version_id" should
probably follow the expected-by-the-library-author lookup, but currently
I think will not.
Maybe windows_iterate_over_objfiles_in_search_order should just be the
default. I am not sure.
Tom
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/4] Handle copy relocations
2019-07-30 21:28 ` Tom Tromey
@ 2019-07-31 19:21 ` Tom Tromey
0 siblings, 0 replies; 14+ messages in thread
From: Tom Tromey @ 2019-07-31 19:21 UTC (permalink / raw)
To: Tom Tromey; +Cc: Pedro Alves, gdb-patches
Tom> Digging a bit deeper, the culprit seems to be lookup_global_symbol.
Tom> I think this function should respect a block that's passed in.
I managed to get the new test completely passing.
I changed basic_lookup_symbol_nonlocal to search the block's global
block, and I removed the long comments fro 2002-3 about why this isn't
done. TBH I think all of that was just mistaken.
This change got to just 2 failures. The last two were both the
"hidden0...main0" variants, which print:
get_version_1: &this_version_id=0x7ffff7fca010, this_version_id=104
get_version_2: &this_version_id=0x7ffff7fca010, this_version_id=104
So, I think for these the answer is to change the new get_symbol_address
to search all objfiles in order. I'm slightly unhappy with this but it
seems correct enough.
I'm re-testing the series and will resubmit it once that's done,
assuming there aren't regressions.
thanks,
Tom
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/4] Change SYMBOL_VALUE_ADDRESS to be an lvalue
2019-06-27 14:52 [PATCH 0/4] Handle copy relocations and add $_ada_exception Tom Tromey
2019-06-27 14:52 ` [PATCH 2/4] Handle copy relocations Tom Tromey
@ 2019-06-27 14:52 ` Tom Tromey
2019-07-20 23:17 ` Simon Marchi
2019-06-27 14:52 ` [PATCH 3/4] Back out earlier Ada exception change Tom Tromey
2019-06-27 15:02 ` [PATCH 4/4] Add $_ada_exception convenience variable Tom Tromey
3 siblings, 1 reply; 14+ messages in thread
From: Tom Tromey @ 2019-06-27 14:52 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This changes SYMBOL_VALUE_ADDRESS to be an lvalue. 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-06-27 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 lvalue.
(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 85f2b1dfc47..2945a2d090d 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -21390,15 +21390,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;
}
@@ -21512,7 +21517,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 b59db591d9f..9f76c184e24 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 e4ee7271a15..7b3bb5d3775 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] 14+ messages in thread* Re: [PATCH 1/4] Change SYMBOL_VALUE_ADDRESS to be an lvalue
2019-06-27 14:52 ` [PATCH 1/4] Change SYMBOL_VALUE_ADDRESS to be an lvalue Tom Tromey
@ 2019-07-20 23:17 ` Simon Marchi
2019-07-23 15:53 ` Tom Tromey
0 siblings, 1 reply; 14+ messages in thread
From: Simon Marchi @ 2019-07-20 23:17 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
On 2019-06-27 10:52, Tom Tromey wrote:
> This changes SYMBOL_VALUE_ADDRESS to be an lvalue. 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.
Is the title (and commit message) backwards? SYMBOL_VALUE_ADDRESS(x) is
currently an lvalue, and you are making it a not-lvalue, no?
Simon
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/4] Change SYMBOL_VALUE_ADDRESS to be an lvalue
2019-07-20 23:17 ` Simon Marchi
@ 2019-07-23 15:53 ` Tom Tromey
0 siblings, 0 replies; 14+ messages in thread
From: Tom Tromey @ 2019-07-23 15:53 UTC (permalink / raw)
To: Simon Marchi; +Cc: Tom Tromey, gdb-patches
>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:
Simon> On 2019-06-27 10:52, Tom Tromey wrote:
>> This changes SYMBOL_VALUE_ADDRESS to be an lvalue. 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.
Simon> Is the title (and commit message) backwards? SYMBOL_VALUE_ADDRESS(x)
Simon> is currently an lvalue, and you are making it a not-lvalue, no?
Yeah, I meant rvalue. I've updated the patch to fix this.
Tom
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/4] Back out earlier Ada exception change
2019-06-27 14:52 [PATCH 0/4] Handle copy relocations and add $_ada_exception Tom Tromey
2019-06-27 14:52 ` [PATCH 2/4] Handle copy relocations Tom Tromey
2019-06-27 14:52 ` [PATCH 1/4] Change SYMBOL_VALUE_ADDRESS to be an lvalue Tom Tromey
@ 2019-06-27 14:52 ` Tom Tromey
2019-06-27 15:02 ` [PATCH 4/4] Add $_ada_exception convenience variable Tom Tromey
3 siblings, 0 replies; 14+ messages in thread
From: Tom Tromey @ 2019-06-27 14:52 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.
2019-06-27 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 5a055fced7b..4ff62cd8c95 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
@@ -12320,6 +12290,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;
@@ -12328,45 +12300,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)
@@ -13028,18 +12983,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
@@ -13058,35 +13013,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] 14+ messages in thread* [PATCH 4/4] Add $_ada_exception convenience variable
2019-06-27 14:52 [PATCH 0/4] Handle copy relocations and add $_ada_exception Tom Tromey
` (2 preceding siblings ...)
2019-06-27 14:52 ` [PATCH 3/4] Back out earlier Ada exception change Tom Tromey
@ 2019-06-27 15:02 ` Tom Tromey
2019-06-27 17:35 ` Eli Zaretskii
3 siblings, 1 reply; 14+ messages in thread
From: Tom Tromey @ 2019-06-27 15:02 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-06-27 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-06-27 Tom Tromey <tromey@adacore.com>
* gdb.texinfo (Set Catchpoints, Convenience Vars): Document
$_ada_exception.
gdb/testsuite/ChangeLog
2019-06-27 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 | 20 +-
gdb/testsuite/ChangeLog | 4 +
gdb/testsuite/gdb.ada/catch_ex_std.exp | 3 +
7 files changed, 143 insertions(+), 231 deletions(-)
diff --git a/gdb/NEWS b/gdb/NEWS
index 2cc82e86560..7379fd78b41 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 4ff62cd8c95..c792644f829 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -12279,8 +12279,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
@@ -12340,8 +12348,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);
}
@@ -12350,7 +12357,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;
@@ -12360,7 +12367,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
@@ -12375,6 +12382,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;
@@ -12408,7 +12439,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);
}
@@ -12417,7 +12448,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;
@@ -12443,13 +12474,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)
@@ -12473,7 +12505,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);
}
@@ -12506,8 +12538,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;
@@ -12522,7 +12553,7 @@ print_one_exception (enum ada_exception_catchpoint_kind ex,
annotate_field (5);
*last_loc = b->loc;
- switch (ex)
+ switch (c->m_kind)
{
case ada_catch_exception:
if (!c->excep_string.empty ())
@@ -12566,8 +12597,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;
@@ -12577,7 +12607,7 @@ print_mention_exception (enum ada_exception_catchpoint_kind ex,
uiout->field_int ("bkptno", b->number);
uiout->text (": ");
- switch (ex)
+ switch (c->m_kind)
{
case ada_catch_exception:
if (!c->excep_string.empty ())
@@ -12620,12 +12650,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");
@@ -12651,192 +12680,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;
/* Split the arguments specified in a "catch exception" command.
@@ -13099,7 +12946,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;
@@ -14290,43 +14137,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 55be2ef6920..1a69c444e37 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -4695,10 +4695,18 @@ 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.
+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 handlers @r{[}@var{name}@r{]}
@kindex catch handlers
@cindex Ada exception handlers catching
@@ -4719,9 +4727,14 @@ 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} holds the address of
+the exception being thrown. This can be useful when setting a
+condition for such a catchpoint.
+
@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
@@ -11592,6 +11605,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] 14+ messages in thread* Re: [PATCH 4/4] Add $_ada_exception convenience variable
2019-06-27 15:02 ` [PATCH 4/4] Add $_ada_exception convenience variable Tom Tromey
@ 2019-06-27 17:35 ` Eli Zaretskii
2019-06-28 18:46 ` Tom Tromey
0 siblings, 1 reply; 14+ messages in thread
From: Eli Zaretskii @ 2019-06-27 17:35 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
> From: Tom Tromey <tromey@adacore.com>
> Cc: Tom Tromey <tromey@adacore.com>
> Date: Thu, 27 Jun 2019 08:52:35 -0600
>
> diff --git a/gdb/NEWS b/gdb/NEWS
> index 2cc82e86560..7379fd78b41 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.
> +
This part is OK.
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 55be2ef6920..1a69c444e37 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -4695,10 +4695,18 @@ 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.
>
> +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 handlers @r{[}@var{name}@r{]}
> @kindex catch handlers
> @cindex Ada exception handlers catching
> @@ -4719,9 +4727,14 @@ 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} holds the address of
> +the exception being thrown. This can be useful when setting a
> +condition for such a catchpoint.
Here I wonder why we need to have the exact same text 3 times within
less than 25 lines. Can't we say this only once?
Otherwise the documentation parts are fine, thanks.
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 4/4] Add $_ada_exception convenience variable
2019-06-27 17:35 ` Eli Zaretskii
@ 2019-06-28 18:46 ` Tom Tromey
2019-06-28 19:43 ` Eli Zaretskii
0 siblings, 1 reply; 14+ messages in thread
From: Tom Tromey @ 2019-06-28 18:46 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: Tom Tromey, gdb-patches
>>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:
>> +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.
Eli> Here I wonder why we need to have the exact same text 3 times within
Eli> less than 25 lines. Can't we say this only once?
The reason I repeated it is that this section is pretty big -- all the
catchpoints are documented in a single large table.
I could split this up somehow. Maybe sub-nodes?
Or, I could mention it in the first Ada catchpoint documentation and
then have the other ones refer back to that.
What would you prefer?
Tom
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 4/4] Add $_ada_exception convenience variable
2019-06-28 18:46 ` Tom Tromey
@ 2019-06-28 19:43 ` Eli Zaretskii
0 siblings, 0 replies; 14+ messages in thread
From: Eli Zaretskii @ 2019-06-28 19:43 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
> From: Tom Tromey <tromey@adacore.com>
> Cc: Tom Tromey <tromey@adacore.com>, gdb-patches@sourceware.org
> Date: Fri, 28 Jun 2019 12:46:01 -0600
>
> Eli> Here I wonder why we need to have the exact same text 3 times within
> Eli> less than 25 lines. Can't we say this only once?
>
> The reason I repeated it is that this section is pretty big -- all the
> catchpoints are documented in a single large table.
>
> I could split this up somehow. Maybe sub-nodes?
>
> Or, I could mention it in the first Ada catchpoint documentation and
> then have the other ones refer back to that.
>
> What would you prefer?
The latter, I think.
Thanks.
^ permalink raw reply [flat|nested] 14+ messages in thread