This is the mail archive of the gdb-prs@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[Bug gdb/22670] regressions in Ada caused by introduction of wild matching in C++ patch series


https://sourceware.org/bugzilla/show_bug.cgi?id=22670

--- Comment #8 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
The gdb-8.1-branch branch has been updated by Pedro Alves
<palves@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=a76cf1f43d27f022562d9e70eb91e212dbd0da4a

commit a76cf1f43d27f022562d9e70eb91e212dbd0da4a
Author: Pedro Alves <palves@redhat.com>
Date:   Fri Jan 5 16:19:17 2018 +0000

    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.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]