Re: [PATCH v2 2/7] Dwarf: Fortran, support DW_TAG_entry_point.

[Paweł Wódkowski <pwodkowski at pl dot sii dot eu>]

On 27.11.2018 23:08, Andrew Burgess wrote:
> * Pawel Wodkowski <pwodkowski@pl.sii.eu> [2018-11-19 22:38:45 +0100]:
> 
>> From: Bernhard Heckel <bernhard.heckel@intel.com>
>>
>> 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.
> 

As I checked gfortran will not generate entry point tags so for now it 
is only for Intel ifort compiler. The entry-point.f90 example compiled 
using ifort produce following entry point tag:

...
<2><9e>: Abbrev Number: 5 (DW_TAG_entry_point)
     <9f>   DW_AT_decl_line   : 35
     <a0>   DW_AT_decl_file   : 1
     <a1>   DW_AT_name        : foo
     <a5>   DW_AT_low_pc      : 0x402ba8
...
<3><c9>: Abbrev Number: 4 (DW_TAG_formal_parameter)
     <ca>   DW_AT_decl_line   : 35
     <cb>   DW_AT_decl_file   : 1
     <cc>   DW_AT_type        : <0x120>
     <d0>   DW_AT_name        : l
     <d2>   DW_AT_location    : 4 byte block: 76 f4 7e 6 
(DW_OP_breg6 (rbp): -140; DW_OP_deref)
...

I will add this to commit message.

Thanks
Pawel

>>
>> 2016-06-01  Bernhard Heckel  <bernhard.heckel@intel.com>
>>
>> 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 <http://www.gnu.org/licenses/>.
>> +
>> +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 <http://www.gnu.org/licenses/>.
>> +
>> +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
>