* [RFC] regresssion(internal-error) printing subprogram argument
@ 2017-12-13 10:37 Joel Brobecker
2017-12-14 19:02 ` Pedro Alves
0 siblings, 1 reply; 27+ messages in thread
From: Joel Brobecker @ 2017-12-13 10:37 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 4531 bytes --]
Hi Pedro,
I don't know if you remember about the patch series that introduces
wild matching for C++ as well? I havent' verified this in details,
because there is a series of patches, and they are fairly long, but
I have a feeling that they may have introduced a regression. I can
bisect tomorrow if needed.
In any case, consider the following code which first declares
a tagged type (the equivalent of a class in Ada), and then
a procedure which takes a pointer (access) to this type's 'Class.
package Pck is
type Top_T is tagged record
N : Integer := 1;
end record;
procedure Inspect (Obj: access Top_T'Class);
end Pck;
Putting a breakpoint in that procedure and then running to it triggers
an internal error:
(gdb) break inspect
(gdb) continue
Breakpoint 1, pck.inspect (obj=0x63e010
/[...]/gdb/stack.c:621: internal-error: void print_frame_args(symbol*, frame_info*, int, ui_file*): Assertion `nsym != NULL' failed.
What's special about this subprogram is that it takes an access
to what we call a 'Class type, and for implementation reasons,
the compiler adds an extra argument named "objL". If you are
curious why, it allows the compiler for perform dynamic accessibility
checks that are mandated by the language.
If we look at the location where we get the internal error
(in stack.c), we find that we are looping over the symbol of
each parameter, and for each parameter, we do:
/* We have to look up the symbol because arguments can have
two entries (one a parameter, one a local) and the one we
want is the local, which lookup_symbol will find for us.
[...]
nsym = lookup_symbol (SYMBOL_LINKAGE_NAME (sym),
b, VAR_DOMAIN, NULL).symbol;
gdb_assert (nsym != NULL);
The lookup_symbol goes through the lookup structure, which means
the symbol's linkage name ("objL") gets transformed into
a lookup_name_info object (in block_lookup_symbol), before
it gets fed to the block symbol dictionary iterators. This,
in turn, triggers the symbol matching by comparing the
"lookup" name which, for Ada, means among other things, lowercasing
the given name to "objl". It is this transformation that causes
the lookup find no matches, and therefore trip this assertion.
Going back to the "offending" call to lookup_symbol in stack.c,
what we are trying to do, here, is do a lookup by linkage name.
So, I think what we mean to be doing is a completely litteral
symbol lookup, so maybe not even strcmp_iw, but actually just
plain strcmp???
There doesn't seem to be a way to do just that anymore,
unfortunately. While this wasn't necessarily part of the spec,
in the past, in practice, you could get that effect by doing
a lookup using the C language. But that doesn't work, because
we still end up somehow using Ada's lookup_name routine which
transforms "objL".
So, ideally, as I hinted before, I think what we need is a way
to perform a litteral lookup so that searches by linkage names
like the above can be performed. But this might be a fairly
involved change (maybe adding a new kind of lookup, and then
making adjustments so we know which kind of name to use for
name matching).
Failing that, the attached prototype takes the easy approach
of just making Ada special, and adjusting the name used for
the lookup to encode in the name itself the fact that the lookup
should be litteral. I tested it with both the official testsuite
as well as AdaCore's testsuite, and it fixed the issue without
regression that I could see. It's not ideal (not clean, IMO),
but gets us back with minimal fuss. We could have that while
we work on extending the current framework to allow litteral
lookups -- I would just clean it up and post an official RFA
for it.
Can you tell me what you think?
I'm sorry I didn't notice this when I did my review and round
of testing. I did the best I could, but the current version of
GDB showing 300+ failures in AdaCore's testsuite at that time,
I used manual report comparison, and I must have missed
these differences (7 failures).
Note also that I consider this issue blocking for 8.1, as these
are not just limited to access to 'Class parameters. These kinds
of internal parameters can also be generated in other situations,
and in particular when using tasking (the equivalent of threads).
Tasking is fairly prevalent in Ada programs. We might even want
to defer branching if we elect to take the more general fix of
adding litteral lookup support...
Thanks!
--
Joel
[-- Attachment #2: 0001-WIP-nsym-NULL-assert-failure-in-stack.c-print_frame_.patch --]
[-- Type: text/x-diff, Size: 6325 bytes --]
From c48fd2d065cddafba771c6bead7ee16806505089 Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Wed, 13 Dec 2017 03:53:13 -0500
Subject: [PATCH] WIP: `nsym != NULL' assert failure in
stack.c::print_frame_args
---
gdb/stack.c | 6 +++-
gdb/testsuite/gdb.ada/access_tagged_param.exp | 37 +++++++++++++++++++++++
gdb/testsuite/gdb.ada/access_tagged_param/foo.adb | 20 ++++++++++++
gdb/testsuite/gdb.ada/access_tagged_param/pck.adb | 21 +++++++++++++
gdb/testsuite/gdb.ada/access_tagged_param/pck.ads | 21 +++++++++++++
5 files changed, 104 insertions(+), 1 deletion(-)
create mode 100644 gdb/testsuite/gdb.ada/access_tagged_param.exp
create mode 100644 gdb/testsuite/gdb.ada/access_tagged_param/foo.adb
create mode 100644 gdb/testsuite/gdb.ada/access_tagged_param/pck.adb
create mode 100644 gdb/testsuite/gdb.ada/access_tagged_param/pck.ads
diff --git a/gdb/stack.c b/gdb/stack.c
index 6bd0d45..521cef1 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -615,8 +615,12 @@ print_frame_args (struct symbol *func, struct frame_info *frame,
if (*SYMBOL_LINKAGE_NAME (sym))
{
struct symbol *nsym;
+ std::string sym_linkage_name (SYMBOL_LINKAGE_NAME (sym));
- nsym = lookup_symbol (SYMBOL_LINKAGE_NAME (sym),
+ if (SYMBOL_LANGUAGE (sym) == language_ada)
+ sym_linkage_name = std::string("<") + sym_linkage_name + '>';
+
+ nsym = lookup_symbol (sym_linkage_name.c_str (),
b, VAR_DOMAIN, NULL).symbol;
gdb_assert (nsym != NULL);
if (SYMBOL_CLASS (nsym) == LOC_REGISTER
diff --git a/gdb/testsuite/gdb.ada/access_tagged_param.exp b/gdb/testsuite/gdb.ada/access_tagged_param.exp
new file mode 100644
index 0000000..a5e399f
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/access_tagged_param.exp
@@ -0,0 +1,37 @@
+# Copyright 2017 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/>.
+
+load_lib "ada.exp"
+
+standard_ada_testfile foo
+
+if {[gdb_compile_ada "${srcfile}" "${binfile}" executable [list debug]] != "" } {
+ return -1
+}
+
+clean_restart ${testfile}
+
+if ![runto "foo"] then {
+ perror "Couldn't run ${testfile}"
+ return
+}
+
+gdb_breakpoint "pck.inspect"
+
+# Continue until we reach the breakpoint, and verify that we can print
+# the value of all the parameters.
+
+gdb_test "continue" \
+ ".*Breakpoint $decimal, pck\\.inspect \\(obj=$hex, <objL>=2\\) at .*"
diff --git a/gdb/testsuite/gdb.ada/access_tagged_param/foo.adb b/gdb/testsuite/gdb.ada/access_tagged_param/foo.adb
new file mode 100644
index 0000000..bd7e641
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/access_tagged_param/foo.adb
@@ -0,0 +1,20 @@
+-- Copyright 2017 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/>.
+
+with Pck; use Pck;
+procedure Foo is
+begin
+ Inspect (new Top_T'(N => 2));
+end Foo;
diff --git a/gdb/testsuite/gdb.ada/access_tagged_param/pck.adb b/gdb/testsuite/gdb.ada/access_tagged_param/pck.adb
new file mode 100644
index 0000000..88fa970
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/access_tagged_param/pck.adb
@@ -0,0 +1,21 @@
+-- Copyright 2017 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/>.
+
+package body Pck is
+ procedure Inspect (Obj: access Top_T'Class) is
+ begin
+ null;
+ end Inspect;
+end Pck;
diff --git a/gdb/testsuite/gdb.ada/access_tagged_param/pck.ads b/gdb/testsuite/gdb.ada/access_tagged_param/pck.ads
new file mode 100644
index 0000000..bbb5c8d
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/access_tagged_param/pck.ads
@@ -0,0 +1,21 @@
+-- Copyright 2017 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/>.
+
+package Pck is
+ type Top_T is tagged record
+ N : Integer := 1;
+ end record;
+ procedure Inspect (Obj: access Top_T'Class);
+end Pck;
--
2.1.4
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC] regresssion(internal-error) printing subprogram argument
2017-12-13 10:37 [RFC] regresssion(internal-error) printing subprogram argument Joel Brobecker
@ 2017-12-14 19:02 ` Pedro Alves
2017-12-14 19:41 ` Pedro Alves
2017-12-15 7:54 ` Joel Brobecker
0 siblings, 2 replies; 27+ messages in thread
From: Pedro Alves @ 2017-12-14 19:02 UTC (permalink / raw)
To: Joel Brobecker; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 5677 bytes --]
On 12/13/2017 10:36 AM, Joel Brobecker wrote:
> Hi Pedro,
>
> I don't know if you remember about the patch series that introduces
> wild matching for C++ as well? I havent' verified this in details,
> because there is a series of patches, and they are fairly long, but
> I have a feeling that they may have introduced a regression. I can
> bisect tomorrow if needed.
>
> In any case, consider the following code which first declares
> a tagged type (the equivalent of a class in Ada), and then
> a procedure which takes a pointer (access) to this type's 'Class.
>
> package Pck is
> type Top_T is tagged record
> N : Integer := 1;
> end record;
> procedure Inspect (Obj: access Top_T'Class);
> end Pck;
>
> Putting a breakpoint in that procedure and then running to it triggers
> an internal error:
>
> (gdb) break inspect
> (gdb) continue
> Breakpoint 1, pck.inspect (obj=0x63e010
> /[...]/gdb/stack.c:621: internal-error: void print_frame_args(symbol*, frame_info*, int, ui_file*): Assertion `nsym != NULL' failed.
>
> What's special about this subprogram is that it takes an access
> to what we call a 'Class type, and for implementation reasons,
> the compiler adds an extra argument named "objL". If you are
> curious why, it allows the compiler for perform dynamic accessibility
> checks that are mandated by the language.
>
> If we look at the location where we get the internal error
> (in stack.c), we find that we are looping over the symbol of
> each parameter, and for each parameter, we do:
>
> /* We have to look up the symbol because arguments can have
> two entries (one a parameter, one a local) and the one we
> want is the local, which lookup_symbol will find for us.
> [...]
> nsym = lookup_symbol (SYMBOL_LINKAGE_NAME (sym),
> b, VAR_DOMAIN, NULL).symbol;
> gdb_assert (nsym != NULL);
>
> The lookup_symbol goes through the lookup structure, which means
> the symbol's linkage name ("objL") gets transformed into
> a lookup_name_info object (in block_lookup_symbol), before
> it gets fed to the block symbol dictionary iterators. This,
> in turn, triggers the symbol matching by comparing the
> "lookup" name which, for Ada, means among other things, lowercasing
> the given name to "objl". It is this transformation that causes
> the lookup find no matches, and therefore trip this assertion.
>
> Going back to the "offending" call to lookup_symbol in stack.c,
> what we are trying to do, here, is do a lookup by linkage name.
> So, I think what we mean to be doing is a completely litteral
> symbol lookup, so maybe not even strcmp_iw, but actually just
> plain strcmp???
>
> There doesn't seem to be a way to do just that anymore,
> unfortunately. While this wasn't necessarily part of the spec,
> in the past, in practice, you could get that effect by doing
> a lookup using the C language. But that doesn't work, because
> we still end up somehow using Ada's lookup_name routine which
> transforms "objL".
>
> So, ideally, as I hinted before, I think what we need is a way
> to perform a litteral lookup so that searches by linkage names
> like the above can be performed. But this might be a fairly
> involved change (maybe adding a new kind of lookup, and then
> making adjustments so we know which kind of name to use for
> name matching).
Indeed.
I gave the "literal" lookup type a try today, and it turns out
not being so bad, I think. See patch below.
Thinking through this and experimenting a bit, I think I convinced
myself that with the current symbol tables infrastructure,
it's not literal _linkage_ names that we want to pass down, but
instead literal _search_ names. I.e., notice that I've switched
the lookup_symbol call in question to pass down
SYMBOL_SEARCH_NAME instead of SYMBOL_LINKAGE_NAME. See
comments in symtab.h in the patch.
I was thinking about trying to add a symbol_name_match_type::LINKAGE
and support searching by linkage name for any language, but the
thing is that the dictionaries only work with SYMBOL_SEARCH_NAME,
AFAICT, because that's what is used for hashing. We'd need
separate dictionaries for hashed linkage names.
The patch is not complete in the sense that there are
symbol-lookup entry points that could be tweaked to pass
down a symbol_name_match_type instead of assuming FULL.
And also, I know there are places in ada-lang.c that are doing
what you were doing here too (the "<...>" verbatim trick) which
could be adjusted. But at least this fixes your testcase too,
and causes no regressions. So maybe we could do this incrementally
and pass adjust functions to pass around a symbol_name_match_type
as we find a need? Functions that we miss passing down simply
end up behaving like symbols_name_match_type::FULL, as today.
> Can you tell me what you think?
Your turn. :-)
>
> I'm sorry I didn't notice this when I did my review and round
> of testing. I did the best I could, but the current version of
> GDB showing 300+ failures in AdaCore's testsuite at that time,
> I used manual report comparison, and I must have missed
> these differences (7 failures).
Well, no worries for me. :-)
> Note also that I consider this issue blocking for 8.1, as these
> are not just limited to access to 'Class parameters. These kinds
> of internal parameters can also be generated in other situations,
> and in particular when using tasking (the equivalent of threads).
> Tasking is fairly prevalent in Ada programs. We might even want
> to defer branching if we elect to take the more general fix of
> adding litteral lookup support...
Thanks,
Pedro Alves
[-- Attachment #2: 0001-symbol_name_match_type-LITERAL.patch --]
[-- Type: text/x-patch, Size: 15487 bytes --]
From f5a7a45a8e4979d7c47a48bf9a10d6cda3be5a73 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Thu, 14 Dec 2017 18:22:42 +0000
Subject: [PATCH] symbol_name_match_type::LITERAL
---
gdb/block.c | 3 ++-
gdb/block.h | 1 +
gdb/c-valprint.c | 9 ++++++---
gdb/compile/compile-object-load.c | 6 +++++-
gdb/cp-namespace.c | 4 +++-
gdb/infrun.c | 2 +-
gdb/language.c | 27 +++++++++++++++++++++++++
gdb/p-valprint.c | 10 ++++++----
gdb/psymtab.c | 6 ++++--
gdb/stack.c | 8 ++++----
gdb/symtab.c | 42 +++++++++++++++++++++++++++++----------
gdb/symtab.h | 22 ++++++++++++++++++++
12 files changed, 113 insertions(+), 27 deletions(-)
diff --git a/gdb/block.c b/gdb/block.c
index a8075a1..41a0c0c 100644
--- a/gdb/block.c
+++ b/gdb/block.c
@@ -673,12 +673,13 @@ block_iter_match_next (const lookup_name_info &name,
struct symbol *
block_lookup_symbol (const struct block *block, const char *name,
+ symbol_name_match_type match_type,
const domain_enum domain)
{
struct block_iterator iter;
struct symbol *sym;
- lookup_name_info lookup_name (name, symbol_name_match_type::FULL);
+ lookup_name_info lookup_name (name, match_type);
if (!BLOCK_FUNCTION (block))
{
diff --git a/gdb/block.h b/gdb/block.h
index 0326c18..66729dd 100644
--- a/gdb/block.h
+++ b/gdb/block.h
@@ -258,6 +258,7 @@ extern struct symbol *block_iter_match_next
extern struct symbol *block_lookup_symbol (const struct block *block,
const char *name,
+ symbol_name_match_type match_type,
const domain_enum domain);
/* Search BLOCK for symbol NAME in DOMAIN but only in primary symbol table of
diff --git a/gdb/c-valprint.c b/gdb/c-valprint.c
index 653fed6..9b2911a 100644
--- a/gdb/c-valprint.c
+++ b/gdb/c-valprint.c
@@ -198,14 +198,17 @@ print_unpacked_pointer (struct type *type, struct type *elttype,
struct symbol *wsym = NULL;
struct type *wtype;
struct block *block = NULL;
- struct field_of_this_result is_this_fld;
if (want_space)
fputs_filtered (" ", stream);
if (msymbol.minsym != NULL)
- wsym = lookup_symbol (MSYMBOL_LINKAGE_NAME(msymbol.minsym), block,
- VAR_DOMAIN, &is_this_fld).symbol;
+ {
+ const char *search_name
+ = MSYMBOL_SEARCH_NAME (msymbol.minsym);
+ wsym = lookup_symbol_literal (search_name, block,
+ VAR_DOMAIN).symbol;
+ }
if (wsym)
{
diff --git a/gdb/compile/compile-object-load.c b/gdb/compile/compile-object-load.c
index f1c8ccd..5f6b03f 100644
--- a/gdb/compile/compile-object-load.c
+++ b/gdb/compile/compile-object-load.c
@@ -439,7 +439,10 @@ get_out_value_type (struct symbol *func_sym, struct objfile *objfile,
block = BLOCKVECTOR_BLOCK (bv, block_loop);
if (BLOCK_FUNCTION (block) != NULL)
continue;
- gdb_val_sym = block_lookup_symbol (block, COMPILE_I_EXPR_VAL, VAR_DOMAIN);
+ gdb_val_sym = block_lookup_symbol (block,
+ COMPILE_I_EXPR_VAL,
+ symbol_name_match_type::LITERAL,
+ VAR_DOMAIN);
if (gdb_val_sym == NULL)
continue;
@@ -466,6 +469,7 @@ get_out_value_type (struct symbol *func_sym, struct objfile *objfile,
gdb_type = check_typedef (gdb_type);
gdb_ptr_type_sym = block_lookup_symbol (block, COMPILE_I_EXPR_PTR_TYPE,
+ symbol_name_match_type::LITERAL,
VAR_DOMAIN);
if (gdb_ptr_type_sym == NULL)
error (_("No \"%s\" symbol found"), COMPILE_I_EXPR_PTR_TYPE);
diff --git a/gdb/cp-namespace.c b/gdb/cp-namespace.c
index 2a3ffef..b82481b 100644
--- a/gdb/cp-namespace.c
+++ b/gdb/cp-namespace.c
@@ -141,7 +141,9 @@ cp_basic_lookup_symbol (const char *name, const struct block *block,
if (global_block != NULL)
{
- sym.symbol = lookup_symbol_in_block (name, global_block, domain);
+ sym.symbol = lookup_symbol_in_block (name,
+ symbol_name_match_type::FULL,
+ global_block, domain);
sym.block = global_block;
}
}
diff --git a/gdb/infrun.c b/gdb/infrun.c
index d7df3c7..7dd6667 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -7489,7 +7489,7 @@ insert_exception_resume_breakpoint (struct thread_info *tp,
CORE_ADDR handler;
struct breakpoint *bp;
- vsym = lookup_symbol (SYMBOL_LINKAGE_NAME (sym), b, VAR_DOMAIN, NULL);
+ vsym = lookup_symbol_literal (SYMBOL_SEARCH_NAME (sym), b, VAR_DOMAIN);
value = read_var_value (vsym.symbol, vsym.block, frame);
/* If the value was optimized out, revert to the old behavior. */
if (! value_optimized_out (value))
diff --git a/gdb/language.c b/gdb/language.c
index c3872fc..5bcdfb9 100644
--- a/gdb/language.c
+++ b/gdb/language.c
@@ -724,14 +724,41 @@ default_symbol_name_matcher (const char *symbol_search_name,
return false;
}
+/* A name matcher that matches the symbol name exactly, with
+ strcmp. */
+
+static bool
+literal_symbol_name_matcher (const char *symbol_search_name,
+ const lookup_name_info &lookup_name,
+ completion_match_result *comp_match_res)
+{
+ const std::string &name = lookup_name.name ();
+
+ int cmp = (lookup_name.completion_mode ()
+ ? strncmp (symbol_search_name, name.c_str (), name.size ())
+ : strcmp (symbol_search_name, name.c_str ()));
+ if (cmp == 0)
+ {
+ if (comp_match_res != NULL)
+ comp_match_res->set_match (symbol_search_name);
+ return true;
+ }
+ else
+ return false;
+}
+
/* See language.h. */
symbol_name_matcher_ftype *
language_get_symbol_name_matcher (const language_defn *lang,
const lookup_name_info &lookup_name)
{
+ if (lookup_name.match_type () == symbol_name_match_type::LITERAL)
+ return literal_symbol_name_matcher;
+
if (lang->la_get_symbol_name_matcher != nullptr)
return lang->la_get_symbol_name_matcher (lookup_name);
+
return default_symbol_name_matcher;
}
diff --git a/gdb/p-valprint.c b/gdb/p-valprint.c
index d12b636..8810d67 100644
--- a/gdb/p-valprint.c
+++ b/gdb/p-valprint.c
@@ -246,15 +246,17 @@ pascal_val_print (struct type *type,
struct symbol *wsym = NULL;
struct type *wtype;
struct block *block = NULL;
- struct field_of_this_result is_this_fld;
if (want_space)
fputs_filtered (" ", stream);
if (msymbol.minsym != NULL)
- wsym = lookup_symbol (MSYMBOL_LINKAGE_NAME (msymbol.minsym),
- block,
- VAR_DOMAIN, &is_this_fld).symbol;
+ {
+ const char *search_name
+ = MSYMBOL_SEARCH_NAME (msymbol.minsym);
+ wsym = lookup_symbol_literal (search_name, block,
+ VAR_DOMAIN).symbol;
+ }
if (wsym)
{
diff --git a/gdb/psymtab.c b/gdb/psymtab.c
index c87ef25..862360e 100644
--- a/gdb/psymtab.c
+++ b/gdb/psymtab.c
@@ -2254,7 +2254,8 @@ maintenance_check_psymtabs (const char *ignore, int from_tty)
length = ps->n_static_syms;
while (length--)
{
- sym = block_lookup_symbol (b, SYMBOL_LINKAGE_NAME (*psym),
+ sym = block_lookup_symbol (b, SYMBOL_SEARCH_NAME (*psym),
+ symbol_name_match_type::LITERAL,
SYMBOL_DOMAIN (*psym));
if (!sym)
{
@@ -2271,7 +2272,8 @@ maintenance_check_psymtabs (const char *ignore, int from_tty)
length = ps->n_global_syms;
while (length--)
{
- sym = block_lookup_symbol (b, SYMBOL_LINKAGE_NAME (*psym),
+ sym = block_lookup_symbol (b, SYMBOL_SEARCH_NAME (*psym),
+ symbol_name_match_type::LITERAL,
SYMBOL_DOMAIN (*psym));
if (!sym)
{
diff --git a/gdb/stack.c b/gdb/stack.c
index 6bd0d45..a51cf4f 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -616,8 +616,8 @@ print_frame_args (struct symbol *func, struct frame_info *frame,
{
struct symbol *nsym;
- nsym = lookup_symbol (SYMBOL_LINKAGE_NAME (sym),
- b, VAR_DOMAIN, NULL).symbol;
+ nsym = lookup_symbol_literal (SYMBOL_SEARCH_NAME (sym),
+ b, VAR_DOMAIN).symbol;
gdb_assert (nsym != NULL);
if (SYMBOL_CLASS (nsym) == LOC_REGISTER
&& !SYMBOL_IS_ARGUMENT (nsym))
@@ -2141,8 +2141,8 @@ iterate_over_block_arg_vars (const struct block *b,
float). There are also LOC_ARG/LOC_REGISTER pairs which
are not combined in symbol-reading. */
- sym2 = lookup_symbol (SYMBOL_LINKAGE_NAME (sym),
- b, VAR_DOMAIN, NULL).symbol;
+ sym2 = lookup_symbol_literal (SYMBOL_SEARCH_NAME (sym),
+ b, VAR_DOMAIN).symbol;
(*cb) (SYMBOL_PRINT_NAME (sym), sym2, cb_data);
}
}
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 220ae09..78018a1 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -75,6 +75,7 @@ static int find_line_common (struct linetable *, int, int *, int);
static struct block_symbol
lookup_symbol_aux (const char *name,
+ symbol_name_match_type match_type,
const struct block *block,
const domain_enum domain,
enum language language,
@@ -82,6 +83,7 @@ static struct block_symbol
static
struct block_symbol lookup_local_symbol (const char *name,
+ symbol_name_match_type match_type,
const struct block *block,
const domain_enum domain,
enum language language);
@@ -1876,7 +1878,9 @@ lookup_symbol_in_language (const char *name, const struct block *block,
demangle_result_storage storage;
const char *modified_name = demangle_for_lookup (name, lang, storage);
- return lookup_symbol_aux (modified_name, block, domain, lang,
+ return lookup_symbol_aux (modified_name,
+ symbol_name_match_type::FULL,
+ block, domain, lang,
is_a_field_of_this);
}
@@ -1895,6 +1899,16 @@ lookup_symbol (const char *name, const struct block *block,
/* See symtab.h. */
struct block_symbol
+lookup_symbol_literal (const char *name, const struct block *block,
+ domain_enum domain)
+{
+ return lookup_symbol_aux (name, symbol_name_match_type::LITERAL,
+ block, domain, language_asm, NULL);
+}
+
+/* See symtab.h. */
+
+struct block_symbol
lookup_language_this (const struct language_defn *lang,
const struct block *block)
{
@@ -1915,7 +1929,8 @@ lookup_language_this (const struct language_defn *lang,
{
struct symbol *sym;
- sym = block_lookup_symbol (block, lang->la_name_of_this, VAR_DOMAIN);
+ sym = block_lookup_symbol (block, lang->la_name_of_this,
+ symbol_name_match_type::LITERAL, VAR_DOMAIN);
if (sym != NULL)
{
if (symbol_lookup_debug > 1)
@@ -1986,7 +2001,8 @@ check_field (struct type *type, const char *name,
(e.g., demangled name) of the symbol that we're looking for. */
static struct block_symbol
-lookup_symbol_aux (const char *name, const struct block *block,
+lookup_symbol_aux (const char *name, symbol_name_match_type match_type,
+ const struct block *block,
const domain_enum domain, enum language language,
struct field_of_this_result *is_a_field_of_this)
{
@@ -2015,7 +2031,7 @@ lookup_symbol_aux (const char *name, const struct block *block,
/* Search specified block and its superiors. Don't search
STATIC_BLOCK or GLOBAL_BLOCK. */
- result = lookup_local_symbol (name, block, domain, language);
+ result = lookup_local_symbol (name, match_type, block, domain, language);
if (result.symbol != NULL)
{
if (symbol_lookup_debug)
@@ -2097,7 +2113,9 @@ lookup_symbol_aux (const char *name, const struct block *block,
Don't search STATIC_BLOCK or GLOBAL_BLOCK. */
static struct block_symbol
-lookup_local_symbol (const char *name, const struct block *block,
+lookup_local_symbol (const char *name,
+ symbol_name_match_type match_type,
+ const struct block *block,
const domain_enum domain,
enum language language)
{
@@ -2112,7 +2130,7 @@ lookup_local_symbol (const char *name, const struct block *block,
while (block != static_block)
{
- sym = lookup_symbol_in_block (name, block, domain);
+ sym = lookup_symbol_in_block (name, match_type, block, domain);
if (sym != NULL)
return (struct block_symbol) {sym, block};
@@ -2165,7 +2183,8 @@ lookup_objfile_from_block (const struct block *block)
/* See symtab.h. */
struct symbol *
-lookup_symbol_in_block (const char *name, const struct block *block,
+lookup_symbol_in_block (const char *name, symbol_name_match_type match_type,
+ const struct block *block,
const domain_enum domain)
{
struct symbol *sym;
@@ -2181,7 +2200,7 @@ lookup_symbol_in_block (const char *name, const struct block *block,
domain_name (domain));
}
- sym = block_lookup_symbol (block, name, domain);
+ sym = block_lookup_symbol (block, name, match_type, domain);
if (sym)
{
if (symbol_lookup_debug > 1)
@@ -2370,7 +2389,8 @@ lookup_symbol_via_quick_fns (struct objfile *objfile, int block_index,
bv = COMPUNIT_BLOCKVECTOR (cust);
block = BLOCKVECTOR_BLOCK (bv, block_index);
- result.symbol = block_lookup_symbol (block, name, domain);
+ result.symbol = block_lookup_symbol (block, name,
+ symbol_name_match_type::FULL, domain);
if (result.symbol == NULL)
error_in_psymtab_expansion (block_index, name, cust);
@@ -2483,7 +2503,9 @@ lookup_symbol_in_static_block (const char *name,
domain_name (domain));
}
- sym = lookup_symbol_in_block (name, static_block, domain);
+ sym = lookup_symbol_in_block (name,
+ symbol_name_match_type::FULL,
+ static_block, domain);
if (symbol_lookup_debug)
{
fprintf_unfiltered (gdb_stdlog,
diff --git a/gdb/symtab.h b/gdb/symtab.h
index 239a479..2a1fac6 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -60,6 +60,15 @@ enum class symbol_name_match_type
namespace/module/package. */
FULL,
+ /* Literal symbol matching. This is like FULL, but the search name
+ did not come from the user; instead it is already a search name
+ retrieved from a SYMBOL_SEARCH_NAME/MSYMBOL_SEARCH_NAME call.
+ Matches the symbol name exactly, with strcmp. The language
+ vector's get_symbol_name_matcher routines never see this -- it is
+ handled by the common language_get_symbol_name_matcher routine
+ instead. */
+ LITERAL,
+
/* Expression matching. The same as FULL matching in most
languages. The same as WILD matching in Ada. */
EXPRESSION,
@@ -1554,6 +1563,18 @@ extern struct block_symbol lookup_symbol (const char *,
const domain_enum,
struct field_of_this_result *);
+/* Find the definition for a specified symbol search name SEARCH_NAME
+ in domain DOMAIN, visible from lexical block BLOCK if non-NULL or
+ from global/static blocks if BLOCK is NULL. The symbol name is
+ matched literally, i.e., with a straight strcmp. See definition of
+ symbol_name_match_type::LITERAL. Returns the struct symbol
+ pointer, or NULL if no symbol is found. The symbol's section is
+ fixed up if necessary. */
+
+extern struct block_symbol lookup_symbol_literal (const char *name,
+ const struct block *block,
+ domain_enum domain);
+
/* A default version of lookup_symbol_nonlocal for use by languages
that can't think of anything better to do.
This implements the C lookup rules. */
@@ -1603,6 +1624,7 @@ extern struct block_symbol
extern struct symbol *
lookup_symbol_in_block (const char *name,
+ symbol_name_match_type match_type,
const struct block *block,
const domain_enum domain);
--
2.5.5
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC] regresssion(internal-error) printing subprogram argument
2017-12-14 19:02 ` Pedro Alves
@ 2017-12-14 19:41 ` Pedro Alves
2017-12-15 9:48 ` Joel Brobecker
2017-12-15 7:54 ` Joel Brobecker
1 sibling, 1 reply; 27+ messages in thread
From: Pedro Alves @ 2017-12-14 19:41 UTC (permalink / raw)
To: Joel Brobecker; +Cc: gdb-patches
On 12/14/2017 07:02 PM, Pedro Alves wrote:
> @@ -2271,7 +2272,8 @@ maintenance_check_psymtabs (const char *ignore, int from_tty)
> length = ps->n_global_syms;
> while (length--)
> {
> - sym = block_lookup_symbol (b, SYMBOL_LINKAGE_NAME (*psym),
> + sym = block_lookup_symbol (b, SYMBOL_SEARCH_NAME (*psym),
> + symbol_name_match_type::LITERAL,
> SYMBOL_DOMAIN (*psym));
> if (!sym)
> {
Reading back the patch on the list, I realized that this must be
fixing "maint check-psymtabs" for Ada. And indeed, without my
patch, I get here:
$ gdb ./testsuite/outputs/gdb.ada/var_arr_typedef/var_arr_typedef
(gdb) start
...
(gdb) maint check-psymtabs
Global symbol `adaS' only found in gdb/testsuite/outputs/gdb.ada/var_arr_typedef/b~var_arr_typedef.adb psymtab
Global symbol `interfacesS' only found in gdb/testsuite/outputs/gdb.ada/var_arr_typedef/b~var_arr_typedef.adb psymtab
Global symbol `packB' only found in gdb/testsuite/outputs/gdb.ada/var_arr_typedef/b~var_arr_typedef.adb psymtab
Global symbol `packS' only found in gdb/testsuite/outputs/gdb.ada/var_arr_typedef/b~var_arr_typedef.adb psymtab
Global symbol `systemS' only found in gdb/testsuite/outputs/gdb.ada/var_arr_typedef/b~var_arr_typedef.adb psymtab
Global symbol `var_arr_typedefB' only found in gdb/testsuite/outputs/gdb.ada/var_arr_typedef/b~var_arr_typedef.adb psymtab
(gdb)
After:
(gdb) start
...
(gdb) maint check-psymtabs
(gdb
Looks like we only test that command for C, currently...
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC] regresssion(internal-error) printing subprogram argument
2017-12-14 19:02 ` Pedro Alves
2017-12-14 19:41 ` Pedro Alves
@ 2017-12-15 7:54 ` Joel Brobecker
1 sibling, 0 replies; 27+ messages in thread
From: Joel Brobecker @ 2017-12-15 7:54 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
> I gave the "literal" lookup type a try today, and it turns out
> not being so bad, I think. See patch below.
Thanks!
> Thinking through this and experimenting a bit, I think I convinced
> myself that with the current symbol tables infrastructure,
> it's not literal _linkage_ names that we want to pass down, but
> instead literal _search_ names. I.e., notice that I've switched
> the lookup_symbol call in question to pass down
> SYMBOL_SEARCH_NAME instead of SYMBOL_LINKAGE_NAME. See
> comments in symtab.h in the patch.
I forgot about how the hashes were calculated, so what you are
saying makes sense.
I did a round of testing, both with the official testsuite as well
as ours, and I confirm what you see: issue fixed with no regression.
I reviewed the patch as well, and it looks good. In particular,
the choices you made in terms of what type of lookup to do, and
what name to pass made sense to me.
> The patch is not complete in the sense that there are
> symbol-lookup entry points that could be tweaked to pass
> down a symbol_name_match_type instead of assuming FULL.
>
> And also, I know there are places in ada-lang.c that are doing
> what you were doing here too (the "<...>" verbatim trick) which
> could be adjusted. But at least this fixes your testcase too,
> and causes no regressions. So maybe we could do this incrementally
> and pass adjust functions to pass around a symbol_name_match_type
> as we find a need? Functions that we miss passing down simply
> end up behaving like symbols_name_match_type::FULL, as today.
I like this approach. I'll try going through ada-lang.c after
this change gets in to see if I can spot some of the "<...">
tricks, and simplify them by using the new symbols_name_match_type
instead.
--
Joel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC] regresssion(internal-error) printing subprogram argument
2017-12-14 19:41 ` Pedro Alves
@ 2017-12-15 9:48 ` Joel Brobecker
2017-12-15 11:02 ` Pedro Alves
2017-12-15 18:31 ` Pedro Alves
0 siblings, 2 replies; 27+ messages in thread
From: Joel Brobecker @ 2017-12-15 9:48 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 2624 bytes --]
> Reading back the patch on the list, I realized that this must be
> fixing "maint check-psymtabs" for Ada. And indeed, without my
> patch, I get here:
>
> $ gdb ./testsuite/outputs/gdb.ada/var_arr_typedef/var_arr_typedef
> (gdb) start
> ...
> (gdb) maint check-psymtabs
> Global symbol `adaS' only found in gdb/testsuite/outputs/gdb.ada/var_arr_typedef/b~var_arr_typedef.adb psymtab
> Global symbol `interfacesS' only found in gdb/testsuite/outputs/gdb.ada/var_arr_typedef/b~var_arr_typedef.adb psymtab
> Global symbol `packB' only found in gdb/testsuite/outputs/gdb.ada/var_arr_typedef/b~var_arr_typedef.adb psymtab
> Global symbol `packS' only found in gdb/testsuite/outputs/gdb.ada/var_arr_typedef/b~var_arr_typedef.adb psymtab
> Global symbol `systemS' only found in gdb/testsuite/outputs/gdb.ada/var_arr_typedef/b~var_arr_typedef.adb psymtab
> Global symbol `var_arr_typedefB' only found in gdb/testsuite/outputs/gdb.ada/var_arr_typedef/b~var_arr_typedef.adb psymtab
> (gdb)
>
> After:
>
> (gdb) start
> ...
> (gdb) maint check-psymtabs
> (gdb
>
> Looks like we only test that command for C, currently...
Good point!
Here is a commit which adds a testcase.
Sadly, unlike you, I still get an error:
(gdb) maintenance check-psymtabs
Global symbol `interfaces__cS' only found in /[...]/maint_with_ada/b~var_arr_typedef.adb psymtab
I am not sure why this is happening just yet; the symbol, at first,
looked like it had an interesting feature, which is both a DW_AT_name
and a DW_AT_linkage name:
<1><ad2>: Abbrev Number: 35 (DW_TAG_variable)
<ad3> DW_AT_name : (indirect string, offset: 0x476): ada_main__u00047
<ad7> DW_AT_decl_file : 5
<ad8> DW_AT_decl_line : 132
<ad9> DW_AT_linkage_name: (indirect string, offset: 0x1b7e): interfaces__cS
<add> DW_AT_type : <0x79>
<ae1> DW_AT_external : 1
<ae1> DW_AT_location : 9 byte block: 3 20 1 0 0 0 0 0 0 (DW_OP_addr: 120)
However, there are plenty of other similar symbols, for instance:
<1><b04>: Abbrev Number: 35 (DW_TAG_variable)
<b05> DW_AT_name : (indirect string, offset: 0x4b9): ada_main__u00049
<b09> DW_AT_decl_file : 5
<b0a> DW_AT_decl_line : 136
<b0b> DW_AT_linkage_name: (indirect string, offset: 0x17cc): system__bounded_stringsS
<b0f> DW_AT_type : <0x79>
<b13> DW_AT_external : 1
<b13> DW_AT_location : 9 byte block: 3 28 1 0 0 0 0 0 0 (DW_OP_addr: 128)
So I'm still not sure what makes interfaces__cS special. I will look
into it when I have a chance...
--
Joel
[-- Attachment #2: 0001-gdb.ada-maint_with_ada.exp-New-testcase.patch --]
[-- Type: text/x-diff, Size: 6181 bytes --]
From e0a28e2429b23fd03723be5ab2833ea4aeece19a Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Fri, 15 Dec 2017 04:38:39 -0500
Subject: [PATCH] gdb.ada/maint_with_ada.exp: New testcase
---
gdb/testsuite/gdb.ada/maint_with_ada.exp | 37 ++++++++++++++++++++++
gdb/testsuite/gdb.ada/maint_with_ada/pack.adb | 25 +++++++++++++++
gdb/testsuite/gdb.ada/maint_with_ada/pack.ads | 29 +++++++++++++++++
.../gdb.ada/maint_with_ada/var_arr_typedef.adb | 28 ++++++++++++++++
4 files changed, 119 insertions(+)
create mode 100644 gdb/testsuite/gdb.ada/maint_with_ada.exp
create mode 100644 gdb/testsuite/gdb.ada/maint_with_ada/pack.adb
create mode 100644 gdb/testsuite/gdb.ada/maint_with_ada/pack.ads
create mode 100644 gdb/testsuite/gdb.ada/maint_with_ada/var_arr_typedef.adb
diff --git a/gdb/testsuite/gdb.ada/maint_with_ada.exp b/gdb/testsuite/gdb.ada/maint_with_ada.exp
new file mode 100644
index 0000000..9ede035
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/maint_with_ada.exp
@@ -0,0 +1,37 @@
+# Copyright 2015-2017 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/>.
+
+load_lib "ada.exp"
+
+standard_ada_testfile var_arr_typedef
+
+if {[gdb_compile_ada "${srcfile}" "${binfile}" executable [list debug ]] != "" } {
+ return -1
+}
+
+clean_restart ${testfile}
+
+# Insert a breakpoint in each compilation unit, to force their psymtab's
+# expansion to a full symtab. This will allow the check-psymtabs command
+# to perform a more extensive check regarding those units which are in
+# Ada.
+
+gdb_breakpoint "adainit"
+gdb_breakpoint "Var_Arr_Typedef"
+gdb_breakpoint "Do_Nothing"
+
+gdb_test_no_output "maintenance check-psymtabs"
+
+gdb_test_no_output "maintenance check-symtabs"
diff --git a/gdb/testsuite/gdb.ada/maint_with_ada/pack.adb b/gdb/testsuite/gdb.ada/maint_with_ada/pack.adb
new file mode 100644
index 0000000..dc6c732
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/maint_with_ada/pack.adb
@@ -0,0 +1,25 @@
+-- Copyright 2015-2017 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/>.
+
+package body Pack is
+
+ function Identity (I : Integer) return Integer is
+ begin
+ return I;
+ end Identity;
+
+ procedure Do_Nothing (A : Array_Type) is null;
+
+end Pack;
diff --git a/gdb/testsuite/gdb.ada/maint_with_ada/pack.ads b/gdb/testsuite/gdb.ada/maint_with_ada/pack.ads
new file mode 100644
index 0000000..efd73a4
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/maint_with_ada/pack.ads
@@ -0,0 +1,29 @@
+-- Copyright 2015-2017 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/>.
+
+package Pack is
+ type Rec_Type is record
+ I : Integer;
+ B : Boolean;
+ end record;
+
+ type Vec_Type is array (1 .. 4) of Rec_Type;
+
+ type Array_Type is array (Positive range <>) of Vec_Type;
+
+ procedure Do_Nothing (A : Array_Type);
+ function Identity (I : Integer) return Integer;
+
+end Pack;
diff --git a/gdb/testsuite/gdb.ada/maint_with_ada/var_arr_typedef.adb b/gdb/testsuite/gdb.ada/maint_with_ada/var_arr_typedef.adb
new file mode 100644
index 0000000..224e78f
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/maint_with_ada/var_arr_typedef.adb
@@ -0,0 +1,28 @@
+-- Copyright 2015-2017 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/>.
+
+with Pack; use Pack;
+
+procedure Var_Arr_Typedef is
+ RA : constant Rec_Type := (3, False);
+ RB : constant Rec_Type := (2, True);
+
+ VA : constant Vec_Type := (RA, RA, RB, RB);
+ VB : constant Vec_Type := (RB, RB, RA, RA);
+
+ A : constant Array_Type (1 .. Identity (4)) := (VA, VA, VB, VB);
+begin
+ Do_Nothing (A); -- BREAK
+end Var_Arr_Typedef;
--
2.1.4
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC] regresssion(internal-error) printing subprogram argument
2017-12-15 9:48 ` Joel Brobecker
@ 2017-12-15 11:02 ` Pedro Alves
2017-12-15 18:57 ` Pedro Alves
2017-12-15 18:31 ` Pedro Alves
1 sibling, 1 reply; 27+ messages in thread
From: Pedro Alves @ 2017-12-15 11:02 UTC (permalink / raw)
To: Joel Brobecker; +Cc: gdb-patches
Hi Joel,
I woke up realizing that I completely forgot that psymbols have no
overload/function parameter info in C++, so a straight strcmp probably
doesn't work properly for C++. Indeed, I remembered now to do
"maint check-psymtabs" when debugging gdb, and I get back a few
problems:
(top-gdb) maint check-psymtabs
Global symbol `__gnu_cxx::operator!=<symtab_and_line*, std::vector<symtab_and_line> >' only found in src/gdb/cli/cli-cmds.c psymtab
Global symbol `__gnu_cxx::operator-<const symtab_and_line*, std::vector<symtab_and_line> >' only found in src/gdb/cli/cli-cmds.c psymtab
[...]
Maybe what we need is to be a little less aggressive then and
add a new symbol_name_match_type::SEARCH_SYMBOL instead that takes as
input a non-user-input search symbol like symbol_name_match_type::LITERAL
was, and then we skip any decoding/demangling steps (like LITERAL) and make:
- Ada treat that as a verbatim match,
- other languages treat it as symbol_name_match_type::FULL.
I can't look at this right now, though I'll try to play with it a bit
more later today. I'm a bit worried on timing since I'm going to be
away next week (today's my last official day before xmas holiday). :-/
No idea offhand on the issue below.
Thanks,
Pedro Alves
On 12/15/2017 09:47 AM, Joel Brobecker wrote:
>> Reading back the patch on the list, I realized that this must be
>> fixing "maint check-psymtabs" for Ada. And indeed, without my
>> patch, I get here:
>>
>> $ gdb ./testsuite/outputs/gdb.ada/var_arr_typedef/var_arr_typedef
>> (gdb) start
>> ...
>> (gdb) maint check-psymtabs
>> Global symbol `adaS' only found in gdb/testsuite/outputs/gdb.ada/var_arr_typedef/b~var_arr_typedef.adb psymtab
>> Global symbol `interfacesS' only found in gdb/testsuite/outputs/gdb.ada/var_arr_typedef/b~var_arr_typedef.adb psymtab
>> Global symbol `packB' only found in gdb/testsuite/outputs/gdb.ada/var_arr_typedef/b~var_arr_typedef.adb psymtab
>> Global symbol `packS' only found in gdb/testsuite/outputs/gdb.ada/var_arr_typedef/b~var_arr_typedef.adb psymtab
>> Global symbol `systemS' only found in gdb/testsuite/outputs/gdb.ada/var_arr_typedef/b~var_arr_typedef.adb psymtab
>> Global symbol `var_arr_typedefB' only found in gdb/testsuite/outputs/gdb.ada/var_arr_typedef/b~var_arr_typedef.adb psymtab
>> (gdb)
>>
>> After:
>>
>> (gdb) start
>> ...
>> (gdb) maint check-psymtabs
>> (gdb
>>
>> Looks like we only test that command for C, currently...
>
> Good point!
>
> Here is a commit which adds a testcase.
>
> Sadly, unlike you, I still get an error:
>
> (gdb) maintenance check-psymtabs
> Global symbol `interfaces__cS' only found in /[...]/maint_with_ada/b~var_arr_typedef.adb psymtab
>
> I am not sure why this is happening just yet; the symbol, at first,
> looked like it had an interesting feature, which is both a DW_AT_name
> and a DW_AT_linkage name:
>
> <1><ad2>: Abbrev Number: 35 (DW_TAG_variable)
> <ad3> DW_AT_name : (indirect string, offset: 0x476): ada_main__u00047
> <ad7> DW_AT_decl_file : 5
> <ad8> DW_AT_decl_line : 132
> <ad9> DW_AT_linkage_name: (indirect string, offset: 0x1b7e): interfaces__cS
> <add> DW_AT_type : <0x79>
> <ae1> DW_AT_external : 1
> <ae1> DW_AT_location : 9 byte block: 3 20 1 0 0 0 0 0 0 (DW_OP_addr: 120)
>
> However, there are plenty of other similar symbols, for instance:
>
> <1><b04>: Abbrev Number: 35 (DW_TAG_variable)
> <b05> DW_AT_name : (indirect string, offset: 0x4b9): ada_main__u00049
> <b09> DW_AT_decl_file : 5
> <b0a> DW_AT_decl_line : 136
> <b0b> DW_AT_linkage_name: (indirect string, offset: 0x17cc): system__bounded_stringsS
> <b0f> DW_AT_type : <0x79>
> <b13> DW_AT_external : 1
> <b13> DW_AT_location : 9 byte block: 3 28 1 0 0 0 0 0 0 (DW_OP_addr: 128)
>
> So I'm still not sure what makes interfaces__cS special. I will look
> into it when I have a chance...
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC] regresssion(internal-error) printing subprogram argument
2017-12-15 9:48 ` Joel Brobecker
2017-12-15 11:02 ` Pedro Alves
@ 2017-12-15 18:31 ` Pedro Alves
1 sibling, 0 replies; 27+ messages in thread
From: Pedro Alves @ 2017-12-15 18:31 UTC (permalink / raw)
To: Joel Brobecker; +Cc: gdb-patches
On 12/15/2017 09:47 AM, Joel Brobecker wrote:
> However, there are plenty of other similar symbols, for instance:
>
> <1><b04>: Abbrev Number: 35 (DW_TAG_variable)
> <b05> DW_AT_name : (indirect string, offset: 0x4b9): ada_main__u00049
> <b09> DW_AT_decl_file : 5
> <b0a> DW_AT_decl_line : 136
> <b0b> DW_AT_linkage_name: (indirect string, offset: 0x17cc): system__bounded_stringsS
> <b0f> DW_AT_type : <0x79>
> <b13> DW_AT_external : 1
> <b13> DW_AT_location : 9 byte block: 3 28 1 0 0 0 0 0 0 (DW_OP_addr: 128)
>
> So I'm still not sure what makes interfaces__cS special. I will look
> into it when I have a chance...
I wonder whether it's because it can demangle as a C++ symbol (using
some older mangling scheme):
$ echo interfaces__cS | c++filt
interfaces(char, signed)
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC] regresssion(internal-error) printing subprogram argument
2017-12-15 11:02 ` Pedro Alves
@ 2017-12-15 18:57 ` Pedro Alves
2018-01-03 4:33 ` Joel Brobecker
0 siblings, 1 reply; 27+ messages in thread
From: Pedro Alves @ 2017-12-15 18:57 UTC (permalink / raw)
To: Joel Brobecker; +Cc: gdb-patches
On 12/15/2017 11:02 AM, Pedro Alves wrote:
> Hi Joel,
>
> I woke up realizing that I completely forgot that psymbols have no
> overload/function parameter info in C++, so a straight strcmp probably
> doesn't work properly for C++. Indeed, I remembered now to do
> "maint check-psymtabs" when debugging gdb, and I get back a few
> problems:
>
> (top-gdb) maint check-psymtabs
> Global symbol `__gnu_cxx::operator!=<symtab_and_line*, std::vector<symtab_and_line> >' only found in src/gdb/cli/cli-cmds.c psymtab
> Global symbol `__gnu_cxx::operator-<const symtab_and_line*, std::vector<symtab_and_line> >' only found in src/gdb/cli/cli-cmds.c psymtab
> [...]
>
> Maybe what we need is to be a little less aggressive then and
> add a new symbol_name_match_type::SEARCH_SYMBOL instead that takes as
> input a non-user-input search symbol like symbol_name_match_type::LITERAL
> was, and then we skip any decoding/demangling steps (like LITERAL) and make:
> - Ada treat that as a verbatim match,
> - other languages treat it as symbol_name_match_type::FULL.
>
> I can't look at this right now, though I'll try to play with it a bit
> more later today. I'm a bit worried on timing since I'm going to be
> away next week (today's my last official day before xmas holiday). :-/
>
Errr, it turns out that I had done the above on the wrong branch...
I get the above output even if I use the system gdb (7.10) to debug
the new gdb, and then do "maint check-psymtabs". So those are not
a regression.
However, (as suspected) with the LITERAL patch things indeed got
worse for C++:
Global symbol `error' only found in src/gdb/common/errors.c psymtab
Global symbol `internal_error' only found in src/gdb/common/errors.c psymtab
Global symbol `internal_warning' only found in src/gdb/common/errors.c psymtab
Global symbol `warning' only found in src/gdb/common/errors.c psymtab
Static symbol `info_command' only found in src/gdb/cli/cli-cmds.c psymtab
Static symbol `show_command' only found in src/gdb/cli/cli-cmds.c psymtab
Static symbol `help_command' only found in src/gdb/cli/cli-cmds.c psymtab
Static symbol `complete_command' only found in src/gdb/cli/cli-cmds.c psymtab
Static symbol `show_version' only found in src/gdb/cli/cli-cmds.c psymtab
Static symbol `show_configuration' only found in src/gdb/cli/cli-cmds.c psymtab
Static symbol `pwd_command' only found in src/gdb/cli/cli-cmds.c psymtab
Static symbol `show_script_ext_mode' only found in src/gdb/cli/cli-cmds.c psymtab
Static symbol `source_script_from_stream' only found in src/gdb/cli/cli-cmds.c psymtab
[...many more...]
With the patch below applied on top, which is a minimal version of
the design change I suggested (LITERAL -> SEARCH_SYMBOL), I now get
again the same "maint check-psymtabs" output that I get with system gdb.
So seems like this is on the good track.
Your internal-error test still passes as well as the new
"maint check-psymtabs" for gdb.ada/ testcase.
I've pushed the current state to users/palves/literal-matching.
I probably won't be able to look at this more today.
From 6b4025034b0b0e8b2511e4c355365c9ff69be822 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Fri, 15 Dec 2017 17:46:56 +0000
Subject: [PATCH] Move literal matching to Ada
---
gdb/ada-lang.c | 8 ++++++++
gdb/cp-support.c | 1 +
gdb/language.c | 9 +++------
gdb/symtab.c | 12 ++++++++----
4 files changed, 20 insertions(+), 10 deletions(-)
diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 9e637eb..5f4e255 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -14065,12 +14065,20 @@ ada_symbol_name_matches (const char *symbol_search_name,
comp_match_res);
}
+bool
+literal_symbol_name_matcher (const char *symbol_search_name,
+ const lookup_name_info &lookup_name,
+ completion_match_result *comp_match_res);
+
/* Implement the "la_get_symbol_name_matcher" language_defn method for
Ada. */
static symbol_name_matcher_ftype *
ada_get_symbol_name_matcher (const lookup_name_info &lookup_name)
{
+ if (lookup_name.match_type () == symbol_name_match_type::LITERAL)
+ return literal_symbol_name_matcher;
+
if (lookup_name.completion_mode ())
return ada_symbol_name_matches;
else
diff --git a/gdb/cp-support.c b/gdb/cp-support.c
index 34a8439..7c14423 100644
--- a/gdb/cp-support.c
+++ b/gdb/cp-support.c
@@ -1793,6 +1793,7 @@ cp_get_symbol_name_matcher (const lookup_name_info &lookup_name)
{
case symbol_name_match_type::FULL:
case symbol_name_match_type::EXPRESSION:
+ case symbol_name_match_type::LITERAL:
return cp_fq_symbol_name_matches;
case symbol_name_match_type::WILD:
return cp_symbol_name_matches;
diff --git a/gdb/language.c b/gdb/language.c
index 5bcdfb9..44afdef 100644
--- a/gdb/language.c
+++ b/gdb/language.c
@@ -727,7 +727,7 @@ default_symbol_name_matcher (const char *symbol_search_name,
/* A name matcher that matches the symbol name exactly, with
strcmp. */
-static bool
+bool
literal_symbol_name_matcher (const char *symbol_search_name,
const lookup_name_info &lookup_name,
completion_match_result *comp_match_res)
@@ -753,13 +753,10 @@ symbol_name_matcher_ftype *
language_get_symbol_name_matcher (const language_defn *lang,
const lookup_name_info &lookup_name)
{
- if (lookup_name.match_type () == symbol_name_match_type::LITERAL)
- return literal_symbol_name_matcher;
-
if (lang->la_get_symbol_name_matcher != nullptr)
return lang->la_get_symbol_name_matcher (lookup_name);
-
- return default_symbol_name_matcher;
+ else
+ return default_symbol_name_matcher;
}
/* Define the language that is no language. */
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 78018a1..6957757 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -1775,14 +1775,18 @@ demangle_for_lookup_info::demangle_for_lookup_info
if (without_params != NULL)
{
- m_demangled_name = demangle_for_lookup (without_params.get (),
- lang, storage);
+ if (lookup_name.match_type () != symbol_name_match_type::LITERAL)
+ m_demangled_name = demangle_for_lookup (without_params.get (),
+ lang, storage);
return;
}
}
- m_demangled_name = demangle_for_lookup (lookup_name.name ().c_str (),
- lang, storage);
+ if (lookup_name.match_type () == symbol_name_match_type::LITERAL)
+ m_demangled_name = lookup_name.name ();
+ else
+ m_demangled_name = demangle_for_lookup (lookup_name.name ().c_str (),
+ lang, storage);
}
/* See symtab.h. */
--
2.5.5
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC] regresssion(internal-error) printing subprogram argument
2017-12-15 18:57 ` Pedro Alves
@ 2018-01-03 4:33 ` Joel Brobecker
2018-01-04 9:48 ` Joel Brobecker
2018-01-05 16:28 ` Pedro Alves
0 siblings, 2 replies; 27+ messages in thread
From: Joel Brobecker @ 2018-01-03 4:33 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
Hi Pedro,
> > Maybe what we need is to be a little less aggressive then and
> > add a new symbol_name_match_type::SEARCH_SYMBOL instead that takes as
> > input a non-user-input search symbol like symbol_name_match_type::LITERAL
> > was, and then we skip any decoding/demangling steps (like LITERAL) and make:
> > - Ada treat that as a verbatim match,
> > - other languages treat it as symbol_name_match_type::FULL.
[...]
> With the patch below applied on top, which is a minimal version of
> the design change I suggested (LITERAL -> SEARCH_SYMBOL), I now get
> again the same "maint check-psymtabs" output that I get with system gdb.
> So seems like this is on the good track.
>
> Your internal-error test still passes as well as the new
> "maint check-psymtabs" for gdb.ada/ testcase.
Sounds like we are indeed on the right track!
I ran the changes through both the official testsuite and AdaCore's
testsuite, and I can confirm no regression :). Is there something
else I can do to help?
--
Joel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC] regresssion(internal-error) printing subprogram argument
2018-01-03 4:33 ` Joel Brobecker
@ 2018-01-04 9:48 ` Joel Brobecker
2018-01-04 12:39 ` Pedro Alves
2018-01-05 16:28 ` Pedro Alves
1 sibling, 1 reply; 27+ messages in thread
From: Joel Brobecker @ 2018-01-04 9:48 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
Hi Pedro,
FYI, I just pushed 3 commits that add more testing in gdb.ada
demonstrating issues introduced by the wild-matching patch series
I hadn't noticed before. See:
https://www.sourceware.org/ml/gdb-patches/2018-01/msg00052.html
As hinted by the ChangeLog entries, I have also created PR gdb/22670
so as to be able to KFAIL the expected failures. And now that we have
this PR, we can use it to track the fixes as well ;-).
I also pushed the patch which adds "maint check psymtabs" testing;
also with a setup_kfail.
Here is my plan: I will continue going through the results I get
when running AdaCore's GDB testsuite (almost there). If there are
other tests related to this patch series, I will try to contribute
those as well. Once I'm doing analyzing the current results, I will
start investigating those failures one by one, and let you know
what I find. For now, all I know is that litteral matching is not
enough to fix those, so I am not sure what is happening just yet.
I will let you know!
--
Joel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC] regresssion(internal-error) printing subprogram argument
2018-01-04 9:48 ` Joel Brobecker
@ 2018-01-04 12:39 ` Pedro Alves
0 siblings, 0 replies; 27+ messages in thread
From: Pedro Alves @ 2018-01-04 12:39 UTC (permalink / raw)
To: Joel Brobecker; +Cc: gdb-patches
Hi Joel,
On 01/04/2018 09:48 AM, Joel Brobecker wrote:
> Hi Pedro,
>
> FYI, I just pushed 3 commits that add more testing in gdb.ada
> demonstrating issues introduced by the wild-matching patch series
> I hadn't noticed before. See:
>
> https://www.sourceware.org/ml/gdb-patches/2018-01/msg00052.html
>
> As hinted by the ChangeLog entries, I have also created PR gdb/22670
> so as to be able to KFAIL the expected failures. And now that we have
> this PR, we can use it to track the fixes as well ;-).
>
> I also pushed the patch which adds "maint check psymtabs" testing;
> also with a setup_kfail.
>
> Here is my plan: I will continue going through the results I get
> when running AdaCore's GDB testsuite (almost there). If there are
> other tests related to this patch series, I will try to contribute
> those as well. Once I'm doing analyzing the current results, I will
> start investigating those failures one by one, and let you know
> what I find. For now, all I know is that litteral matching is not
> enough to fix those, so I am not sure what is happening just yet.
> I will let you know!
Thanks! I'm catching up to this and will take a look at the
new tests.
Pedro Alves
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC] regresssion(internal-error) printing subprogram argument
2018-01-03 4:33 ` Joel Brobecker
2018-01-04 9:48 ` Joel Brobecker
@ 2018-01-05 16:28 ` Pedro Alves
2018-01-16 11:54 ` Pedro Alves
1 sibling, 1 reply; 27+ messages in thread
From: Pedro Alves @ 2018-01-05 16:28 UTC (permalink / raw)
To: Joel Brobecker; +Cc: gdb-patches
On 01/03/2018 04:33 AM, Joel Brobecker wrote:
> Hi Pedro,
>
>>> Maybe what we need is to be a little less aggressive then and
>>> add a new symbol_name_match_type::SEARCH_SYMBOL instead that takes as
>>> input a non-user-input search symbol like symbol_name_match_type::LITERAL
>>> was, and then we skip any decoding/demangling steps (like LITERAL) and make:
>>> - Ada treat that as a verbatim match,
>>> - other languages treat it as symbol_name_match_type::FULL.
> [...]
>> With the patch below applied on top, which is a minimal version of
>> the design change I suggested (LITERAL -> SEARCH_SYMBOL), I now get
>> again the same "maint check-psymtabs" output that I get with system gdb.
>> So seems like this is on the good track.
>>
>> Your internal-error test still passes as well as the new
>> "maint check-psymtabs" for gdb.ada/ testcase.
>
> Sounds like we are indeed on the right track!
>
> I ran the changes through both the official testsuite and AdaCore's
> testsuite, and I can confirm no regression :). Is there something
> else I can do to help?
I just needed to fold both patches in, do the renaming thing,
update comments and compose git log + ChangeLog. After looking at
the other regressions and convincing myself that this is good
enough, that is. :-)
Here's the resulting patch, which I merged to both master and
branch now.
I remembered to update the copyright year in the new testcase,
and to include a short description of what the testcase is about
in the .exp file.
From de63c46b549d1cf4f7851e47872cb759a12983f4 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Fri, 5 Jan 2018 14:04:09 +0000
Subject: [PATCH] Fix regresssion(internal-error) printing subprogram argument
(PR gdb/22670)
At <https://sourceware.org/ml/gdb-patches/2017-12/msg00298.html>, Joel
wrote:
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Consider the following code which first declares a tagged type (the
equivalent of a class in Ada), and then a procedure which takes a
pointer (access) to this type's 'Class.
package Pck is
type Top_T is tagged record
N : Integer := 1;
end record;
procedure Inspect (Obj: access Top_T'Class);
end Pck;
Putting a breakpoint in that procedure and then running to it triggers
an internal error:
(gdb) break inspect
(gdb) continue
Breakpoint 1, pck.inspect (obj=0x63e010
/[...]/gdb/stack.c:621: internal-error: void print_frame_args(symbol*, frame_info*, int, ui_file*): Assertion `nsym != NULL' failed.
What's special about this subprogram is that it takes an access to
what we call a 'Class type, and for implementation reasons, the
compiler adds an extra argument named "objL". If you are curious why,
it allows the compiler for perform dynamic accessibility checks that
are mandated by the language.
If we look at the location where we get the internal error (in
stack.c), we find that we are looping over the symbol of each
parameter, and for each parameter, we do:
/* We have to look up the symbol because arguments can have
two entries (one a parameter, one a local) and the one we
want is the local, which lookup_symbol will find for us.
[...]
nsym = lookup_symbol (SYMBOL_LINKAGE_NAME (sym),
b, VAR_DOMAIN, NULL).symbol;
gdb_assert (nsym != NULL);
The lookup_symbol goes through the lookup structure, which means the
symbol's linkage name ("objL") gets transformed into a
lookup_name_info object (in block_lookup_symbol), before it gets fed
to the block symbol dictionary iterators. This, in turn, triggers the
symbol matching by comparing the "lookup" name which, for Ada, means
among other things, lowercasing the given name to "objl". It is this
transformation that causes the lookup find no matches, and therefore
trip this assertion.
Going back to the "offending" call to lookup_symbol in stack.c, what
we are trying to do, here, is do a lookup by linkage name. So, I
think what we mean to be doing is a completely literal symbol lookup,
so maybe not even strcmp_iw, but actually just plain strcmp???
In the past, in practice, you could get that effect by doing a lookup
using the C language. But that doesn't work, because we still end up
somehow using Ada's lookup_name routine which transforms "objL".
So, ideally, as I hinted before, I think what we need is a way to
perform a literal lookup so that searches by linkage names like the
above can be performed.
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
This commit fixes the problem by implementing something similar to
Joel's literal idea, but with some important differences.
I considered adding a symbol_name_match_type::LINKAGE and supporting
searching by linkage name for any language, but the problem with that
is that the dictionaries only work with SYMBOL_SEARCH_NAME, because
that's what is used for hashing. We'd need separate dictionaries for
hashed linkage names.
So with the current symbol tables infrastructure, it's not literal
linkage names that we want to pass down, but instead literal _search_
names (SYMBOL_SEARCH_NAME, etc.).
However, psymbols have no overload/function parameter info in C++, so
a straight strcmp doesn't work properly for C++ name matching.
So what we do is be a little less aggressive then and add a new
symbol_name_match_type::SEARCH_SYMBOL instead that takes as input a
non-user-input search symbol, and then we skip any decoding/demangling
steps and make:
- Ada treat that as a verbatim match,
- other languages treat it as symbol_name_match_type::FULL.
This also fixes the new '"maint check-psymtabs" for Ada' testcase for
me (gdb.ada/maint_with_ada.exp). I've not removed the kfail yet
because Joel still sees that testcase failing with this patch.
That'll be fixed in follow up patches.
gdb/ChangeLog:
2018-01-05 Pedro Alves <palves@redhat.com>
PR gdb/22670
* ada-lang.c (literal_symbol_name_matcher): New function.
(ada_get_symbol_name_matcher): Use it for
symbol_name_match_type::SEARCH_NAME.
* block.c (block_lookup_symbol): New parameter 'match_type'. Pass
it down instead of assuming symbol_name_match_type::FULL.
* block.h (block_lookup_symbol): New parameter 'match_type'.
* c-valprint.c (print_unpacked_pointer): Use
lookup_symbol_search_name instead of lookup_symbol.
* compile/compile-object-load.c (get_out_value_type): Pass down
symbol_name_match_type::SEARCH_NAME.
* cp-namespace.c (cp_basic_lookup_symbol): Pass down
symbol_name_match_type::FULL.
* cp-support.c (cp_get_symbol_name_matcher): Handle
symbol_name_match_type::SEARCH_NAME.
* infrun.c (insert_exception_resume_breakpoint): Use
lookup_symbol_search_name.
* p-valprint.c (pascal_val_print): Use lookup_symbol_search_name.
* psymtab.c (maintenance_check_psymtabs): Use
symbol_name_match_type::SEARCH_NAME and SYMBOL_SEARCH_NAME.
* stack.c (print_frame_args): Use lookup_symbol_search_name and
SYMBOL_SEARCH_NAME.
* symtab.c (lookup_local_symbol): Don't demangle the lookup name
if symbol_name_match_type::SEARCH_NAME.
(lookup_symbol_in_language): Pass down
symbol_name_match_type::FULL.
(lookup_symbol_search_name): New.
(lookup_language_this): Pass down
symbol_name_match_type::SEARCH_NAME.
(lookup_symbol_aux, lookup_local_symbol): New parameter
'match_type'. Pass it down.
* symtab.h (symbol_name_match_type::SEARCH_NAME): New enumerator.
(lookup_symbol_search_name): New declaration.
(lookup_symbol_in_block): New 'match_type' parameter.
gdb/testsuite/ChangeLog:
2018-01-05 Joel Brobecker <brobecker@adacore.com>
PR gdb/22670
* gdb.ada/access_tagged_param.exp: New file.
* gdb.ada/access_tagged_param/foo.adb: New file.
---
gdb/ChangeLog | 37 +++++++++++++++
gdb/testsuite/ChangeLog | 6 +++
gdb/ada-lang.c | 26 +++++++++++
gdb/block.c | 3 +-
gdb/block.h | 1 +
gdb/c-valprint.c | 9 ++--
gdb/compile/compile-object-load.c | 6 ++-
gdb/cp-namespace.c | 4 +-
gdb/cp-support.c | 1 +
gdb/infrun.c | 3 +-
gdb/p-valprint.c | 10 +++--
gdb/psymtab.c | 6 ++-
gdb/stack.c | 8 ++--
gdb/symtab.c | 55 +++++++++++++++++------
gdb/symtab.h | 25 +++++++++++
gdb/testsuite/gdb.ada/access_tagged_param.exp | 40 +++++++++++++++++
gdb/testsuite/gdb.ada/access_tagged_param/foo.adb | 20 +++++++++
gdb/testsuite/gdb.ada/access_tagged_param/pck.adb | 21 +++++++++
gdb/testsuite/gdb.ada/access_tagged_param/pck.ads | 21 +++++++++
19 files changed, 271 insertions(+), 31 deletions(-)
create mode 100644 gdb/testsuite/gdb.ada/access_tagged_param.exp
create mode 100644 gdb/testsuite/gdb.ada/access_tagged_param/foo.adb
create mode 100644 gdb/testsuite/gdb.ada/access_tagged_param/pck.adb
create mode 100644 gdb/testsuite/gdb.ada/access_tagged_param/pck.ads
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index badced8f061..10aabcde3bb 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,40 @@
+2018-01-05 Pedro Alves <palves@redhat.com>
+
+ PR gdb/22670
+ * ada-lang.c (literal_symbol_name_matcher): New function.
+ (ada_get_symbol_name_matcher): Use it for
+ symbol_name_match_type::SEARCH_NAME.
+ * block.c (block_lookup_symbol): New parameter 'match_type'. Pass
+ it down instead of assuming symbol_name_match_type::FULL.
+ * block.h (block_lookup_symbol): New parameter 'match_type'.
+ * c-valprint.c (print_unpacked_pointer): Use
+ lookup_symbol_search_name instead of lookup_symbol.
+ * compile/compile-object-load.c (get_out_value_type): Pass down
+ symbol_name_match_type::SEARCH_NAME.
+ * cp-namespace.c (cp_basic_lookup_symbol): Pass down
+ symbol_name_match_type::FULL.
+ * cp-support.c (cp_get_symbol_name_matcher): Handle
+ symbol_name_match_type::SEARCH_NAME.
+ * infrun.c (insert_exception_resume_breakpoint): Use
+ lookup_symbol_search_name.
+ * p-valprint.c (pascal_val_print): Use lookup_symbol_search_name.
+ * psymtab.c (maintenance_check_psymtabs): Use
+ symbol_name_match_type::SEARCH_NAME and SYMBOL_SEARCH_NAME.
+ * stack.c (print_frame_args): Use lookup_symbol_search_name and
+ SYMBOL_SEARCH_NAME.
+ * symtab.c (lookup_local_symbol): Don't demangle the lookup name
+ if symbol_name_match_type::SEARCH_NAME.
+ (lookup_symbol_in_language): Pass down
+ symbol_name_match_type::FULL.
+ (lookup_symbol_search_name): New.
+ (lookup_language_this): Pass down
+ symbol_name_match_type::SEARCH_NAME.
+ (lookup_symbol_aux, lookup_local_symbol): New parameter
+ 'match_type'. Pass it down.
+ * symtab.h (symbol_name_match_type::SEARCH_NAME): New enumerator.
+ (lookup_symbol_search_name): New declaration.
+ (lookup_symbol_in_block): New 'match_type' parameter.
+
2018-01-05 Pedro Alves <palves@redhat.com>
PR gdb/22670
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 94486316d24..18ad7403b49 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2018-01-05 Joel Brobecker <brobecker@adacore.com>
+
+ PR gdb/22670
+ * gdb.ada/access_tagged_param.exp: New file.
+ * gdb.ada/access_tagged_param/foo.adb: New file.
+
2018-01-05 Pedro Alves <palves@redhat.com>
PR gdb/22670
diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index e7c2197ca7d..622cfd0a81e 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -14412,12 +14412,38 @@ ada_symbol_name_matches (const char *symbol_search_name,
comp_match_res);
}
+/* A name matcher that matches the symbol name exactly, with
+ strcmp. */
+
+static bool
+literal_symbol_name_matcher (const char *symbol_search_name,
+ const lookup_name_info &lookup_name,
+ completion_match_result *comp_match_res)
+{
+ const std::string &name = lookup_name.name ();
+
+ int cmp = (lookup_name.completion_mode ()
+ ? strncmp (symbol_search_name, name.c_str (), name.size ())
+ : strcmp (symbol_search_name, name.c_str ()));
+ if (cmp == 0)
+ {
+ if (comp_match_res != NULL)
+ comp_match_res->set_match (symbol_search_name);
+ return true;
+ }
+ else
+ return false;
+}
+
/* Implement the "la_get_symbol_name_matcher" language_defn method for
Ada. */
static symbol_name_matcher_ftype *
ada_get_symbol_name_matcher (const lookup_name_info &lookup_name)
{
+ if (lookup_name.match_type () == symbol_name_match_type::SEARCH_NAME)
+ return literal_symbol_name_matcher;
+
if (lookup_name.completion_mode ())
return ada_symbol_name_matches;
else
diff --git a/gdb/block.c b/gdb/block.c
index fb91eb9fee0..57da7ba7393 100644
--- a/gdb/block.c
+++ b/gdb/block.c
@@ -673,12 +673,13 @@ block_iter_match_next (const lookup_name_info &name,
struct symbol *
block_lookup_symbol (const struct block *block, const char *name,
+ symbol_name_match_type match_type,
const domain_enum domain)
{
struct block_iterator iter;
struct symbol *sym;
- lookup_name_info lookup_name (name, symbol_name_match_type::FULL);
+ lookup_name_info lookup_name (name, match_type);
if (!BLOCK_FUNCTION (block))
{
diff --git a/gdb/block.h b/gdb/block.h
index 8f400cf8c0f..ff127256ad7 100644
--- a/gdb/block.h
+++ b/gdb/block.h
@@ -258,6 +258,7 @@ extern struct symbol *block_iter_match_next
extern struct symbol *block_lookup_symbol (const struct block *block,
const char *name,
+ symbol_name_match_type match_type,
const domain_enum domain);
/* Search BLOCK for symbol NAME in DOMAIN but only in primary symbol table of
diff --git a/gdb/c-valprint.c b/gdb/c-valprint.c
index 7f8154e6601..c4c0918e26a 100644
--- a/gdb/c-valprint.c
+++ b/gdb/c-valprint.c
@@ -198,14 +198,17 @@ print_unpacked_pointer (struct type *type, struct type *elttype,
struct symbol *wsym = NULL;
struct type *wtype;
struct block *block = NULL;
- struct field_of_this_result is_this_fld;
if (want_space)
fputs_filtered (" ", stream);
if (msymbol.minsym != NULL)
- wsym = lookup_symbol (MSYMBOL_LINKAGE_NAME(msymbol.minsym), block,
- VAR_DOMAIN, &is_this_fld).symbol;
+ {
+ const char *search_name
+ = MSYMBOL_SEARCH_NAME (msymbol.minsym);
+ wsym = lookup_symbol_search_name (search_name, block,
+ VAR_DOMAIN).symbol;
+ }
if (wsym)
{
diff --git a/gdb/compile/compile-object-load.c b/gdb/compile/compile-object-load.c
index 0b7215b3236..fac16d70dde 100644
--- a/gdb/compile/compile-object-load.c
+++ b/gdb/compile/compile-object-load.c
@@ -439,7 +439,10 @@ get_out_value_type (struct symbol *func_sym, struct objfile *objfile,
block = BLOCKVECTOR_BLOCK (bv, block_loop);
if (BLOCK_FUNCTION (block) != NULL)
continue;
- gdb_val_sym = block_lookup_symbol (block, COMPILE_I_EXPR_VAL, VAR_DOMAIN);
+ gdb_val_sym = block_lookup_symbol (block,
+ COMPILE_I_EXPR_VAL,
+ symbol_name_match_type::SEARCH_NAME,
+ VAR_DOMAIN);
if (gdb_val_sym == NULL)
continue;
@@ -466,6 +469,7 @@ get_out_value_type (struct symbol *func_sym, struct objfile *objfile,
gdb_type = check_typedef (gdb_type);
gdb_ptr_type_sym = block_lookup_symbol (block, COMPILE_I_EXPR_PTR_TYPE,
+ symbol_name_match_type::SEARCH_NAME,
VAR_DOMAIN);
if (gdb_ptr_type_sym == NULL)
error (_("No \"%s\" symbol found"), COMPILE_I_EXPR_PTR_TYPE);
diff --git a/gdb/cp-namespace.c b/gdb/cp-namespace.c
index 0751fda7e63..185607e4eb8 100644
--- a/gdb/cp-namespace.c
+++ b/gdb/cp-namespace.c
@@ -141,7 +141,9 @@ cp_basic_lookup_symbol (const char *name, const struct block *block,
if (global_block != NULL)
{
- sym.symbol = lookup_symbol_in_block (name, global_block, domain);
+ sym.symbol = lookup_symbol_in_block (name,
+ symbol_name_match_type::FULL,
+ global_block, domain);
sym.block = global_block;
}
}
diff --git a/gdb/cp-support.c b/gdb/cp-support.c
index 2efc38557ac..dde417be80e 100644
--- a/gdb/cp-support.c
+++ b/gdb/cp-support.c
@@ -1793,6 +1793,7 @@ cp_get_symbol_name_matcher (const lookup_name_info &lookup_name)
{
case symbol_name_match_type::FULL:
case symbol_name_match_type::EXPRESSION:
+ case symbol_name_match_type::SEARCH_NAME:
return cp_fq_symbol_name_matches;
case symbol_name_match_type::WILD:
return cp_symbol_name_matches;
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 3f7d185240b..7e8d8da5884 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -7489,7 +7489,8 @@ insert_exception_resume_breakpoint (struct thread_info *tp,
CORE_ADDR handler;
struct breakpoint *bp;
- vsym = lookup_symbol (SYMBOL_LINKAGE_NAME (sym), b, VAR_DOMAIN, NULL);
+ vsym = lookup_symbol_search_name (SYMBOL_SEARCH_NAME (sym),
+ b, VAR_DOMAIN);
value = read_var_value (vsym.symbol, vsym.block, frame);
/* If the value was optimized out, revert to the old behavior. */
if (! value_optimized_out (value))
diff --git a/gdb/p-valprint.c b/gdb/p-valprint.c
index 6d5358630e1..933dbfb6c4d 100644
--- a/gdb/p-valprint.c
+++ b/gdb/p-valprint.c
@@ -246,15 +246,17 @@ pascal_val_print (struct type *type,
struct symbol *wsym = NULL;
struct type *wtype;
struct block *block = NULL;
- struct field_of_this_result is_this_fld;
if (want_space)
fputs_filtered (" ", stream);
if (msymbol.minsym != NULL)
- wsym = lookup_symbol (MSYMBOL_LINKAGE_NAME (msymbol.minsym),
- block,
- VAR_DOMAIN, &is_this_fld).symbol;
+ {
+ const char *search_name
+ = MSYMBOL_SEARCH_NAME (msymbol.minsym);
+ wsym = lookup_symbol_search_name (search_name, block,
+ VAR_DOMAIN).symbol;
+ }
if (wsym)
{
diff --git a/gdb/psymtab.c b/gdb/psymtab.c
index 3075be29b77..88d234a7da2 100644
--- a/gdb/psymtab.c
+++ b/gdb/psymtab.c
@@ -2254,7 +2254,8 @@ maintenance_check_psymtabs (const char *ignore, int from_tty)
length = ps->n_static_syms;
while (length--)
{
- sym = block_lookup_symbol (b, SYMBOL_LINKAGE_NAME (*psym),
+ sym = block_lookup_symbol (b, SYMBOL_SEARCH_NAME (*psym),
+ symbol_name_match_type::SEARCH_NAME,
SYMBOL_DOMAIN (*psym));
if (!sym)
{
@@ -2271,7 +2272,8 @@ maintenance_check_psymtabs (const char *ignore, int from_tty)
length = ps->n_global_syms;
while (length--)
{
- sym = block_lookup_symbol (b, SYMBOL_LINKAGE_NAME (*psym),
+ sym = block_lookup_symbol (b, SYMBOL_SEARCH_NAME (*psym),
+ symbol_name_match_type::SEARCH_NAME,
SYMBOL_DOMAIN (*psym));
if (!sym)
{
diff --git a/gdb/stack.c b/gdb/stack.c
index 263e7bb0fc6..9993ae654ad 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -616,8 +616,8 @@ print_frame_args (struct symbol *func, struct frame_info *frame,
{
struct symbol *nsym;
- nsym = lookup_symbol (SYMBOL_LINKAGE_NAME (sym),
- b, VAR_DOMAIN, NULL).symbol;
+ nsym = lookup_symbol_search_name (SYMBOL_SEARCH_NAME (sym),
+ b, VAR_DOMAIN).symbol;
gdb_assert (nsym != NULL);
if (SYMBOL_CLASS (nsym) == LOC_REGISTER
&& !SYMBOL_IS_ARGUMENT (nsym))
@@ -2141,8 +2141,8 @@ iterate_over_block_arg_vars (const struct block *b,
float). There are also LOC_ARG/LOC_REGISTER pairs which
are not combined in symbol-reading. */
- sym2 = lookup_symbol (SYMBOL_LINKAGE_NAME (sym),
- b, VAR_DOMAIN, NULL).symbol;
+ sym2 = lookup_symbol_search_name (SYMBOL_SEARCH_NAME (sym),
+ b, VAR_DOMAIN).symbol;
(*cb) (SYMBOL_PRINT_NAME (sym), sym2, cb_data);
}
}
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 3865420cd1e..146dc2e4213 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -75,6 +75,7 @@ static int find_line_common (struct linetable *, int, int *, int);
static struct block_symbol
lookup_symbol_aux (const char *name,
+ symbol_name_match_type match_type,
const struct block *block,
const domain_enum domain,
enum language language,
@@ -82,6 +83,7 @@ static struct block_symbol
static
struct block_symbol lookup_local_symbol (const char *name,
+ symbol_name_match_type match_type,
const struct block *block,
const domain_enum domain,
enum language language);
@@ -1773,14 +1775,18 @@ demangle_for_lookup_info::demangle_for_lookup_info
if (without_params != NULL)
{
- m_demangled_name = demangle_for_lookup (without_params.get (),
- lang, storage);
+ if (lookup_name.match_type () != symbol_name_match_type::SEARCH_NAME)
+ m_demangled_name = demangle_for_lookup (without_params.get (),
+ lang, storage);
return;
}
}
- m_demangled_name = demangle_for_lookup (lookup_name.name ().c_str (),
- lang, storage);
+ if (lookup_name.match_type () == symbol_name_match_type::SEARCH_NAME)
+ m_demangled_name = lookup_name.name ();
+ else
+ m_demangled_name = demangle_for_lookup (lookup_name.name ().c_str (),
+ lang, storage);
}
/* See symtab.h. */
@@ -1876,7 +1882,9 @@ lookup_symbol_in_language (const char *name, const struct block *block,
demangle_result_storage storage;
const char *modified_name = demangle_for_lookup (name, lang, storage);
- return lookup_symbol_aux (modified_name, block, domain, lang,
+ return lookup_symbol_aux (modified_name,
+ symbol_name_match_type::FULL,
+ block, domain, lang,
is_a_field_of_this);
}
@@ -1894,6 +1902,16 @@ lookup_symbol (const char *name, const struct block *block,
/* See symtab.h. */
+struct block_symbol
+lookup_symbol_search_name (const char *search_name, const struct block *block,
+ domain_enum domain)
+{
+ return lookup_symbol_aux (search_name, symbol_name_match_type::SEARCH_NAME,
+ block, domain, language_asm, NULL);
+}
+
+/* See symtab.h. */
+
struct block_symbol
lookup_language_this (const struct language_defn *lang,
const struct block *block)
@@ -1915,7 +1933,9 @@ lookup_language_this (const struct language_defn *lang,
{
struct symbol *sym;
- sym = block_lookup_symbol (block, lang->la_name_of_this, VAR_DOMAIN);
+ sym = block_lookup_symbol (block, lang->la_name_of_this,
+ symbol_name_match_type::SEARCH_NAME,
+ VAR_DOMAIN);
if (sym != NULL)
{
if (symbol_lookup_debug > 1)
@@ -1986,7 +2006,8 @@ check_field (struct type *type, const char *name,
(e.g., demangled name) of the symbol that we're looking for. */
static struct block_symbol
-lookup_symbol_aux (const char *name, const struct block *block,
+lookup_symbol_aux (const char *name, symbol_name_match_type match_type,
+ const struct block *block,
const domain_enum domain, enum language language,
struct field_of_this_result *is_a_field_of_this)
{
@@ -2015,7 +2036,7 @@ lookup_symbol_aux (const char *name, const struct block *block,
/* Search specified block and its superiors. Don't search
STATIC_BLOCK or GLOBAL_BLOCK. */
- result = lookup_local_symbol (name, block, domain, language);
+ result = lookup_local_symbol (name, match_type, block, domain, language);
if (result.symbol != NULL)
{
if (symbol_lookup_debug)
@@ -2097,7 +2118,9 @@ lookup_symbol_aux (const char *name, const struct block *block,
Don't search STATIC_BLOCK or GLOBAL_BLOCK. */
static struct block_symbol
-lookup_local_symbol (const char *name, const struct block *block,
+lookup_local_symbol (const char *name,
+ symbol_name_match_type match_type,
+ const struct block *block,
const domain_enum domain,
enum language language)
{
@@ -2112,7 +2135,7 @@ lookup_local_symbol (const char *name, const struct block *block,
while (block != static_block)
{
- sym = lookup_symbol_in_block (name, block, domain);
+ sym = lookup_symbol_in_block (name, match_type, block, domain);
if (sym != NULL)
return (struct block_symbol) {sym, block};
@@ -2165,7 +2188,8 @@ lookup_objfile_from_block (const struct block *block)
/* See symtab.h. */
struct symbol *
-lookup_symbol_in_block (const char *name, const struct block *block,
+lookup_symbol_in_block (const char *name, symbol_name_match_type match_type,
+ const struct block *block,
const domain_enum domain)
{
struct symbol *sym;
@@ -2181,7 +2205,7 @@ lookup_symbol_in_block (const char *name, const struct block *block,
domain_name (domain));
}
- sym = block_lookup_symbol (block, name, domain);
+ sym = block_lookup_symbol (block, name, match_type, domain);
if (sym)
{
if (symbol_lookup_debug > 1)
@@ -2370,7 +2394,8 @@ lookup_symbol_via_quick_fns (struct objfile *objfile, int block_index,
bv = COMPUNIT_BLOCKVECTOR (cust);
block = BLOCKVECTOR_BLOCK (bv, block_index);
- result.symbol = block_lookup_symbol (block, name, domain);
+ result.symbol = block_lookup_symbol (block, name,
+ symbol_name_match_type::FULL, domain);
if (result.symbol == NULL)
error_in_psymtab_expansion (block_index, name, cust);
@@ -2483,7 +2508,9 @@ lookup_symbol_in_static_block (const char *name,
domain_name (domain));
}
- sym = lookup_symbol_in_block (name, static_block, domain);
+ sym = lookup_symbol_in_block (name,
+ symbol_name_match_type::FULL,
+ static_block, domain);
if (symbol_lookup_debug)
{
fprintf_unfiltered (gdb_stdlog,
diff --git a/gdb/symtab.h b/gdb/symtab.h
index b49d56ba2a8..a7b1ed2131c 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -60,6 +60,16 @@ enum class symbol_name_match_type
namespace/module/package. */
FULL,
+ /* Search name matching. This is like FULL, but the search name did
+ not come from the user; instead it is already a search name
+ retrieved from a SYMBOL_SEARCH_NAME/MSYMBOL_SEARCH_NAME call.
+ For Ada, this avoids re-encoding an already-encoded search name
+ (which would potentially incorrectly lowercase letters in the
+ linkage/search name that should remain uppercase). For C++, it
+ avoids trying to demangle a name we already know is
+ demangled. */
+ SEARCH_NAME,
+
/* Expression matching. The same as FULL matching in most
languages. The same as WILD matching in Ada. */
EXPRESSION,
@@ -1554,6 +1564,20 @@ extern struct block_symbol lookup_symbol (const char *,
const domain_enum,
struct field_of_this_result *);
+/* Find the definition for a specified symbol search name in domain
+ DOMAIN, visible from lexical block BLOCK if non-NULL or from
+ global/static blocks if BLOCK is NULL. The passed-in search name
+ should not come from the user; instead it should already be a
+ search name as retrieved from a
+ SYMBOL_SEARCH_NAME/MSYMBOL_SEARCH_NAME call. See definition of
+ symbol_name_match_type::SEARCH_NAME. Returns the struct symbol
+ pointer, or NULL if no symbol is found. The symbol's section is
+ fixed up if necessary. */
+
+extern struct block_symbol lookup_symbol_search_name (const char *search_name,
+ const struct block *block,
+ domain_enum domain);
+
/* A default version of lookup_symbol_nonlocal for use by languages
that can't think of anything better to do.
This implements the C lookup rules. */
@@ -1603,6 +1627,7 @@ extern struct block_symbol
extern struct symbol *
lookup_symbol_in_block (const char *name,
+ symbol_name_match_type match_type,
const struct block *block,
const domain_enum domain);
diff --git a/gdb/testsuite/gdb.ada/access_tagged_param.exp b/gdb/testsuite/gdb.ada/access_tagged_param.exp
new file mode 100644
index 00000000000..598cf89b4ec
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/access_tagged_param.exp
@@ -0,0 +1,40 @@
+# Copyright 2017-2018 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/>.
+
+# Check that we can print values of parameters of type 'pointer
+# (access) to tagged type'. See PR gdb/22670.
+
+load_lib "ada.exp"
+
+standard_ada_testfile foo
+
+if {[gdb_compile_ada "${srcfile}" "${binfile}" executable [list debug]] != "" } {
+ return -1
+}
+
+clean_restart ${testfile}
+
+if ![runto "foo"] then {
+ perror "Couldn't run ${testfile}"
+ return
+}
+
+gdb_breakpoint "pck.inspect"
+
+# Continue until we reach the breakpoint, and verify that we can print
+# the value of all the parameters.
+
+gdb_test "continue" \
+ ".*Breakpoint $decimal, pck\\.inspect \\(obj=$hex, <objL>=2\\) at .*"
diff --git a/gdb/testsuite/gdb.ada/access_tagged_param/foo.adb b/gdb/testsuite/gdb.ada/access_tagged_param/foo.adb
new file mode 100644
index 00000000000..76d738c373c
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/access_tagged_param/foo.adb
@@ -0,0 +1,20 @@
+-- Copyright 2017-2018 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/>.
+
+with Pck; use Pck;
+procedure Foo is
+begin
+ Inspect (new Top_T'(N => 2));
+end Foo;
diff --git a/gdb/testsuite/gdb.ada/access_tagged_param/pck.adb b/gdb/testsuite/gdb.ada/access_tagged_param/pck.adb
new file mode 100644
index 00000000000..d156cee34b7
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/access_tagged_param/pck.adb
@@ -0,0 +1,21 @@
+-- Copyright 2017-2018 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/>.
+
+package body Pck is
+ procedure Inspect (Obj: access Top_T'Class) is
+ begin
+ null;
+ end Inspect;
+end Pck;
diff --git a/gdb/testsuite/gdb.ada/access_tagged_param/pck.ads b/gdb/testsuite/gdb.ada/access_tagged_param/pck.ads
new file mode 100644
index 00000000000..f1973d201aa
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/access_tagged_param/pck.ads
@@ -0,0 +1,21 @@
+-- Copyright 2017-2018 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/>.
+
+package Pck is
+ type Top_T is tagged record
+ N : Integer := 1;
+ end record;
+ procedure Inspect (Obj: access Top_T'Class);
+end Pck;
--
2.14.3
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC] regresssion(internal-error) printing subprogram argument
2018-01-05 16:28 ` Pedro Alves
@ 2018-01-16 11:54 ` Pedro Alves
2018-01-17 9:13 ` Joel Brobecker
0 siblings, 1 reply; 27+ messages in thread
From: Pedro Alves @ 2018-01-16 11:54 UTC (permalink / raw)
To: Joel Brobecker; +Cc: gdb-patches
Hi Joel,
On 01/05/2018 04:28 PM, Pedro Alves wrote:
>
> This also fixes the new '"maint check-psymtabs" for Ada' testcase for
> me (gdb.ada/maint_with_ada.exp). I've not removed the kfail yet
> because Joel still sees that testcase failing with this patch.
> That'll be fixed in follow up patches.
I'm curious to know if after all the fixing you still see
the KFAIL on your end. I get:
KPASS: gdb.ada/maint_with_ada.exp: maintenance check-psymtabs (PRMS gdb/22670)
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC] regresssion(internal-error) printing subprogram argument
2018-01-16 11:54 ` Pedro Alves
@ 2018-01-17 9:13 ` Joel Brobecker
2018-01-26 3:51 ` Joel Brobecker
2018-01-26 4:50 ` [RFC] regresssion(internal-error) printing subprogram argument Joel Brobecker
0 siblings, 2 replies; 27+ messages in thread
From: Joel Brobecker @ 2018-01-17 9:13 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
Hi Pedro,
> I'm curious to know if after all the fixing you still see
> the KFAIL on your end. I get:
>
> KPASS: gdb.ada/maint_with_ada.exp: maintenance check-psymtabs (PRMS gdb/22670)
Strange, I still get the error:
(gdb) maintenance check-psymtabs
Global symbol `interfaces__cS' only found in /[...]/gdb.ada/maint_with_ada/b~var_arr_typedef.adb psymtab
(gdb) KFAIL: gdb.ada/maint_with_ada.exp: maintenance check-psymtabs (PRMS: gdb/22670)
I'll set some time aside next week to look into this, and let you know
what I find.
--
Joel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC] regresssion(internal-error) printing subprogram argument
2018-01-17 9:13 ` Joel Brobecker
@ 2018-01-26 3:51 ` Joel Brobecker
2018-01-26 11:28 ` Pedro Alves
2018-01-26 4:50 ` [RFC] regresssion(internal-error) printing subprogram argument Joel Brobecker
1 sibling, 1 reply; 27+ messages in thread
From: Joel Brobecker @ 2018-01-26 3:51 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
Hi Pedro,
> > I'm curious to know if after all the fixing you still see
> > the KFAIL on your end. I get:
> >
> > KPASS: gdb.ada/maint_with_ada.exp: maintenance check-psymtabs (PRMS gdb/22670)
I just started looking at this, and I am wondering whether your binder-
generated file looks like, and in particular whether it has the
following kind of declaration in b~var_arr_typedef.ads:
u00045 : constant Version_32 := 16#f60287af#;
pragma Export (C, u00045, "interfaces__cS");
IIRC, these are version consistency checks that are used by
the distributed system annex. If you don't have those in this file,
for some reason, then it would explain why you don't see the issue.
Continuing the investigation...
--
Joel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC] regresssion(internal-error) printing subprogram argument
2018-01-17 9:13 ` Joel Brobecker
2018-01-26 3:51 ` Joel Brobecker
@ 2018-01-26 4:50 ` Joel Brobecker
1 sibling, 0 replies; 27+ messages in thread
From: Joel Brobecker @ 2018-01-26 4:50 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 2831 bytes --]
> > I'm curious to know if after all the fixing you still see
> > the KFAIL on your end. I get:
> >
> > KPASS: gdb.ada/maint_with_ada.exp: maintenance check-psymtabs (PRMS gdb/22670)
>
> Strange, I still get the error:
>
> (gdb) maintenance check-psymtabs
> Global symbol `interfaces__cS' only found in /[...]/gdb.ada/maint_with_ada/b~var_arr_typedef.adb psymtab
> (gdb) KFAIL: gdb.ada/maint_with_ada.exp: maintenance check-psymtabs (PRMS: gdb/22670)
For the sake of this investigation, I used a simpler program:
| $ cat a.adb
| procedure A is
| begin
| null;
| end A;
The symbol in question is declared in b~a.ads as follow:
| u00045 : constant Version_32 := 16#f60287af#;
| pragma Export (C, u00045, "interfaces__cS");
The debugging information for this variable looks like this:
| <1><a9f>: Abbrev Number: 35 (DW_TAG_variable)
| <aa0> DW_AT_name : (indirect string, offset: 0x3fd): ada_main__u00045
| <aa4> DW_AT_decl_file : 5
| <aa5> DW_AT_decl_line : 128
| <aa6> DW_AT_linkage_name: (indirect string, offset: 0x198c): interfaces__cS
| <aaa> DW_AT_type : <0x79>
| <aae> DW_AT_external : 1
| <aae> DW_AT_location : 9 byte block: 3 80 e5 41 0 0 0 0 0 (DW_OP_addr: 41e580)
The important bit is that it has a DW_AT_linkage_name attribute.
When we look at dwarf2read.c::new_symbol, we can see:
| /* Cache this symbol's name and the name's demangled form (if any). */
| SYMBOL_SET_LANGUAGE (sym, cu->language, &objfile->objfile_obstack);
| linkagename = dwarf2_physname (name, die, cu);
| SYMBOL_SET_NAMES (sym, linkagename, strlen (linkagename), 0, objfile);
In our case, LINKAGENAMES is "interfaces(char, signed)"; as you
intuitively guessed, C++ demangling was applied on the symbol.
Following that thread and looking at dwarf2_physname, we see that
it basically calls gdb_demangle, and that works, then returns
its value as the physname.
Continuing on, gdb_demangle is just a wrapper for bfd_demangle,
and neither take any language information as a parameter. And
looking at bfd_demangle I found that it is just a wrapper around
cplus_demangle. Since Ada does not follow the same encoding
technique, this should not be done for Ada symbols.
Attached is a prototype patch which hacks that idea into the current
code. It was tested on x86_64-linux, and both fixes the KFAIL and
shows no regression. I don't understand the command regarding go;
it was just a convenient location for adding the condition.
The part that worries me is that we have several other other locations
where gdb_demangle is being called. Half of them are from language-
specific areas, so probably not an issue for Ada. But I think I will
review the other ones...
To be continued. Thoughts?
--
Joel
[-- Attachment #2: 0001-WIP-dwarf2read.c-dwarf2_physname-do-not-call-gdb_dem.patch --]
[-- Type: text/x-diff, Size: 883 bytes --]
From d98976141c88b05f414bc8975ee2d77e3e655708 Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Thu, 25 Jan 2018 23:23:54 -0500
Subject: [PATCH] WIP: dwarf2read.c::dwarf2_physname: do not call gdb_demangle
with Ada symbols
---
gdb/dwarf2read.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 96026a8..dfed170 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -11127,7 +11127,7 @@ dwarf2_physname (const char *name, struct die_info *die, struct dwarf2_cu *cu)
variant `long name(params)' does not have the proper inferior type.
*/
- if (cu->language == language_go)
+ if (cu->language == language_go || cu->language == language_ada)
{
/* This is a lie, but we already lie to the caller new_symbol.
new_symbol assumes we return the mangled name.
--
2.1.4
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC] regresssion(internal-error) printing subprogram argument
2018-01-26 3:51 ` Joel Brobecker
@ 2018-01-26 11:28 ` Pedro Alves
2018-01-29 10:38 ` Joel Brobecker
0 siblings, 1 reply; 27+ messages in thread
From: Pedro Alves @ 2018-01-26 11:28 UTC (permalink / raw)
To: Joel Brobecker; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 1347 bytes --]
On 01/26/2018 03:50 AM, Joel Brobecker wrote:
> Hi Pedro,
>
>>> I'm curious to know if after all the fixing you still see
>>> the KFAIL on your end. I get:
>>>
>>> KPASS: gdb.ada/maint_with_ada.exp: maintenance check-psymtabs (PRMS gdb/22670)
>
> I just started looking at this, and I am wondering whether your binder-
> generated file looks like, and in particular whether it has the
> following kind of declaration in b~var_arr_typedef.ads:
>
> u00045 : constant Version_32 := 16#f60287af#;
> pragma Export (C, u00045, "interfaces__cS");
>
> IIRC, these are version consistency checks that are used by
> the distributed system annex. If you don't have those in this file,
> for some reason, then it would explain why you don't see the issue.
Indeed, looks like I don't have that. I have "interfacesS".
I've attached the whole file.
Running:
$ grep Export ./testsuite/outputs/gdb.ada/maint_with_ada/b~var_arr_typedef.ads | sed 's/.*, \"//' | sed 's/").*//g' | c++filt
I notice that none of the Export symbols I have demangles as a C++ symbol,
while "interfaces__cS" (the one you have) does:
$ echo "interfaces__cS" | c++filt
interfaces(char, signed)
So it may be that we still need to add another special case
for Ada somewhere. Would old GDB from before the C++
wildmatching pass this test for you?
Thanks,
Pedro Alves
[-- Attachment #2: b~var_arr_typedef.ads --]
[-- Type: text/x-adasrc, Size: 7862 bytes --]
pragma Ada_95;
pragma Warnings (Off);
with System;
package ada_main is
gnat_argc : Integer;
gnat_argv : System.Address;
gnat_envp : System.Address;
pragma Import (C, gnat_argc);
pragma Import (C, gnat_argv);
pragma Import (C, gnat_envp);
gnat_exit_status : Integer;
pragma Import (C, gnat_exit_status);
GNAT_Version : constant String :=
"GNAT Version: 7.2.1 20170915 (Red Hat 7.2.1-2)" & ASCII.NUL;
pragma Export (C, GNAT_Version, "__gnat_version");
Ada_Main_Program_Name : constant String := "_ada_var_arr_typedef" & ASCII.NUL;
pragma Export (C, Ada_Main_Program_Name, "__gnat_ada_main_program_name");
procedure adainit;
pragma Export (C, adainit, "adainit");
procedure adafinal;
pragma Export (C, adafinal, "adafinal");
function main
(argc : Integer;
argv : System.Address;
envp : System.Address)
return Integer;
pragma Export (C, main, "main");
type Version_32 is mod 2 ** 32;
u00001 : constant Version_32 := 16#73e42c92#;
pragma Export (C, u00001, "var_arr_typedefB");
u00002 : constant Version_32 := 16#b6df930e#;
pragma Export (C, u00002, "system__standard_libraryB");
u00003 : constant Version_32 := 16#7ec093d3#;
pragma Export (C, u00003, "system__standard_libraryS");
u00004 : constant Version_32 := 16#59841384#;
pragma Export (C, u00004, "packB");
u00005 : constant Version_32 := 16#be226d3a#;
pragma Export (C, u00005, "packS");
u00006 : constant Version_32 := 16#4635ec04#;
pragma Export (C, u00006, "systemS");
u00007 : constant Version_32 := 16#a6359005#;
pragma Export (C, u00007, "system__memoryB");
u00008 : constant Version_32 := 16#1f488a30#;
pragma Export (C, u00008, "system__memoryS");
u00009 : constant Version_32 := 16#c2326fda#;
pragma Export (C, u00009, "ada__exceptionsB");
u00010 : constant Version_32 := 16#6e98a13f#;
pragma Export (C, u00010, "ada__exceptionsS");
u00011 : constant Version_32 := 16#76789da1#;
pragma Export (C, u00011, "adaS");
u00012 : constant Version_32 := 16#e947e6a9#;
pragma Export (C, u00012, "ada__exceptions__last_chance_handlerB");
u00013 : constant Version_32 := 16#41e5552e#;
pragma Export (C, u00013, "ada__exceptions__last_chance_handlerS");
u00014 : constant Version_32 := 16#4e7785b8#;
pragma Export (C, u00014, "system__soft_linksB");
u00015 : constant Version_32 := 16#d8b13451#;
pragma Export (C, u00015, "system__soft_linksS");
u00016 : constant Version_32 := 16#b01dad17#;
pragma Export (C, u00016, "system__parametersB");
u00017 : constant Version_32 := 16#381fe17b#;
pragma Export (C, u00017, "system__parametersS");
u00018 : constant Version_32 := 16#30ad09e5#;
pragma Export (C, u00018, "system__secondary_stackB");
u00019 : constant Version_32 := 16#fca7137e#;
pragma Export (C, u00019, "system__secondary_stackS");
u00020 : constant Version_32 := 16#f103f468#;
pragma Export (C, u00020, "system__storage_elementsB");
u00021 : constant Version_32 := 16#6bf6a600#;
pragma Export (C, u00021, "system__storage_elementsS");
u00022 : constant Version_32 := 16#41837d1e#;
pragma Export (C, u00022, "system__stack_checkingB");
u00023 : constant Version_32 := 16#c88a87ec#;
pragma Export (C, u00023, "system__stack_checkingS");
u00024 : constant Version_32 := 16#87a448ff#;
pragma Export (C, u00024, "system__exception_tableB");
u00025 : constant Version_32 := 16#1b9b8546#;
pragma Export (C, u00025, "system__exception_tableS");
u00026 : constant Version_32 := 16#ce4af020#;
pragma Export (C, u00026, "system__exceptionsB");
u00027 : constant Version_32 := 16#2e5681f2#;
pragma Export (C, u00027, "system__exceptionsS");
u00028 : constant Version_32 := 16#843d48dc#;
pragma Export (C, u00028, "system__exceptions__machineS");
u00029 : constant Version_32 := 16#aa0563fc#;
pragma Export (C, u00029, "system__exceptions_debugB");
u00030 : constant Version_32 := 16#38bf15c0#;
pragma Export (C, u00030, "system__exceptions_debugS");
u00031 : constant Version_32 := 16#6c2f8802#;
pragma Export (C, u00031, "system__img_intB");
u00032 : constant Version_32 := 16#44ee0cc6#;
pragma Export (C, u00032, "system__img_intS");
u00033 : constant Version_32 := 16#39df8c17#;
pragma Export (C, u00033, "system__tracebackB");
u00034 : constant Version_32 := 16#181732c0#;
pragma Export (C, u00034, "system__tracebackS");
u00035 : constant Version_32 := 16#9ed49525#;
pragma Export (C, u00035, "system__traceback_entriesB");
u00036 : constant Version_32 := 16#466e1a74#;
pragma Export (C, u00036, "system__traceback_entriesS");
u00037 : constant Version_32 := 16#6fd210f2#;
pragma Export (C, u00037, "system__traceback__symbolicB");
u00038 : constant Version_32 := 16#dd19f67a#;
pragma Export (C, u00038, "system__traceback__symbolicS");
u00039 : constant Version_32 := 16#701f9d88#;
pragma Export (C, u00039, "ada__exceptions__tracebackB");
u00040 : constant Version_32 := 16#20245e75#;
pragma Export (C, u00040, "ada__exceptions__tracebackS");
u00041 : constant Version_32 := 16#9f00b3d3#;
pragma Export (C, u00041, "system__address_imageB");
u00042 : constant Version_32 := 16#e7d9713e#;
pragma Export (C, u00042, "system__address_imageS");
u00043 : constant Version_32 := 16#8c33a517#;
pragma Export (C, u00043, "system__wch_conB");
u00044 : constant Version_32 := 16#5d48ced6#;
pragma Export (C, u00044, "system__wch_conS");
u00045 : constant Version_32 := 16#9721e840#;
pragma Export (C, u00045, "system__wch_stwB");
u00046 : constant Version_32 := 16#7059e2d7#;
pragma Export (C, u00046, "system__wch_stwS");
u00047 : constant Version_32 := 16#a831679c#;
pragma Export (C, u00047, "system__wch_cnvB");
u00048 : constant Version_32 := 16#52ff7425#;
pragma Export (C, u00048, "system__wch_cnvS");
u00049 : constant Version_32 := 16#5ab55268#;
pragma Export (C, u00049, "interfacesS");
u00050 : constant Version_32 := 16#ece6fdb6#;
pragma Export (C, u00050, "system__wch_jisB");
u00051 : constant Version_32 := 16#d28f6d04#;
pragma Export (C, u00051, "system__wch_jisS");
u00052 : constant Version_32 := 16#36a43a0a#;
pragma Export (C, u00052, "system__crtlS");
-- BEGIN ELABORATION ORDER
-- ada%s
-- interfaces%s
-- system%s
-- system.img_int%s
-- system.img_int%b
-- system.parameters%s
-- system.parameters%b
-- system.crtl%s
-- system.storage_elements%s
-- system.storage_elements%b
-- system.stack_checking%s
-- system.stack_checking%b
-- system.traceback_entries%s
-- system.traceback_entries%b
-- system.wch_con%s
-- system.wch_con%b
-- system.wch_jis%s
-- system.wch_jis%b
-- system.wch_cnv%s
-- system.wch_cnv%b
-- system.traceback%s
-- system.traceback%b
-- system.wch_stw%s
-- system.standard_library%s
-- system.exceptions_debug%s
-- system.exceptions_debug%b
-- ada.exceptions%s
-- system.wch_stw%b
-- ada.exceptions.traceback%s
-- system.soft_links%s
-- system.exception_table%s
-- system.exception_table%b
-- system.exceptions%s
-- system.exceptions%b
-- system.secondary_stack%s
-- system.address_image%s
-- system.soft_links%b
-- ada.exceptions.last_chance_handler%s
-- system.memory%s
-- system.memory%b
-- system.standard_library%b
-- ada.exceptions.traceback%b
-- system.traceback.symbolic%s
-- system.traceback.symbolic%b
-- system.exceptions.machine%s
-- system.secondary_stack%b
-- system.address_image%b
-- ada.exceptions.last_chance_handler%b
-- ada.exceptions%b
-- pack%s
-- pack%b
-- var_arr_typedef%b
-- END ELABORATION ORDER
end ada_main;
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC] regresssion(internal-error) printing subprogram argument
2018-01-26 11:28 ` Pedro Alves
@ 2018-01-29 10:38 ` Joel Brobecker
2018-01-29 15:33 ` Pedro Alves
0 siblings, 1 reply; 27+ messages in thread
From: Joel Brobecker @ 2018-01-29 10:38 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 1369 bytes --]
> Indeed, looks like I don't have that. I have "interfacesS".
> I've attached the whole file.
That explains it :).
> So it may be that we still need to add another special case
> for Ada somewhere. Would old GDB from before the C++
> wildmatching pass this test for you?
I went binary searching for the source of the regression and
when I found that it was "caused" by the change requiring variables
without debugging information to be cast before they can be printed,
I gently head-slapped myself, adjusted the testcase to use something
other than an integer variable, and voila - even GDB 7.10 suffers
from the problem. We just didn't see it for integer variables simply
because we were lucky!
I ran out of time again today, but at least the WIP patch got
augmented with a testcase that currently fails before the patch
is applied.
I think the patch itself is probably correct, although I'd like
to do some archeology to understand the comment attached to
that location. I'm pretty confused by it, when we could simply
say that symbols from languages that do not follow the C++ mangling
should not be demangled by gdb_demangle -- at least for as long
as gdb_demangle is equivalent to cplusplus_demangle in practice...
And just as a reminder for myself, I said in my other email
last week that I wanted also to review all the calls to gdb_demangle.
--
Joel
[-- Attachment #2: 0001-WIP-dwarf2read.c-dwarf2_physname-do-not-call-gdb_dem.patch --]
[-- Type: text/x-diff, Size: 7678 bytes --]
From 6f24c49e7c3ba2c6246aa3de92ef48c1315778f8 Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Thu, 25 Jan 2018 23:23:54 -0500
Subject: [PATCH] WIP: dwarf2read.c::dwarf2_physname: do not call gdb_demangle
with Ada symbols
---
gdb/dwarf2read.c | 2 +-
gdb/testsuite/gdb.ada/notcplusplus.exp | 45 ++++++++++++++++++++++++++++++
gdb/testsuite/gdb.ada/notcplusplus/foo.adb | 21 ++++++++++++++
gdb/testsuite/gdb.ada/notcplusplus/pck.adb | 21 ++++++++++++++
gdb/testsuite/gdb.ada/notcplusplus/pck.ads | 19 +++++++++++++
gdb/testsuite/gdb.ada/notcplusplus/ver.ads | 22 +++++++++++++++
6 files changed, 129 insertions(+), 1 deletion(-)
create mode 100644 gdb/testsuite/gdb.ada/notcplusplus.exp
create mode 100644 gdb/testsuite/gdb.ada/notcplusplus/foo.adb
create mode 100644 gdb/testsuite/gdb.ada/notcplusplus/pck.adb
create mode 100644 gdb/testsuite/gdb.ada/notcplusplus/pck.ads
create mode 100644 gdb/testsuite/gdb.ada/notcplusplus/ver.ads
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 51d0f39..7febade 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -11150,7 +11150,7 @@ dwarf2_physname (const char *name, struct die_info *die, struct dwarf2_cu *cu)
variant `long name(params)' does not have the proper inferior type.
*/
- if (cu->language == language_go)
+ if (cu->language == language_go || cu->language == language_ada)
{
/* This is a lie, but we already lie to the caller new_symbol.
new_symbol assumes we return the mangled name.
diff --git a/gdb/testsuite/gdb.ada/notcplusplus.exp b/gdb/testsuite/gdb.ada/notcplusplus.exp
new file mode 100644
index 0000000..b2a24e8
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/notcplusplus.exp
@@ -0,0 +1,45 @@
+# Copyright 2018 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/>.
+
+load_lib "ada.exp"
+
+if { [skip_ada_tests] } { return -1 }
+
+standard_ada_testfile foo
+
+if {[gdb_compile_ada "${srcfile}" "${binfile}" executable {debug}] != ""} {
+ return -1
+}
+
+clean_restart ${testfile}
+
+gdb_test "print /x <symada__cS>" \
+ "= \\(a => 0x60287af\\)" \
+ "print <symada__cS> before loading symbols from ver.ads"
+
+# Force the partial symbosl from ver.ads to be expanded into full symbols.
+
+gdb_test \
+ "list ver.ads:16" \
+ [multi_line ".*" \
+ "16\\s+package Ver is" \
+ "17\\s+type Wrapper is record" \
+ "18\\s+A : Integer;" \
+ "19\\s+end record;" \
+ "20\\s+u00045 : constant Wrapper := \\(A => 16#060287af#\\);"]
+
+gdb_test "print /x <symada__cS>" \
+ "= \\(a => 0x60287af\\)" \
+ "print <symada__cS> after loading symbols from ver.ads"
diff --git a/gdb/testsuite/gdb.ada/notcplusplus/foo.adb b/gdb/testsuite/gdb.ada/notcplusplus/foo.adb
new file mode 100644
index 0000000..89e42f9
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/notcplusplus/foo.adb
@@ -0,0 +1,21 @@
+-- Copyright 2018 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/>.
+
+with Pck; use Pck;
+with Ver; use Ver;
+procedure Foo is
+begin
+ Do_Nothing (u00045'Address);
+end Foo;
diff --git a/gdb/testsuite/gdb.ada/notcplusplus/pck.adb b/gdb/testsuite/gdb.ada/notcplusplus/pck.adb
new file mode 100644
index 0000000..dcfb306
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/notcplusplus/pck.adb
@@ -0,0 +1,21 @@
+-- Copyright 2018 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/>.
+
+package body Pck is
+ procedure Do_Nothing (A : System.Address) is
+ begin
+ null;
+ end Do_Nothing;
+end Pck;
diff --git a/gdb/testsuite/gdb.ada/notcplusplus/pck.ads b/gdb/testsuite/gdb.ada/notcplusplus/pck.ads
new file mode 100644
index 0000000..33e369e
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/notcplusplus/pck.ads
@@ -0,0 +1,19 @@
+-- Copyright 2018 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/>.
+
+with System;
+package Pck is
+ procedure Do_Nothing (A : System.Address);
+end Pck;
diff --git a/gdb/testsuite/gdb.ada/notcplusplus/ver.ads b/gdb/testsuite/gdb.ada/notcplusplus/ver.ads
new file mode 100644
index 0000000..8f264d0
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/notcplusplus/ver.ads
@@ -0,0 +1,22 @@
+-- Copyright 2018 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/>.
+
+package Ver is
+ type Wrapper is record
+ A : Integer;
+ end record;
+ u00045 : constant Wrapper := (A => 16#060287af#);
+ pragma Export (C, u00045, "symada__cS");
+end Ver;
--
2.1.4
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC] regresssion(internal-error) printing subprogram argument
2018-01-29 10:38 ` Joel Brobecker
@ 2018-01-29 15:33 ` Pedro Alves
2018-01-29 15:52 ` Joel Brobecker
2018-01-30 3:56 ` Joel Brobecker
0 siblings, 2 replies; 27+ messages in thread
From: Pedro Alves @ 2018-01-29 15:33 UTC (permalink / raw)
To: Joel Brobecker; +Cc: gdb-patches
On 01/29/2018 10:38 AM, Joel Brobecker wrote:
>> Indeed, looks like I don't have that. I have "interfacesS".
>> I've attached the whole file.
>
> That explains it :).
>
>> So it may be that we still need to add another special case
>> for Ada somewhere. Would old GDB from before the C++
>> wildmatching pass this test for you?
>
> I went binary searching for the source of the regression and
> when I found that it was "caused" by the change requiring variables
> without debugging information to be cast before they can be printed,
> I gently head-slapped myself, adjusted the testcase to use something
> other than an integer variable, and voila - even GDB 7.10 suffers
> from the problem. We just didn't see it for integer variables simply
> because we were lucky!
>
> I ran out of time again today, but at least the WIP patch got
> augmented with a testcase that currently fails before the patch
> is applied.
>
> I think the patch itself is probably correct, although I'd like
> to do some archeology to understand the comment attached to
> that location. I'm pretty confused by it, when we could simply
> say that symbols from languages that do not follow the C++ mangling
> should not be demangled by gdb_demangle -- at least for as long
> as gdb_demangle is equivalent to cplusplus_demangle in practice...
Yeah, the commentary around that code isn't exactly clear.
Why doesn't that code use language->la_demangle instead,
for example.
The other day I noticed that gdb_demangle -> bfd_demangle ->
cplus_demangle does have support for demangling other languages.
For Ada, see the GNAT_DEMANGLING handling in
libiberty.c:cplus-dem.c:cplus_demangle, which takes you to:
/* Demangle ada names. The encoding is documented in gcc/ada/exp_dbug.ads. */
char *
ada_demangle (const char *mangled, int option ATTRIBUTE_UNUSED)
{
When I saw that, I wondered why is it that GDB has a separate
implementation of Ada decoding in gdb/ada-lang.c. The only plausible
explanation I came up with is that gdb's version decodes into a
buffer that is shared between invocations while libiberty's version
always heap-allocates the result. Maybe it was an efficiency thing?
Do you know?
I ran into that around this discussion
<https://sourceware.org/ml/gdb/2017-11/msg00029.html> where I
was wondering whether we could speed up demangling by letting
bfd_demangle / cplus_demangle try all languages and return back
which one worked:
~~~
An idea I had would be to try to combine most the language_sniff_from_mangled_name
implementations in one call, by deferring to cplus_demangle which seems
like could take care of gnu v3, rust, java, gnat, dlang and old C++ mangling
schemes all at once. This would avoid the overhead of gdb_demangle and
bfd_demangle for each language-specific demangling call, at least. Not sure.
~~~
Because ada_decode comes up high in profiles today...
>
> And just as a reminder for myself, I said in my other email
> last week that I wanted also to review all the calls to gdb_demangle.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC] regresssion(internal-error) printing subprogram argument
2018-01-29 15:33 ` Pedro Alves
@ 2018-01-29 15:52 ` Joel Brobecker
2018-01-30 3:56 ` Joel Brobecker
1 sibling, 0 replies; 27+ messages in thread
From: Joel Brobecker @ 2018-01-29 15:52 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 269 bytes --]
Hi Pedro,
It's the end of my day, now, and I haven't had a chance to look at
your comments just yet. I just wanted to post version 3 of the WIP
patch, because it also includes a testcase in C. A bit on the
artificial side, but I thought could be useful too.
--
Joel
[-- Attachment #2: 0001-WIPv3-dwarf2read.c-dwarf2_physname-do-not-call-gdb_d.patch --]
[-- Type: text/x-diff, Size: 11530 bytes --]
From 98f782214fcd6916f9163936b1fcf308edb8c522 Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Thu, 25 Jan 2018 23:23:54 -0500
Subject: [PATCH] WIPv3: dwarf2read.c::dwarf2_physname: do not call
gdb_demangle with Ada symbols
---
gdb/dwarf2read.c | 2 +-
gdb/testsuite/gdb.ada/notcplusplus.exp | 45 ++++++++++++++++++++++++++++
gdb/testsuite/gdb.ada/notcplusplus/foo.adb | 21 +++++++++++++
gdb/testsuite/gdb.ada/notcplusplus/pck.adb | 21 +++++++++++++
gdb/testsuite/gdb.ada/notcplusplus/pck.ads | 19 ++++++++++++
gdb/testsuite/gdb.ada/notcplusplus/ver.ads | 22 ++++++++++++++
gdb/testsuite/gdb.base/c-linkage-name.c | 44 ++++++++++++++++++++++++++++
gdb/testsuite/gdb.base/c-linkage-name.exp | 47 ++++++++++++++++++++++++++++++
8 files changed, 220 insertions(+), 1 deletion(-)
create mode 100644 gdb/testsuite/gdb.ada/notcplusplus.exp
create mode 100644 gdb/testsuite/gdb.ada/notcplusplus/foo.adb
create mode 100644 gdb/testsuite/gdb.ada/notcplusplus/pck.adb
create mode 100644 gdb/testsuite/gdb.ada/notcplusplus/pck.ads
create mode 100644 gdb/testsuite/gdb.ada/notcplusplus/ver.ads
create mode 100644 gdb/testsuite/gdb.base/c-linkage-name.c
create mode 100644 gdb/testsuite/gdb.base/c-linkage-name.exp
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 51d0f39..7febade 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -11150,7 +11150,7 @@ dwarf2_physname (const char *name, struct die_info *die, struct dwarf2_cu *cu)
variant `long name(params)' does not have the proper inferior type.
*/
- if (cu->language == language_go)
+ if (cu->language == language_go || cu->language == language_ada)
{
/* This is a lie, but we already lie to the caller new_symbol.
new_symbol assumes we return the mangled name.
diff --git a/gdb/testsuite/gdb.ada/notcplusplus.exp b/gdb/testsuite/gdb.ada/notcplusplus.exp
new file mode 100644
index 0000000..b2a24e8
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/notcplusplus.exp
@@ -0,0 +1,45 @@
+# Copyright 2018 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/>.
+
+load_lib "ada.exp"
+
+if { [skip_ada_tests] } { return -1 }
+
+standard_ada_testfile foo
+
+if {[gdb_compile_ada "${srcfile}" "${binfile}" executable {debug}] != ""} {
+ return -1
+}
+
+clean_restart ${testfile}
+
+gdb_test "print /x <symada__cS>" \
+ "= \\(a => 0x60287af\\)" \
+ "print <symada__cS> before loading symbols from ver.ads"
+
+# Force the partial symbosl from ver.ads to be expanded into full symbols.
+
+gdb_test \
+ "list ver.ads:16" \
+ [multi_line ".*" \
+ "16\\s+package Ver is" \
+ "17\\s+type Wrapper is record" \
+ "18\\s+A : Integer;" \
+ "19\\s+end record;" \
+ "20\\s+u00045 : constant Wrapper := \\(A => 16#060287af#\\);"]
+
+gdb_test "print /x <symada__cS>" \
+ "= \\(a => 0x60287af\\)" \
+ "print <symada__cS> after loading symbols from ver.ads"
diff --git a/gdb/testsuite/gdb.ada/notcplusplus/foo.adb b/gdb/testsuite/gdb.ada/notcplusplus/foo.adb
new file mode 100644
index 0000000..89e42f9
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/notcplusplus/foo.adb
@@ -0,0 +1,21 @@
+-- Copyright 2018 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/>.
+
+with Pck; use Pck;
+with Ver; use Ver;
+procedure Foo is
+begin
+ Do_Nothing (u00045'Address);
+end Foo;
diff --git a/gdb/testsuite/gdb.ada/notcplusplus/pck.adb b/gdb/testsuite/gdb.ada/notcplusplus/pck.adb
new file mode 100644
index 0000000..dcfb306
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/notcplusplus/pck.adb
@@ -0,0 +1,21 @@
+-- Copyright 2018 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/>.
+
+package body Pck is
+ procedure Do_Nothing (A : System.Address) is
+ begin
+ null;
+ end Do_Nothing;
+end Pck;
diff --git a/gdb/testsuite/gdb.ada/notcplusplus/pck.ads b/gdb/testsuite/gdb.ada/notcplusplus/pck.ads
new file mode 100644
index 0000000..33e369e
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/notcplusplus/pck.ads
@@ -0,0 +1,19 @@
+-- Copyright 2018 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/>.
+
+with System;
+package Pck is
+ procedure Do_Nothing (A : System.Address);
+end Pck;
diff --git a/gdb/testsuite/gdb.ada/notcplusplus/ver.ads b/gdb/testsuite/gdb.ada/notcplusplus/ver.ads
new file mode 100644
index 0000000..8f264d0
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/notcplusplus/ver.ads
@@ -0,0 +1,22 @@
+-- Copyright 2018 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/>.
+
+package Ver is
+ type Wrapper is record
+ A : Integer;
+ end record;
+ u00045 : constant Wrapper := (A => 16#060287af#);
+ pragma Export (C, u00045, "symada__cS");
+end Ver;
diff --git a/gdb/testsuite/gdb.base/c-linkage-name.c b/gdb/testsuite/gdb.base/c-linkage-name.c
new file mode 100644
index 0000000..925004c
--- /dev/null
+++ b/gdb/testsuite/gdb.base/c-linkage-name.c
@@ -0,0 +1,44 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2018 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/>. */
+
+struct wrapper
+{
+ int a;
+};
+
+/* Create a global variable whose name in the assembly code
+ (aka the "linkage name") is different from the name in
+ the source code. The goal is to create a symbol described
+ in DWARF using a DW_AT_linkage_name attribute, with a name
+ which follows the C++ mangling.
+
+ In this particular case, we chose "symada__cS" which, if it were
+ demangled, would translate to "symada (char, signed)". */
+struct wrapper mundane asm ("symada__cS") = {0x060287af};
+
+void
+do_something (struct wrapper *w)
+{
+ w->a++;
+}
+
+int
+main (void)
+{
+ do_something (&mundane);
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.base/c-linkage-name.exp b/gdb/testsuite/gdb.base/c-linkage-name.exp
new file mode 100644
index 0000000..c80a530
--- /dev/null
+++ b/gdb/testsuite/gdb.base/c-linkage-name.exp
@@ -0,0 +1,47 @@
+# Copyright 2018 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/>.
+
+# This file is part of the gdb testsuite. It is intended to test that
+# gdb can correctly print arrays with indexes for each element of the
+# array.
+
+standard_testfile .c
+
+if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
+ untested "failed to compile"
+ return -1
+}
+
+clean_restart ${binfile}
+
+# Try to print MUNDANE, but using its linkage name.
+
+gdb_test "print symada__cS" \
+ " = {a = 100829103}" \
+ "print symada__cS before partial symtab expansion"
+
+# Force the symbols to be expanded for the unit that contains
+# our symada__cS symbol by, e.g. inserting a breakpoint on one
+# of the founction also provided by the same using.
+
+gdb_test "break main" \
+ "Breakpoint $decimal at $hex: file .*$srcfile, line $decimal\\."
+
+# Try to print MUNDANE using its linkage name again, after partial
+# symtab expansion.
+
+gdb_test "print symada__cS" \
+ " = {a = 100829103}" \
+ "print symada__cS after partial symtab expansion"
--
2.1.4
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC] regresssion(internal-error) printing subprogram argument
2018-01-29 15:33 ` Pedro Alves
2018-01-29 15:52 ` Joel Brobecker
@ 2018-01-30 3:56 ` Joel Brobecker
2018-02-05 9:57 ` Joel Brobecker
1 sibling, 1 reply; 27+ messages in thread
From: Joel Brobecker @ 2018-01-30 3:56 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
> The other day I noticed that gdb_demangle -> bfd_demangle ->
> cplus_demangle does have support for demangling other languages.
> For Ada, see the GNAT_DEMANGLING handling in
> libiberty.c:cplus-dem.c:cplus_demangle, which takes you to:
>
> /* Demangle ada names. The encoding is documented in gcc/ada/exp_dbug.ads. */
>
> char *
> ada_demangle (const char *mangled, int option ATTRIBUTE_UNUSED)
> {
Ha! Thanks for digging; who would have thought...
> When I saw that, I wondered why is it that GDB has a separate
> implementation of Ada decoding in gdb/ada-lang.c. The only plausible
> explanation I came up with is that gdb's version decodes into a
> buffer that is shared between invocations while libiberty's version
> always heap-allocates the result. Maybe it was an efficiency thing?
> Do you know?
I don't unfortunately. I'm pretty sure GDB's ada_decode was already
present before I joined AdaCore...
It is true that start up performance is critical, particularly in Ada,
because one of the strengths of Ada is to help design very large apps.
I think that, eventually, we're going to have to accept that computing
power is gained by adding parallelism, and try to take advantage of that
to speed things up (with the dangers in terms of stability that it
entails), so that aspect might become less relevant once we make that
transition.
> I ran into that around this discussion
> <https://sourceware.org/ml/gdb/2017-11/msg00029.html> where I
> was wondering whether we could speed up demangling by letting
> bfd_demangle / cplus_demangle try all languages and return back
> which one worked:
>
> ~~~
> An idea I had would be to try to combine most the language_sniff_from_mangled_name
> implementations in one call, by deferring to cplus_demangle which seems
> like could take care of gnu v3, rust, java, gnat, dlang and old C++ mangling
> schemes all at once. This would avoid the overhead of gdb_demangle and
> bfd_demangle for each language-specific demangling call, at least. Not sure.
> ~~~
>
> Because ada_decode comes up high in profiles today...
Can we avoid the calls to most demanglers entirely by passing
the language to symbol_set_names? I didn't do an extensive search
of all the callers. We could have a fallback where language_unknown
means try all the demanglers, but for the cases we care about,
which is mostly DWARF debugging info, that could save a lot of
unnecessary attempts, like you say.
My feeling on the global situation with those demanglers is that
we take a risk each time we use the demanglers themselves to
try to determine which language the symbol belongs to. Both
the C++ and the Ada mangling/encoding algorithms are not exclusive
enough, and we're seeing in this thread how easy it is to get
it wrong. For minimal symbols, we don't have the information,
so that's one valid situation where we have no choice but to guess.
But when reading DWARF debugging information, for instance, we do
have the language information, so we should take advantage of it.
Thinking out loud, this leads me to wonder whether it's a good idea
to store the demangled name instead of the linkage name. You'd do
the search on the mangled name rather than on the demangled one,
so you would not have to guess when loading the symbols. Maybe it's
a good idea if it helps performance, and perhaps we can plug that
weakness differently...
--
Joel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC] regresssion(internal-error) printing subprogram argument
2018-01-30 3:56 ` Joel Brobecker
@ 2018-02-05 9:57 ` Joel Brobecker
2018-02-09 9:09 ` [RFA/RFC] fix PR gdb/22670 (pb looking up some symbols when they have a linkage name) (was: "Re: [RFC] regresssion(internal-error) printing subprogram argument") Joel Brobecker
0 siblings, 1 reply; 27+ messages in thread
From: Joel Brobecker @ 2018-02-05 9:57 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 5228 bytes --]
Here is the latest update for this thread. I'm pretty much ready
to work on an official patch, but I'd like some guidance.
A quick summary:
We noticed that, with Ada programs, "maintainance check-psymtabs"
reports consistency check error for symbols such as "interfaces__cS".
This happens for symbols whose linkage name resembles a C++ mangled
name. In that situation, dwarf2physname finds the linkage name,
and then processes it through gdb_demangle, regardless of the
unit's language. As a result, we end up storing the symbol
linkage name using the demangled name, which makes no sense
for Ada, and thus triggers the consistency check failure.
The fix for the various testcases (one in C, one in the existing
Ada testcase) is to patch dwarf2_physname to avoid calling gdb_demangle
for languages where it doesn't make sense. The big question is:
which languages should we exclude? For now, I've decided to only
touch languages where I am sure: Ada, of course, but also
C, Asm, and "minimal".
There was the question of Go, but I'm not sure what go does in terms
of mangling. If you remember, symbols from Go units do not process
symbols' linkage name through gdb_demangle, but for reasons that are
unclear. Here is the comment:
if (cu->language == language_go)
{
/* This is a lie, but we already lie to the caller new_symbol.
new_symbol assumes we return the mangled name.
This just undoes that lie until things are cleaned up. */
I went looking for the origin of this comment, and unfortunately, it
was introduced as part of the large patch that introduced GO support
(circa Apr 2012). Not much information there.
To play it safe, I decided to fix dwarf2_physname independently
of go, so that go's comment doesn't bleed into this fix.
I also spent some time investigating all the calls to gdb_demangle.
The vast majority are obviously in a situation where we're dealing
with C++ types, or at least language-specific types where the language
is known. So the assumption is that we know what we're doing and
the call is OK.
As a result, there were only the 3 instances in dwarf2read.c.
One of them is the dwarf2_physname function, which is fixed here.
The other two are:
(a) fixup_partial_die:
| /* GCC might emit a nameless struct or union that has a linkage
| name. See http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47510. */
| if (part_die->name == NULL
| && (part_die->tag == DW_TAG_class_type
| || part_die->tag == DW_TAG_interface_type
| || part_die->tag == DW_TAG_structure_type
| || part_die->tag == DW_TAG_union_type)
| && part_die->linkage_name != NULL)
(b) dwarf2_name:
| case DW_TAG_class_type:
| case DW_TAG_interface_type:
| case DW_TAG_structure_type:
| case DW_TAG_union_type:
| [...]
| /* GCC might emit a nameless typedef that has a linkage name. See
| http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47510. */
| if (!attr || DW_STRING (attr) == NULL)
Theoretically, we might have an issue where we might be calling
gdb_demangle improperly for those types, and we could try to fix that.
However, the fix in those two situations is not clearly obvious.
And to top it all off, the equivalent situation might not exist
outside of certain versions of C++. So I would like to suggest
we leave those areas alone for now.
In terms of the fix, we have several alternatives. I discarded
the idea of hard-coding the list of languages we want to exclude
in dwarf2_physname as we might need that list elsewhere.
We have the option of a function (like in the attached patch),
probably to be moved to symtab.h/symtab.c. I chose a name that
spoke to me, but I'm completely familiar with the new lookup
system entirely yet, so better name suggestions are welcome.
For instance, instead of talking of symbol name storage format,
we could have something like...
symbol_lookup_uses_demanged_name_p (enum language lang);
All in all, I think a better solution would be to put that information
directly in the language_defn itself, via a new "attribute".
Something like a...
/* True if symbol search names should be stored in demangled
format. False otherwise. */
const bool symbol_stored_in_demangle_form_p;
Then, I'd set it to "true" for all languages, except the languages
we selected (Ada, C, Asm, minimal). I didn't implement that
just yet, as this requires a larger number of files being modified,
so I thought I'd seek opinions before going ahead.
My vote: a new language_defn attribute. Thoughts?
gdb/ChangeLog:
PR gdb/22670:
* dwarf2read.c (symbols_stored_in_demangled_form_p): New function.
(dwarf2_physname): Do not call gdb_demangle if
symbols_stored_in_demangled_form_p returns false for
the cu's language.
gdb/testsuite/ChangeLog:
* gdb.ada/notcplusplus: New testcase.
* gdb.ada/maint_with_ada.exp: Remove setup_kfail.
* gdb.base/c-linkage-name.c: New file.
* gdb.base/c-linkage-name.exp: New testcase.
Tested on x86_64-linux, no regression.
Thank you!
--
Joel
[-- Attachment #2: 0001-WIPv4-dwarf2read.c-dwarf2_physname-conditionalize-ca.patch --]
[-- Type: text/x-diff, Size: 13139 bytes --]
From 4b3af7739001893595ec2b81b5cfb504b0b58859 Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Thu, 25 Jan 2018 23:23:54 -0500
Subject: [PATCH] WIPv4: dwarf2read.c::dwarf2_physname: conditionalize call to
gdb_demangle
This patch avoids the call to gdb_demangle of the DW_AT_linkage_name
for languages where either: demangling does not make sense, or where
the language implementation chose to store symbol names in mangled
form.
---
gdb/dwarf2read.c | 26 ++++++++++++++++-
gdb/testsuite/gdb.ada/maint_with_ada.exp | 1 -
gdb/testsuite/gdb.ada/notcplusplus.exp | 45 ++++++++++++++++++++++++++++
gdb/testsuite/gdb.ada/notcplusplus/foo.adb | 21 +++++++++++++
gdb/testsuite/gdb.ada/notcplusplus/pck.adb | 21 +++++++++++++
gdb/testsuite/gdb.ada/notcplusplus/pck.ads | 19 ++++++++++++
gdb/testsuite/gdb.ada/notcplusplus/ver.ads | 22 ++++++++++++++
gdb/testsuite/gdb.base/c-linkage-name.c | 44 ++++++++++++++++++++++++++++
gdb/testsuite/gdb.base/c-linkage-name.exp | 47 ++++++++++++++++++++++++++++++
9 files changed, 244 insertions(+), 2 deletions(-)
create mode 100644 gdb/testsuite/gdb.ada/notcplusplus.exp
create mode 100644 gdb/testsuite/gdb.ada/notcplusplus/foo.adb
create mode 100644 gdb/testsuite/gdb.ada/notcplusplus/pck.adb
create mode 100644 gdb/testsuite/gdb.ada/notcplusplus/pck.ads
create mode 100644 gdb/testsuite/gdb.ada/notcplusplus/ver.ads
create mode 100644 gdb/testsuite/gdb.base/c-linkage-name.c
create mode 100644 gdb/testsuite/gdb.base/c-linkage-name.exp
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index d651725..91e7862 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -11109,6 +11109,26 @@ dwarf2_full_name (const char *name, struct die_info *die, struct dwarf2_cu *cu)
return dwarf2_compute_name (name, die, cu, 0);
}
+static bool
+symbols_stored_in_demangled_form_p (enum language lang)
+{
+ if (lang == language_asm
+ || lang == language_minimal
+ || lang == language_c)
+ {
+ /* No mangling for those languages. */
+ return false;
+ }
+ if (lang == language_ada)
+ {
+ /* For Ada, we store the symbol names in encoded form
+ and then perform the lookups using that form. */
+ return false;
+ }
+
+ return true;
+}
+
/* Construct a physname for the given DIE in CU. NAME may either be
from a previous call to dwarf2_name or NULL. The result will be
allocated on the objfile_objstack or NULL if the DIE does not have a
@@ -11142,7 +11162,11 @@ dwarf2_physname (const char *name, struct die_info *die, struct dwarf2_cu *cu)
if (mangled != NULL)
{
- if (cu->language == language_go)
+ if (! symbols_stored_in_demangled_form_p (cu->language))
+ {
+ /* Do nothing (do not demangle the symbol name). */
+ }
+ else if (cu->language == language_go)
{
/* This is a lie, but we already lie to the caller new_symbol.
new_symbol assumes we return the mangled name.
diff --git a/gdb/testsuite/gdb.ada/maint_with_ada.exp b/gdb/testsuite/gdb.ada/maint_with_ada.exp
index 73da613..9ede035 100644
--- a/gdb/testsuite/gdb.ada/maint_with_ada.exp
+++ b/gdb/testsuite/gdb.ada/maint_with_ada.exp
@@ -32,7 +32,6 @@ gdb_breakpoint "adainit"
gdb_breakpoint "Var_Arr_Typedef"
gdb_breakpoint "Do_Nothing"
-setup_kfail gdb/22670 "*-*-*"
gdb_test_no_output "maintenance check-psymtabs"
gdb_test_no_output "maintenance check-symtabs"
diff --git a/gdb/testsuite/gdb.ada/notcplusplus.exp b/gdb/testsuite/gdb.ada/notcplusplus.exp
new file mode 100644
index 0000000..b2a24e8
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/notcplusplus.exp
@@ -0,0 +1,45 @@
+# Copyright 2018 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/>.
+
+load_lib "ada.exp"
+
+if { [skip_ada_tests] } { return -1 }
+
+standard_ada_testfile foo
+
+if {[gdb_compile_ada "${srcfile}" "${binfile}" executable {debug}] != ""} {
+ return -1
+}
+
+clean_restart ${testfile}
+
+gdb_test "print /x <symada__cS>" \
+ "= \\(a => 0x60287af\\)" \
+ "print <symada__cS> before loading symbols from ver.ads"
+
+# Force the partial symbosl from ver.ads to be expanded into full symbols.
+
+gdb_test \
+ "list ver.ads:16" \
+ [multi_line ".*" \
+ "16\\s+package Ver is" \
+ "17\\s+type Wrapper is record" \
+ "18\\s+A : Integer;" \
+ "19\\s+end record;" \
+ "20\\s+u00045 : constant Wrapper := \\(A => 16#060287af#\\);"]
+
+gdb_test "print /x <symada__cS>" \
+ "= \\(a => 0x60287af\\)" \
+ "print <symada__cS> after loading symbols from ver.ads"
diff --git a/gdb/testsuite/gdb.ada/notcplusplus/foo.adb b/gdb/testsuite/gdb.ada/notcplusplus/foo.adb
new file mode 100644
index 0000000..89e42f9
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/notcplusplus/foo.adb
@@ -0,0 +1,21 @@
+-- Copyright 2018 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/>.
+
+with Pck; use Pck;
+with Ver; use Ver;
+procedure Foo is
+begin
+ Do_Nothing (u00045'Address);
+end Foo;
diff --git a/gdb/testsuite/gdb.ada/notcplusplus/pck.adb b/gdb/testsuite/gdb.ada/notcplusplus/pck.adb
new file mode 100644
index 0000000..dcfb306
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/notcplusplus/pck.adb
@@ -0,0 +1,21 @@
+-- Copyright 2018 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/>.
+
+package body Pck is
+ procedure Do_Nothing (A : System.Address) is
+ begin
+ null;
+ end Do_Nothing;
+end Pck;
diff --git a/gdb/testsuite/gdb.ada/notcplusplus/pck.ads b/gdb/testsuite/gdb.ada/notcplusplus/pck.ads
new file mode 100644
index 0000000..33e369e
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/notcplusplus/pck.ads
@@ -0,0 +1,19 @@
+-- Copyright 2018 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/>.
+
+with System;
+package Pck is
+ procedure Do_Nothing (A : System.Address);
+end Pck;
diff --git a/gdb/testsuite/gdb.ada/notcplusplus/ver.ads b/gdb/testsuite/gdb.ada/notcplusplus/ver.ads
new file mode 100644
index 0000000..8f264d0
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/notcplusplus/ver.ads
@@ -0,0 +1,22 @@
+-- Copyright 2018 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/>.
+
+package Ver is
+ type Wrapper is record
+ A : Integer;
+ end record;
+ u00045 : constant Wrapper := (A => 16#060287af#);
+ pragma Export (C, u00045, "symada__cS");
+end Ver;
diff --git a/gdb/testsuite/gdb.base/c-linkage-name.c b/gdb/testsuite/gdb.base/c-linkage-name.c
new file mode 100644
index 0000000..925004c
--- /dev/null
+++ b/gdb/testsuite/gdb.base/c-linkage-name.c
@@ -0,0 +1,44 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2018 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/>. */
+
+struct wrapper
+{
+ int a;
+};
+
+/* Create a global variable whose name in the assembly code
+ (aka the "linkage name") is different from the name in
+ the source code. The goal is to create a symbol described
+ in DWARF using a DW_AT_linkage_name attribute, with a name
+ which follows the C++ mangling.
+
+ In this particular case, we chose "symada__cS" which, if it were
+ demangled, would translate to "symada (char, signed)". */
+struct wrapper mundane asm ("symada__cS") = {0x060287af};
+
+void
+do_something (struct wrapper *w)
+{
+ w->a++;
+}
+
+int
+main (void)
+{
+ do_something (&mundane);
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.base/c-linkage-name.exp b/gdb/testsuite/gdb.base/c-linkage-name.exp
new file mode 100644
index 0000000..c80a530
--- /dev/null
+++ b/gdb/testsuite/gdb.base/c-linkage-name.exp
@@ -0,0 +1,47 @@
+# Copyright 2018 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/>.
+
+# This file is part of the gdb testsuite. It is intended to test that
+# gdb can correctly print arrays with indexes for each element of the
+# array.
+
+standard_testfile .c
+
+if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
+ untested "failed to compile"
+ return -1
+}
+
+clean_restart ${binfile}
+
+# Try to print MUNDANE, but using its linkage name.
+
+gdb_test "print symada__cS" \
+ " = {a = 100829103}" \
+ "print symada__cS before partial symtab expansion"
+
+# Force the symbols to be expanded for the unit that contains
+# our symada__cS symbol by, e.g. inserting a breakpoint on one
+# of the founction also provided by the same using.
+
+gdb_test "break main" \
+ "Breakpoint $decimal at $hex: file .*$srcfile, line $decimal\\."
+
+# Try to print MUNDANE using its linkage name again, after partial
+# symtab expansion.
+
+gdb_test "print symada__cS" \
+ " = {a = 100829103}" \
+ "print symada__cS after partial symtab expansion"
--
2.1.4
^ permalink raw reply [flat|nested] 27+ messages in thread
* [RFA/RFC] fix PR gdb/22670 (pb looking up some symbols when they have a linkage name) (was: "Re: [RFC] regresssion(internal-error) printing subprogram argument")
2018-02-05 9:57 ` Joel Brobecker
@ 2018-02-09 9:09 ` Joel Brobecker
2018-02-21 3:02 ` PING: " Joel Brobecker
0 siblings, 1 reply; 27+ messages in thread
From: Joel Brobecker @ 2018-02-09 9:09 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 1079 bytes --]
> All in all, I think a better solution would be to put that information
> directly in the language_defn itself, via a new "attribute".
> Something like a...
I ended up choosing this approach. lookup names is still a bit
foggy for me, so the comments I wrote and/or the name of the
new language_defn attribute might be a bit off. As always, I am
grateful for suggestions :).
gdb/ChangeLog:
PR gdb/22670
* dwarf2read.c (dwarf2_physname): Do not return the demangled
symbol name is the CU's language stores symbol names in linkage
format.
* language.h (struct language_defn)
<la_store_sym_names_in_linkage_form_p>: New field. Adjust
all instances of this struct.
gdb/testsuite/ChangeLog:
* gdb.ada/maint_with_ada.exp: Remove PR gdb/22670 setup_kfail.
* gdb.ada/notcplusplus: New testcase.
* gdb.base/c-linkage-name.c: New file.
* gdb.base/c-linkage-name.exp: New testcase.
Tested on x86_64-linux.
This also passes AdaCore's internal GDB testsuite.
OK to commit?
Thank you!
--
Joel
[-- Attachment #2: 0001-problem-looking-up-some-symbols-when-they-have-a-lin.patch --]
[-- Type: text/x-diff, Size: 24886 bytes --]
From c22aef84cc86097d6d8b654b0586a3fbc1ff6af3 Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Fri, 9 Feb 2018 02:13:34 -0500
Subject: [PATCH] problem looking up some symbols when they have a linkage name
This patch fixes a known failure in gdb.ada/maint_with_ada.exp
(maintenance check-psymtabs). Another way to witness the same
issue is by considering the following Ada declarations...
type Wrapper is record
A : Integer;
end record;
u00045 : constant Wrapper := (A => 16#060287af#);
pragma Export (C, u00045, "symada__cS");
... which declares a variable name "u00045" but with a linkage
name which is "symada__cS". This variable is a record with one
component, the Ada equivalent of a struct with one field in C.
Trying to print that variable's value currently yields:
(gdb) p /x <symada__cS>
'symada(char, signed)' has unknown type; cast it to its declared type
This indicates that GDB was only able to find the minimal symbol,
but not the full symbol. The expected output is:
(gdb) print /x <symada__cS>
$1 = (a => 0x60287af)
The error message gives a hint about what's happening: We processed
the symbol through gdb_demangle, which in the case of this particular
symbol name, ends up matching the C++ naming scheme. As a result,
the demangler transforms our symbol name into 'symada(char, signed)',
thus breaking Ada lookups.
This patch fixes the issue by first introducing a new language_defn
attribute called la_store_sym_names_in_linkage_form_p, which is a boolean
to be set to true for the few languages that do not want their symbols
to have their names stored in demangled form, and false otherwise.
We then use this language attribute to skip the call to gdb_demangle
for all languages whose la_store_sym_names_in_linkage_form_p is true.
In terms of the selection of languages for which the new attribute
is set to true, the selection errs on the side of preserving the
existing behavior, and only changes the behavior for the languages
where we are certain storing symbol names in demangling form is not
needed. It is conceivable that other languages might be in the same
situation, but I not knowing in detail the symbol name enconding
strategy, I decided to play it safe and let other language maintainers
potentially adjust their language if it makes sense to do so.
gdb/ChangeLog:
PR gdb/22670
* dwarf2read.c (dwarf2_physname): Do not return the demangled
symbol name is the CU's language stores symbol names in linkage
format.
* language.h (struct language_defn)
<la_store_sym_names_in_linkage_form_p>: New field. Adjust
all instances of this struct.
gdb/testsuite/ChangeLog:
* gdb.ada/maint_with_ada.exp: Remove PR gdb/22670 setup_kfail.
* gdb.ada/notcplusplus: New testcase.
* gdb.base/c-linkage-name.c: New file.
* gdb.base/c-linkage-name.exp: New testcase.
Tested on x86_64-linux.
This also passes AdaCore's internal GDB testsuite.
---
gdb/ada-lang.c | 1 +
gdb/c-lang.c | 4 +++
gdb/d-lang.c | 1 +
gdb/dwarf2read.c | 6 +++-
gdb/f-lang.c | 1 +
gdb/go-lang.c | 1 +
gdb/language.c | 2 ++
gdb/language.h | 20 +++++++++++++
gdb/m2-lang.c | 1 +
gdb/objc-lang.c | 1 +
gdb/opencl-lang.c | 1 +
gdb/p-lang.c | 1 +
gdb/rust-lang.c | 1 +
gdb/testsuite/gdb.ada/maint_with_ada.exp | 1 -
gdb/testsuite/gdb.ada/notcplusplus.exp | 45 ++++++++++++++++++++++++++++
gdb/testsuite/gdb.ada/notcplusplus/foo.adb | 21 +++++++++++++
gdb/testsuite/gdb.ada/notcplusplus/pck.adb | 21 +++++++++++++
gdb/testsuite/gdb.ada/notcplusplus/pck.ads | 19 ++++++++++++
gdb/testsuite/gdb.ada/notcplusplus/ver.ads | 22 ++++++++++++++
gdb/testsuite/gdb.base/c-linkage-name.c | 44 ++++++++++++++++++++++++++++
gdb/testsuite/gdb.base/c-linkage-name.exp | 47 ++++++++++++++++++++++++++++++
21 files changed, 259 insertions(+), 2 deletions(-)
create mode 100644 gdb/testsuite/gdb.ada/notcplusplus.exp
create mode 100644 gdb/testsuite/gdb.ada/notcplusplus/foo.adb
create mode 100644 gdb/testsuite/gdb.ada/notcplusplus/pck.adb
create mode 100644 gdb/testsuite/gdb.ada/notcplusplus/pck.ads
create mode 100644 gdb/testsuite/gdb.ada/notcplusplus/ver.ads
create mode 100644 gdb/testsuite/gdb.base/c-linkage-name.c
create mode 100644 gdb/testsuite/gdb.base/c-linkage-name.exp
diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 0da58d9..7f99a2e 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -14521,6 +14521,7 @@ extern const struct language_defn ada_language_defn = {
ada_read_var_value, /* la_read_var_value */
NULL, /* Language specific skip_trampoline */
NULL, /* name_of_this */
+ true, /* la_store_sym_names_in_linkage_form_p */
ada_lookup_symbol_nonlocal, /* Looking up non-local symbols. */
basic_lookup_transparent_type, /* lookup_transparent_type */
ada_la_decode, /* Language specific symbol demangler */
diff --git a/gdb/c-lang.c b/gdb/c-lang.c
index a0b553e..658c7f7 100644
--- a/gdb/c-lang.c
+++ b/gdb/c-lang.c
@@ -853,6 +853,7 @@ extern const struct language_defn c_language_defn =
default_read_var_value, /* la_read_var_value */
NULL, /* Language specific skip_trampoline */
NULL, /* name_of_this */
+ true, /* la_store_sym_names_in_linkage_form_p */
basic_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */
basic_lookup_transparent_type,/* lookup_transparent_type */
NULL, /* Language specific symbol demangler */
@@ -998,6 +999,7 @@ extern const struct language_defn cplus_language_defn =
default_read_var_value, /* la_read_var_value */
cplus_skip_trampoline, /* Language specific skip_trampoline */
"this", /* name_of_this */
+ false, /* la_store_sym_names_in_linkage_form_p */
cp_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */
cp_lookup_transparent_type, /* lookup_transparent_type */
gdb_demangle, /* Language specific symbol demangler */
@@ -1052,6 +1054,7 @@ extern const struct language_defn asm_language_defn =
default_read_var_value, /* la_read_var_value */
NULL, /* Language specific skip_trampoline */
NULL, /* name_of_this */
+ true, /* la_store_sym_names_in_linkage_form_p */
basic_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */
basic_lookup_transparent_type,/* lookup_transparent_type */
NULL, /* Language specific symbol demangler */
@@ -1106,6 +1109,7 @@ extern const struct language_defn minimal_language_defn =
default_read_var_value, /* la_read_var_value */
NULL, /* Language specific skip_trampoline */
NULL, /* name_of_this */
+ true, /* la_store_sym_names_in_linkage_form_p */
basic_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */
basic_lookup_transparent_type,/* lookup_transparent_type */
NULL, /* Language specific symbol demangler */
diff --git a/gdb/d-lang.c b/gdb/d-lang.c
index e0afe48..688ae98 100644
--- a/gdb/d-lang.c
+++ b/gdb/d-lang.c
@@ -229,6 +229,7 @@ extern const struct language_defn d_language_defn =
default_read_var_value, /* la_read_var_value */
NULL, /* Language specific skip_trampoline. */
"this",
+ false, /* la_store_sym_names_in_linkage_form_p */
d_lookup_symbol_nonlocal,
basic_lookup_transparent_type,
d_demangle, /* Language specific symbol demangler. */
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index d651725..b4c087f 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -11142,7 +11142,11 @@ dwarf2_physname (const char *name, struct die_info *die, struct dwarf2_cu *cu)
if (mangled != NULL)
{
- if (cu->language == language_go)
+ if (language_def (cu->language)->la_store_sym_names_in_linkage_form_p)
+ {
+ /* Do nothing (do not demangle the symbol name). */
+ }
+ else if (cu->language == language_go)
{
/* This is a lie, but we already lie to the caller new_symbol.
new_symbol assumes we return the mangled name.
diff --git a/gdb/f-lang.c b/gdb/f-lang.c
index 74f5622..81922f7 100644
--- a/gdb/f-lang.c
+++ b/gdb/f-lang.c
@@ -269,6 +269,7 @@ extern const struct language_defn f_language_defn =
default_read_var_value, /* la_read_var_value */
NULL, /* Language specific skip_trampoline */
NULL, /* name_of_this */
+ false, /* la_store_sym_names_in_linkage_form_p */
cp_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */
basic_lookup_transparent_type,/* lookup_transparent_type */
diff --git a/gdb/go-lang.c b/gdb/go-lang.c
index 022b8de..e9cba77 100644
--- a/gdb/go-lang.c
+++ b/gdb/go-lang.c
@@ -590,6 +590,7 @@ extern const struct language_defn go_language_defn =
default_read_var_value, /* la_read_var_value */
NULL, /* Language specific skip_trampoline. */
NULL, /* name_of_this */
+ false, /* la_store_sym_names_in_linkage_form_p */
basic_lookup_symbol_nonlocal,
basic_lookup_transparent_type,
go_demangle, /* Language specific symbol demangler. */
diff --git a/gdb/language.c b/gdb/language.c
index 0d8604b..22199e0 100644
--- a/gdb/language.c
+++ b/gdb/language.c
@@ -864,6 +864,7 @@ const struct language_defn unknown_language_defn =
default_read_var_value, /* la_read_var_value */
unk_lang_trampoline, /* Language specific skip_trampoline */
"this", /* name_of_this */
+ true, /* store_sym_names_in_linkage_form_p */
basic_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */
basic_lookup_transparent_type,/* lookup_transparent_type */
unk_lang_demangle, /* Language specific symbol demangler */
@@ -915,6 +916,7 @@ const struct language_defn auto_language_defn =
default_read_var_value, /* la_read_var_value */
unk_lang_trampoline, /* Language specific skip_trampoline */
"this", /* name_of_this */
+ false, /* store_sym_names_in_linkage_form_p */
basic_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */
basic_lookup_transparent_type,/* lookup_transparent_type */
unk_lang_demangle, /* Language specific symbol demangler */
diff --git a/gdb/language.h b/gdb/language.h
index 06b42ae..029de4a 100644
--- a/gdb/language.h
+++ b/gdb/language.h
@@ -264,6 +264,26 @@ struct language_defn
const char *la_name_of_this;
+ /* True if the symbols names should be stored in GDB's data structures
+ for minimal/partial/full symbols using their linkage (aka mangled)
+ form; false if the symbol names should be demangled first.
+
+ Most languages implement symbol lookup by comparing the demangled
+ names, in which case it is advantageous to store that information
+ already demangled, and so would set this field to false.
+
+ On the other hand, some languages have opted for doing symbol
+ lookups by comparing mangled names instead, for reasons usually
+ specific to the language. Those languages should set this field
+ to true.
+
+ And finally, other languages such as C or Asm do not have
+ the concept of mangled vs demangled name, so those languages
+ should set this field to true as well, to prevent any accidental
+ demangling through an unrelated language's demangler. */
+
+ const bool la_store_sym_names_in_linkage_form_p;
+
/* This is a function that lookup_symbol will call when it gets to
the part of symbol lookup where C looks up static and global
variables. */
diff --git a/gdb/m2-lang.c b/gdb/m2-lang.c
index 11ccab3..6e6434b 100644
--- a/gdb/m2-lang.c
+++ b/gdb/m2-lang.c
@@ -377,6 +377,7 @@ extern const struct language_defn m2_language_defn =
default_read_var_value, /* la_read_var_value */
NULL, /* Language specific skip_trampoline */
NULL, /* name_of_this */
+ false, /* la_store_sym_names_in_linkage_form_p */
basic_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */
basic_lookup_transparent_type,/* lookup_transparent_type */
NULL, /* Language specific symbol demangler */
diff --git a/gdb/objc-lang.c b/gdb/objc-lang.c
index c714ec3..4f7dc36 100644
--- a/gdb/objc-lang.c
+++ b/gdb/objc-lang.c
@@ -389,6 +389,7 @@ extern const struct language_defn objc_language_defn = {
default_read_var_value, /* la_read_var_value */
objc_skip_trampoline, /* Language specific skip_trampoline */
"self", /* name_of_this */
+ false, /* la_store_sym_names_in_linkage_form_p */
basic_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */
basic_lookup_transparent_type,/* lookup_transparent_type */
objc_demangle, /* Language specific symbol demangler */
diff --git a/gdb/opencl-lang.c b/gdb/opencl-lang.c
index 268c3c5..9c5b90c 100644
--- a/gdb/opencl-lang.c
+++ b/gdb/opencl-lang.c
@@ -1065,6 +1065,7 @@ extern const struct language_defn opencl_language_defn =
default_read_var_value, /* la_read_var_value */
NULL, /* Language specific skip_trampoline */
NULL, /* name_of_this */
+ false, /* la_store_sym_names_in_linkage_form_p */
basic_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */
basic_lookup_transparent_type,/* lookup_transparent_type */
NULL, /* Language specific symbol demangler */
diff --git a/gdb/p-lang.c b/gdb/p-lang.c
index 03db2df..3ff7f56 100644
--- a/gdb/p-lang.c
+++ b/gdb/p-lang.c
@@ -439,6 +439,7 @@ extern const struct language_defn pascal_language_defn =
default_read_var_value, /* la_read_var_value */
NULL, /* Language specific skip_trampoline */
"this", /* name_of_this */
+ false, /* la_store_sym_names_in_linkage_form_p */
basic_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */
basic_lookup_transparent_type,/* lookup_transparent_type */
NULL, /* Language specific symbol demangler */
diff --git a/gdb/rust-lang.c b/gdb/rust-lang.c
index 5ff80b2..2d7c1f7 100644
--- a/gdb/rust-lang.c
+++ b/gdb/rust-lang.c
@@ -2290,6 +2290,7 @@ extern const struct language_defn rust_language_defn =
default_read_var_value, /* la_read_var_value */
NULL, /* Language specific skip_trampoline */
NULL, /* name_of_this */
+ false, /* la_store_sym_names_in_linkage_form_p */
rust_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */
basic_lookup_transparent_type,/* lookup_transparent_type */
gdb_demangle, /* Language specific symbol demangler */
diff --git a/gdb/testsuite/gdb.ada/maint_with_ada.exp b/gdb/testsuite/gdb.ada/maint_with_ada.exp
index 73da613..9ede035 100644
--- a/gdb/testsuite/gdb.ada/maint_with_ada.exp
+++ b/gdb/testsuite/gdb.ada/maint_with_ada.exp
@@ -32,7 +32,6 @@ gdb_breakpoint "adainit"
gdb_breakpoint "Var_Arr_Typedef"
gdb_breakpoint "Do_Nothing"
-setup_kfail gdb/22670 "*-*-*"
gdb_test_no_output "maintenance check-psymtabs"
gdb_test_no_output "maintenance check-symtabs"
diff --git a/gdb/testsuite/gdb.ada/notcplusplus.exp b/gdb/testsuite/gdb.ada/notcplusplus.exp
new file mode 100644
index 0000000..b2a24e8
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/notcplusplus.exp
@@ -0,0 +1,45 @@
+# Copyright 2018 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/>.
+
+load_lib "ada.exp"
+
+if { [skip_ada_tests] } { return -1 }
+
+standard_ada_testfile foo
+
+if {[gdb_compile_ada "${srcfile}" "${binfile}" executable {debug}] != ""} {
+ return -1
+}
+
+clean_restart ${testfile}
+
+gdb_test "print /x <symada__cS>" \
+ "= \\(a => 0x60287af\\)" \
+ "print <symada__cS> before loading symbols from ver.ads"
+
+# Force the partial symbosl from ver.ads to be expanded into full symbols.
+
+gdb_test \
+ "list ver.ads:16" \
+ [multi_line ".*" \
+ "16\\s+package Ver is" \
+ "17\\s+type Wrapper is record" \
+ "18\\s+A : Integer;" \
+ "19\\s+end record;" \
+ "20\\s+u00045 : constant Wrapper := \\(A => 16#060287af#\\);"]
+
+gdb_test "print /x <symada__cS>" \
+ "= \\(a => 0x60287af\\)" \
+ "print <symada__cS> after loading symbols from ver.ads"
diff --git a/gdb/testsuite/gdb.ada/notcplusplus/foo.adb b/gdb/testsuite/gdb.ada/notcplusplus/foo.adb
new file mode 100644
index 0000000..89e42f9
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/notcplusplus/foo.adb
@@ -0,0 +1,21 @@
+-- Copyright 2018 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/>.
+
+with Pck; use Pck;
+with Ver; use Ver;
+procedure Foo is
+begin
+ Do_Nothing (u00045'Address);
+end Foo;
diff --git a/gdb/testsuite/gdb.ada/notcplusplus/pck.adb b/gdb/testsuite/gdb.ada/notcplusplus/pck.adb
new file mode 100644
index 0000000..dcfb306
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/notcplusplus/pck.adb
@@ -0,0 +1,21 @@
+-- Copyright 2018 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/>.
+
+package body Pck is
+ procedure Do_Nothing (A : System.Address) is
+ begin
+ null;
+ end Do_Nothing;
+end Pck;
diff --git a/gdb/testsuite/gdb.ada/notcplusplus/pck.ads b/gdb/testsuite/gdb.ada/notcplusplus/pck.ads
new file mode 100644
index 0000000..33e369e
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/notcplusplus/pck.ads
@@ -0,0 +1,19 @@
+-- Copyright 2018 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/>.
+
+with System;
+package Pck is
+ procedure Do_Nothing (A : System.Address);
+end Pck;
diff --git a/gdb/testsuite/gdb.ada/notcplusplus/ver.ads b/gdb/testsuite/gdb.ada/notcplusplus/ver.ads
new file mode 100644
index 0000000..8f264d0
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/notcplusplus/ver.ads
@@ -0,0 +1,22 @@
+-- Copyright 2018 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/>.
+
+package Ver is
+ type Wrapper is record
+ A : Integer;
+ end record;
+ u00045 : constant Wrapper := (A => 16#060287af#);
+ pragma Export (C, u00045, "symada__cS");
+end Ver;
diff --git a/gdb/testsuite/gdb.base/c-linkage-name.c b/gdb/testsuite/gdb.base/c-linkage-name.c
new file mode 100644
index 0000000..925004c
--- /dev/null
+++ b/gdb/testsuite/gdb.base/c-linkage-name.c
@@ -0,0 +1,44 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2018 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/>. */
+
+struct wrapper
+{
+ int a;
+};
+
+/* Create a global variable whose name in the assembly code
+ (aka the "linkage name") is different from the name in
+ the source code. The goal is to create a symbol described
+ in DWARF using a DW_AT_linkage_name attribute, with a name
+ which follows the C++ mangling.
+
+ In this particular case, we chose "symada__cS" which, if it were
+ demangled, would translate to "symada (char, signed)". */
+struct wrapper mundane asm ("symada__cS") = {0x060287af};
+
+void
+do_something (struct wrapper *w)
+{
+ w->a++;
+}
+
+int
+main (void)
+{
+ do_something (&mundane);
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.base/c-linkage-name.exp b/gdb/testsuite/gdb.base/c-linkage-name.exp
new file mode 100644
index 0000000..c80a530
--- /dev/null
+++ b/gdb/testsuite/gdb.base/c-linkage-name.exp
@@ -0,0 +1,47 @@
+# Copyright 2018 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/>.
+
+# This file is part of the gdb testsuite. It is intended to test that
+# gdb can correctly print arrays with indexes for each element of the
+# array.
+
+standard_testfile .c
+
+if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
+ untested "failed to compile"
+ return -1
+}
+
+clean_restart ${binfile}
+
+# Try to print MUNDANE, but using its linkage name.
+
+gdb_test "print symada__cS" \
+ " = {a = 100829103}" \
+ "print symada__cS before partial symtab expansion"
+
+# Force the symbols to be expanded for the unit that contains
+# our symada__cS symbol by, e.g. inserting a breakpoint on one
+# of the founction also provided by the same using.
+
+gdb_test "break main" \
+ "Breakpoint $decimal at $hex: file .*$srcfile, line $decimal\\."
+
+# Try to print MUNDANE using its linkage name again, after partial
+# symtab expansion.
+
+gdb_test "print symada__cS" \
+ " = {a = 100829103}" \
+ "print symada__cS after partial symtab expansion"
--
2.1.4
^ permalink raw reply [flat|nested] 27+ messages in thread
* PING: [RFA/RFC] fix PR gdb/22670 (pb looking up some symbols when they have a linkage name) (was: "Re: [RFC] regresssion(internal-error) printing subprogram argument")
2018-02-09 9:09 ` [RFA/RFC] fix PR gdb/22670 (pb looking up some symbols when they have a linkage name) (was: "Re: [RFC] regresssion(internal-error) printing subprogram argument") Joel Brobecker
@ 2018-02-21 3:02 ` Joel Brobecker
2018-03-19 21:22 ` PING^2: " Joel Brobecker
0 siblings, 1 reply; 27+ messages in thread
From: Joel Brobecker @ 2018-02-21 3:02 UTC (permalink / raw)
To: gdb-patches
> > All in all, I think a better solution would be to put that information
> > directly in the language_defn itself, via a new "attribute".
> > Something like a...
>
> I ended up choosing this approach. lookup names is still a bit
> foggy for me, so the comments I wrote and/or the name of the
> new language_defn attribute might be a bit off. As always, I am
> grateful for suggestions :).
>
> gdb/ChangeLog:
>
> PR gdb/22670
> * dwarf2read.c (dwarf2_physname): Do not return the demangled
> symbol name is the CU's language stores symbol names in linkage
> format.
> * language.h (struct language_defn)
> <la_store_sym_names_in_linkage_form_p>: New field. Adjust
> all instances of this struct.
>
> gdb/testsuite/ChangeLog:
>
> * gdb.ada/maint_with_ada.exp: Remove PR gdb/22670 setup_kfail.
>
> * gdb.ada/notcplusplus: New testcase.
>
> * gdb.base/c-linkage-name.c: New file.
> * gdb.base/c-linkage-name.exp: New testcase.
>
> Tested on x86_64-linux.
> This also passes AdaCore's internal GDB testsuite.
Any comment on this patch?
Thanks!
> >From c22aef84cc86097d6d8b654b0586a3fbc1ff6af3 Mon Sep 17 00:00:00 2001
> From: Joel Brobecker <brobecker@adacore.com>
> Date: Fri, 9 Feb 2018 02:13:34 -0500
> Subject: [PATCH] problem looking up some symbols when they have a linkage name
>
> This patch fixes a known failure in gdb.ada/maint_with_ada.exp
> (maintenance check-psymtabs). Another way to witness the same
> issue is by considering the following Ada declarations...
>
> type Wrapper is record
> A : Integer;
> end record;
> u00045 : constant Wrapper := (A => 16#060287af#);
> pragma Export (C, u00045, "symada__cS");
>
> ... which declares a variable name "u00045" but with a linkage
> name which is "symada__cS". This variable is a record with one
> component, the Ada equivalent of a struct with one field in C.
> Trying to print that variable's value currently yields:
>
> (gdb) p /x <symada__cS>
> 'symada(char, signed)' has unknown type; cast it to its declared type
>
> This indicates that GDB was only able to find the minimal symbol,
> but not the full symbol. The expected output is:
>
> (gdb) print /x <symada__cS>
> $1 = (a => 0x60287af)
>
> The error message gives a hint about what's happening: We processed
> the symbol through gdb_demangle, which in the case of this particular
> symbol name, ends up matching the C++ naming scheme. As a result,
> the demangler transforms our symbol name into 'symada(char, signed)',
> thus breaking Ada lookups.
>
> This patch fixes the issue by first introducing a new language_defn
> attribute called la_store_sym_names_in_linkage_form_p, which is a boolean
> to be set to true for the few languages that do not want their symbols
> to have their names stored in demangled form, and false otherwise.
> We then use this language attribute to skip the call to gdb_demangle
> for all languages whose la_store_sym_names_in_linkage_form_p is true.
>
> In terms of the selection of languages for which the new attribute
> is set to true, the selection errs on the side of preserving the
> existing behavior, and only changes the behavior for the languages
> where we are certain storing symbol names in demangling form is not
> needed. It is conceivable that other languages might be in the same
> situation, but I not knowing in detail the symbol name enconding
> strategy, I decided to play it safe and let other language maintainers
> potentially adjust their language if it makes sense to do so.
>
> gdb/ChangeLog:
>
> PR gdb/22670
> * dwarf2read.c (dwarf2_physname): Do not return the demangled
> symbol name is the CU's language stores symbol names in linkage
> format.
> * language.h (struct language_defn)
> <la_store_sym_names_in_linkage_form_p>: New field. Adjust
> all instances of this struct.
>
> gdb/testsuite/ChangeLog:
>
> * gdb.ada/maint_with_ada.exp: Remove PR gdb/22670 setup_kfail.
>
> * gdb.ada/notcplusplus: New testcase.
>
> * gdb.base/c-linkage-name.c: New file.
> * gdb.base/c-linkage-name.exp: New testcase.
>
> Tested on x86_64-linux.
> This also passes AdaCore's internal GDB testsuite.
> ---
> gdb/ada-lang.c | 1 +
> gdb/c-lang.c | 4 +++
> gdb/d-lang.c | 1 +
> gdb/dwarf2read.c | 6 +++-
> gdb/f-lang.c | 1 +
> gdb/go-lang.c | 1 +
> gdb/language.c | 2 ++
> gdb/language.h | 20 +++++++++++++
> gdb/m2-lang.c | 1 +
> gdb/objc-lang.c | 1 +
> gdb/opencl-lang.c | 1 +
> gdb/p-lang.c | 1 +
> gdb/rust-lang.c | 1 +
> gdb/testsuite/gdb.ada/maint_with_ada.exp | 1 -
> gdb/testsuite/gdb.ada/notcplusplus.exp | 45 ++++++++++++++++++++++++++++
> gdb/testsuite/gdb.ada/notcplusplus/foo.adb | 21 +++++++++++++
> gdb/testsuite/gdb.ada/notcplusplus/pck.adb | 21 +++++++++++++
> gdb/testsuite/gdb.ada/notcplusplus/pck.ads | 19 ++++++++++++
> gdb/testsuite/gdb.ada/notcplusplus/ver.ads | 22 ++++++++++++++
> gdb/testsuite/gdb.base/c-linkage-name.c | 44 ++++++++++++++++++++++++++++
> gdb/testsuite/gdb.base/c-linkage-name.exp | 47 ++++++++++++++++++++++++++++++
> 21 files changed, 259 insertions(+), 2 deletions(-)
> create mode 100644 gdb/testsuite/gdb.ada/notcplusplus.exp
> create mode 100644 gdb/testsuite/gdb.ada/notcplusplus/foo.adb
> create mode 100644 gdb/testsuite/gdb.ada/notcplusplus/pck.adb
> create mode 100644 gdb/testsuite/gdb.ada/notcplusplus/pck.ads
> create mode 100644 gdb/testsuite/gdb.ada/notcplusplus/ver.ads
> create mode 100644 gdb/testsuite/gdb.base/c-linkage-name.c
> create mode 100644 gdb/testsuite/gdb.base/c-linkage-name.exp
>
> diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
> index 0da58d9..7f99a2e 100644
> --- a/gdb/ada-lang.c
> +++ b/gdb/ada-lang.c
> @@ -14521,6 +14521,7 @@ extern const struct language_defn ada_language_defn = {
> ada_read_var_value, /* la_read_var_value */
> NULL, /* Language specific skip_trampoline */
> NULL, /* name_of_this */
> + true, /* la_store_sym_names_in_linkage_form_p */
> ada_lookup_symbol_nonlocal, /* Looking up non-local symbols. */
> basic_lookup_transparent_type, /* lookup_transparent_type */
> ada_la_decode, /* Language specific symbol demangler */
> diff --git a/gdb/c-lang.c b/gdb/c-lang.c
> index a0b553e..658c7f7 100644
> --- a/gdb/c-lang.c
> +++ b/gdb/c-lang.c
> @@ -853,6 +853,7 @@ extern const struct language_defn c_language_defn =
> default_read_var_value, /* la_read_var_value */
> NULL, /* Language specific skip_trampoline */
> NULL, /* name_of_this */
> + true, /* la_store_sym_names_in_linkage_form_p */
> basic_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */
> basic_lookup_transparent_type,/* lookup_transparent_type */
> NULL, /* Language specific symbol demangler */
> @@ -998,6 +999,7 @@ extern const struct language_defn cplus_language_defn =
> default_read_var_value, /* la_read_var_value */
> cplus_skip_trampoline, /* Language specific skip_trampoline */
> "this", /* name_of_this */
> + false, /* la_store_sym_names_in_linkage_form_p */
> cp_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */
> cp_lookup_transparent_type, /* lookup_transparent_type */
> gdb_demangle, /* Language specific symbol demangler */
> @@ -1052,6 +1054,7 @@ extern const struct language_defn asm_language_defn =
> default_read_var_value, /* la_read_var_value */
> NULL, /* Language specific skip_trampoline */
> NULL, /* name_of_this */
> + true, /* la_store_sym_names_in_linkage_form_p */
> basic_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */
> basic_lookup_transparent_type,/* lookup_transparent_type */
> NULL, /* Language specific symbol demangler */
> @@ -1106,6 +1109,7 @@ extern const struct language_defn minimal_language_defn =
> default_read_var_value, /* la_read_var_value */
> NULL, /* Language specific skip_trampoline */
> NULL, /* name_of_this */
> + true, /* la_store_sym_names_in_linkage_form_p */
> basic_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */
> basic_lookup_transparent_type,/* lookup_transparent_type */
> NULL, /* Language specific symbol demangler */
> diff --git a/gdb/d-lang.c b/gdb/d-lang.c
> index e0afe48..688ae98 100644
> --- a/gdb/d-lang.c
> +++ b/gdb/d-lang.c
> @@ -229,6 +229,7 @@ extern const struct language_defn d_language_defn =
> default_read_var_value, /* la_read_var_value */
> NULL, /* Language specific skip_trampoline. */
> "this",
> + false, /* la_store_sym_names_in_linkage_form_p */
> d_lookup_symbol_nonlocal,
> basic_lookup_transparent_type,
> d_demangle, /* Language specific symbol demangler. */
> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> index d651725..b4c087f 100644
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
> @@ -11142,7 +11142,11 @@ dwarf2_physname (const char *name, struct die_info *die, struct dwarf2_cu *cu)
> if (mangled != NULL)
> {
>
> - if (cu->language == language_go)
> + if (language_def (cu->language)->la_store_sym_names_in_linkage_form_p)
> + {
> + /* Do nothing (do not demangle the symbol name). */
> + }
> + else if (cu->language == language_go)
> {
> /* This is a lie, but we already lie to the caller new_symbol.
> new_symbol assumes we return the mangled name.
> diff --git a/gdb/f-lang.c b/gdb/f-lang.c
> index 74f5622..81922f7 100644
> --- a/gdb/f-lang.c
> +++ b/gdb/f-lang.c
> @@ -269,6 +269,7 @@ extern const struct language_defn f_language_defn =
> default_read_var_value, /* la_read_var_value */
> NULL, /* Language specific skip_trampoline */
> NULL, /* name_of_this */
> + false, /* la_store_sym_names_in_linkage_form_p */
> cp_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */
> basic_lookup_transparent_type,/* lookup_transparent_type */
>
> diff --git a/gdb/go-lang.c b/gdb/go-lang.c
> index 022b8de..e9cba77 100644
> --- a/gdb/go-lang.c
> +++ b/gdb/go-lang.c
> @@ -590,6 +590,7 @@ extern const struct language_defn go_language_defn =
> default_read_var_value, /* la_read_var_value */
> NULL, /* Language specific skip_trampoline. */
> NULL, /* name_of_this */
> + false, /* la_store_sym_names_in_linkage_form_p */
> basic_lookup_symbol_nonlocal,
> basic_lookup_transparent_type,
> go_demangle, /* Language specific symbol demangler. */
> diff --git a/gdb/language.c b/gdb/language.c
> index 0d8604b..22199e0 100644
> --- a/gdb/language.c
> +++ b/gdb/language.c
> @@ -864,6 +864,7 @@ const struct language_defn unknown_language_defn =
> default_read_var_value, /* la_read_var_value */
> unk_lang_trampoline, /* Language specific skip_trampoline */
> "this", /* name_of_this */
> + true, /* store_sym_names_in_linkage_form_p */
> basic_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */
> basic_lookup_transparent_type,/* lookup_transparent_type */
> unk_lang_demangle, /* Language specific symbol demangler */
> @@ -915,6 +916,7 @@ const struct language_defn auto_language_defn =
> default_read_var_value, /* la_read_var_value */
> unk_lang_trampoline, /* Language specific skip_trampoline */
> "this", /* name_of_this */
> + false, /* store_sym_names_in_linkage_form_p */
> basic_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */
> basic_lookup_transparent_type,/* lookup_transparent_type */
> unk_lang_demangle, /* Language specific symbol demangler */
> diff --git a/gdb/language.h b/gdb/language.h
> index 06b42ae..029de4a 100644
> --- a/gdb/language.h
> +++ b/gdb/language.h
> @@ -264,6 +264,26 @@ struct language_defn
>
> const char *la_name_of_this;
>
> + /* True if the symbols names should be stored in GDB's data structures
> + for minimal/partial/full symbols using their linkage (aka mangled)
> + form; false if the symbol names should be demangled first.
> +
> + Most languages implement symbol lookup by comparing the demangled
> + names, in which case it is advantageous to store that information
> + already demangled, and so would set this field to false.
> +
> + On the other hand, some languages have opted for doing symbol
> + lookups by comparing mangled names instead, for reasons usually
> + specific to the language. Those languages should set this field
> + to true.
> +
> + And finally, other languages such as C or Asm do not have
> + the concept of mangled vs demangled name, so those languages
> + should set this field to true as well, to prevent any accidental
> + demangling through an unrelated language's demangler. */
> +
> + const bool la_store_sym_names_in_linkage_form_p;
> +
> /* This is a function that lookup_symbol will call when it gets to
> the part of symbol lookup where C looks up static and global
> variables. */
> diff --git a/gdb/m2-lang.c b/gdb/m2-lang.c
> index 11ccab3..6e6434b 100644
> --- a/gdb/m2-lang.c
> +++ b/gdb/m2-lang.c
> @@ -377,6 +377,7 @@ extern const struct language_defn m2_language_defn =
> default_read_var_value, /* la_read_var_value */
> NULL, /* Language specific skip_trampoline */
> NULL, /* name_of_this */
> + false, /* la_store_sym_names_in_linkage_form_p */
> basic_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */
> basic_lookup_transparent_type,/* lookup_transparent_type */
> NULL, /* Language specific symbol demangler */
> diff --git a/gdb/objc-lang.c b/gdb/objc-lang.c
> index c714ec3..4f7dc36 100644
> --- a/gdb/objc-lang.c
> +++ b/gdb/objc-lang.c
> @@ -389,6 +389,7 @@ extern const struct language_defn objc_language_defn = {
> default_read_var_value, /* la_read_var_value */
> objc_skip_trampoline, /* Language specific skip_trampoline */
> "self", /* name_of_this */
> + false, /* la_store_sym_names_in_linkage_form_p */
> basic_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */
> basic_lookup_transparent_type,/* lookup_transparent_type */
> objc_demangle, /* Language specific symbol demangler */
> diff --git a/gdb/opencl-lang.c b/gdb/opencl-lang.c
> index 268c3c5..9c5b90c 100644
> --- a/gdb/opencl-lang.c
> +++ b/gdb/opencl-lang.c
> @@ -1065,6 +1065,7 @@ extern const struct language_defn opencl_language_defn =
> default_read_var_value, /* la_read_var_value */
> NULL, /* Language specific skip_trampoline */
> NULL, /* name_of_this */
> + false, /* la_store_sym_names_in_linkage_form_p */
> basic_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */
> basic_lookup_transparent_type,/* lookup_transparent_type */
> NULL, /* Language specific symbol demangler */
> diff --git a/gdb/p-lang.c b/gdb/p-lang.c
> index 03db2df..3ff7f56 100644
> --- a/gdb/p-lang.c
> +++ b/gdb/p-lang.c
> @@ -439,6 +439,7 @@ extern const struct language_defn pascal_language_defn =
> default_read_var_value, /* la_read_var_value */
> NULL, /* Language specific skip_trampoline */
> "this", /* name_of_this */
> + false, /* la_store_sym_names_in_linkage_form_p */
> basic_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */
> basic_lookup_transparent_type,/* lookup_transparent_type */
> NULL, /* Language specific symbol demangler */
> diff --git a/gdb/rust-lang.c b/gdb/rust-lang.c
> index 5ff80b2..2d7c1f7 100644
> --- a/gdb/rust-lang.c
> +++ b/gdb/rust-lang.c
> @@ -2290,6 +2290,7 @@ extern const struct language_defn rust_language_defn =
> default_read_var_value, /* la_read_var_value */
> NULL, /* Language specific skip_trampoline */
> NULL, /* name_of_this */
> + false, /* la_store_sym_names_in_linkage_form_p */
> rust_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */
> basic_lookup_transparent_type,/* lookup_transparent_type */
> gdb_demangle, /* Language specific symbol demangler */
> diff --git a/gdb/testsuite/gdb.ada/maint_with_ada.exp b/gdb/testsuite/gdb.ada/maint_with_ada.exp
> index 73da613..9ede035 100644
> --- a/gdb/testsuite/gdb.ada/maint_with_ada.exp
> +++ b/gdb/testsuite/gdb.ada/maint_with_ada.exp
> @@ -32,7 +32,6 @@ gdb_breakpoint "adainit"
> gdb_breakpoint "Var_Arr_Typedef"
> gdb_breakpoint "Do_Nothing"
>
> -setup_kfail gdb/22670 "*-*-*"
> gdb_test_no_output "maintenance check-psymtabs"
>
> gdb_test_no_output "maintenance check-symtabs"
> diff --git a/gdb/testsuite/gdb.ada/notcplusplus.exp b/gdb/testsuite/gdb.ada/notcplusplus.exp
> new file mode 100644
> index 0000000..b2a24e8
> --- /dev/null
> +++ b/gdb/testsuite/gdb.ada/notcplusplus.exp
> @@ -0,0 +1,45 @@
> +# Copyright 2018 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/>.
> +
> +load_lib "ada.exp"
> +
> +if { [skip_ada_tests] } { return -1 }
> +
> +standard_ada_testfile foo
> +
> +if {[gdb_compile_ada "${srcfile}" "${binfile}" executable {debug}] != ""} {
> + return -1
> +}
> +
> +clean_restart ${testfile}
> +
> +gdb_test "print /x <symada__cS>" \
> + "= \\(a => 0x60287af\\)" \
> + "print <symada__cS> before loading symbols from ver.ads"
> +
> +# Force the partial symbosl from ver.ads to be expanded into full symbols.
> +
> +gdb_test \
> + "list ver.ads:16" \
> + [multi_line ".*" \
> + "16\\s+package Ver is" \
> + "17\\s+type Wrapper is record" \
> + "18\\s+A : Integer;" \
> + "19\\s+end record;" \
> + "20\\s+u00045 : constant Wrapper := \\(A => 16#060287af#\\);"]
> +
> +gdb_test "print /x <symada__cS>" \
> + "= \\(a => 0x60287af\\)" \
> + "print <symada__cS> after loading symbols from ver.ads"
> diff --git a/gdb/testsuite/gdb.ada/notcplusplus/foo.adb b/gdb/testsuite/gdb.ada/notcplusplus/foo.adb
> new file mode 100644
> index 0000000..89e42f9
> --- /dev/null
> +++ b/gdb/testsuite/gdb.ada/notcplusplus/foo.adb
> @@ -0,0 +1,21 @@
> +-- Copyright 2018 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/>.
> +
> +with Pck; use Pck;
> +with Ver; use Ver;
> +procedure Foo is
> +begin
> + Do_Nothing (u00045'Address);
> +end Foo;
> diff --git a/gdb/testsuite/gdb.ada/notcplusplus/pck.adb b/gdb/testsuite/gdb.ada/notcplusplus/pck.adb
> new file mode 100644
> index 0000000..dcfb306
> --- /dev/null
> +++ b/gdb/testsuite/gdb.ada/notcplusplus/pck.adb
> @@ -0,0 +1,21 @@
> +-- Copyright 2018 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/>.
> +
> +package body Pck is
> + procedure Do_Nothing (A : System.Address) is
> + begin
> + null;
> + end Do_Nothing;
> +end Pck;
> diff --git a/gdb/testsuite/gdb.ada/notcplusplus/pck.ads b/gdb/testsuite/gdb.ada/notcplusplus/pck.ads
> new file mode 100644
> index 0000000..33e369e
> --- /dev/null
> +++ b/gdb/testsuite/gdb.ada/notcplusplus/pck.ads
> @@ -0,0 +1,19 @@
> +-- Copyright 2018 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/>.
> +
> +with System;
> +package Pck is
> + procedure Do_Nothing (A : System.Address);
> +end Pck;
> diff --git a/gdb/testsuite/gdb.ada/notcplusplus/ver.ads b/gdb/testsuite/gdb.ada/notcplusplus/ver.ads
> new file mode 100644
> index 0000000..8f264d0
> --- /dev/null
> +++ b/gdb/testsuite/gdb.ada/notcplusplus/ver.ads
> @@ -0,0 +1,22 @@
> +-- Copyright 2018 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/>.
> +
> +package Ver is
> + type Wrapper is record
> + A : Integer;
> + end record;
> + u00045 : constant Wrapper := (A => 16#060287af#);
> + pragma Export (C, u00045, "symada__cS");
> +end Ver;
> diff --git a/gdb/testsuite/gdb.base/c-linkage-name.c b/gdb/testsuite/gdb.base/c-linkage-name.c
> new file mode 100644
> index 0000000..925004c
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/c-linkage-name.c
> @@ -0,0 +1,44 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> + Copyright 2018 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/>. */
> +
> +struct wrapper
> +{
> + int a;
> +};
> +
> +/* Create a global variable whose name in the assembly code
> + (aka the "linkage name") is different from the name in
> + the source code. The goal is to create a symbol described
> + in DWARF using a DW_AT_linkage_name attribute, with a name
> + which follows the C++ mangling.
> +
> + In this particular case, we chose "symada__cS" which, if it were
> + demangled, would translate to "symada (char, signed)". */
> +struct wrapper mundane asm ("symada__cS") = {0x060287af};
> +
> +void
> +do_something (struct wrapper *w)
> +{
> + w->a++;
> +}
> +
> +int
> +main (void)
> +{
> + do_something (&mundane);
> + return 0;
> +}
> diff --git a/gdb/testsuite/gdb.base/c-linkage-name.exp b/gdb/testsuite/gdb.base/c-linkage-name.exp
> new file mode 100644
> index 0000000..c80a530
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/c-linkage-name.exp
> @@ -0,0 +1,47 @@
> +# Copyright 2018 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/>.
> +
> +# This file is part of the gdb testsuite. It is intended to test that
> +# gdb can correctly print arrays with indexes for each element of the
> +# array.
> +
> +standard_testfile .c
> +
> +if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
> + untested "failed to compile"
> + return -1
> +}
> +
> +clean_restart ${binfile}
> +
> +# Try to print MUNDANE, but using its linkage name.
> +
> +gdb_test "print symada__cS" \
> + " = {a = 100829103}" \
> + "print symada__cS before partial symtab expansion"
> +
> +# Force the symbols to be expanded for the unit that contains
> +# our symada__cS symbol by, e.g. inserting a breakpoint on one
> +# of the founction also provided by the same using.
> +
> +gdb_test "break main" \
> + "Breakpoint $decimal at $hex: file .*$srcfile, line $decimal\\."
> +
> +# Try to print MUNDANE using its linkage name again, after partial
> +# symtab expansion.
> +
> +gdb_test "print symada__cS" \
> + " = {a = 100829103}" \
> + "print symada__cS after partial symtab expansion"
> --
> 2.1.4
>
--
Joel
^ permalink raw reply [flat|nested] 27+ messages in thread
* PING^2: [RFA/RFC] fix PR gdb/22670 (pb looking up some symbols when they have a linkage name) (was: "Re: [RFC] regresssion(internal-error) printing subprogram argument")
2018-02-21 3:02 ` PING: " Joel Brobecker
@ 2018-03-19 21:22 ` Joel Brobecker
2018-03-26 14:26 ` PING^2: [RFA/RFC] fix PR gdb/22670 (pb looking up some symbols when they have a linkage name) Pedro Alves
0 siblings, 1 reply; 27+ messages in thread
From: Joel Brobecker @ 2018-03-19 21:22 UTC (permalink / raw)
To: gdb-patches
On Wed, Feb 21, 2018 at 07:02:11AM +0400, Joel Brobecker wrote:
> > > All in all, I think a better solution would be to put that information
> > > directly in the language_defn itself, via a new "attribute".
> > > Something like a...
> >
> > I ended up choosing this approach. lookup names is still a bit
> > foggy for me, so the comments I wrote and/or the name of the
> > new language_defn attribute might be a bit off. As always, I am
> > grateful for suggestions :).
> >
> > gdb/ChangeLog:
> >
> > PR gdb/22670
> > * dwarf2read.c (dwarf2_physname): Do not return the demangled
> > symbol name is the CU's language stores symbol names in linkage
> > format.
> > * language.h (struct language_defn)
> > <la_store_sym_names_in_linkage_form_p>: New field. Adjust
> > all instances of this struct.
> >
> > gdb/testsuite/ChangeLog:
> >
> > * gdb.ada/maint_with_ada.exp: Remove PR gdb/22670 setup_kfail.
> >
> > * gdb.ada/notcplusplus: New testcase.
> >
> > * gdb.base/c-linkage-name.c: New file.
> > * gdb.base/c-linkage-name.exp: New testcase.
> >
> > Tested on x86_64-linux.
> > This also passes AdaCore's internal GDB testsuite.
I'm thinking of pushing this patch at some point, since it has been
waiting for a review for a quite a while, now. If it's lack of time
and you'd like a little more time before I push, please let me know!
> Any comment on this patch?
>
> Thanks!
>
> > >From c22aef84cc86097d6d8b654b0586a3fbc1ff6af3 Mon Sep 17 00:00:00 2001
> > From: Joel Brobecker <brobecker@adacore.com>
> > Date: Fri, 9 Feb 2018 02:13:34 -0500
> > Subject: [PATCH] problem looking up some symbols when they have a linkage name
> >
> > This patch fixes a known failure in gdb.ada/maint_with_ada.exp
> > (maintenance check-psymtabs). Another way to witness the same
> > issue is by considering the following Ada declarations...
> >
> > type Wrapper is record
> > A : Integer;
> > end record;
> > u00045 : constant Wrapper := (A => 16#060287af#);
> > pragma Export (C, u00045, "symada__cS");
> >
> > ... which declares a variable name "u00045" but with a linkage
> > name which is "symada__cS". This variable is a record with one
> > component, the Ada equivalent of a struct with one field in C.
> > Trying to print that variable's value currently yields:
> >
> > (gdb) p /x <symada__cS>
> > 'symada(char, signed)' has unknown type; cast it to its declared type
> >
> > This indicates that GDB was only able to find the minimal symbol,
> > but not the full symbol. The expected output is:
> >
> > (gdb) print /x <symada__cS>
> > $1 = (a => 0x60287af)
> >
> > The error message gives a hint about what's happening: We processed
> > the symbol through gdb_demangle, which in the case of this particular
> > symbol name, ends up matching the C++ naming scheme. As a result,
> > the demangler transforms our symbol name into 'symada(char, signed)',
> > thus breaking Ada lookups.
> >
> > This patch fixes the issue by first introducing a new language_defn
> > attribute called la_store_sym_names_in_linkage_form_p, which is a boolean
> > to be set to true for the few languages that do not want their symbols
> > to have their names stored in demangled form, and false otherwise.
> > We then use this language attribute to skip the call to gdb_demangle
> > for all languages whose la_store_sym_names_in_linkage_form_p is true.
> >
> > In terms of the selection of languages for which the new attribute
> > is set to true, the selection errs on the side of preserving the
> > existing behavior, and only changes the behavior for the languages
> > where we are certain storing symbol names in demangling form is not
> > needed. It is conceivable that other languages might be in the same
> > situation, but I not knowing in detail the symbol name enconding
> > strategy, I decided to play it safe and let other language maintainers
> > potentially adjust their language if it makes sense to do so.
> >
> > gdb/ChangeLog:
> >
> > PR gdb/22670
> > * dwarf2read.c (dwarf2_physname): Do not return the demangled
> > symbol name is the CU's language stores symbol names in linkage
> > format.
> > * language.h (struct language_defn)
> > <la_store_sym_names_in_linkage_form_p>: New field. Adjust
> > all instances of this struct.
> >
> > gdb/testsuite/ChangeLog:
> >
> > * gdb.ada/maint_with_ada.exp: Remove PR gdb/22670 setup_kfail.
> >
> > * gdb.ada/notcplusplus: New testcase.
> >
> > * gdb.base/c-linkage-name.c: New file.
> > * gdb.base/c-linkage-name.exp: New testcase.
> >
> > Tested on x86_64-linux.
> > This also passes AdaCore's internal GDB testsuite.
> > ---
> > gdb/ada-lang.c | 1 +
> > gdb/c-lang.c | 4 +++
> > gdb/d-lang.c | 1 +
> > gdb/dwarf2read.c | 6 +++-
> > gdb/f-lang.c | 1 +
> > gdb/go-lang.c | 1 +
> > gdb/language.c | 2 ++
> > gdb/language.h | 20 +++++++++++++
> > gdb/m2-lang.c | 1 +
> > gdb/objc-lang.c | 1 +
> > gdb/opencl-lang.c | 1 +
> > gdb/p-lang.c | 1 +
> > gdb/rust-lang.c | 1 +
> > gdb/testsuite/gdb.ada/maint_with_ada.exp | 1 -
> > gdb/testsuite/gdb.ada/notcplusplus.exp | 45 ++++++++++++++++++++++++++++
> > gdb/testsuite/gdb.ada/notcplusplus/foo.adb | 21 +++++++++++++
> > gdb/testsuite/gdb.ada/notcplusplus/pck.adb | 21 +++++++++++++
> > gdb/testsuite/gdb.ada/notcplusplus/pck.ads | 19 ++++++++++++
> > gdb/testsuite/gdb.ada/notcplusplus/ver.ads | 22 ++++++++++++++
> > gdb/testsuite/gdb.base/c-linkage-name.c | 44 ++++++++++++++++++++++++++++
> > gdb/testsuite/gdb.base/c-linkage-name.exp | 47 ++++++++++++++++++++++++++++++
> > 21 files changed, 259 insertions(+), 2 deletions(-)
> > create mode 100644 gdb/testsuite/gdb.ada/notcplusplus.exp
> > create mode 100644 gdb/testsuite/gdb.ada/notcplusplus/foo.adb
> > create mode 100644 gdb/testsuite/gdb.ada/notcplusplus/pck.adb
> > create mode 100644 gdb/testsuite/gdb.ada/notcplusplus/pck.ads
> > create mode 100644 gdb/testsuite/gdb.ada/notcplusplus/ver.ads
> > create mode 100644 gdb/testsuite/gdb.base/c-linkage-name.c
> > create mode 100644 gdb/testsuite/gdb.base/c-linkage-name.exp
> >
> > diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
> > index 0da58d9..7f99a2e 100644
> > --- a/gdb/ada-lang.c
> > +++ b/gdb/ada-lang.c
> > @@ -14521,6 +14521,7 @@ extern const struct language_defn ada_language_defn = {
> > ada_read_var_value, /* la_read_var_value */
> > NULL, /* Language specific skip_trampoline */
> > NULL, /* name_of_this */
> > + true, /* la_store_sym_names_in_linkage_form_p */
> > ada_lookup_symbol_nonlocal, /* Looking up non-local symbols. */
> > basic_lookup_transparent_type, /* lookup_transparent_type */
> > ada_la_decode, /* Language specific symbol demangler */
> > diff --git a/gdb/c-lang.c b/gdb/c-lang.c
> > index a0b553e..658c7f7 100644
> > --- a/gdb/c-lang.c
> > +++ b/gdb/c-lang.c
> > @@ -853,6 +853,7 @@ extern const struct language_defn c_language_defn =
> > default_read_var_value, /* la_read_var_value */
> > NULL, /* Language specific skip_trampoline */
> > NULL, /* name_of_this */
> > + true, /* la_store_sym_names_in_linkage_form_p */
> > basic_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */
> > basic_lookup_transparent_type,/* lookup_transparent_type */
> > NULL, /* Language specific symbol demangler */
> > @@ -998,6 +999,7 @@ extern const struct language_defn cplus_language_defn =
> > default_read_var_value, /* la_read_var_value */
> > cplus_skip_trampoline, /* Language specific skip_trampoline */
> > "this", /* name_of_this */
> > + false, /* la_store_sym_names_in_linkage_form_p */
> > cp_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */
> > cp_lookup_transparent_type, /* lookup_transparent_type */
> > gdb_demangle, /* Language specific symbol demangler */
> > @@ -1052,6 +1054,7 @@ extern const struct language_defn asm_language_defn =
> > default_read_var_value, /* la_read_var_value */
> > NULL, /* Language specific skip_trampoline */
> > NULL, /* name_of_this */
> > + true, /* la_store_sym_names_in_linkage_form_p */
> > basic_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */
> > basic_lookup_transparent_type,/* lookup_transparent_type */
> > NULL, /* Language specific symbol demangler */
> > @@ -1106,6 +1109,7 @@ extern const struct language_defn minimal_language_defn =
> > default_read_var_value, /* la_read_var_value */
> > NULL, /* Language specific skip_trampoline */
> > NULL, /* name_of_this */
> > + true, /* la_store_sym_names_in_linkage_form_p */
> > basic_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */
> > basic_lookup_transparent_type,/* lookup_transparent_type */
> > NULL, /* Language specific symbol demangler */
> > diff --git a/gdb/d-lang.c b/gdb/d-lang.c
> > index e0afe48..688ae98 100644
> > --- a/gdb/d-lang.c
> > +++ b/gdb/d-lang.c
> > @@ -229,6 +229,7 @@ extern const struct language_defn d_language_defn =
> > default_read_var_value, /* la_read_var_value */
> > NULL, /* Language specific skip_trampoline. */
> > "this",
> > + false, /* la_store_sym_names_in_linkage_form_p */
> > d_lookup_symbol_nonlocal,
> > basic_lookup_transparent_type,
> > d_demangle, /* Language specific symbol demangler. */
> > diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> > index d651725..b4c087f 100644
> > --- a/gdb/dwarf2read.c
> > +++ b/gdb/dwarf2read.c
> > @@ -11142,7 +11142,11 @@ dwarf2_physname (const char *name, struct die_info *die, struct dwarf2_cu *cu)
> > if (mangled != NULL)
> > {
> >
> > - if (cu->language == language_go)
> > + if (language_def (cu->language)->la_store_sym_names_in_linkage_form_p)
> > + {
> > + /* Do nothing (do not demangle the symbol name). */
> > + }
> > + else if (cu->language == language_go)
> > {
> > /* This is a lie, but we already lie to the caller new_symbol.
> > new_symbol assumes we return the mangled name.
> > diff --git a/gdb/f-lang.c b/gdb/f-lang.c
> > index 74f5622..81922f7 100644
> > --- a/gdb/f-lang.c
> > +++ b/gdb/f-lang.c
> > @@ -269,6 +269,7 @@ extern const struct language_defn f_language_defn =
> > default_read_var_value, /* la_read_var_value */
> > NULL, /* Language specific skip_trampoline */
> > NULL, /* name_of_this */
> > + false, /* la_store_sym_names_in_linkage_form_p */
> > cp_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */
> > basic_lookup_transparent_type,/* lookup_transparent_type */
> >
> > diff --git a/gdb/go-lang.c b/gdb/go-lang.c
> > index 022b8de..e9cba77 100644
> > --- a/gdb/go-lang.c
> > +++ b/gdb/go-lang.c
> > @@ -590,6 +590,7 @@ extern const struct language_defn go_language_defn =
> > default_read_var_value, /* la_read_var_value */
> > NULL, /* Language specific skip_trampoline. */
> > NULL, /* name_of_this */
> > + false, /* la_store_sym_names_in_linkage_form_p */
> > basic_lookup_symbol_nonlocal,
> > basic_lookup_transparent_type,
> > go_demangle, /* Language specific symbol demangler. */
> > diff --git a/gdb/language.c b/gdb/language.c
> > index 0d8604b..22199e0 100644
> > --- a/gdb/language.c
> > +++ b/gdb/language.c
> > @@ -864,6 +864,7 @@ const struct language_defn unknown_language_defn =
> > default_read_var_value, /* la_read_var_value */
> > unk_lang_trampoline, /* Language specific skip_trampoline */
> > "this", /* name_of_this */
> > + true, /* store_sym_names_in_linkage_form_p */
> > basic_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */
> > basic_lookup_transparent_type,/* lookup_transparent_type */
> > unk_lang_demangle, /* Language specific symbol demangler */
> > @@ -915,6 +916,7 @@ const struct language_defn auto_language_defn =
> > default_read_var_value, /* la_read_var_value */
> > unk_lang_trampoline, /* Language specific skip_trampoline */
> > "this", /* name_of_this */
> > + false, /* store_sym_names_in_linkage_form_p */
> > basic_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */
> > basic_lookup_transparent_type,/* lookup_transparent_type */
> > unk_lang_demangle, /* Language specific symbol demangler */
> > diff --git a/gdb/language.h b/gdb/language.h
> > index 06b42ae..029de4a 100644
> > --- a/gdb/language.h
> > +++ b/gdb/language.h
> > @@ -264,6 +264,26 @@ struct language_defn
> >
> > const char *la_name_of_this;
> >
> > + /* True if the symbols names should be stored in GDB's data structures
> > + for minimal/partial/full symbols using their linkage (aka mangled)
> > + form; false if the symbol names should be demangled first.
> > +
> > + Most languages implement symbol lookup by comparing the demangled
> > + names, in which case it is advantageous to store that information
> > + already demangled, and so would set this field to false.
> > +
> > + On the other hand, some languages have opted for doing symbol
> > + lookups by comparing mangled names instead, for reasons usually
> > + specific to the language. Those languages should set this field
> > + to true.
> > +
> > + And finally, other languages such as C or Asm do not have
> > + the concept of mangled vs demangled name, so those languages
> > + should set this field to true as well, to prevent any accidental
> > + demangling through an unrelated language's demangler. */
> > +
> > + const bool la_store_sym_names_in_linkage_form_p;
> > +
> > /* This is a function that lookup_symbol will call when it gets to
> > the part of symbol lookup where C looks up static and global
> > variables. */
> > diff --git a/gdb/m2-lang.c b/gdb/m2-lang.c
> > index 11ccab3..6e6434b 100644
> > --- a/gdb/m2-lang.c
> > +++ b/gdb/m2-lang.c
> > @@ -377,6 +377,7 @@ extern const struct language_defn m2_language_defn =
> > default_read_var_value, /* la_read_var_value */
> > NULL, /* Language specific skip_trampoline */
> > NULL, /* name_of_this */
> > + false, /* la_store_sym_names_in_linkage_form_p */
> > basic_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */
> > basic_lookup_transparent_type,/* lookup_transparent_type */
> > NULL, /* Language specific symbol demangler */
> > diff --git a/gdb/objc-lang.c b/gdb/objc-lang.c
> > index c714ec3..4f7dc36 100644
> > --- a/gdb/objc-lang.c
> > +++ b/gdb/objc-lang.c
> > @@ -389,6 +389,7 @@ extern const struct language_defn objc_language_defn = {
> > default_read_var_value, /* la_read_var_value */
> > objc_skip_trampoline, /* Language specific skip_trampoline */
> > "self", /* name_of_this */
> > + false, /* la_store_sym_names_in_linkage_form_p */
> > basic_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */
> > basic_lookup_transparent_type,/* lookup_transparent_type */
> > objc_demangle, /* Language specific symbol demangler */
> > diff --git a/gdb/opencl-lang.c b/gdb/opencl-lang.c
> > index 268c3c5..9c5b90c 100644
> > --- a/gdb/opencl-lang.c
> > +++ b/gdb/opencl-lang.c
> > @@ -1065,6 +1065,7 @@ extern const struct language_defn opencl_language_defn =
> > default_read_var_value, /* la_read_var_value */
> > NULL, /* Language specific skip_trampoline */
> > NULL, /* name_of_this */
> > + false, /* la_store_sym_names_in_linkage_form_p */
> > basic_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */
> > basic_lookup_transparent_type,/* lookup_transparent_type */
> > NULL, /* Language specific symbol demangler */
> > diff --git a/gdb/p-lang.c b/gdb/p-lang.c
> > index 03db2df..3ff7f56 100644
> > --- a/gdb/p-lang.c
> > +++ b/gdb/p-lang.c
> > @@ -439,6 +439,7 @@ extern const struct language_defn pascal_language_defn =
> > default_read_var_value, /* la_read_var_value */
> > NULL, /* Language specific skip_trampoline */
> > "this", /* name_of_this */
> > + false, /* la_store_sym_names_in_linkage_form_p */
> > basic_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */
> > basic_lookup_transparent_type,/* lookup_transparent_type */
> > NULL, /* Language specific symbol demangler */
> > diff --git a/gdb/rust-lang.c b/gdb/rust-lang.c
> > index 5ff80b2..2d7c1f7 100644
> > --- a/gdb/rust-lang.c
> > +++ b/gdb/rust-lang.c
> > @@ -2290,6 +2290,7 @@ extern const struct language_defn rust_language_defn =
> > default_read_var_value, /* la_read_var_value */
> > NULL, /* Language specific skip_trampoline */
> > NULL, /* name_of_this */
> > + false, /* la_store_sym_names_in_linkage_form_p */
> > rust_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */
> > basic_lookup_transparent_type,/* lookup_transparent_type */
> > gdb_demangle, /* Language specific symbol demangler */
> > diff --git a/gdb/testsuite/gdb.ada/maint_with_ada.exp b/gdb/testsuite/gdb.ada/maint_with_ada.exp
> > index 73da613..9ede035 100644
> > --- a/gdb/testsuite/gdb.ada/maint_with_ada.exp
> > +++ b/gdb/testsuite/gdb.ada/maint_with_ada.exp
> > @@ -32,7 +32,6 @@ gdb_breakpoint "adainit"
> > gdb_breakpoint "Var_Arr_Typedef"
> > gdb_breakpoint "Do_Nothing"
> >
> > -setup_kfail gdb/22670 "*-*-*"
> > gdb_test_no_output "maintenance check-psymtabs"
> >
> > gdb_test_no_output "maintenance check-symtabs"
> > diff --git a/gdb/testsuite/gdb.ada/notcplusplus.exp b/gdb/testsuite/gdb.ada/notcplusplus.exp
> > new file mode 100644
> > index 0000000..b2a24e8
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.ada/notcplusplus.exp
> > @@ -0,0 +1,45 @@
> > +# Copyright 2018 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/>.
> > +
> > +load_lib "ada.exp"
> > +
> > +if { [skip_ada_tests] } { return -1 }
> > +
> > +standard_ada_testfile foo
> > +
> > +if {[gdb_compile_ada "${srcfile}" "${binfile}" executable {debug}] != ""} {
> > + return -1
> > +}
> > +
> > +clean_restart ${testfile}
> > +
> > +gdb_test "print /x <symada__cS>" \
> > + "= \\(a => 0x60287af\\)" \
> > + "print <symada__cS> before loading symbols from ver.ads"
> > +
> > +# Force the partial symbosl from ver.ads to be expanded into full symbols.
> > +
> > +gdb_test \
> > + "list ver.ads:16" \
> > + [multi_line ".*" \
> > + "16\\s+package Ver is" \
> > + "17\\s+type Wrapper is record" \
> > + "18\\s+A : Integer;" \
> > + "19\\s+end record;" \
> > + "20\\s+u00045 : constant Wrapper := \\(A => 16#060287af#\\);"]
> > +
> > +gdb_test "print /x <symada__cS>" \
> > + "= \\(a => 0x60287af\\)" \
> > + "print <symada__cS> after loading symbols from ver.ads"
> > diff --git a/gdb/testsuite/gdb.ada/notcplusplus/foo.adb b/gdb/testsuite/gdb.ada/notcplusplus/foo.adb
> > new file mode 100644
> > index 0000000..89e42f9
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.ada/notcplusplus/foo.adb
> > @@ -0,0 +1,21 @@
> > +-- Copyright 2018 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/>.
> > +
> > +with Pck; use Pck;
> > +with Ver; use Ver;
> > +procedure Foo is
> > +begin
> > + Do_Nothing (u00045'Address);
> > +end Foo;
> > diff --git a/gdb/testsuite/gdb.ada/notcplusplus/pck.adb b/gdb/testsuite/gdb.ada/notcplusplus/pck.adb
> > new file mode 100644
> > index 0000000..dcfb306
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.ada/notcplusplus/pck.adb
> > @@ -0,0 +1,21 @@
> > +-- Copyright 2018 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/>.
> > +
> > +package body Pck is
> > + procedure Do_Nothing (A : System.Address) is
> > + begin
> > + null;
> > + end Do_Nothing;
> > +end Pck;
> > diff --git a/gdb/testsuite/gdb.ada/notcplusplus/pck.ads b/gdb/testsuite/gdb.ada/notcplusplus/pck.ads
> > new file mode 100644
> > index 0000000..33e369e
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.ada/notcplusplus/pck.ads
> > @@ -0,0 +1,19 @@
> > +-- Copyright 2018 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/>.
> > +
> > +with System;
> > +package Pck is
> > + procedure Do_Nothing (A : System.Address);
> > +end Pck;
> > diff --git a/gdb/testsuite/gdb.ada/notcplusplus/ver.ads b/gdb/testsuite/gdb.ada/notcplusplus/ver.ads
> > new file mode 100644
> > index 0000000..8f264d0
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.ada/notcplusplus/ver.ads
> > @@ -0,0 +1,22 @@
> > +-- Copyright 2018 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/>.
> > +
> > +package Ver is
> > + type Wrapper is record
> > + A : Integer;
> > + end record;
> > + u00045 : constant Wrapper := (A => 16#060287af#);
> > + pragma Export (C, u00045, "symada__cS");
> > +end Ver;
> > diff --git a/gdb/testsuite/gdb.base/c-linkage-name.c b/gdb/testsuite/gdb.base/c-linkage-name.c
> > new file mode 100644
> > index 0000000..925004c
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.base/c-linkage-name.c
> > @@ -0,0 +1,44 @@
> > +/* This testcase is part of GDB, the GNU debugger.
> > +
> > + Copyright 2018 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/>. */
> > +
> > +struct wrapper
> > +{
> > + int a;
> > +};
> > +
> > +/* Create a global variable whose name in the assembly code
> > + (aka the "linkage name") is different from the name in
> > + the source code. The goal is to create a symbol described
> > + in DWARF using a DW_AT_linkage_name attribute, with a name
> > + which follows the C++ mangling.
> > +
> > + In this particular case, we chose "symada__cS" which, if it were
> > + demangled, would translate to "symada (char, signed)". */
> > +struct wrapper mundane asm ("symada__cS") = {0x060287af};
> > +
> > +void
> > +do_something (struct wrapper *w)
> > +{
> > + w->a++;
> > +}
> > +
> > +int
> > +main (void)
> > +{
> > + do_something (&mundane);
> > + return 0;
> > +}
> > diff --git a/gdb/testsuite/gdb.base/c-linkage-name.exp b/gdb/testsuite/gdb.base/c-linkage-name.exp
> > new file mode 100644
> > index 0000000..c80a530
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.base/c-linkage-name.exp
> > @@ -0,0 +1,47 @@
> > +# Copyright 2018 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/>.
> > +
> > +# This file is part of the gdb testsuite. It is intended to test that
> > +# gdb can correctly print arrays with indexes for each element of the
> > +# array.
> > +
> > +standard_testfile .c
> > +
> > +if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
> > + untested "failed to compile"
> > + return -1
> > +}
> > +
> > +clean_restart ${binfile}
> > +
> > +# Try to print MUNDANE, but using its linkage name.
> > +
> > +gdb_test "print symada__cS" \
> > + " = {a = 100829103}" \
> > + "print symada__cS before partial symtab expansion"
> > +
> > +# Force the symbols to be expanded for the unit that contains
> > +# our symada__cS symbol by, e.g. inserting a breakpoint on one
> > +# of the founction also provided by the same using.
> > +
> > +gdb_test "break main" \
> > + "Breakpoint $decimal at $hex: file .*$srcfile, line $decimal\\."
> > +
> > +# Try to print MUNDANE using its linkage name again, after partial
> > +# symtab expansion.
> > +
> > +gdb_test "print symada__cS" \
> > + " = {a = 100829103}" \
> > + "print symada__cS after partial symtab expansion"
> > --
> > 2.1.4
> >
>
>
> --
> Joel
--
Joel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: PING^2: [RFA/RFC] fix PR gdb/22670 (pb looking up some symbols when they have a linkage name)
2018-03-19 21:22 ` PING^2: " Joel Brobecker
@ 2018-03-26 14:26 ` Pedro Alves
2018-03-27 14:02 ` Joel Brobecker
0 siblings, 1 reply; 27+ messages in thread
From: Pedro Alves @ 2018-03-26 14:26 UTC (permalink / raw)
To: Joel Brobecker, gdb-patches
Hi Joel,
On 03/19/2018 09:21 PM, Joel Brobecker wrote:
> On Wed, Feb 21, 2018 at 07:02:11AM +0400, Joel Brobecker wrote:
>>>> All in all, I think a better solution would be to put that information
>>>> directly in the language_defn itself, via a new "attribute".
>>>> Something like a...
>>>
>>> I ended up choosing this approach. lookup names is still a bit
>>> foggy for me, so the comments I wrote and/or the name of the
>>> new language_defn attribute might be a bit off. As always, I am
>>> grateful for suggestions :).
I looked again at this today, and I couldn't think of a better
option right now. So I think you should go ahead and push this in.
Note that this C++ mangled form like in "symada__cS", is not
actually how C++ symbols are mangled nowadays. The modern Itanium
mangling scheme has linkage names that always start with "_Z". In this
case, "void symada(char, signed)" mangles as "_Z6symadaci".
It's possible that we no longer need to support that older (and ambiguous
with Ada and other languages) mangling scheme for C++, I don't know.
I wish Ada linkage names also had some kind of leading prefix, but
that can't happen without an ABI break, of course...
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: PING^2: [RFA/RFC] fix PR gdb/22670 (pb looking up some symbols when they have a linkage name)
2018-03-26 14:26 ` PING^2: [RFA/RFC] fix PR gdb/22670 (pb looking up some symbols when they have a linkage name) Pedro Alves
@ 2018-03-27 14:02 ` Joel Brobecker
0 siblings, 0 replies; 27+ messages in thread
From: Joel Brobecker @ 2018-03-27 14:02 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
> >>> I ended up choosing this approach. lookup names is still a bit
> >>> foggy for me, so the comments I wrote and/or the name of the
> >>> new language_defn attribute might be a bit off. As always, I am
> >>> grateful for suggestions :).
>
> I looked again at this today, and I couldn't think of a better
> option right now. So I think you should go ahead and push this in.
Thanks, Pedro.
I re-tested the patch on x86_64-linux, and then pushed it to master.
> Note that this C++ mangled form like in "symada__cS", is not
> actually how C++ symbols are mangled nowadays. The modern Itanium
> mangling scheme has linkage names that always start with "_Z". In this
> case, "void symada(char, signed)" mangles as "_Z6symadaci".
Interesting.
> It's possible that we no longer need to support that older (and
> ambiguous with Ada and other languages) mangling scheme for C++, I
> don't know. I wish Ada linkage names also had some kind of leading
> prefix, but that can't happen without an ABI break, of course...
Yeah, it's a good idea, but making that kind of change happen is
going to be tough, especially since we know there are consumers
of our debugging information that we have no control of. I'll mention
it to the compiler team, though.
Thanks Pedro.
--
Joel
^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2018-03-27 14:02 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-13 10:37 [RFC] regresssion(internal-error) printing subprogram argument Joel Brobecker
2017-12-14 19:02 ` Pedro Alves
2017-12-14 19:41 ` Pedro Alves
2017-12-15 9:48 ` Joel Brobecker
2017-12-15 11:02 ` Pedro Alves
2017-12-15 18:57 ` Pedro Alves
2018-01-03 4:33 ` Joel Brobecker
2018-01-04 9:48 ` Joel Brobecker
2018-01-04 12:39 ` Pedro Alves
2018-01-05 16:28 ` Pedro Alves
2018-01-16 11:54 ` Pedro Alves
2018-01-17 9:13 ` Joel Brobecker
2018-01-26 3:51 ` Joel Brobecker
2018-01-26 11:28 ` Pedro Alves
2018-01-29 10:38 ` Joel Brobecker
2018-01-29 15:33 ` Pedro Alves
2018-01-29 15:52 ` Joel Brobecker
2018-01-30 3:56 ` Joel Brobecker
2018-02-05 9:57 ` Joel Brobecker
2018-02-09 9:09 ` [RFA/RFC] fix PR gdb/22670 (pb looking up some symbols when they have a linkage name) (was: "Re: [RFC] regresssion(internal-error) printing subprogram argument") Joel Brobecker
2018-02-21 3:02 ` PING: " Joel Brobecker
2018-03-19 21:22 ` PING^2: " Joel Brobecker
2018-03-26 14:26 ` PING^2: [RFA/RFC] fix PR gdb/22670 (pb looking up some symbols when they have a linkage name) Pedro Alves
2018-03-27 14:02 ` Joel Brobecker
2018-01-26 4:50 ` [RFC] regresssion(internal-error) printing subprogram argument Joel Brobecker
2017-12-15 18:31 ` Pedro Alves
2017-12-15 7:54 ` Joel Brobecker
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).