[PATCH] DWARF: Add .debug_addr table header for dwarf_version >= 5.

[PATCH] DWARF: Add .debug_addr table header for dwarf_version >= 5.

[Mark Wielaard]

GNU DebugFission didn't add table headers for the .debug_addr
tables, but DWARF5 does. The table header make it possible
for a DWARF consumer to parse the address tables without having
to index all .debug_info CUs first. We can keep using the
.debug_addr section label as is, because the DW_AT_addr_base
attribute points at the actual address index, which starts
right after the table header. So the label is generated at
the correct location whether the header is added first or not.

gcc/ChangeLog

	* dwarf2out.c (dwarf2out_finish): Add .debug_addr table header for
	dwarf_version >= 5.

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index d3d925d..51d0ca4 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -31293,6 +31293,21 @@ dwarf2out_finish (const char *)
 	}
 
       switch_to_section (debug_addr_section);
+      /* GNU DebugFission didn't have a header for .debug_addr.  */
+      if (dwarf_version >= 5)
+	{
+	  unsigned long addrs_length =
+	    addr_index_table->elements () * DWARF2_ADDR_SIZE + 4;
+
+	  if (DWARF_INITIAL_LENGTH_SIZE - DWARF_OFFSET_SIZE == 4)
+	    dw2_asm_output_data (4, 0xffffffff,
+				 "Escape value for 64-bit DWARF extension");
+	  dw2_asm_output_data (DWARF_OFFSET_SIZE, addrs_length,
+			       "Length of Address Unit");
+	  dw2_asm_output_data (2, 5, "DWARF addr version");
+	  dw2_asm_output_data (1, DWARF2_ADDR_SIZE, "Size of Address");
+	  dw2_asm_output_data (1, 0, "Size of Segment Descriptor");
+	}
       ASM_OUTPUT_LABEL (asm_out_file, debug_addr_section_label);
       output_addr_table ();
     }

Re: [PATCH] DWARF: Add .debug_addr table header for dwarf_version >= 5.

[Eric Botcazou]

> index d3d925d..51d0ca4 100644
> --- a/gcc/dwarf2out.c
> +++ b/gcc/dwarf2out.c
> @@ -31293,6 +31293,21 @@ dwarf2out_finish (const char *)
>  	}
> 
>        switch_to_section (debug_addr_section);
> +      /* GNU DebugFission didn't have a header for .debug_addr.  */
> +      if (dwarf_version >= 5)

That sounds LLVMish...  Did you mean -gsplit-dwarf instead?

-- 
Eric Botcazou

Re: [PATCH] DWARF: Add .debug_addr table header for dwarf_version >= 5.

[Mark Wielaard]

Hi Eric,

On Sat, 2018-04-28 at 18:00 +0200, Eric Botcazou wrote:
> > index d3d925d..51d0ca4 100644
> > --- a/gcc/dwarf2out.c
> > +++ b/gcc/dwarf2out.c
> > @@ -31293,6 +31293,21 @@ dwarf2out_finish (const char *)
> >  	}
> > 
> >        switch_to_section (debug_addr_section);
> > +      /* GNU DebugFission didn't have a header for .debug_addr.  */
> > +      if (dwarf_version >= 5)
> 
> That sounds LLVMish...  Did you mean -gsplit-dwarf instead?

I don't know what LLVMish does, or whether it implements the GNU
DebugFission variant of split DWARF, the DWARF5 variant or something
else. For GCC -gsplit-dwarf generates GNU DebugFission output
(including .debug_addr) for older DWARF versions.
It is described here: https://gcc.gnu.org/wiki/DebugFission

This got standardized for DWARF5. But there are some differences. One
particular change is that GNU DebugFission doesn't have address table
headers in .debug_addr sections. But DWARF5 does. This patch fixes
that.

Or did you mean that you expected the code to be guarded by 
if (dwarf_split_debug_info)? It is, just a bit higher up in the code.

Cheers,

Mark

Re: [PATCH] DWARF: Add .debug_addr table header for dwarf_version >= 5.

[Jakub Jelinek]

On Sat, Apr 28, 2018 at 05:23:12PM +0200, Mark Wielaard wrote:
> GNU DebugFission didn't add table headers for the .debug_addr
> tables, but DWARF5 does. The table header make it possible
> for a DWARF consumer to parse the address tables without having
> to index all .debug_info CUs first. We can keep using the
> .debug_addr section label as is, because the DW_AT_addr_base
> attribute points at the actual address index, which starts
> right after the table header. So the label is generated at
> the correct location whether the header is added first or not.
> 
> gcc/ChangeLog
> 
> 	* dwarf2out.c (dwarf2out_finish): Add .debug_addr table header for
> 	dwarf_version >= 5.
> 
> diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
> index d3d925d..51d0ca4 100644
> --- a/gcc/dwarf2out.c
> +++ b/gcc/dwarf2out.c
> @@ -31293,6 +31293,21 @@ dwarf2out_finish (const char *)
>  	}
>  
>        switch_to_section (debug_addr_section);
> +      /* GNU DebugFission didn't have a header for .debug_addr.  */
> +      if (dwarf_version >= 5)
> +	{
> +	  unsigned long addrs_length =
> +	    addr_index_table->elements () * DWARF2_ADDR_SIZE + 4;

No = at the end of line, it should be at the start of the next line, i.e.
	  unsigned long addrs_length
	    = addr_index_table->elements () * DWARF2_ADDR_SIZE + 4;

> +
> +	  if (DWARF_INITIAL_LENGTH_SIZE - DWARF_OFFSET_SIZE == 4)
> +	    dw2_asm_output_data (4, 0xffffffff,
> +				 "Escape value for 64-bit DWARF extension");
> +	  dw2_asm_output_data (DWARF_OFFSET_SIZE, addrs_length,
> +			       "Length of Address Unit");
> +	  dw2_asm_output_data (2, 5, "DWARF addr version");
> +	  dw2_asm_output_data (1, DWARF2_ADDR_SIZE, "Size of Address");
> +	  dw2_asm_output_data (1, 0, "Size of Segment Descriptor");
> +	}
>        ASM_OUTPUT_LABEL (asm_out_file, debug_addr_section_label);
>        output_addr_table ();
>      }

Shouldn't we together with the addition of the .debug_addr section header
also switch to using DW_AT_addr_base rather than DW_AT_GNU_addr_base, i.e.
add DW_AT_addr_base into dwarf_AT and use dwarf_AT (DW_AT_addr_base)
instead of DW_AT_GNU_addr_base?

	Jakub

Re: [PATCH] DWARF: Add .debug_addr table header for dwarf_version >= 5.

[Eric Botcazou]

> I don't know what LLVMish does, or whether it implements the GNU
> DebugFission variant of split DWARF, the DWARF5 variant or something
> else. For GCC -gsplit-dwarf generates GNU DebugFission output
> (including .debug_addr) for older DWARF versions.
> It is described here: https://gcc.gnu.org/wiki/DebugFission

Yes, that's what I was referring to.  DebugFission is the name of the Wiki 
page so let's use "GNU split DWARF" or -gsplit-dwarf in the comment please.

-- 
Eric Botcazou

Re: [PATCH] DWARF: Add .debug_addr table header for dwarf_version >= 5.

[Mark Wielaard]

On Sat, Apr 28, 2018 at 06:36:02PM +0200, Jakub Jelinek wrote:
> On Sat, Apr 28, 2018 at 05:23:12PM +0200, Mark Wielaard wrote:
> >        switch_to_section (debug_addr_section);
> > +      /* GNU DebugFission didn't have a header for .debug_addr.  */
> > +      if (dwarf_version >= 5)
> > +	{
> > +	  unsigned long addrs_length =
> > +	    addr_index_table->elements () * DWARF2_ADDR_SIZE + 4;
> 
> No = at the end of line, it should be at the start of the next line, i.e.
> 	  unsigned long addrs_length
> 	    = addr_index_table->elements () * DWARF2_ADDR_SIZE + 4;

Oops. Fixed.

> > +	  if (DWARF_INITIAL_LENGTH_SIZE - DWARF_OFFSET_SIZE == 4)
> > +	    dw2_asm_output_data (4, 0xffffffff,
> > +				 "Escape value for 64-bit DWARF extension");
> > +	  dw2_asm_output_data (DWARF_OFFSET_SIZE, addrs_length,
> > +			       "Length of Address Unit");
> > +	  dw2_asm_output_data (2, 5, "DWARF addr version");
> > +	  dw2_asm_output_data (1, DWARF2_ADDR_SIZE, "Size of Address");
> > +	  dw2_asm_output_data (1, 0, "Size of Segment Descriptor");
> > +	}
> >        ASM_OUTPUT_LABEL (asm_out_file, debug_addr_section_label);
> >        output_addr_table ();
> >      }
> 
> Shouldn't we together with the addition of the .debug_addr section header
> also switch to using DW_AT_addr_base rather than DW_AT_GNU_addr_base, i.e.
> add DW_AT_addr_base into dwarf_AT and use dwarf_AT (DW_AT_addr_base)
> instead of DW_AT_GNU_addr_base?

Yes of course. I was actually checking the DWARF version of the CU, but
having the correct attribute is obviously better.

Updated patch:

GNU DebugFission didn't add table headers for the .debug_addr
tables, but DWARF5 does. The table header makes it possible
for a DWARF consumer to parse the address tables without having
to index all .debug_info CUs first.

We can keep using the .debug_addr section label as is, because the
DW_AT_[GNU_]addr_base attribute points at the actual address index,
which starts right after the table header. So the label is generated
at the correct location whether the header is added first or not.

Add DW_AT_addr_base instead of DW_AT_GNU_addr_base to the skeleton
CU DIE for DWARF5.

gcc/ChangeLog

	* dwarf2out.c (dwarf2out_finish): Add .debug_addr table header for
	dwarf_version >= 5.
	(dwarf_AT): Handle DW_AT_addr_base.
	(add_top_level_skeleton_die_attrs): Use dwarf_AT for DW_AT_addr_base.

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index d3d925d..c172387 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -1724,6 +1724,11 @@ dwarf_AT (enum dwarf_attribute at)
 	return DW_AT_GNU_dwo_name;
       break;
 
+    case DW_AT_addr_base:
+      if (dwarf_version < 5)
+	return DW_AT_GNU_addr_base;
+      break;
+
     default:
       break;
     }
@@ -11106,7 +11111,7 @@ add_top_level_skeleton_die_attrs (dw_die_ref die)
   if (comp_dir != NULL)
     add_skeleton_AT_string (die, DW_AT_comp_dir, comp_dir);
   add_AT_pubnames (die);
-  add_AT_lineptr (die, DW_AT_GNU_addr_base, debug_addr_section_label);
+  add_AT_lineptr (die, dwarf_AT (DW_AT_addr_base), debug_addr_section_label);
 }
 
 /* Output skeleton debug sections that point to the dwo file.  */
@@ -31293,6 +31298,21 @@ dwarf2out_finish (const char *)
 	}
 
       switch_to_section (debug_addr_section);
+      /* GNU DebugFission didn't have a header for .debug_addr.  */
+      if (dwarf_version >= 5)
+	{
+	  unsigned long addrs_length
+	    = addr_index_table->elements () * DWARF2_ADDR_SIZE + 4;
+
+	  if (DWARF_INITIAL_LENGTH_SIZE - DWARF_OFFSET_SIZE == 4)
+	    dw2_asm_output_data (4, 0xffffffff,
+				 "Escape value for 64-bit DWARF extension");
+	  dw2_asm_output_data (DWARF_OFFSET_SIZE, addrs_length,
+			       "Length of Address Unit");
+	  dw2_asm_output_data (2, 5, "DWARF addr version");
+	  dw2_asm_output_data (1, DWARF2_ADDR_SIZE, "Size of Address");
+	  dw2_asm_output_data (1, 0, "Size of Segment Descriptor");
+	}
       ASM_OUTPUT_LABEL (asm_out_file, debug_addr_section_label);
       output_addr_table ();
     }

Re: [PATCH] DWARF: Add .debug_addr table header for dwarf_version >= 5.

[Jakub Jelinek]

On Sat, Apr 28, 2018 at 09:01:45PM +0200, Mark Wielaard wrote:
> gcc/ChangeLog
> 
> 	* dwarf2out.c (dwarf2out_finish): Add .debug_addr table header for
> 	dwarf_version >= 5.
> 	(dwarf_AT): Handle DW_AT_addr_base.
> 	(add_top_level_skeleton_die_attrs): Use dwarf_AT for DW_AT_addr_base.

Ok for trunk.

	Jakub

Re: [PATCH] DWARF: Add .debug_addr table header for dwarf_version >= 5.

[Mark Wielaard]

On Sat, Apr 28, 2018 at 08:54:00PM +0200, Eric Botcazou wrote:
> > For GCC -gsplit-dwarf generates GNU DebugFission output
> > (including .debug_addr) for older DWARF versions.
> > It is described here: https://gcc.gnu.org/wiki/DebugFission
> 
> Yes, that's what I was referring to.  DebugFission is the name of the Wiki 
> page so let's use "GNU split DWARF" or -gsplit-dwarf in the comment please.

I expanded the comment into a little paragraph explaining the terminology
used and included the URL for background information. Hope that makes
things more clear.

Thanks,

Mark

Re: [PATCH] DWARF: Add .debug_addr table header for dwarf_version >= 5.

[Jakub Jelinek]

On Sat, Apr 28, 2018 at 08:54:00PM +0200, Eric Botcazou wrote:
> > I don't know what LLVMish does, or whether it implements the GNU
> > DebugFission variant of split DWARF, the DWARF5 variant or something
> > else. For GCC -gsplit-dwarf generates GNU DebugFission output
> > (including .debug_addr) for older DWARF versions.
> > It is described here: https://gcc.gnu.org/wiki/DebugFission
> 
> Yes, that's what I was referring to.  DebugFission is the name of the Wiki 
> page so let's use "GNU split DWARF" or -gsplit-dwarf in the comment please.

-gsplit-dwarf is not appropriate, because that includes the DWARF5 variant
thereof.  GNU split DWARF extension is ok.

	Jakub