RFC: Support DW_TAG_entry_point

RFC: Support DW_TAG_entry_point

[H.J. Lu]

I have a patch to support DW_TAG_entry_point. Should I submit it
with a ChangeLog entry?


H.J.
--- gdb/dwarf2read.c.entry	2008-05-07 10:41:46.000000000 -0700
+++ gdb/dwarf2read.c	2008-05-07 11:26:14.000000000 -0700
@@ -1780,6 +1780,7 @@ scan_partial_symbols (struct partial_die
 	  switch (pdi->tag)
 	    {
 	    case DW_TAG_subprogram:
+	    case DW_TAG_entry_point:
 	      if (pdi->has_pc_info)
 		{
 		  if (pdi->lowpc < *lowpc)
@@ -1829,6 +1830,14 @@ scan_partial_symbols (struct partial_die
 	      break;
 	    }
 	}
+      
+      /* A DW_TAG_subprogram DIE may have DW_TAG_entry_point DIEs
+	 as children.  */ 
+      if (pdi->tag == DW_TAG_subprogram
+	  && pdi->has_children
+	  && pdi->die_child
+	  && pdi->die_child->tag == DW_TAG_entry_point)
+	scan_partial_symbols (pdi->die_child, lowpc, highpc, cu);
 
       /* If the die has a sibling, skip to the sibling.  */
 
@@ -1981,6 +1990,29 @@ add_partial_symbol (struct partial_die_i
 				      cu->language, objfile);
 	}
       break;
+    case DW_TAG_entry_point:
+      if (pdi->die_parent->tag == DW_TAG_subprogram)
+	{
+	  if (pdi->die_parent->is_external)
+	    /*prim_record_minimal_symbol (actual_name, pdi->lowpc + baseaddr,
+	      mst_text, objfile); */
+	    psym = add_psymbol_to_list (actual_name,
+					strlen (actual_name),
+					VAR_DOMAIN, LOC_BLOCK,
+					&objfile->global_psymbols,
+					0, pdi->lowpc + baseaddr,
+					cu->language, objfile);
+	  else
+	    /*prim_record_minimal_symbol (actual_name, pdi->lowpc + baseaddr,
+	      mst_file_text, objfile); */
+	    psym = add_psymbol_to_list (actual_name,
+					strlen (actual_name),
+					VAR_DOMAIN, LOC_BLOCK,
+					&objfile->static_psymbols,
+					0, pdi->lowpc + baseaddr,
+					cu->language, objfile);
+	}
+      break;
     case DW_TAG_variable:
       if (pdi->is_external)
 	{
@@ -2713,6 +2745,7 @@ process_die (struct die_info *die, struc
       read_file_scope (die, cu);
       break;
     case DW_TAG_subprogram:
+    case DW_TAG_entry_point:
       read_func_scope (die, cu);
       break;
     case DW_TAG_inlined_subroutine:
@@ -3276,6 +3309,27 @@ dwarf2_get_pc_bounds (struct die_info *d
 	  /* Found discontinuous range of addresses.  */
 	  ret = -1;
 	}
+      else if (die->tag == DW_TAG_entry_point)
+	{
+	  /* DW_TAG_entry_point DIE only has DW_AT_low_pc. We get
+	     HIGH from its parent.  */
+	  attr = dwarf2_attr (die, DW_AT_low_pc, cu);
+	  if (attr)
+	    {
+	      low = DW_ADDR (attr);
+	      if (die->parent->tag == DW_TAG_subprogram)
+		{
+		  CORE_ADDR parent_low = 0;
+		  CORE_ADDR parent_high = 0;
+		  if (dwarf2_get_pc_bounds (die->parent, &parent_low,
+					    &parent_high, cu))
+		    {
+		      high = parent_high;
+		      ret = 1;
+		    }
+		}
+	    } 
+	}
     }
 
   if (high < low)
@@ -3323,6 +3377,7 @@ get_scope_pc_bounds (struct die_info *di
 	{
 	  switch (child->tag) {
 	  case DW_TAG_subprogram:
+	  case DW_TAG_entry_point:
 	    if (dwarf2_get_pc_bounds (child, &current_low, &current_high, cu))
 	      {
 		best_low = min (best_low, current_low);
@@ -5581,6 +5636,7 @@ load_partial_dies (bfd *abfd, gdb_byte *
 	  && !is_type_tag_for_partial (abbrev->tag)
 	  && abbrev->tag != DW_TAG_enumerator
 	  && abbrev->tag != DW_TAG_subprogram
+	  && abbrev->tag != DW_TAG_entry_point
 	  && abbrev->tag != DW_TAG_variable
 	  && abbrev->tag != DW_TAG_namespace
 	  && abbrev->tag != DW_TAG_member)
@@ -5688,6 +5744,7 @@ load_partial_dies (bfd *abfd, gdb_byte *
 
       if (load_all
 	  || abbrev->tag == DW_TAG_subprogram
+	  || abbrev->tag == DW_TAG_entry_point
 	  || abbrev->tag == DW_TAG_variable
 	  || abbrev->tag == DW_TAG_namespace
 	  || part_die->is_declaration)
@@ -5713,6 +5770,7 @@ load_partial_dies (bfd *abfd, gdb_byte *
 	      || (cu->language != language_c
 		  && (last_die->tag == DW_TAG_class_type
 		      || last_die->tag == DW_TAG_interface_type
+		      || last_die->tag == DW_TAG_subprogram
 		      || last_die->tag == DW_TAG_structure_type
 		      || last_die->tag == DW_TAG_union_type))))
 	{
@@ -6028,6 +6086,16 @@ fixup_partial_die (struct partial_die_in
       || part_die->tag == DW_TAG_class_type
       || part_die->tag == DW_TAG_union_type)
     guess_structure_name (part_die, cu);
+
+  if (part_die->tag == DW_TAG_entry_point)
+    {
+      if (part_die->die_parent->tag == DW_TAG_subprogram)
+	{
+	  part_die->highpc = part_die->die_parent->highpc;
+	  if (part_die->lowpc < part_die->highpc)
+	    part_die->has_pc_info = 1;
+	}
+    }
 }
 
 /* Read the die from the .debug_info section buffer.  Set DIEP to
@@ -7437,6 +7505,19 @@ new_symbol (struct die_info *die, struct
 	      add_symbol_to_list (sym, cu->list_in_scope);
 	    }
 	  break;
+	case DW_TAG_entry_point:
+	  if (die->parent->tag == DW_TAG_subprogram)
+	    {
+	      /* SYMBOL_BLOCK_VALUE (sym) will be filled in later by
+		 finish_block.  */
+	      SYMBOL_CLASS (sym) = LOC_BLOCK;
+	      attr2 = dwarf2_attr (die->parent, DW_AT_external, cu);
+	      if (attr2 && (DW_UNSND (attr2) != 0))
+		add_symbol_to_list (sym, &global_symbols);
+	      else
+		add_symbol_to_list (sym, cu->list_in_scope);
+	    }
+	  break;
 	case DW_TAG_variable:
 	  /* Compilation with minimal debug info may result in variables
 	     with missing type entries. Change the misleading `void' type
@@ -7839,6 +7920,7 @@ read_type_die (struct die_info *die, str
       break;
     case DW_TAG_subprogram:
     case DW_TAG_subroutine_type:
+    case DW_TAG_entry_point:
       this_type = read_subroutine_type (die, cu);
       break;
     case DW_TAG_array_type:

Re: RFC: Support DW_TAG_entry_point

[Tom Tromey]

>>>>> "H" == H J Lu <hongjiu.lu@intel.com> writes:

HJ> I have a patch to support DW_TAG_entry_point. Should I submit it
HJ> with a ChangeLog entry?

Yes.  A rationale for the patch would also be helpful, at least to me.

Tom

Re: RFC: Support DW_TAG_entry_point

[H.J. Lu]

On Fri, Mar 20, 2009 at 5:50 PM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "H" == H J Lu <hongjiu.lu@intel.com> writes:
>
> HJ> I have a patch to support DW_TAG_entry_point. Should I submit it
> HJ> with a ChangeLog entry?
>
> Yes.  A rationale for the patch would also be helpful, at least to me.
>

DW_TAG_entry_point may be used by Fortran compiler. But gcc
doesn't use it.  I guess one reason is gdb doesn't support it. Is
this good enough?


-- 
H.J.

Re: RFC: Support DW_TAG_entry_point

[Tom Tromey]

>>>>> "H.J." == H J Lu <hjl.tools@gmail.com> writes:

Tom> Yes.  A rationale for the patch would also be helpful, at least to me.

H.J.> DW_TAG_entry_point may be used by Fortran compiler. But gcc
H.J.> doesn't use it.  I guess one reason is gdb doesn't support it. Is
H.J.> this good enough?

Yeah.  You also need that ChangeLog entry though...

Tom

Re: RFC: Support DW_TAG_entry_point

[H.J. Lu]

On Mon, Mar 23, 2009 at 11:14 AM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "H.J." == H J Lu <hjl.tools@gmail.com> writes:
>
> Tom> Yes.  A rationale for the patch would also be helpful, at least to me.
>
> H.J.> DW_TAG_entry_point may be used by Fortran compiler. But gcc
> H.J.> doesn't use it.  I guess one reason is gdb doesn't support it. Is
> H.J.> this good enough?
>
> Yeah.  You also need that ChangeLog entry though...

Here is ChangeLog entry.  OK to install?

Thanks.

H.J.
---
2009-03-23  H.J. Lu  <hongjiu.lu@intel.com>

	* dwarf2read.c (scan_partial_symbols): Make a recursive call
	if a DIE has children. Handle DW_TAG_entry_point.
	(load_partial_dies): Don't skip DW_TAG_entry_point. Follow
	children of DW_TAG_subprogram if not C.
	(add_partial_symbol): Handle DW_TAG_entry_point.
	(process_die): Likewise.
	(dwarf2_get_pc_bounds): Likewise.
	(get_scope_pc_bounds): Likewise.
	(fixup_partial_die): Likewise.
	(new_symbol): Likewise.
	(read_type_die): Likewise.

Re: RFC: Support DW_TAG_entry_point

[Tom Tromey]

>>>>> "HJ" == H J Lu <hongjiu.lu@intel.com> writes:

HJ> I have a patch to support DW_TAG_entry_point.

I don't really understand most of this patch; perhaps someone more
familiar with dwarf2read.c could review it.

However:

HJ> +      /* A DW_TAG_subprogram DIE may have DW_TAG_entry_point DIEs
HJ> +	 as children.  */ 
HJ> +      if (pdi->tag == DW_TAG_subprogram
HJ> +	  && pdi->has_children
HJ> +	  && pdi->die_child
HJ> +	  && pdi->die_child->tag == DW_TAG_entry_point)
HJ> +	scan_partial_symbols (pdi->die_child, lowpc, highpc, cu);

This looks strange to me.  IIUC, there's no requirement that
DW_TAG_entry_point be the first child.

Tom

Re: RFC: Support DW_TAG_entry_point

[Joel Brobecker]

> HJ> I have a patch to support DW_TAG_entry_point.
> 
> I don't really understand most of this patch; perhaps someone more
> familiar with dwarf2read.c could review it.

I started reviewing it a while ago, but never got to the end of it.

> However:
> 
> HJ> +      /* A DW_TAG_subprogram DIE may have DW_TAG_entry_point DIEs
> HJ> +	 as children.  */ 
> HJ> +      if (pdi->tag == DW_TAG_subprogram
> HJ> +	  && pdi->has_children
> HJ> +	  && pdi->die_child
> HJ> +	  && pdi->die_child->tag == DW_TAG_entry_point)
> HJ> +	scan_partial_symbols (pdi->die_child, lowpc, highpc, cu);
> 
> This looks strange to me.  IIUC, there's no requirement that
> DW_TAG_entry_point be the first child.

That was my main observation at the time.

-- 
Joel

Re: RFC: Support DW_TAG_entry_point

[Jan Kratochvil]

On Thu, 23 Apr 2009 08:10:03 +0200, Joel Brobecker wrote:
> On Wed, 22 Apr 2009 23:13:27 +0200, Tom Tromey wrote:
> > This looks strange to me.  IIUC, there's no requirement that
> > DW_TAG_entry_point be the first child.
> 
> That was my main observation at the time.

I guess the whole patch tagets just one compiler's (ifort's?) use of
DW_TAG_entry_point.  DWARF standard also says neither that DW_TAG_entry_point
should be a child of DW_TAG_subprogram nor that it should not have its own
DW_AT_high_pc.  Therefore assuming DW_TAG_entry_point will be the first child
DIE is ensured for the target compiler this patch was made for.

Still this patch is a clear improvement of the current GDB state while having
no regressions.  Shouldn't be a more complete support left as a possible next
patch on top of it?


Just my $0.02,
Jan

Re: RFC: Support DW_TAG_entry_point

[Tom Tromey]

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> I guess the whole patch tagets just one compiler's (ifort's?) use
Jan> of DW_TAG_entry_point.  DWARF standard also says neither that
Jan> DW_TAG_entry_point should be a child of DW_TAG_subprogram nor
Jan> that it should not have its own DW_AT_high_pc.  Therefore
Jan> assuming DW_TAG_entry_point will be the first child DIE is
Jan> ensured for the target compiler this patch was made for.

Ok, I went and looked through DWARF 3 again to try to understand more.

I think this means that the compiler in question is emitting invalid
DWARF, or at least using its own extension.  In that case I suppose I
would be more inclined to allow this, provided that it doesn't impact
the possibility of correctly implementing DW_TAG_entry_point in the
future (maybe the patch already does this too, I really don't know).

Assuming this is an extension, I would like a comment to that effect,
mentioning the compiler.  A test case wouldn't hurt, either.

Tom

Re: RFC: Support DW_TAG_entry_point

[H.J. Lu]

On Thu, Apr 23, 2009 at 4:42 PM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:
>
> Jan> I guess the whole patch tagets just one compiler's (ifort's?) use
> Jan> of DW_TAG_entry_point.  DWARF standard also says neither that
> Jan> DW_TAG_entry_point should be a child of DW_TAG_subprogram nor
> Jan> that it should not have its own DW_AT_high_pc.  Therefore
> Jan> assuming DW_TAG_entry_point will be the first child DIE is
> Jan> ensured for the target compiler this patch was made for.
>
> Ok, I went and looked through DWARF 3 again to try to understand more.
>
> I think this means that the compiler in question is emitting invalid
> DWARF, or at least using its own extension.  In that case I suppose I
> would be more inclined to allow this, provided that it doesn't impact
> the possibility of correctly implementing DW_TAG_entry_point in the
> future (maybe the patch already does this too, I really don't know).

I will take a look.

> Assuming this is an extension, I would like a comment to that effect,
> mentioning the compiler.  A test case wouldn't hurt, either.
>

I do have a testcase. But you need ifort to use it.


-- 
H.J.

Re: RFC: Support DW_TAG_entry_point

[Joel Brobecker]

> > Assuming this is an extension, I would like a comment to that effect,
> > mentioning the compiler.  A test case wouldn't hurt, either.
> >
> 
> I do have a testcase. But you need ifort to use it.

We have a gdb.dwarf2 section where you can provide hard-coded dwarf2
data. That would be very helpful in making sure that we don't break
GDB with ifort's output later on.

-- 
Joel