This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFC] regresssion(internal-error) printing subprogram argument
- From: Pedro Alves <palves at redhat dot com>
- To: Joel Brobecker <brobecker at adacore dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Thu, 14 Dec 2017 19:02:44 +0000
- Subject: Re: [RFC] regresssion(internal-error) printing subprogram argument
- Authentication-results: sourceware.org; auth=none
- References: <20171213103655.msbaxfrykc36f4a7@adacore.com>
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
>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