From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 96004 invoked by alias); 27 Nov 2018 22:09:28 -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 95994 invoked by uid 89); 27 Nov 2018 22:09:27 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.4 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_SHORT,KAM_STOCKGEN,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=deserves X-HELO: mail-wm1-f65.google.com Received: from mail-wm1-f65.google.com (HELO mail-wm1-f65.google.com) (209.85.128.65) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 27 Nov 2018 22:09:22 +0000 Received: by mail-wm1-f65.google.com with SMTP id z18so562724wmc.4 for ; Tue, 27 Nov 2018 14:09:22 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=DLZh5LtKNa/U1mHOvgYNHr6A6jUteA2zMfftKHM+36I=; b=W7tV30rBJEzJdXEuTCjrfupppcYVQopeX4sDEo6ypbswpHC2aB68w48wjIa3rRqkY5 QbenMFNUxFruaJwdU3HOGqZJedTZs0UATmberhY08Zp3cFKxU7X94vqMecesKBS7RNn3 xvNKlo5cF9aLS9qG3cHUn3kSueipM60CEaOeV/Fc2ROBF4hnl+8xslhJmBi6SIhDkEru MrHdwLWcsKJoAOjo3m/CrvKS2XB4MJGes+JImmziiXzQN+32E/ql+ZmDdvoee+tXk/pq gLVoS8SsN3lEWuZG9mdB0MbEcMACQAoU/jHQnww2FemIUDuB1a9h/gl7LHKLuGTqKvjZ gVcA== Return-Path: Received: from localhost (host86-156-236-171.range86-156.btcentralplus.com. [86.156.236.171]) by smtp.gmail.com with ESMTPSA id x10sm4827028wrn.29.2018.11.27.14.09.18 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 27 Nov 2018 14:09:19 -0800 (PST) Date: Tue, 27 Nov 2018 22:09:00 -0000 From: Andrew Burgess To: Pawel Wodkowski Cc: gdb-patches@sourceware.org, murbanski@pl.sii.eu, sbasierski@pl.sii.eu, tim.wiederhake@intel.com, dragos.carciumaru@intel.com, Bernhard Heckel Subject: Re: [PATCH v2 2/7] Dwarf: Fortran, support DW_TAG_entry_point. Message-ID: <20181127220917.GC18841@embecosm.com> References: <1542663530-140490-1-git-send-email-pwodkowski@pl.sii.eu> <1542663530-140490-2-git-send-email-pwodkowski@pl.sii.eu> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1542663530-140490-2-git-send-email-pwodkowski@pl.sii.eu> X-Fortune: The ring needs another token X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] User-Agent: Mutt/1.9.2 (2017-12-15) X-IsSubscribed: yes X-SW-Source: 2018-11/txt/msg00489.txt.bz2 * Pawel Wodkowski [2018-11-19 22:38:45 +0100]: > From: Bernhard Heckel > > Fortran provides additional entry-points to an subprogram. > Those entry-points may have only a subset of parameters > of the original subprogram as well. > Add support for parsing DW_TAG_entry_point's for Fortran. As with patch #1 it's probably worth documenting which compilers you see DW_TAG_entry_point in, along with an example of what the generated DWARF looks like. This means that in the future if/when others start to use this DWARF feature we can figure out which other compilers we need to test. > > 2016-06-01 Bernhard Heckel > > gdb/Changelog: > * gdb/dwarf2read.c (add_partial_symbol): Handle DW_TAG_entry_point. > (add_partial_entry_point): New. > (add_partial_subprogram): Search for entry_points. > (process_die): Handle DW_TAG_entry_point. > (dwarf2_get_pc_bounds): Update low pc from DWARF. > (load_partial_dies): Save DW_TAG_entry_point's. > (load_partial_dies): Save DW_TAG_entry_point to hash table. > (load_partial_dies): Look into child's of DW_TAG_sub_program > for fortran. > (new_symbol_full): Process DW_TAG_entry_point. > (read_type_die_1): Handle DW_TAG_entry_point. > > gdb/Testsuite/Changelog: > * gdb.fortran/entry_point.f90: New. > * gdb.fortran/entry_point.exp: New. > --- > gdb/dwarf2read.c | 98 ++++++++++++++++++++++++++++++- > gdb/testsuite/gdb.fortran/entry-point.exp | 70 ++++++++++++++++++++++ > gdb/testsuite/gdb.fortran/entry-point.f90 | 48 +++++++++++++++ > 3 files changed, 215 insertions(+), 1 deletion(-) > create mode 100644 gdb/testsuite/gdb.fortran/entry-point.exp > create mode 100644 gdb/testsuite/gdb.fortran/entry-point.f90 > > diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c > index 89fd4ae15e80..88e57d7ab68e 100644 > --- a/gdb/dwarf2read.c > +++ b/gdb/dwarf2read.c > @@ -1473,6 +1473,10 @@ static void add_partial_module (struct partial_die_info *pdi, CORE_ADDR *lowpc, > static void add_partial_enumeration (struct partial_die_info *enum_pdi, > struct dwarf2_cu *cu); > > +static void add_partial_entry_point (struct partial_die_info *pdi, > + CORE_ADDR *lowpc, CORE_ADDR *highpc, > + int need_pc, struct dwarf2_cu *cu); > + > static void add_partial_subprogram (struct partial_die_info *pdi, > CORE_ADDR *lowpc, CORE_ADDR *highpc, > int need_pc, struct dwarf2_cu *cu); > @@ -8876,6 +8880,32 @@ add_partial_symbol (struct partial_die_info *pdi, struct dwarf2_cu *cu) > > switch (pdi->tag) > { > + case DW_TAG_entry_point: > + /* Don't know any other language than fortran which is > + using DW_TAG_entry_point. */ > + if (cu->language == language_fortran) > + { I'm not sure the language check is needed here. The description for DW_TAG_entry_point in the DWARF spec seems pretty generic. I don't see how a different language would expect significantly different results, and so, I would suggest we should just add this as general purpose code. > + addr = gdbarch_adjust_dwarf2_addr (gdbarch, pdi->lowpc + baseaddr); When compared to DW_TAG_subprogram, this line doesn't include a '- baseaddr'. I think that deserves a comment explaining why. > + /* DW_TAG_entry_point provides an additional entry_point to an > + existing sub_program. Therefore, we inherit the "external" > + attribute from the sub_program to which the entry_point > + belongs to. */ > + if (pdi->die_parent->is_external) > + add_psymbol_to_list (actual_name, strlen (actual_name), > + built_actual_name != nullptr, > + VAR_DOMAIN, LOC_BLOCK, > + SECT_OFF_TEXT (objfile), > + &objfile->global_psymbols, > + addr, cu->language, objfile); > + else > + add_psymbol_to_list (actual_name, strlen (actual_name), > + built_actual_name != nullptr, > + VAR_DOMAIN, LOC_BLOCK, > + SECT_OFF_TEXT (objfile), > + &objfile->static_psymbols, > + addr, cu->language, objfile); I think these two add_psymbol_to_list calls, and the two for DW_TAG_subprogram should be factored out. Just my opinion though... > + } > + break; > case DW_TAG_inlined_subroutine: > case DW_TAG_subprogram: > addr = (gdbarch_adjust_dwarf2_addr (gdbarch, pdi->lowpc + baseaddr) > @@ -9082,6 +9112,17 @@ add_partial_module (struct partial_die_info *pdi, CORE_ADDR *lowpc, > scan_partial_symbols (pdi->die_child, lowpc, highpc, set_addrmap, cu); > } > > +static void > +add_partial_entry_point (struct partial_die_info *pdi, > + CORE_ADDR *p_lowpc, CORE_ADDR *p_highpc, > + int set_addrmap, struct dwarf2_cu *cu) > +{ > + if (pdi->name == nullptr) > + complaint (_("DW_TAG_entry_point must have a name")); > + else > + add_partial_symbol (pdi, cu); > +} This function should have a header comment, however, a quick look at add_partial_subprogram seems to indicate that at this point GDB is not complaining, but just ignoring weird looking DWARF. For example this code in add_partial_subprogram: /* Ignore subprogram DIEs that do not have a name, they are illegal. Do not emit a complaint at this point, we will do so when we convert this psymtab into a symtab. */ if (pdi->name) add_partial_symbol (pdi, cu); I think the same logic should apply for DW_TAG_entry_point maybe? So, add_partial_entry_point is possibly redundant. > + > /* 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 > @@ -9158,6 +9199,16 @@ add_partial_subprogram (struct partial_die_info *pdi, > pdi = pdi->die_sibling; > } > } > + else if (cu->language == language_fortran) > + { Like before, I'm not convinced we should tie this to Fortran... > + pdi = pdi->die_child; > + while (pdi != nullptr) > + { > + if (pdi->tag == DW_TAG_entry_point) > + add_partial_entry_point (pdi, lowpc, highpc, set_addrmap, cu); You can probably just make the add_partial_symbol call directly here (after a check that pdi->name is not null. > + pdi = pdi->die_sibling; > + } > + } > } > > /* Read a partial die corresponding to an enumeration type. */ > @@ -10596,6 +10647,7 @@ process_die (struct die_info *die, struct dwarf2_cu *cu) > case DW_TAG_type_unit: > read_type_unit_scope (die, cu); > break; > + case DW_TAG_entry_point: > case DW_TAG_subprogram: > case DW_TAG_inlined_subroutine: > read_func_scope (die, cu); > @@ -14669,6 +14721,26 @@ dwarf2_get_pc_bounds (struct die_info *die, CORE_ADDR *lowpc, > CORE_ADDR high = 0; > enum pc_bounds_kind ret; > > + if (die->tag == DW_TAG_entry_point) > + { > + /* Entry_point is embedded in an subprogram. Therefore, we can use > + the highpc from it's enveloping subprogram and get the > + lowpc from DWARF. */ > + if (0 == dwarf2_get_pc_bounds (die->parent, lowpc, highpc, cu, pst)) Compare to 'PC_BOUNDS_NOT_PRESENT' not 0. > + return PC_BOUNDS_NOT_PRESENT; > + > + attr = dwarf2_attr (die, DW_AT_low_pc, cu); > + if (!attr) > + { > + complaint (_("DW_TAG_entry_point is missing DW_AT_low_pc")); > + return PC_BOUNDS_NOT_PRESENT; > + } > + low = attr_value_as_address (attr); > + *lowpc = low; > + > + return PC_BOUNDS_HIGH_LOW; > + } > + > attr_high = dwarf2_attr (die, DW_AT_high_pc, cu); > if (attr_high) > { > @@ -18399,6 +18471,7 @@ load_partial_dies (const struct die_reader_specs *reader, > && !is_type_tag_for_partial (abbrev->tag) > && abbrev->tag != DW_TAG_constant > && abbrev->tag != DW_TAG_enumerator > + && abbrev->tag != DW_TAG_entry_point > && abbrev->tag != DW_TAG_subprogram > && abbrev->tag != DW_TAG_inlined_subroutine > && abbrev->tag != DW_TAG_lexical_block > @@ -18529,6 +18602,7 @@ load_partial_dies (const struct die_reader_specs *reader, > > if (load_all > || abbrev->tag == DW_TAG_constant > + || abbrev->tag == DW_TAG_entry_point > || abbrev->tag == DW_TAG_subprogram > || abbrev->tag == DW_TAG_variable > || abbrev->tag == DW_TAG_namespace > @@ -18570,7 +18644,9 @@ load_partial_dies (const struct die_reader_specs *reader, > || last_die->tag == DW_TAG_union_type)) > || (cu->language == language_ada > && (last_die->tag == DW_TAG_subprogram > - || last_die->tag == DW_TAG_lexical_block)))) > + || last_die->tag == DW_TAG_lexical_block)) > + || (cu->language == language_fortran > + && last_die->tag == DW_TAG_subprogram))) > { > nesting_level++; > parent_die = last_die; > @@ -21442,6 +21518,25 @@ new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu, > SYMBOL_ACLASS_INDEX (sym) = LOC_LABEL; > dw2_add_symbol_to_list (sym, cu->list_in_scope); > break; > + case DW_TAG_entry_point: > + /* Don't know any other language than fortran which is > + using DW_TAG_entry_point. */ > + if (cu->language == language_fortran) > + { > + /* SYMBOL_BLOCK_VALUE (sym) will be filled in later by > + finish_block. */ > + SYMBOL_ACLASS_INDEX (sym) = LOC_BLOCK; > + /* DW_TAG_entry_point provides an additional entry_point to an > + existing sub_program. Therefore, we inherit the "external" > + attribute from the sub_program to which the entry_point > + belongs to. */ > + attr2 = dwarf2_attr (die->parent, DW_AT_external, cu); > + if (attr2 && (DW_UNSND (attr2) != 0)) > + list_to_add = cu->builder->get_global_symbols (); > + else > + list_to_add = cu->list_in_scope; > + } > + break; > case DW_TAG_subprogram: > /* SYMBOL_BLOCK_VALUE (sym) will be filled in later by > finish_block. */ > @@ -22129,6 +22224,7 @@ read_type_die_1 (struct die_info *die, struct dwarf2_cu *cu) > case DW_TAG_enumeration_type: > this_type = read_enumeration_type (die, cu); > break; > + case DW_TAG_entry_point: > case DW_TAG_subprogram: > case DW_TAG_subroutine_type: > case DW_TAG_inlined_subroutine: > diff --git a/gdb/testsuite/gdb.fortran/entry-point.exp b/gdb/testsuite/gdb.fortran/entry-point.exp > new file mode 100644 > index 000000000000..a1f3f2bebdb9 > --- /dev/null > +++ b/gdb/testsuite/gdb.fortran/entry-point.exp > @@ -0,0 +1,70 @@ > +# 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 . > + > +if { [skip_fortran_tests] } { return -1 } > + > +standard_testfile .f90 > +load_lib "fortran.exp" > + > +if {[prepare_for_testing $testfile.exp $testfile $srcfile {debug f90}]} { > + return -1 > +} > + > +if ![runto MAIN__] then { > + perror "couldn't run to breakpoint MAIN__" > + continue > +} > + > +# Test if we can set a breakpoint via entry-point name > +set ept_name "foo" > +gdb_breakpoint $ept_name > +gdb_test "continue" \ > + [multi_line "Breakpoint $decimal, $ept_name \\(j=1, k=2, l=3, i1=4\\) at .*" \ > + ".*"] \ > + "continue to breakpoint: $ept_name" > + > +gdb_test "print j" "= 1" "print j, entered via $ept_name" > +gdb_test "print k" "= 2" "print k, entered via $ept_name" > +gdb_test "print l" "= 3" "print l, entered via $ept_name" > +gdb_test "print i1" "= 4" "print i1, entered via $ept_name" > +gdb_test "info args" \ > + [multi_line "j = 1" \ > + "k = 2" \ > + "l = 3" \ > + "i1 = 4"] \ > + "info args, entered via $ept_name" > + > +# Test if we can set a breakpoint via function name > +set ept_name "bar" > +gdb_breakpoint $ept_name > +gdb_test "continue" \ > + [multi_line "Breakpoint $decimal, $ept_name \\(i=4, j=5, k=6, i1=7\\) at .*" \ > + ".*"] \ > + "continue to breakpoint: $ept_name" > + > +gdb_test "print i" "= 4" "print i, entered via $ept_name" > +gdb_test "print j" "= 5" "print j, entered via $ept_name" > +gdb_test "print k" "= 6" "print k, entered via $ept_name" > +gdb_test "print i1" "= 7" "print i1, entered via $ept_name" > + > +set ept_name "tim" > +gdb_breakpoint $ept_name > +gdb_test "continue" \ > + [multi_line "Breakpoint $decimal, $ept_name \\(j=1\\) at .*" \ > + ".*"] \ > + "continue to breakpoint: $ept_name" > + > +gdb_test "print j" "= 1" "print j, entered via $ept_name" > +gdb_test "info args" "j = 1" "info args, entered via $ept_name" > diff --git a/gdb/testsuite/gdb.fortran/entry-point.f90 b/gdb/testsuite/gdb.fortran/entry-point.f90 > new file mode 100644 > index 000000000000..cb663b956982 > --- /dev/null > +++ b/gdb/testsuite/gdb.fortran/entry-point.f90 > @@ -0,0 +1,48 @@ > +! 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 . > + > +program TestEntryPoint > + > + call foo(1,2,3,4) > + call bar(4,5,6,7) > + call tim(1) > + > +end program TestEntryPoint > + > + subroutine bar(I,J,K,I1) > + INTEGER I,J,K,L,I1 > + INTEGER A > + REAL C > + > + A = 0 > + C = 0.0 > + > + A = I + K + I1 > + goto 1000 > + > + entry foo(J,K,L,I1) > + A = J + K + L + I1 > + > +200 C = J > + goto 1000 > + > + entry tim(J) > + goto 200 > + > +1000 A = C + 1 > + C = J * 1.5 > + > + return > + end subroutine > -- > 2.7.4 > Thanks, Andrew