This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA] (Ada) fix breakpoint add on inlined function using function name.
- From: Joel Brobecker <brobecker at adacore dot com>
- To: Xavier Roirand <roirand at adacore dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Wed, 20 Dec 2017 15:23:22 +0400
- Subject: Re: [RFA] (Ada) fix breakpoint add on inlined function using function name.
- Authentication-results: sourceware.org; auth=none
- References: <1513765319-17005-1-git-send-email-roirand@adacore.com>
On Wed, Dec 20, 2017 at 11:21:59AM +0100, Xavier Roirand wrote:
> Using this Ada example:
>
> package B is
> procedure Read_Small with Inline_Always;
> end B;
>
> package body B is
> Total : Natural := 0;
> procedure Read_Small is
> begin
> Total := Total + 1;
> end Read_Small;
> end B;
>
> and
>
> with B;
>
> procedure M is
> begin
> B.Read_Small;
> end M;
>
> % gnatmake -g -O0 -m m.adb -cargs -gnatn
> % gdb m
>
> Inserting a breakpoint on Read_Small inlined function does not work:
>
> (gdb) b read_small
> Breakpoint 1 at 0x40250e: file b.adb, line 5.
> (gdb) info b
> Num Type Disp Enb Address What
> 1 breakpoint keep y 0x000000000040250e in b.doit at b.adb:5
> (gdb)
>
> In this exemple we should have two breakpoints set, one in package B and
> the other one in the inlined instance inside procedure M), like below:
>
> (gdb) b read_small
> Breakpoint 1 at 0x40250e: b.adb:5. (2 locations)
> (gdb) info b
> Num Type Disp Enb Address What
> 1 breakpoint keep y <MULTIPLE>
> 1.1 y 0x000000000040250e in b.doit at b.adb:5
> 1.2 y 0x0000000000402540 in m at b.adb:5
> (gdb)
>
> Looking at the DWARF info for inlined instance of Read_Small:
>
> <1><1526>: Abbrev Number: 2 (DW_TAG_subprogram)
> <1527> DW_AT_name : ([...], offset: 0x1e82): b__read_small
> <152b> DW_AT_decl_file : 2
> <152c> DW_AT_decl_line : 3
> <152d> DW_AT_inline : 3 (declared as inline and inlined)
> [...]
> <2><1547>: Abbrev Number: 4 (DW_TAG_inlined_subroutine)
> <1548> DW_AT_abstract_origin: <0x1526>
> <154c> DW_AT_low_pc : 0x402552
> <1554> DW_AT_high_pc : 0x2b
> <155c> DW_AT_call_file : 1
> <155d> DW_AT_call_line : 5
> <2><155e>: Abbrev Number: 0
>
> During the parsing of DWARF info in order to produce partial DIE linked
> list, the DW_TAG_inlined_subroutine were skipped thus not present in the
> final partial dies.
> Taking DW_TAG_inlined_subroutine in account during the parsing process
> fixes the problem.
>
> gdb/ChangeLog:
>
> * dwarf2read.c (scan_partial_symbols, add_partial_symbol)
> (add_partial_subprogram, load_partial_dies): Add
> DW_TAG_inlined_subroutine handling.
>
> gdb/testsuite/ChangeLog:
>
> * gdb.ada/bp_inlined_func: New testcase.
>
> For QB08-006.
>
> Change-Id: I07db06d43be56cc4493167ef53b4e942a784d8f6
Please remove the two lines above ;-)
Other than that, the patch looks good to me. But give others
a couple of weeks so they also get a chance to review the patch
as well. Here is what I propose: Send a v2 patch, with the revision
log fixed, so we don't forget about fixing (revision logs cannot
be changed once pushed without rewriting history), mentioning
that I conditionally OK-ed the patch but asked for us to wait
another couple of weeks to give everyone some time to look at it
also.
It wouldn't hurt as well to confirm to us how you tested this patch,
and on which platform.
Thank you,
--
Joel
> ---
> gdb/dwarf2read.c | 21 ++++++----
> gdb/testsuite/gdb.ada/bp_inlined_func.exp | 55 +++++++++++++++++++++++++++
> gdb/testsuite/gdb.ada/bp_inlined_func/b.adb | 28 ++++++++++++++
> gdb/testsuite/gdb.ada/bp_inlined_func/b.ads | 19 +++++++++
> gdb/testsuite/gdb.ada/bp_inlined_func/c.adb | 27 +++++++++++++
> gdb/testsuite/gdb.ada/bp_inlined_func/c.ads | 19 +++++++++
> gdb/testsuite/gdb.ada/bp_inlined_func/foo.adb | 23 +++++++++++
> 7 files changed, 184 insertions(+), 8 deletions(-)
> create mode 100644 gdb/testsuite/gdb.ada/bp_inlined_func.exp
> create mode 100644 gdb/testsuite/gdb.ada/bp_inlined_func/b.adb
> create mode 100644 gdb/testsuite/gdb.ada/bp_inlined_func/b.ads
> create mode 100644 gdb/testsuite/gdb.ada/bp_inlined_func/c.adb
> create mode 100644 gdb/testsuite/gdb.ada/bp_inlined_func/c.ads
> create mode 100644 gdb/testsuite/gdb.ada/bp_inlined_func/foo.adb
>
> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> index 484cbce..694f3fc 100644
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
> @@ -8961,11 +8961,13 @@ scan_partial_symbols (struct partial_die_info *first_die, CORE_ADDR *lowpc,
>
> if (pdi->name != NULL || pdi->tag == DW_TAG_namespace
> || pdi->tag == DW_TAG_module || pdi->tag == DW_TAG_enumeration_type
> - || pdi->tag == DW_TAG_imported_unit)
> + || pdi->tag == DW_TAG_imported_unit
> + || pdi->tag == DW_TAG_inlined_subroutine)
> {
> switch (pdi->tag)
> {
> case DW_TAG_subprogram:
> + case DW_TAG_inlined_subroutine:
> add_partial_subprogram (pdi, lowpc, highpc, set_addrmap, cu);
> break;
> case DW_TAG_constant:
> @@ -9198,6 +9200,7 @@ add_partial_symbol (struct partial_die_info *pdi, struct dwarf2_cu *cu)
>
> switch (pdi->tag)
> {
> + case DW_TAG_inlined_subroutine:
> case DW_TAG_subprogram:
> addr = gdbarch_adjust_dwarf2_addr (gdbarch, pdi->lowpc + baseaddr);
> if (pdi->is_external || cu->language == language_ada)
> @@ -9395,12 +9398,12 @@ add_partial_module (struct partial_die_info *pdi, CORE_ADDR *lowpc,
> scan_partial_symbols (pdi->die_child, lowpc, highpc, set_addrmap, cu);
> }
>
> -/* Read a partial die corresponding to a subprogram and create a partial
> - symbol for that subprogram. When the CU language allows it, this
> - routine also defines a partial symbol for each nested subprogram
> - that this subprogram contains. If SET_ADDRMAP is true, record the
> - covered ranges in the addrmap. Set *LOWPC and *HIGHPC to the lowest
> - and highest PC values found in PDI.
> +/* Read a partial die corresponding to a subprogram or an inlined
> + subprogram and create a partial symbol for that subprogram.
> + When the CU language allows it, this routine also defines a partial
> + symbol for each nested subprogram that this subprogram contains.
> + If SET_ADDRMAP is true, record the covered ranges in the addrmap.
> + Set *LOWPC and *HIGHPC to the lowest and highest PC values found in PDI.
>
> PDI may also be a lexical block, in which case we simply search
> recursively for subprograms defined inside that lexical block.
> @@ -9412,7 +9415,7 @@ add_partial_subprogram (struct partial_die_info *pdi,
> CORE_ADDR *lowpc, CORE_ADDR *highpc,
> int set_addrmap, struct dwarf2_cu *cu)
> {
> - if (pdi->tag == DW_TAG_subprogram)
> + if (pdi->tag == DW_TAG_subprogram || DW_TAG_inlined_subroutine)
> {
> if (pdi->has_pc_info)
> {
> @@ -9460,6 +9463,7 @@ add_partial_subprogram (struct partial_die_info *pdi,
> {
> fixup_partial_die (pdi, cu);
> if (pdi->tag == DW_TAG_subprogram
> + || pdi->tag == DW_TAG_inlined_subroutine
> || pdi->tag == DW_TAG_lexical_block)
> add_partial_subprogram (pdi, lowpc, highpc, set_addrmap, cu);
> pdi = pdi->die_sibling;
> @@ -18223,6 +18227,7 @@ load_partial_dies (const struct die_reader_specs *reader,
> && abbrev->tag != DW_TAG_constant
> && abbrev->tag != DW_TAG_enumerator
> && abbrev->tag != DW_TAG_subprogram
> + && abbrev->tag != DW_TAG_inlined_subroutine
> && abbrev->tag != DW_TAG_lexical_block
> && abbrev->tag != DW_TAG_variable
> && abbrev->tag != DW_TAG_namespace
> diff --git a/gdb/testsuite/gdb.ada/bp_inlined_func.exp b/gdb/testsuite/gdb.ada/bp_inlined_func.exp
> new file mode 100644
> index 0000000..299d25a
> --- /dev/null
> +++ b/gdb/testsuite/gdb.ada/bp_inlined_func.exp
> @@ -0,0 +1,55 @@
> +# 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
> +}
> +
> +# Test inserting breakpoint on read_small inlined function.
> +
> +clean_restart ${testfile}
> +
> +if ![runto_main] then {
> + fail "Cannot run to main, testcase aborted"
> + return 0
> +}
> +
> +gdb_test "break read_small" \
> + "Breakpoint $decimal at $hex: read_small\\. \\(4 locations\\)" \
> + "set breakpoint at read_small"
> +
> +gdb_test "continue" \
> + "Breakpoint $decimal, b\\.doit \\(\\).*" \
> + "Hitting first call of read_small"
> +
> +gdb_test "continue" \
> + "Breakpoint $decimal, foo \\(\\).*" \
> + "Hitting second call of read_small"
> +
> +gdb_test "continue" \
> + "Breakpoint $decimal, c\\.c_doit \\(\\).*" \
> + "Hitting third call of read_small"
> +
> +gdb_test "continue" \
> + "Breakpoint $decimal, c\\.c_doit2 \\(\\).*" \
> + "Hitting fourth call of read_small"
> +
> +gdb_test "continue" \
> + "Continuing\..*$inferior_exited_re.*" \
> + "continuing to program completion"
> diff --git a/gdb/testsuite/gdb.ada/bp_inlined_func/b.adb b/gdb/testsuite/gdb.ada/bp_inlined_func/b.adb
> new file mode 100644
> index 0000000..49a5374
> --- /dev/null
> +++ b/gdb/testsuite/gdb.ada/bp_inlined_func/b.adb
> @@ -0,0 +1,28 @@
> +-- 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 B is
> + Total : Natural := 0;
> + procedure Read_Small is
> + begin
> + Total := Total + 1; -- BREAK
> + end Read_Small;
> +
> + procedure Doit is
> + begin
> + Read_Small;
> + null;
> + end Doit;
> +end B;
> diff --git a/gdb/testsuite/gdb.ada/bp_inlined_func/b.ads b/gdb/testsuite/gdb.ada/bp_inlined_func/b.ads
> new file mode 100644
> index 0000000..2987c97
> --- /dev/null
> +++ b/gdb/testsuite/gdb.ada/bp_inlined_func/b.ads
> @@ -0,0 +1,19 @@
> +-- 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 B is
> + procedure Read_Small with Inline_Always;
> + procedure Doit;
> +end B;
> diff --git a/gdb/testsuite/gdb.ada/bp_inlined_func/c.adb b/gdb/testsuite/gdb.ada/bp_inlined_func/c.adb
> new file mode 100644
> index 0000000..12784ea
> --- /dev/null
> +++ b/gdb/testsuite/gdb.ada/bp_inlined_func/c.adb
> @@ -0,0 +1,27 @@
> +-- 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 B;
> +package body C is
> + procedure C_Doit is
> + begin
> + B.Read_Small;
> + C_Doit2;
> + end;
> + procedure C_Doit2 is
> + begin
> + B.Read_Small;
> + end;
> +end C;
> diff --git a/gdb/testsuite/gdb.ada/bp_inlined_func/c.ads b/gdb/testsuite/gdb.ada/bp_inlined_func/c.ads
> new file mode 100644
> index 0000000..fd6dfc6
> --- /dev/null
> +++ b/gdb/testsuite/gdb.ada/bp_inlined_func/c.ads
> @@ -0,0 +1,19 @@
> +-- 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 C is
> + procedure C_Doit;
> + procedure C_Doit2;
> +end C;
> diff --git a/gdb/testsuite/gdb.ada/bp_inlined_func/foo.adb b/gdb/testsuite/gdb.ada/bp_inlined_func/foo.adb
> new file mode 100644
> index 0000000..bf44275
> --- /dev/null
> +++ b/gdb/testsuite/gdb.ada/bp_inlined_func/foo.adb
> @@ -0,0 +1,23 @@
> +-- 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 B; use B;
> +with C;
> +procedure FOO is
> +begin
> + Doit;
> + B.Read_Small;
> + C.C_Doit;
> +end FOO;
> --
> 2.7.4
--
Joel