[RFA/commit] (Ada) fix GDB crash printing packed array

Message ID 1549786901-77868-1-git-send-email-brobecker@adacore.com
State New
Headers show
Series
  • [RFA/commit] (Ada) fix GDB crash printing packed array
Related show

Commit Message

Joel Brobecker Feb. 10, 2019, 8:21 a.m.
Hello,

This touches on symbol searching, and in particular the ability
to search for an exact match...

Trying to print a packed array sometimes leads to a crash (see
attached testcase for an example of when this happens):

  | (gdb) p bad
  | [1]    65571 segmentation fault  gdb -q foo

Variable "bad" is declared in the debug information as an array where
the array's type name has an XPnnn suffix:

  | .uleb128 0xc    # (DIE (0x566) DW_TAG_typedef)
  | .long   .LASF200        # DW_AT_name: "pck__t___XP1"
  | [loc info attributes snipped]
  | .long   0x550   # DW_AT_type
  | .byte   0x1     # DW_AT_alignment

The signals to GDB that the debugging information follows a GNAT encoding
used for packed arrays, and an in order to decode it, we need to find
the type whose name is the same minus the "___XPnnn" suffix: "pck__t".

For that, we make a call to ada-lang.c::standard_lookup, which is
a simple function which essentially does:

  | /* Return the result of a standard (literal, C-like) lookup of NAME in
  |    given DOMAIN, visible from lexical block BLOCK.  */
  |
  |   [...]
  |   sym = lookup_symbol_in_language (name, block, domain, language_c, 0);

Unfortunately for us, while the intent of this call was to perform
an exact-match lookup, in our case, it returns ... type pck__t___XP1
instead! In other words, it finds itself back. The reason why it finds
this type is a confluence of two factors:

  (1) Forcing the lookup into language_c currently does not affect
      how symbol matching is done anymore, because we look at the symbol's
      language to determine which kind of matching should be done;

  (2) The lookup searches the local context (via block) first, beforei
      doing a more general lookup. And looking at the debug info for
      the main subprogram, we see that type "pck__t" is not declared
      there, only in the debug info for pck.ads. In other words,
      there is no way that we accidently find "pck__t" by random chance.

I believe Pedro added a new function called ada_lookup_encoded_symbol
for that specific purpose, so I started by replacing the lookup
by language above by this. Unfortunately, still no joy.

This was because, even though ada_lookup_encoded_symbol puts angle-
brackets around the search name to signal that we want a verbatim
search, we end up losing that information in the function called
to compare a symbol with the search name:

  | static bool
  | do_full_match (const char *symbol_search_name,
  |                const lookup_name_info &lookup_name,
  |                completion_match_result *comp_match_res)
  | {
  |   return full_match (symbol_search_name, ada_lookup_name (lookup_name));
                                             ^^^^^^^^^^^^^^^
                                                    |
                                    <=> lookup_name.m_ada.m_encoded_name
                                           (no angle brackets)

The way I fixed this was by introducing a new function called
do_exact_match, and then adjust ada_get_symbol_name_matcher to
return that function when seeing that we have a verbatim non-wild-match
search.

As it happens, this fixes an incorrect test in gdb.ada/homony.exp,
where we were inserting a breakpoint on a symbol using the angle-brackets
notation, and got 2 locations for that breakpoint...

    (gdb) b <homonym__get_value>
    Breakpoint 1 at 0x4029fc: <homonym__get_value>. (2 locations)

...  each location being in a different function:

    (gdb) info break
    Num     Type           Disp Enb Address            What
    1       breakpoint     keep y   <MULTIPLE>
    1.1                         y   0x00000000004029fc in homonym.get_value
                                    at /[...]/homonym.adb:32
    1.2                         y   0x0000000000402a3a in homonym.get_value
                                    at /[...]/homonym.adb:50
    (gdb) x /i 0x00000000004029fc
       0x4029fc <homonym__get_value+8>:     movl   $0x1d,-0x4(%rbp)
    (gdb) x /i 0x0000000000402a3a
       0x402a3a <homonym__get_value__2+8>:  movl   $0x11,-0x4(%rbp)

Since we used angle-brackets, we shouldn't be matching the second one,
something this patch fixes.

gdb/ChangeLog:

        * ada-lang.c (standard_lookup): Use ada_lookup_encoded_symbol
        instead of lookup_symbol_in_language
        (do_exact_match): New function.
        (ada_get_symbol_name_matcher): Return do_exact_match when
        doing a verbatim match.

gdb/testsuite/ChangeLog:

        * gdb.ada/big_packed_array: New testcase.
        * gdb.ada/homonym.exp: Fix incorrect expected output for
        "break <homonym__get_value>" test.

Tested on x86_64-linux.  OK to commit?

Thanks,
-- 
Joel


---
 gdb/ada-lang.c                                     | 14 +++++-
 gdb/testsuite/gdb.ada/big_packed_array.exp         | 35 ++++++++++++++
 .../gdb.ada/big_packed_array/foo_ra24_010.adb      | 24 ++++++++++
 gdb/testsuite/gdb.ada/big_packed_array/pck.adb     | 21 +++++++++
 gdb/testsuite/gdb.ada/big_packed_array/pck.ads     | 53 ++++++++++++++++++++++
 gdb/testsuite/gdb.ada/homonym.exp                  |  2 +-
 6 files changed, 147 insertions(+), 2 deletions(-)
 create mode 100644 gdb/testsuite/gdb.ada/big_packed_array.exp
 create mode 100644 gdb/testsuite/gdb.ada/big_packed_array/foo_ra24_010.adb
 create mode 100644 gdb/testsuite/gdb.ada/big_packed_array/pck.adb
 create mode 100644 gdb/testsuite/gdb.ada/big_packed_array/pck.ads

-- 
2.1.4

Comments

Tom Tromey Feb. 15, 2019, 7:49 p.m. | #1
>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:


Joel>   (1) Forcing the lookup into language_c currently does not affect
Joel>       how symbol matching is done anymore, because we look at the symbol's
Joel>       language to determine which kind of matching should be done;

There's also this code in get_symbol_name_matcher:

  /* If currently in Ada mode, and the lookup name is wrapped in
     '<...>', hijack all symbol name comparisons using the Ada
     matcher, which handles the verbatim matching.  */
  if (current_language->la_language == language_ada
      && lookup_name.ada ().verbatim_p ())
    return current_language->la_get_symbol_name_matcher (lookup_name);

I don't know if this could be removed, but if so it would be better to
do so.

The patch looks fine to me.

Tom
Joel Brobecker Feb. 17, 2019, 1:33 p.m. | #2
> Joel>   (1) Forcing the lookup into language_c currently does not affect

> Joel>       how symbol matching is done anymore, because we look at the symbol's

> Joel>       language to determine which kind of matching should be done;

> 

> There's also this code in get_symbol_name_matcher:

> 

>   /* If currently in Ada mode, and the lookup name is wrapped in

>      '<...>', hijack all symbol name comparisons using the Ada

>      matcher, which handles the verbatim matching.  */

>   if (current_language->la_language == language_ada

>       && lookup_name.ada ().verbatim_p ())

>     return current_language->la_get_symbol_name_matcher (lookup_name);

> 

> I don't know if this could be removed, but if so it would be better to

> do so.


I was absolutely certain, looking at the new code, that we could
remove this, but unfortunately, it turns out there is a regression.

Before removing:

    (gdb) p <MixedCaseFunc>
    $1 = void^M

After:

    (gdb) p <MixedCaseFunc>
    $1 = {<text variable, no debug info>} 0x4029c6 <MixedCaseFunc>

This tells me that the function symbol wasn't found (in the debuging
info), so we used the minsym instead.  Not sure if this is because
the verbatim_p flag might be missing, or because of something else.

We'll put this on our list of things to investigate, one of these days.

> The patch looks fine to me.


Thanks! Now pushed to master.

-- 
Joel
Pedro Franco de Carvalho May 10, 2019, 8:51 p.m. | #3
Hello,

I was running some regression tests for gdb 8.3, and I noticed that
there was a regression introduced by this patch for test
gdb.ada/pckd_arr_ren.exp.

I tried investigating, and it seems that this happened due to the
compiler in my system being relatively old (GNAT 4.9.3).  I don't know
much about Ada, so I couldn't figure out if this is an issue with the
debug symbols generated by the older compiler or if GDB should handle
this, but it worked before the patch.

This is the relevant output from the test:

(gdb) break foo.adb:22
Breakpoint 1 at 0x10002410: file /home/pedromfc/binutils-gdb/gdb/testsuite/gdb.ada/pckd_arr_ren/foo.adb, line 22.
(gdb) run 
Starting program: /home/pedromfc/binutils-gdb/build-bisect/gdb/testsuite/outputs/gdb.ada/pckd_arr_ren/foo 

Breakpoint 1, foo () at /home/pedromfc/binutils-gdb/gdb/testsuite/gdb.ada/pckd_arr_ren/foo.adb:22
22         Do_Nothing (A2'Address); -- STOP
(gdb) print A2
Could not find renamed variable: a1
(gdb) FAIL: gdb.ada/pckd_arr_ren.exp: print var

That error message seems to come from ada-exp.y:write_object_renaming.

Thanks!

--
Pedro Franco de Carvalho

Joel Brobecker <brobecker@adacore.com> writes:

> Hello,

>

> This touches on symbol searching, and in particular the ability

> to search for an exact match...

>

> Trying to print a packed array sometimes leads to a crash (see

> attached testcase for an example of when this happens):

>

>   | (gdb) p bad

>   | [1]    65571 segmentation fault  gdb -q foo

>

> Variable "bad" is declared in the debug information as an array where

> the array's type name has an XPnnn suffix:

>

>   | .uleb128 0xc    # (DIE (0x566) DW_TAG_typedef)

>   | .long   .LASF200        # DW_AT_name: "pck__t___XP1"

>   | [loc info attributes snipped]

>   | .long   0x550   # DW_AT_type

>   | .byte   0x1     # DW_AT_alignment

>

> The signals to GDB that the debugging information follows a GNAT encoding

> used for packed arrays, and an in order to decode it, we need to find

> the type whose name is the same minus the "___XPnnn" suffix: "pck__t".

>

> For that, we make a call to ada-lang.c::standard_lookup, which is

> a simple function which essentially does:

>

>   | /* Return the result of a standard (literal, C-like) lookup of NAME in

>   |    given DOMAIN, visible from lexical block BLOCK.  */

>   |

>   |   [...]

>   |   sym = lookup_symbol_in_language (name, block, domain, language_c, 0);

>

> Unfortunately for us, while the intent of this call was to perform

> an exact-match lookup, in our case, it returns ... type pck__t___XP1

> instead! In other words, it finds itself back. The reason why it finds

> this type is a confluence of two factors:

>

>   (1) Forcing the lookup into language_c currently does not affect

>       how symbol matching is done anymore, because we look at the symbol's

>       language to determine which kind of matching should be done;

>

>   (2) The lookup searches the local context (via block) first, beforei

>       doing a more general lookup. And looking at the debug info for

>       the main subprogram, we see that type "pck__t" is not declared

>       there, only in the debug info for pck.ads. In other words,

>       there is no way that we accidently find "pck__t" by random chance.

>

> I believe Pedro added a new function called ada_lookup_encoded_symbol

> for that specific purpose, so I started by replacing the lookup

> by language above by this. Unfortunately, still no joy.

>

> This was because, even though ada_lookup_encoded_symbol puts angle-

> brackets around the search name to signal that we want a verbatim

> search, we end up losing that information in the function called

> to compare a symbol with the search name:

>

>   | static bool

>   | do_full_match (const char *symbol_search_name,

>   |                const lookup_name_info &lookup_name,

>   |                completion_match_result *comp_match_res)

>   | {

>   |   return full_match (symbol_search_name, ada_lookup_name (lookup_name));

>                                              ^^^^^^^^^^^^^^^

>                                                     |

>                                     <=> lookup_name.m_ada.m_encoded_name

>                                            (no angle brackets)

>

> The way I fixed this was by introducing a new function called

> do_exact_match, and then adjust ada_get_symbol_name_matcher to

> return that function when seeing that we have a verbatim non-wild-match

> search.

>

> As it happens, this fixes an incorrect test in gdb.ada/homony.exp,

> where we were inserting a breakpoint on a symbol using the angle-brackets

> notation, and got 2 locations for that breakpoint...

>

>     (gdb) b <homonym__get_value>

>     Breakpoint 1 at 0x4029fc: <homonym__get_value>. (2 locations)

>

> ...  each location being in a different function:

>

>     (gdb) info break

>     Num     Type           Disp Enb Address            What

>     1       breakpoint     keep y   <MULTIPLE>

>     1.1                         y   0x00000000004029fc in homonym.get_value

>                                     at /[...]/homonym.adb:32

>     1.2                         y   0x0000000000402a3a in homonym.get_value

>                                     at /[...]/homonym.adb:50

>     (gdb) x /i 0x00000000004029fc

>        0x4029fc <homonym__get_value+8>:     movl   $0x1d,-0x4(%rbp)

>     (gdb) x /i 0x0000000000402a3a

>        0x402a3a <homonym__get_value__2+8>:  movl   $0x11,-0x4(%rbp)

>

> Since we used angle-brackets, we shouldn't be matching the second one,

> something this patch fixes.

>

> gdb/ChangeLog:

>

>         * ada-lang.c (standard_lookup): Use ada_lookup_encoded_symbol

>         instead of lookup_symbol_in_language

>         (do_exact_match): New function.

>         (ada_get_symbol_name_matcher): Return do_exact_match when

>         doing a verbatim match.

>

> gdb/testsuite/ChangeLog:

>

>         * gdb.ada/big_packed_array: New testcase.

>         * gdb.ada/homonym.exp: Fix incorrect expected output for

>         "break <homonym__get_value>" test.

>

> Tested on x86_64-linux.  OK to commit?

>

> Thanks,

> -- 

> Joel

>

>

> ---

>  gdb/ada-lang.c                                     | 14 +++++-

>  gdb/testsuite/gdb.ada/big_packed_array.exp         | 35 ++++++++++++++

>  .../gdb.ada/big_packed_array/foo_ra24_010.adb      | 24 ++++++++++

>  gdb/testsuite/gdb.ada/big_packed_array/pck.adb     | 21 +++++++++

>  gdb/testsuite/gdb.ada/big_packed_array/pck.ads     | 53 ++++++++++++++++++++++

>  gdb/testsuite/gdb.ada/homonym.exp                  |  2 +-

>  6 files changed, 147 insertions(+), 2 deletions(-)

>  create mode 100644 gdb/testsuite/gdb.ada/big_packed_array.exp

>  create mode 100644 gdb/testsuite/gdb.ada/big_packed_array/foo_ra24_010.adb

>  create mode 100644 gdb/testsuite/gdb.ada/big_packed_array/pck.adb

>  create mode 100644 gdb/testsuite/gdb.ada/big_packed_array/pck.ads

>

> diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c

> index a878d4d..415dc53 100644

> --- a/gdb/ada-lang.c

> +++ b/gdb/ada-lang.c

> @@ -4760,7 +4760,7 @@ standard_lookup (const char *name, const struct block *block,

>

>    if (lookup_cached_symbol (name, domain, &sym.symbol, NULL))

>      return sym.symbol;

> -  sym = lookup_symbol_in_language (name, block, domain, language_c, 0);

> +  ada_lookup_encoded_symbol (name, block, domain, &sym);

>    cache_symbol (name, domain, sym.symbol, sym.block);

>    return sym.symbol;

>  }

> @@ -14193,6 +14193,16 @@ do_full_match (const char *symbol_search_name,

>    return full_match (symbol_search_name, ada_lookup_name (lookup_name));

>  }

>

> +/* symbol_name_matcher_ftype for exact (verbatim) matches.  */

> +

> +static bool

> +do_exact_match (const char *symbol_search_name,

> +		const lookup_name_info &lookup_name,

> +		completion_match_result *comp_match_res)

> +{

> +  return strcmp (symbol_search_name, ada_lookup_name (lookup_name)) == 0;

> +}

> +

>  /* Build the Ada lookup name for LOOKUP_NAME.  */

>

>  ada_lookup_name_info::ada_lookup_name_info (const lookup_name_info &lookup_name)

> @@ -14303,6 +14313,8 @@ ada_get_symbol_name_matcher (const lookup_name_info &lookup_name)

>      {

>        if (lookup_name.ada ().wild_match_p ())

>  	return do_wild_match;

> +      else if (lookup_name.ada ().verbatim_p ())

> +	return do_exact_match;

>        else

>  	return do_full_match;

>      }

> diff --git a/gdb/testsuite/gdb.ada/big_packed_array.exp b/gdb/testsuite/gdb.ada/big_packed_array.exp

> new file mode 100644

> index 0000000..a2e03bb

> --- /dev/null

> +++ b/gdb/testsuite/gdb.ada/big_packed_array.exp

> @@ -0,0 +1,35 @@

> +# Copyright 2019 Free Software Foundation, Inc.

> +#

> +# This program is free software; you can redistribute it and/or modify

> +# it under the terms of the GNU General Public License as published by

> +# the Free Software Foundation; either version 3 of the License, or

> +# (at your option) any later version.

> +#

> +# This program is distributed in the hope that it will be useful,

> +# but WITHOUT ANY WARRANTY; without even the implied warranty of

> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the

> +# GNU General Public License for more details.

> +#

> +# You should have received a copy of the GNU General Public License

> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.

> +

> +load_lib "ada.exp"

> +

> +if { [skip_ada_tests] } { return -1 }

> +

> +standard_ada_testfile foo_ra24_010

> +

> +if {[gdb_compile_ada "${srcfile}" "${binfile}" executable {debug}] != ""} {

> +    return -1

> +}

> +

> +clean_restart ${testfile}

> +

> +set bp_location [gdb_get_line_number "STOP" ${testdir}/foo_ra24_010.adb]

> +runto "foo_ra24_010.adb:$bp_location"

> +

> +gdb_test "print good" \

> +         "= \\(false <repeats 196 times>\\)" \

> +

> +gdb_test "print bad" \

> +         "= \\(false <repeats 196 times>\\)" \

> diff --git a/gdb/testsuite/gdb.ada/big_packed_array/foo_ra24_010.adb b/gdb/testsuite/gdb.ada/big_packed_array/foo_ra24_010.adb

> new file mode 100644

> index 0000000..7e6a26c

> --- /dev/null

> +++ b/gdb/testsuite/gdb.ada/big_packed_array/foo_ra24_010.adb

> @@ -0,0 +1,24 @@

> +--  Copyright 2019 Free Software Foundation, Inc.

> +--

> +--  This program is free software; you can redistribute it and/or modify

> +--  it under the terms of the GNU General Public License as published by

> +--  the Free Software Foundation; either version 3 of the License, or

> +--  (at your option) any later version.

> +--

> +--  This program is distributed in the hope that it will be useful,

> +--  but WITHOUT ANY WARRANTY; without even the implied warranty of

> +--  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the

> +--  GNU General Public License for more details.

> +--

> +--  You should have received a copy of the GNU General Public License

> +--  along with this program.  If not, see <http://www.gnu.org/licenses/>.

> +

> +with Pck; use Pck;

> +

> +procedure Foo_RA24_010 is

> +   Good : PA := (others => False);

> +   Bad : Bad_Packed_Table := (others => False);

> +begin

> +   Do_Nothing (Good'Address);  -- STOP

> +   Do_Nothing (Bad'Address);

> +end Foo_RA24_010;

> diff --git a/gdb/testsuite/gdb.ada/big_packed_array/pck.adb b/gdb/testsuite/gdb.ada/big_packed_array/pck.adb

> new file mode 100644

> index 0000000..6535991

> --- /dev/null

> +++ b/gdb/testsuite/gdb.ada/big_packed_array/pck.adb

> @@ -0,0 +1,21 @@

> +--  Copyright 2019 Free Software Foundation, Inc.

> +--

> +--  This program is free software; you can redistribute it and/or modify

> +--  it under the terms of the GNU General Public License as published by

> +--  the Free Software Foundation; either version 3 of the License, or

> +--  (at your option) any later version.

> +--

> +--  This program is distributed in the hope that it will be useful,

> +--  but WITHOUT ANY WARRANTY; without even the implied warranty of

> +--  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the

> +--  GNU General Public License for more details.

> +--

> +--  You should have received a copy of the GNU General Public License

> +--  along with this program.  If not, see <http://www.gnu.org/licenses/>.

> +

> +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/big_packed_array/pck.ads b/gdb/testsuite/gdb.ada/big_packed_array/pck.ads

> new file mode 100644

> index 0000000..18b58fb

> --- /dev/null

> +++ b/gdb/testsuite/gdb.ada/big_packed_array/pck.ads

> @@ -0,0 +1,53 @@

> +--  Copyright 2019 Free Software Foundation, Inc.

> +--

> +--  This program is free software; you can redistribute it and/or modify

> +--  it under the terms of the GNU General Public License as published by

> +--  the Free Software Foundation; either version 3 of the License, or

> +--  (at your option) any later version.

> +--

> +--  This program is distributed in the hope that it will be useful,

> +--  but WITHOUT ANY WARRANTY; without even the implied warranty of

> +--  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the

> +--  GNU General Public License for more details.

> +--

> +--  You should have received a copy of the GNU General Public License

> +--  along with this program.  If not, see <http://www.gnu.org/licenses/>.

> +

> +with System;

> +

> +package Pck is

> +   type Enum_Idx is

> +      (e_000, e_001, e_002, e_003, e_004, e_005, e_006, e_007, e_008,

> +       e_009, e_010, e_011, e_012, e_013, e_014, e_015, e_016, e_017,

> +       e_018, e_019, e_020, e_021, e_022, e_023, e_024, e_025, e_026,

> +       e_027, e_028, e_029, e_030, e_031, e_032, e_033, e_034, e_035,

> +       e_036, e_037, e_038, e_039, e_040, e_041, e_042, e_043, e_044,

> +       e_045, e_046, e_047, e_048, e_049, e_050, e_051, e_052, e_053,

> +       e_054, e_055, e_056, e_057, e_058, e_059, e_060, e_061, e_062,

> +       e_063, e_064, e_065, e_066, e_067, e_068, e_069, e_070, e_071,

> +       e_072, e_073, e_074, e_075, e_076, e_077, e_078, e_079, e_080,

> +       e_081, e_082, e_083, e_084, e_085, e_086, e_087, e_088, e_089,

> +       e_090, e_091, e_092, e_093, e_094, e_095, e_096, e_097, e_098,

> +       e_099, e_100, e_101, e_102, e_103, e_104, e_105, e_106, e_107,

> +       e_108, e_109, e_110, e_111, e_112, e_113, e_114, e_115, e_116,

> +       e_117, e_118, e_119, e_120, e_121, e_122, e_123, e_124, e_125,

> +       e_126, e_127, e_128, e_129, e_130, e_131, e_132, e_133, e_134,

> +       e_135, e_136, e_137, e_138, e_139, e_140, e_141, e_142, e_143,

> +       e_144, e_145, e_146, e_147, e_148, e_149, e_150, e_151, e_152,

> +       e_153, e_154, e_155, e_156, e_157, e_158, e_159, e_160, e_161,

> +       e_162, e_163, e_164, e_165, e_166, e_167, e_168, e_169, e_170,

> +       e_171, e_172, e_173, e_174, e_175, e_176, e_177, e_178, e_179,

> +       e_180, e_181, e_182, e_183, e_184, e_185, e_186, e_187, e_188,

> +       e_189, e_190, e_191, e_192, e_193, e_194, e_195);

> +

> +   type PA is array (Enum_Idx) of Boolean;

> +   pragma Pack (PA);

> +

> +   type T is array (Enum_Idx) of Boolean;

> +   pragma Pack (T);

> +   T_Empty : constant T := (others => False);

> +

> +   type Bad_Packed_Table is new T;

> +

> +   Procedure Do_Nothing (A : System.Address);

> +end Pck;

> diff --git a/gdb/testsuite/gdb.ada/homonym.exp b/gdb/testsuite/gdb.ada/homonym.exp

> index 625a4d4..3888090 100644

> --- a/gdb/testsuite/gdb.ada/homonym.exp

> +++ b/gdb/testsuite/gdb.ada/homonym.exp

> @@ -36,7 +36,7 @@ gdb_test "break homonym.adb:Get_Value" \

>      "set breakpoint at homonym.adb:Get_Value"

>

>  gdb_test "break <homonym__get_value>" \

> -    "Breakpoint \[0-9\]+ at $hex: <homonym__get_value>. .2 locations." \

> +    "Breakpoint \[0-9\]+ at $hex: file .*homonym\\.adb, line $decimal\\." \

>      "set breakpoint at <homonym__get_value>"

>

>  delete_breakpoints

> -- 

> 2.1.4
Joel Brobecker May 11, 2019, 5:23 p.m. | #4
Hi Pedro,

> I was running some regression tests for gdb 8.3, and I noticed that

> there was a regression introduced by this patch for test

> gdb.ada/pckd_arr_ren.exp.

> 

> I tried investigating, and it seems that this happened due to the

> compiler in my system being relatively old (GNAT 4.9.3).  I don't know

> much about Ada, so I couldn't figure out if this is an issue with the

> debug symbols generated by the older compiler or if GDB should handle

> this, but it worked before the patch.

> 

> This is the relevant output from the test:

> 

> (gdb) break foo.adb:22

> Breakpoint 1 at 0x10002410: file /home/pedromfc/binutils-gdb/gdb/testsuite/gdb.ada/pckd_arr_ren/foo.adb, line 22.

> (gdb) run 

> Starting program: /home/pedromfc/binutils-gdb/build-bisect/gdb/testsuite/outputs/gdb.ada/pckd_arr_ren/foo 

> 

> Breakpoint 1, foo () at /home/pedromfc/binutils-gdb/gdb/testsuite/gdb.ada/pckd_arr_ren/foo.adb:22

> 22         Do_Nothing (A2'Address); -- STOP

> (gdb) print A2

> Could not find renamed variable: a1

> (gdb) FAIL: gdb.ada/pckd_arr_ren.exp: print var

> 

> That error message seems to come from ada-exp.y:write_object_renaming.


Thanks for the heads up.

Most likely the version of the compiler you are using is generating
the debug info improperly, but why don't you send me privately your
binary, and I will try to take a look, to see what's happening.

-- 
Joel

Patch

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index a878d4d..415dc53 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -4760,7 +4760,7 @@  standard_lookup (const char *name, const struct block *block,
 
   if (lookup_cached_symbol (name, domain, &sym.symbol, NULL))
     return sym.symbol;
-  sym = lookup_symbol_in_language (name, block, domain, language_c, 0);
+  ada_lookup_encoded_symbol (name, block, domain, &sym);
   cache_symbol (name, domain, sym.symbol, sym.block);
   return sym.symbol;
 }
@@ -14193,6 +14193,16 @@  do_full_match (const char *symbol_search_name,
   return full_match (symbol_search_name, ada_lookup_name (lookup_name));
 }
 
+/* symbol_name_matcher_ftype for exact (verbatim) matches.  */
+
+static bool
+do_exact_match (const char *symbol_search_name,
+		const lookup_name_info &lookup_name,
+		completion_match_result *comp_match_res)
+{
+  return strcmp (symbol_search_name, ada_lookup_name (lookup_name)) == 0;
+}
+
 /* Build the Ada lookup name for LOOKUP_NAME.  */
 
 ada_lookup_name_info::ada_lookup_name_info (const lookup_name_info &lookup_name)
@@ -14303,6 +14313,8 @@  ada_get_symbol_name_matcher (const lookup_name_info &lookup_name)
     {
       if (lookup_name.ada ().wild_match_p ())
 	return do_wild_match;
+      else if (lookup_name.ada ().verbatim_p ())
+	return do_exact_match;
       else
 	return do_full_match;
     }
diff --git a/gdb/testsuite/gdb.ada/big_packed_array.exp b/gdb/testsuite/gdb.ada/big_packed_array.exp
new file mode 100644
index 0000000..a2e03bb
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/big_packed_array.exp
@@ -0,0 +1,35 @@ 
+# Copyright 2019 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+load_lib "ada.exp"
+
+if { [skip_ada_tests] } { return -1 }
+
+standard_ada_testfile foo_ra24_010
+
+if {[gdb_compile_ada "${srcfile}" "${binfile}" executable {debug}] != ""} {
+    return -1
+}
+
+clean_restart ${testfile}
+
+set bp_location [gdb_get_line_number "STOP" ${testdir}/foo_ra24_010.adb]
+runto "foo_ra24_010.adb:$bp_location"
+
+gdb_test "print good" \
+         "= \\(false <repeats 196 times>\\)" \
+
+gdb_test "print bad" \
+         "= \\(false <repeats 196 times>\\)" \
diff --git a/gdb/testsuite/gdb.ada/big_packed_array/foo_ra24_010.adb b/gdb/testsuite/gdb.ada/big_packed_array/foo_ra24_010.adb
new file mode 100644
index 0000000..7e6a26c
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/big_packed_array/foo_ra24_010.adb
@@ -0,0 +1,24 @@ 
+--  Copyright 2019 Free Software Foundation, Inc.
+--
+--  This program is free software; you can redistribute it and/or modify
+--  it under the terms of the GNU General Public License as published by
+--  the Free Software Foundation; either version 3 of the License, or
+--  (at your option) any later version.
+--
+--  This program is distributed in the hope that it will be useful,
+--  but WITHOUT ANY WARRANTY; without even the implied warranty of
+--  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+--  GNU General Public License for more details.
+--
+--  You should have received a copy of the GNU General Public License
+--  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+with Pck; use Pck;
+
+procedure Foo_RA24_010 is
+   Good : PA := (others => False);
+   Bad : Bad_Packed_Table := (others => False);
+begin
+   Do_Nothing (Good'Address);  -- STOP
+   Do_Nothing (Bad'Address);
+end Foo_RA24_010;
diff --git a/gdb/testsuite/gdb.ada/big_packed_array/pck.adb b/gdb/testsuite/gdb.ada/big_packed_array/pck.adb
new file mode 100644
index 0000000..6535991
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/big_packed_array/pck.adb
@@ -0,0 +1,21 @@ 
+--  Copyright 2019 Free Software Foundation, Inc.
+--
+--  This program is free software; you can redistribute it and/or modify
+--  it under the terms of the GNU General Public License as published by
+--  the Free Software Foundation; either version 3 of the License, or
+--  (at your option) any later version.
+--
+--  This program is distributed in the hope that it will be useful,
+--  but WITHOUT ANY WARRANTY; without even the implied warranty of
+--  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+--  GNU General Public License for more details.
+--
+--  You should have received a copy of the GNU General Public License
+--  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+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/big_packed_array/pck.ads b/gdb/testsuite/gdb.ada/big_packed_array/pck.ads
new file mode 100644
index 0000000..18b58fb
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/big_packed_array/pck.ads
@@ -0,0 +1,53 @@ 
+--  Copyright 2019 Free Software Foundation, Inc.
+--
+--  This program is free software; you can redistribute it and/or modify
+--  it under the terms of the GNU General Public License as published by
+--  the Free Software Foundation; either version 3 of the License, or
+--  (at your option) any later version.
+--
+--  This program is distributed in the hope that it will be useful,
+--  but WITHOUT ANY WARRANTY; without even the implied warranty of
+--  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+--  GNU General Public License for more details.
+--
+--  You should have received a copy of the GNU General Public License
+--  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+with System;
+
+package Pck is
+   type Enum_Idx is
+      (e_000, e_001, e_002, e_003, e_004, e_005, e_006, e_007, e_008,
+       e_009, e_010, e_011, e_012, e_013, e_014, e_015, e_016, e_017,
+       e_018, e_019, e_020, e_021, e_022, e_023, e_024, e_025, e_026,
+       e_027, e_028, e_029, e_030, e_031, e_032, e_033, e_034, e_035,
+       e_036, e_037, e_038, e_039, e_040, e_041, e_042, e_043, e_044,
+       e_045, e_046, e_047, e_048, e_049, e_050, e_051, e_052, e_053,
+       e_054, e_055, e_056, e_057, e_058, e_059, e_060, e_061, e_062,
+       e_063, e_064, e_065, e_066, e_067, e_068, e_069, e_070, e_071,
+       e_072, e_073, e_074, e_075, e_076, e_077, e_078, e_079, e_080,
+       e_081, e_082, e_083, e_084, e_085, e_086, e_087, e_088, e_089,
+       e_090, e_091, e_092, e_093, e_094, e_095, e_096, e_097, e_098,
+       e_099, e_100, e_101, e_102, e_103, e_104, e_105, e_106, e_107,
+       e_108, e_109, e_110, e_111, e_112, e_113, e_114, e_115, e_116,
+       e_117, e_118, e_119, e_120, e_121, e_122, e_123, e_124, e_125,
+       e_126, e_127, e_128, e_129, e_130, e_131, e_132, e_133, e_134,
+       e_135, e_136, e_137, e_138, e_139, e_140, e_141, e_142, e_143,
+       e_144, e_145, e_146, e_147, e_148, e_149, e_150, e_151, e_152,
+       e_153, e_154, e_155, e_156, e_157, e_158, e_159, e_160, e_161,
+       e_162, e_163, e_164, e_165, e_166, e_167, e_168, e_169, e_170,
+       e_171, e_172, e_173, e_174, e_175, e_176, e_177, e_178, e_179,
+       e_180, e_181, e_182, e_183, e_184, e_185, e_186, e_187, e_188,
+       e_189, e_190, e_191, e_192, e_193, e_194, e_195);
+
+   type PA is array (Enum_Idx) of Boolean;
+   pragma Pack (PA);
+
+   type T is array (Enum_Idx) of Boolean;
+   pragma Pack (T);
+   T_Empty : constant T := (others => False);
+
+   type Bad_Packed_Table is new T;
+
+   Procedure Do_Nothing (A : System.Address);
+end Pck;
diff --git a/gdb/testsuite/gdb.ada/homonym.exp b/gdb/testsuite/gdb.ada/homonym.exp
index 625a4d4..3888090 100644
--- a/gdb/testsuite/gdb.ada/homonym.exp
+++ b/gdb/testsuite/gdb.ada/homonym.exp
@@ -36,7 +36,7 @@  gdb_test "break homonym.adb:Get_Value" \
     "set breakpoint at homonym.adb:Get_Value"
 
 gdb_test "break <homonym__get_value>" \
-    "Breakpoint \[0-9\]+ at $hex: <homonym__get_value>. .2 locations." \
+    "Breakpoint \[0-9\]+ at $hex: file .*homonym\\.adb, line $decimal\\." \
     "set breakpoint at <homonym__get_value>"
 
 delete_breakpoints