RE: [PING][RFC][PATCH 0/1] Fortran entry and DW_TAG_entry_point

RE: [PING][RFC][PATCH 0/1] Fortran entry and DW_TAG_entry_point

[Kempke, Nils-Christian]

Ping!

> -----Original Message-----
> From: Kempke, Nils-Christian <nils-christian.kempke@intel.com>
> Sent: Wednesday, March 23, 2022 11:53 AM
> To: gdb-patches@sourceware.org
> Cc: Kempke, Nils-Christian <nils-christian.kempke@intel.com>
> Subject: [RFC][PATCH 0/1] Fortran entry and DW_TAG_entry_point
> 
> Hi,
> 
> please find attached a patch that enables GDB to parse the
> DW_TAG_entry_point DWARF tag.
> 
> It was already sent to the mailing list a while ago but got lost in
> review (see the commit message of the attached patch).
> 
> The patch was originally written with ifort in mind, the only compiler
> we know of, that actually emits the DW_TAG_entry_point.  It is used to
> describe alternative entry points to functions defined with Fortran's
> 'entry' keyword.  With this patch it becomes possible to define and hit
> breakpoints in an executable that uses the 'entry' feature and has been
> compiled with ifort.
> 
> Gfortran and ifx actually follow a different way to describe Fortran entry
> points.  Both emit a DW_TAG_subprogram instead.
> 
> Sadly, the DWARF emitted by ifort is not quite correct for the given
> test (or any other entry point test we tried).  Some parameters of
> subroutines and entry points have a wrong DWARF location description.  So
> even though hitting breakpoints is possible in the test, printing of the
> passed variables fails and running the test with ifort will still produce
> FAILs.
> 
> With this in mind, we were unsure what to do with this patch, thus the RFC.
> On the one hand we did not want the patch to be lost.  On the other hand
> the
> compiler the patch was written for has some issues in its emitted DWARF
> and we do not know when these issues can/will get fixed.
> 
> Note, that the attached test only fully passes with gfortran.  Ifx, while
> able to define breakpoints at entry points, also has issues producing
> correct DWARF for entry points (using the DW_TAG_subprogram approach).
> 
> The patch was tested with the standard board, unix/-m32,
> navtive-gdbserver and native-extended-gdbserver and we did not find any
> regressions with these.
> 
> Any feedback is highly welcome.
> 
> Cheers!
> 
> Nils
> 
> Nils-Christian Kempke (1):
>   dwarf, fortran: add support for DW_TAG_entry_point
> 
>  gdb/dwarf2/read.c                         | 101 ++++++++++++++++++++++
>  gdb/testsuite/gdb.fortran/entry-point.exp |  67 ++++++++++++++
>  gdb/testsuite/gdb.fortran/entry-point.f90 |  48 ++++++++++
>  3 files changed, 216 insertions(+)
>  create mode 100644 gdb/testsuite/gdb.fortran/entry-point.exp
>  create mode 100644 gdb/testsuite/gdb.fortran/entry-point.f90
> 
> --
> 2.25.1

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

RE: [PING][RFC][PATCH 0/1] Fortran entry and DW_TAG_entry_point

[George, Jini Susan]

[AMD Official Use Only]

Hello Nils,

Thanks for the patch. I am not an approver, but I had a few questions/ observations regarding it.

* In dwarf2/read.c, we would need to modify determine_prefix() to take care of the situation wherein if we have entry points defined inside modules, the module name qualifier gets appended to the name of the entry point. Unless this is done, gdb would not be able to place breakpoints at (or understand) symbol names of the form <module name>::<entry point name>.

* Shouldn't we consider the case of having the call to dwarf2_get_pc_bounds() (for the parent) return PC_BOUNDS_RANGES in  dwarf2_get_pc_bounds_entry_point() for discontiguous addresses ? We return PC_BOUNDS_HIGH_LOW from dwarf2_get_pc_bounds_entry_point() in the error free case, irrespective of whether the DIE has AT_ranges or not.

* With your patch, the breakpoints placed using entry point names get hit for the cases where there are explicit calls to the entry points, but not for the fallthrough cases. The reason seems to be that the breakpoints are getting placed at the start address of the entry point as against after its prologue end. Is this something that you plan to fix ?

Thanks!
Jini


>>-----Original Message-----
>>From: Gdb-patches <gdb-patches-
>>bounces+jigeorge=amd.com@sourceware.org> On Behalf Of Kempke, Nils-
>>Christian via Gdb-patches
>>Sent: Friday, April 8, 2022 6:55 PM
>>To: gdb-patches@sourceware.org
>>Subject: RE: [PING][RFC][PATCH 0/1] Fortran entry and DW_TAG_entry_point
>>
>>[CAUTION: External Email]
>>
>>Ping!
>>
>>> -----Original Message-----
>>> From: Kempke, Nils-Christian <nils-christian.kempke@intel.com>
>>> Sent: Wednesday, March 23, 2022 11:53 AM
>>> To: gdb-patches@sourceware.org
>>> Cc: Kempke, Nils-Christian <nils-christian.kempke@intel.com>
>>> Subject: [RFC][PATCH 0/1] Fortran entry and DW_TAG_entry_point
>>>
>>> Hi,
>>>
>>> please find attached a patch that enables GDB to parse the
>>> DW_TAG_entry_point DWARF tag.
>>>
>>> It was already sent to the mailing list a while ago but got lost in
>>> review (see the commit message of the attached patch).
>>>
>>> The patch was originally written with ifort in mind, the only compiler
>>> we know of, that actually emits the DW_TAG_entry_point.  It is used to
>>> describe alternative entry points to functions defined with Fortran's
>>> 'entry' keyword.  With this patch it becomes possible to define and
>>> hit breakpoints in an executable that uses the 'entry' feature and has
>>> been compiled with ifort.
>>>
>>> Gfortran and ifx actually follow a different way to describe Fortran
>>> entry points.  Both emit a DW_TAG_subprogram instead.
>>>
>>> Sadly, the DWARF emitted by ifort is not quite correct for the given
>>> test (or any other entry point test we tried).  Some parameters of
>>> subroutines and entry points have a wrong DWARF location description.
>>> So even though hitting breakpoints is possible in the test, printing
>>> of the passed variables fails and running the test with ifort will
>>> still produce FAILs.
>>>
>>> With this in mind, we were unsure what to do with this patch, thus the RFC.
>>> On the one hand we did not want the patch to be lost.  On the other
>>> hand the compiler the patch was written for has some issues in its
>>> emitted DWARF and we do not know when these issues can/will get fixed.
>>>
>>> Note, that the attached test only fully passes with gfortran.  Ifx,
>>> while able to define breakpoints at entry points, also has issues
>>> producing correct DWARF for entry points (using the DW_TAG_subprogram
>>approach).
>>>
>>> The patch was tested with the standard board, unix/-m32,
>>> navtive-gdbserver and native-extended-gdbserver and we did not find
>>> any regressions with these.
>>>
>>> Any feedback is highly welcome.
>>>
>>> Cheers!
>>>
>>> Nils
>>>
>>> Nils-Christian Kempke (1):
>>>   dwarf, fortran: add support for DW_TAG_entry_point
>>>
>>>  gdb/dwarf2/read.c                         | 101 ++++++++++++++++++++++
>>>  gdb/testsuite/gdb.fortran/entry-point.exp |  67 ++++++++++++++
>>>  gdb/testsuite/gdb.fortran/entry-point.f90 |  48 ++++++++++
>>>  3 files changed, 216 insertions(+)
>>>  create mode 100644 gdb/testsuite/gdb.fortran/entry-point.exp
>>>  create mode 100644 gdb/testsuite/gdb.fortran/entry-point.f90
>>>
>>> --
>>> 2.25.1
>>
>>Intel Deutschland GmbH
>>Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
>>Tel: +49 89 99 8853-0,
>>https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.int
>>el.de%2F&amp;data=04%7C01%7CJiniSusan.George%40amd.com%7C33d2efb2
>>3add4017189908da19633e06%7C3dd8961fe4884e608e11a82d994e183d%7C0%
>>7C0%7C637850211304702448%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wL
>>jAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;s
>>data=X6zw98TRaS%2B42zrglcOX4sZTcueY6xHIV1woFVBNbxA%3D&amp;reserve
>>d=0
>><https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.in
>>tel.de%2F&amp;data=04%7C01%7CJiniSusan.George%40amd.com%7C33d2efb2
>>3add4017189908da19633e06%7C3dd8961fe4884e608e11a82d994e183d%7C0%
>>7C0%7C637850211304702448%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wL
>>jAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;s
>>data=X6zw98TRaS%2B42zrglcOX4sZTcueY6xHIV1woFVBNbxA%3D&amp;reserve
>>d=0>
>>Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva
>>Chairperson of the Supervisory Board: Nicole Lau Registered Office: Munich
>>Commercial Register: Amtsgericht Muenchen HRB 186928

RE: [PING][RFC][PATCH 0/1] Fortran entry and DW_TAG_entry_point

[Kempke, Nils-Christian]

Hi Jini,

Thanks for the comments!

> From: George, Jini Susan <JiniSusan.George@amd.com>
> Sent: Sunday, April 10, 2022 6:48 PM
> Hello Nils,
> 
> Thanks for the patch. I am not an approver, but I had a few questions/
> observations regarding it.
> 
> * In dwarf2/read.c, we would need to modify determine_prefix() to take
> care of the situation wherein if we have entry points defined inside modules,
> the module name qualifier gets appended to the name of the entry point.
> Unless this is done, gdb would not be able to place breakpoints at (or
> understand) symbol names of the form <module name>::<entry point
> name>.
> 
I think this should normally be the case.  I added DW_TAG_entry_point to the 
die_needs_namespace list which imo should trigger the calling of
determine_prefix(...) in dwarf2_compute_name(...).  There is already a case
for a DW_TAG_module in determine_prefix(...) which should generate
   <module name>::<entry point name>
style prefix if the entry point were actually child of a module name.

But you are still correct (at least for ifort): I checked this and there is actually
no namespace being emitted if I declare the entry points inside a module.  But the
reason is that the entry points are emitted as children of their enclosing subroutine
and not the whole module.  So what is actually needed is a check whether the
enclosing DW_TAG_subprogram is contained in a module when we are looking at an
entry point (so I think the prefix determination should be just like the default
clause in determine_prefix(...)).

Thanks for finding that, I adapted my patch and it should be there in v2. I will also
add a test for this.

> * Shouldn't we consider the case of having the call to
> dwarf2_get_pc_bounds() (for the parent) return PC_BOUNDS_RANGES in
> dwarf2_get_pc_bounds_entry_point() for discontiguous addresses ? We
> return PC_BOUNDS_HIGH_LOW from
> dwarf2_get_pc_bounds_entry_point() in the error free case, irrespective of
> whether the DIE has AT_ranges or not.

Yes, I think this is correct.  The dwarf2_get_pc_bounds_entry_point(...) should
Probably return the PC_BOUNDS_RANGES over PC_BOUNDS_HIGH_LOW depending
on its parent's bounds..

Also updated for v2.

> * With your patch, the breakpoints placed using entry point names get hit for
> the cases where there are explicit calls to the entry points, but not for the
> fallthrough cases. The reason seems to be that the breakpoints are getting
> placed at the start address of the entry point as against after its prologue
> end. Is this something that you plan to fix ?

I am a bit lost here. Here are my thoughts on this:

Looking at the test I sent with my patch and compiling it with ifort I get
running gdb (with my patch and the above 2 fixes) and breaking at foo:

   (gdb) b foo
   Breakpoint 1 at 0x40396d: file ./test.f90, line 3.
   (gdb) info address foo
   Symbol "test_mod::foo" is a function at address 0x403948.

 And I see in the DWARF generated for the example:

<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      : 0x403948

So this seems right at first. Some prologue also seems to have
been skipped when setting the BP.  Hitting disassemble after a BP
hit I get

   (gdb) disassemble
   Dump of assembler code for function foo:
      0x0000000000403948 <+0>:     push   %rbp
      0x0000000000403949 <+1>:     mov    %rsp,%rbp
      0x000000000040394c <+4>:     sub    $0xa0,%rsp
      0x0000000000403953 <+11>:    mov    %rdi,-0x68(%rbp)
      0x0000000000403957 <+15>:    mov    %rsi,-0x60(%rbp)
      0x000000000040395b <+19>:    mov    %rdx,-0x58(%rbp)
      0x000000000040395f <+23>:    mov    %rcx,-0x50(%rbp)
      0x0000000000403963 <+27>:    movl   $0x2,-0xa0(%rbp)
=> 0x000000000040396d <+37>:    mov    -0x68(%rbp),%rax
      0x0000000000403971 <+41>:    mov    %rax,-0x9c(%rbp)

Checking the line-table for the executable (dwarfdump -l) I see

   0x0040393b  [  33, 0] NS
   0x00403948  [  35, 0] NS
   0x0040396d  [  24, 0] NS PE
   0x00403983  [  35, 0] NS
   0x0040398e  [  24, 0] NS
   0x00403999  [  36, 0] NS
   0x004039c0  [  38, 0] NS
   0x004039d2  [  39, 0] NS
   0x004039dc  [  41, 0] NS

So this seems to tell me that the prologue_end for the function with the first
line at 0x403948 is at 0x40396d which would be exactly where I am at in the
executable.  So DWARF/GDB wise, all this seems fine to me.

What could definitely be improved is the way ifort emits the prologue_end here.
You can see, that after the first PE we jump to line 35, then again to line 24 and then
finally to 36.  Only once we reach this 36 (0x00403999), we can see that the
bt command works in gdb.  Before 0x00403999 not all of the variables (j, k, l, and I1)
have been set up and GDB will print stuff like

   (gdb) bt
   #0  foo (j=<error reading variable: Cannot access memory at address 0x0>,
       k=<error reading variable: Cannot access memory at address 0x0>,
       l=<error reading variable: Cannot access memory at address 0x0>,
       i1=<error reading variable: Cannot access memory at address 0x0>) at ./test.f90:24
   #1  0x0000000000403948 in testentrypoint () at ./test.f90:18

 So imo this should be the actual PE and with that we would also
hit the location every time we run through the executable.

But this is an ifort issue I believe and should ideally not be caused by my patch.  We
have opened a ticket for this internally but, as I said in my initial email, it might
take a while until this gets resolved.

Is this the issue you meant?

Thanks again for your comments!

Nils
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928