From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 46433 invoked by alias); 5 Jan 2018 03:49:17 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 46422 invoked by uid 89); 5 Jan 2018 03:49:17 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.4 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=shoes, technically X-HELO: rock.gnat.com Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 05 Jan 2018 03:49:14 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 56E2C1177E4; Thu, 4 Jan 2018 22:49:13 -0500 (EST) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id QW0noJM7rR3D; Thu, 4 Jan 2018 22:49:13 -0500 (EST) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id C96BC117780; Thu, 4 Jan 2018 22:49:12 -0500 (EST) Received: by joel.gnat.com (Postfix, from userid 1000) id 8AA6F808EA; Fri, 5 Jan 2018 07:49:08 +0400 (+04) Date: Fri, 05 Jan 2018 03:49:00 -0000 From: Joel Brobecker To: Xavier Roirand Cc: gdb-patches@sourceware.org Subject: Re: [RFAv3/DWARF] Fix breakpoint add on inlined function using function name. Message-ID: <20180105034908.oxc2ylnttnwjpdvt@adacore.com> References: <1514975165-10270-1-git-send-email-roirand@adacore.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1514975165-10270-1-git-send-email-roirand@adacore.com> User-Agent: NeoMutt/20170113 (1.7.2) X-SW-Source: 2018-01/txt/msg00067.txt.bz2 On Wed, Jan 03, 2018 at 11:26:05AM +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 > 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. The patch looks good. Just a couple of tiny little things you might want to fix. > --- /dev/null > +++ b/gdb/testsuite/gdb.ada/bp_inlined_func.exp > @@ -0,0 +1,61 @@ > +# Copyright 2018 Free Software Foundation, Inc. Technically, it should be 2017-2018, as the copyright year range starts the day the file was committed to a medium (file system). Let's change this file and all the other files this commit is adding. > +# 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 . > + > +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. This is nitpicking but... Now that you added the comment just before the "break read_small" function, I find that comment misplaced: If I put myself in the shoes of somone reading the testcase for the first time, I would see the comment, and then read that the next thing we do is not to insert the breakpoint like the comment says, but restart. The next thing, perhaps? Neither, it's running to main. And then comes your new comment, which says the same thing and more.... I would delete it. The one you have a few lines below is enough. > + > +clean_restart ${testfile} > + > +if ![runto_main] then { > + fail "Cannot run to main, testcase aborted" > + return 0 > +} > + > +# Check that inserting breakpoint on read_small inlined function inserts > +# 4 breakpoints. > + > +gdb_test "break read_small" \ > + "Breakpoint $decimal at $hex: read_small\\. \\(4 locations\\)" \ > + "set breakpoint at read_small" > + > +# We do not verify each breakpoint info, but use continue command instead > +# to verify that we properly stops on each expected breakpoint. "command" -> "commands". "stops" -> "stop". > + > +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..a30292c > --- /dev/null > +++ b/gdb/testsuite/gdb.ada/bp_inlined_func/b.adb > @@ -0,0 +1,28 @@ > +-- 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 . > + > +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..69bd8e8 > --- /dev/null > +++ b/gdb/testsuite/gdb.ada/bp_inlined_func/b.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 . > + > +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..d3bb957 > --- /dev/null > +++ b/gdb/testsuite/gdb.ada/bp_inlined_func/c.adb > @@ -0,0 +1,27 @@ > +-- 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 . > + > +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..632fb31 > --- /dev/null > +++ b/gdb/testsuite/gdb.ada/bp_inlined_func/c.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 . > + > +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..4c7512e > --- /dev/null > +++ b/gdb/testsuite/gdb.ada/bp_inlined_func/foo.adb > @@ -0,0 +1,23 @@ > +-- 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 . > + > +with B; use B; > +with C; > +procedure FOO is > +begin > + Doit; > + B.Read_Small; > + C.C_Doit; > +end FOO; > -- > 2.7.4 -- Joel