public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Remove padding from DWARF5 headers
@ 2017-01-03 23:10 Jakub Jelinek
  2017-01-04 12:58 ` Mark Wielaard
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Jakub Jelinek @ 2017-01-03 23:10 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches, Jan Kratochvil, Mark Wielaard

Hi!

http://dwarfstd.org/ShowIssue.php?issue=161031.2
got approved today, so DWARF5 is changing and the various DW_UT_* kinds
will no longer have the same size of the headers.  So,
DW_UT_compile/DW_UT_partial shrinks by 12/16 bytes (padding 1 and padding 2
is removed; 16 bytes for 64-bit DWARF), DW_UT_type remains the same,
DW_UT_skeleton/DW_UT_split_compile shrink by 4/8 bytes (padding 2 is
removed).  For DW_UT_* kinds consumers don't understand, the first 3 fields
(length, version and ut kind) are required to be present and the only
sensible action is to skip the whole unit (using length field).

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Jan/Mark, are you going to adjust GDB/elfutils etc. correspondingly?

2017-01-03  Jakub Jelinek  <jakub@redhat.com>

	* dwarf2out.c (DWARF_COMPILE_UNIT_HEADER_SIZE): For DWARF5 decrease
	by 12.
	(DWARF_COMDAT_TYPE_UNIT_HEADER_SIZE): Always
	DWARF_COMPILE_UNIT_HEADER_SIZE plus 12.
	(DWARF_COMPILE_UNIT_SKELETON_HEADER_SIZE): Define.
	(calc_base_type_die_sizes): Use DWARF_COMPILE_UNIT_SKELETON_HEADER_SIZE
	for initial die_offset if dwarf_split_debug_info.
	(output_comp_unit): Use DWARF_COMPILE_UNIT_SKELETON_HEADER_SIZE for
	initial next_die_offset if dwo_id is non-NULL.  Don't emit padding
	fields.
	(output_skeleton_debug_sections): Formatting fix.  Use
	DWARF_COMPILE_UNIT_SKELETON_HEADER_SIZE instead of
	DWARF_COMPILE_UNIT_HEADER_SIZE.  Don't emit padding.

--- gcc/dwarf2out.c.jj	2017-01-03 16:04:17.000000000 +0100
+++ gcc/dwarf2out.c	2017-01-03 19:41:45.526194592 +0100
@@ -2996,14 +2996,16 @@ skeleton_chain_node;
 /* Fixed size portion of the DWARF compilation unit header.  */
 #define DWARF_COMPILE_UNIT_HEADER_SIZE \
   (DWARF_INITIAL_LENGTH_SIZE + DWARF_OFFSET_SIZE			\
-   + (dwarf_version >= 5						\
-      ? 4 + DWARF_TYPE_SIGNATURE_SIZE + DWARF_OFFSET_SIZE : 3))
+   + (dwarf_version >= 5 ? 4 : 3))
 
 /* Fixed size portion of the DWARF comdat type unit header.  */
 #define DWARF_COMDAT_TYPE_UNIT_HEADER_SIZE \
   (DWARF_COMPILE_UNIT_HEADER_SIZE					\
-   + (dwarf_version >= 5						\
-      ? 0 : DWARF_TYPE_SIGNATURE_SIZE + DWARF_OFFSET_SIZE))
+   + DWARF_TYPE_SIGNATURE_SIZE + DWARF_OFFSET_SIZE)
+
+/* Fixed size portion of the DWARF skeleton compilation unit header.  */
+#define DWARF_COMPILE_UNIT_SKELETON_HEADER_SIZE \
+  (DWARF_COMPILE_UNIT_HEADER_SIZE + (dwarf_version >= 5 ? 8 : 0))
 
 /* Fixed size portion of public names info.  */
 #define DWARF_PUBNAMES_HEADER_SIZE (2 * DWARF_OFFSET_SIZE + 2)
@@ -9044,7 +9046,9 @@ calc_die_sizes (dw_die_ref die)
 static void
 calc_base_type_die_sizes (void)
 {
-  unsigned long die_offset = DWARF_COMPILE_UNIT_HEADER_SIZE;
+  unsigned long die_offset = (dwarf_split_debug_info
+			      ? DWARF_COMPILE_UNIT_SKELETON_HEADER_SIZE
+			      : DWARF_COMPILE_UNIT_HEADER_SIZE);
   unsigned int i;
   dw_die_ref base_type;
 #if ENABLE_ASSERT_CHECKING
@@ -10302,7 +10306,9 @@ output_comp_unit (dw_die_ref die, int ou
   delete extern_map;
 
   /* Initialize the beginning DIE offset - and calculate sizes/offsets.  */
-  next_die_offset = DWARF_COMPILE_UNIT_HEADER_SIZE;
+  next_die_offset = (dwo_id
+		     ? DWARF_COMPILE_UNIT_SKELETON_HEADER_SIZE
+		     : DWARF_COMPILE_UNIT_HEADER_SIZE);
   calc_die_sizes (die);
 
   oldsym = die->die_id.die_symbol;
@@ -10330,12 +10336,6 @@ output_comp_unit (dw_die_ref die, int ou
       if (dwo_id != NULL)
 	for (int i = 0; i < 8; i++)
 	  dw2_asm_output_data (1, dwo_id[i], i == 0 ? "DWO id" : NULL);
-      else
-	/* Hope all the padding will be removed for DWARF 5 final for
-	   DW_AT_compile and DW_AT_partial.  */
-	dw2_asm_output_data (8, 0, "Padding 1");
-
-      dw2_asm_output_data (DWARF_OFFSET_SIZE, 0, "Padding 2");
     }
   output_die (die);
 
@@ -10430,10 +10430,11 @@ output_skeleton_debug_sections (dw_die_r
      header.  */
   if (DWARF_INITIAL_LENGTH_SIZE - DWARF_OFFSET_SIZE == 4)
     dw2_asm_output_data (4, 0xffffffff,
-      "Initial length escape value indicating 64-bit DWARF extension");
+			 "Initial length escape value indicating 64-bit "
+			 "DWARF extension");
 
   dw2_asm_output_data (DWARF_OFFSET_SIZE,
-                       DWARF_COMPILE_UNIT_HEADER_SIZE
+		       DWARF_COMPILE_UNIT_SKELETON_HEADER_SIZE
                        - DWARF_INITIAL_LENGTH_SIZE
                        + size_of_die (comp_unit),
                       "Length of Compilation Unit Info");
@@ -10449,12 +10450,8 @@ output_skeleton_debug_sections (dw_die_r
   if (dwarf_version < 5)
     dw2_asm_output_data (1, DWARF2_ADDR_SIZE, "Pointer Size (in bytes)");
   else
-    {
-      for (int i = 0; i < 8; i++)
-	dw2_asm_output_data (1, dwo_id[i], i == 0 ? "DWO id" : NULL);
-
-      dw2_asm_output_data (DWARF_OFFSET_SIZE, 0, "Padding 2");
-    }
+    for (int i = 0; i < 8; i++)
+      dw2_asm_output_data (1, dwo_id[i], i == 0 ? "DWO id" : NULL);
 
   comp_unit->die_abbrev = SKELETON_COMP_DIE_ABBREV;
   output_die (comp_unit);

	Jakub

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Remove padding from DWARF5 headers
  2017-01-03 23:10 [PATCH] Remove padding from DWARF5 headers Jakub Jelinek
@ 2017-01-04 12:58 ` Mark Wielaard
  2017-01-04 13:47   ` Jakub Jelinek
  2017-01-04 14:04 ` Jan Kratochvil
  2017-01-11 16:37 ` Jason Merrill
  2 siblings, 1 reply; 6+ messages in thread
From: Mark Wielaard @ 2017-01-04 12:58 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Jason Merrill, gcc-patches, Jan Kratochvil

On Wed, Jan 04, 2017 at 12:09:43AM +0100, Jakub Jelinek wrote:
> http://dwarfstd.org/ShowIssue.php?issue=161031.2
> got approved today, so DWARF5 is changing and the various DW_UT_* kinds
> will no longer have the same size of the headers.  So,
> DW_UT_compile/DW_UT_partial shrinks by 12/16 bytes (padding 1 and padding 2
> is removed; 16 bytes for 64-bit DWARF), DW_UT_type remains the same,
> DW_UT_skeleton/DW_UT_split_compile shrink by 4/8 bytes (padding 2 is
> removed).  For DW_UT_* kinds consumers don't understand, the first 3 fields
> (length, version and ut kind) are required to be present and the only
> sensible action is to skip the whole unit (using length field).

OK, so I assume my alternative proposal to encode which fields are
used in the unit type field got rejected?
http://dwarfstd.org/ShowIssue.php?issue=161130.5
(Any word on other proposals? They don't seem to have been updated
on the website yet.)

Was anything said about what consumers should do when the version is
unknown? Is the length field still valid and can the unit be skipped
or is it game over?

Is any of the range of unit type values reserved for vendor extensions?

> Jan/Mark, are you going to adjust GDB/elfutils etc. correspondingly?

Of course. I was already anticipating this would change.

Thanks,

Mark

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Remove padding from DWARF5 headers
  2017-01-04 12:58 ` Mark Wielaard
@ 2017-01-04 13:47   ` Jakub Jelinek
  2017-01-04 13:51     ` Jakub Jelinek
  0 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2017-01-04 13:47 UTC (permalink / raw)
  To: Mark Wielaard; +Cc: Jason Merrill, gcc-patches, Jan Kratochvil

On Wed, Jan 04, 2017 at 01:58:33PM +0100, Mark Wielaard wrote:
> On Wed, Jan 04, 2017 at 12:09:43AM +0100, Jakub Jelinek wrote:
> > http://dwarfstd.org/ShowIssue.php?issue=161031.2
> > got approved today, so DWARF5 is changing and the various DW_UT_* kinds
> > will no longer have the same size of the headers.  So,
> > DW_UT_compile/DW_UT_partial shrinks by 12/16 bytes (padding 1 and padding 2
> > is removed; 16 bytes for 64-bit DWARF), DW_UT_type remains the same,
> > DW_UT_skeleton/DW_UT_split_compile shrink by 4/8 bytes (padding 2 is
> > removed).  For DW_UT_* kinds consumers don't understand, the first 3 fields
> > (length, version and ut kind) are required to be present and the only
> > sensible action is to skip the whole unit (using length field).
> 
> OK, so I assume my alternative proposal to encode which fields are
> used in the unit type field got rejected?
> http://dwarfstd.org/ShowIssue.php?issue=161130.5
> (Any word on other proposals? They don't seem to have been updated
> on the website yet.)

It will take a while before the web is updated and the changes actually
materialize in the document.

Yes, 161130.5 has been rejected.

As for other proposals, not sure if I remember all the details, I think
160808.1 is in, 161027.1 will say that the hashing of identifiers in
.debug_names (no matter if case sensitive or not) roughly starts by
replacing U+0131 characters with U+0069 (i.e. use i instead of small
dotless i), then applying C + S case folding from Unicode 9.0's
CaseFolding.txt and then hashing the resulting string as described in the
public draft.  At least that is my understanding.  161031.2 accepted,
161031.4 deferred, 161102.1 accepted, 161230.1 accepted, 161122.1
I think we are going to have DW_FORM_strx{1,2,3,4} or something similar
in addition to DW_FORM_strx (i.e. fixes size 1, 2, 3, 4 byte indexes
into .debug_addr), 161130.2 rejected (DW_AT_type should be used instead),
161130.3 rejected, 161206.[123] need more thinking, 161215.[12] rejected,
don't remember others.

> Was anything said about what consumers should do when the version is
> unknown? Is the length field still valid and can the unit be skipped
> or is it game over?

See above, I tried to explain it.  All the units have to have the
unit_length (initial length)
version (uhalf)
unit_type (ubyte)
fields at the beginning of their headers in that order (otherwise you
couldn't even find out what the unit type is), the rest is well defined
only for the unit types documented in the spec.
So, for other unit types (unless they know how to handle them), consumers
should just skip over it (i.e. advance by unit_length from the start of
the version field.

> Is any of the range of unit type values reserved for vendor extensions?

Yes, DW_UT_lo_user ... DW_UT_hi_user.

	Jakub

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Remove padding from DWARF5 headers
  2017-01-04 13:47   ` Jakub Jelinek
@ 2017-01-04 13:51     ` Jakub Jelinek
  0 siblings, 0 replies; 6+ messages in thread
From: Jakub Jelinek @ 2017-01-04 13:51 UTC (permalink / raw)
  To: Mark Wielaard; +Cc: Jason Merrill, gcc-patches, Jan Kratochvil

On Wed, Jan 04, 2017 at 02:46:51PM +0100, Jakub Jelinek wrote:
> As for other proposals, not sure if I remember all the details, I think
> 160808.1 is in, 161027.1 will say that the hashing of identifiers in
> .debug_names (no matter if case sensitive or not) roughly starts by
> replacing U+0131 characters with U+0069 (i.e. use i instead of small

U+0130 as well with U+0069.  Basically the hashing is non-Turkish as well
as Turkish locale compatible, as all of U+0049, U+0069, U+0130 and U+0131
are hashed the same.

	Jakub

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Remove padding from DWARF5 headers
  2017-01-03 23:10 [PATCH] Remove padding from DWARF5 headers Jakub Jelinek
  2017-01-04 12:58 ` Mark Wielaard
@ 2017-01-04 14:04 ` Jan Kratochvil
  2017-01-11 16:37 ` Jason Merrill
  2 siblings, 0 replies; 6+ messages in thread
From: Jan Kratochvil @ 2017-01-04 14:04 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Jason Merrill, gcc-patches, Mark Wielaard

On Wed, 04 Jan 2017 00:09:43 +0100, Jakub Jelinek wrote:
> Jan/Mark, are you going to adjust GDB/elfutils etc. correspondingly?

My GDB DWARF-5 patchset is still off-trunk so that is sure OK.


Jan

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Remove padding from DWARF5 headers
  2017-01-03 23:10 [PATCH] Remove padding from DWARF5 headers Jakub Jelinek
  2017-01-04 12:58 ` Mark Wielaard
  2017-01-04 14:04 ` Jan Kratochvil
@ 2017-01-11 16:37 ` Jason Merrill
  2 siblings, 0 replies; 6+ messages in thread
From: Jason Merrill @ 2017-01-11 16:37 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches List, Jan Kratochvil, Mark Wielaard

OK.

On Tue, Jan 3, 2017 at 6:09 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> http://dwarfstd.org/ShowIssue.php?issue=161031.2
> got approved today, so DWARF5 is changing and the various DW_UT_* kinds
> will no longer have the same size of the headers.  So,
> DW_UT_compile/DW_UT_partial shrinks by 12/16 bytes (padding 1 and padding 2
> is removed; 16 bytes for 64-bit DWARF), DW_UT_type remains the same,
> DW_UT_skeleton/DW_UT_split_compile shrink by 4/8 bytes (padding 2 is
> removed).  For DW_UT_* kinds consumers don't understand, the first 3 fields
> (length, version and ut kind) are required to be present and the only
> sensible action is to skip the whole unit (using length field).
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> Jan/Mark, are you going to adjust GDB/elfutils etc. correspondingly?
>
> 2017-01-03  Jakub Jelinek  <jakub@redhat.com>
>
>         * dwarf2out.c (DWARF_COMPILE_UNIT_HEADER_SIZE): For DWARF5 decrease
>         by 12.
>         (DWARF_COMDAT_TYPE_UNIT_HEADER_SIZE): Always
>         DWARF_COMPILE_UNIT_HEADER_SIZE plus 12.
>         (DWARF_COMPILE_UNIT_SKELETON_HEADER_SIZE): Define.
>         (calc_base_type_die_sizes): Use DWARF_COMPILE_UNIT_SKELETON_HEADER_SIZE
>         for initial die_offset if dwarf_split_debug_info.
>         (output_comp_unit): Use DWARF_COMPILE_UNIT_SKELETON_HEADER_SIZE for
>         initial next_die_offset if dwo_id is non-NULL.  Don't emit padding
>         fields.
>         (output_skeleton_debug_sections): Formatting fix.  Use
>         DWARF_COMPILE_UNIT_SKELETON_HEADER_SIZE instead of
>         DWARF_COMPILE_UNIT_HEADER_SIZE.  Don't emit padding.
>
> --- gcc/dwarf2out.c.jj  2017-01-03 16:04:17.000000000 +0100
> +++ gcc/dwarf2out.c     2017-01-03 19:41:45.526194592 +0100
> @@ -2996,14 +2996,16 @@ skeleton_chain_node;
>  /* Fixed size portion of the DWARF compilation unit header.  */
>  #define DWARF_COMPILE_UNIT_HEADER_SIZE \
>    (DWARF_INITIAL_LENGTH_SIZE + DWARF_OFFSET_SIZE                       \
> -   + (dwarf_version >= 5                                               \
> -      ? 4 + DWARF_TYPE_SIGNATURE_SIZE + DWARF_OFFSET_SIZE : 3))
> +   + (dwarf_version >= 5 ? 4 : 3))
>
>  /* Fixed size portion of the DWARF comdat type unit header.  */
>  #define DWARF_COMDAT_TYPE_UNIT_HEADER_SIZE \
>    (DWARF_COMPILE_UNIT_HEADER_SIZE                                      \
> -   + (dwarf_version >= 5                                               \
> -      ? 0 : DWARF_TYPE_SIGNATURE_SIZE + DWARF_OFFSET_SIZE))
> +   + DWARF_TYPE_SIGNATURE_SIZE + DWARF_OFFSET_SIZE)
> +
> +/* Fixed size portion of the DWARF skeleton compilation unit header.  */
> +#define DWARF_COMPILE_UNIT_SKELETON_HEADER_SIZE \
> +  (DWARF_COMPILE_UNIT_HEADER_SIZE + (dwarf_version >= 5 ? 8 : 0))
>
>  /* Fixed size portion of public names info.  */
>  #define DWARF_PUBNAMES_HEADER_SIZE (2 * DWARF_OFFSET_SIZE + 2)
> @@ -9044,7 +9046,9 @@ calc_die_sizes (dw_die_ref die)
>  static void
>  calc_base_type_die_sizes (void)
>  {
> -  unsigned long die_offset = DWARF_COMPILE_UNIT_HEADER_SIZE;
> +  unsigned long die_offset = (dwarf_split_debug_info
> +                             ? DWARF_COMPILE_UNIT_SKELETON_HEADER_SIZE
> +                             : DWARF_COMPILE_UNIT_HEADER_SIZE);
>    unsigned int i;
>    dw_die_ref base_type;
>  #if ENABLE_ASSERT_CHECKING
> @@ -10302,7 +10306,9 @@ output_comp_unit (dw_die_ref die, int ou
>    delete extern_map;
>
>    /* Initialize the beginning DIE offset - and calculate sizes/offsets.  */
> -  next_die_offset = DWARF_COMPILE_UNIT_HEADER_SIZE;
> +  next_die_offset = (dwo_id
> +                    ? DWARF_COMPILE_UNIT_SKELETON_HEADER_SIZE
> +                    : DWARF_COMPILE_UNIT_HEADER_SIZE);
>    calc_die_sizes (die);
>
>    oldsym = die->die_id.die_symbol;
> @@ -10330,12 +10336,6 @@ output_comp_unit (dw_die_ref die, int ou
>        if (dwo_id != NULL)
>         for (int i = 0; i < 8; i++)
>           dw2_asm_output_data (1, dwo_id[i], i == 0 ? "DWO id" : NULL);
> -      else
> -       /* Hope all the padding will be removed for DWARF 5 final for
> -          DW_AT_compile and DW_AT_partial.  */
> -       dw2_asm_output_data (8, 0, "Padding 1");
> -
> -      dw2_asm_output_data (DWARF_OFFSET_SIZE, 0, "Padding 2");
>      }
>    output_die (die);
>
> @@ -10430,10 +10430,11 @@ output_skeleton_debug_sections (dw_die_r
>       header.  */
>    if (DWARF_INITIAL_LENGTH_SIZE - DWARF_OFFSET_SIZE == 4)
>      dw2_asm_output_data (4, 0xffffffff,
> -      "Initial length escape value indicating 64-bit DWARF extension");
> +                        "Initial length escape value indicating 64-bit "
> +                        "DWARF extension");
>
>    dw2_asm_output_data (DWARF_OFFSET_SIZE,
> -                       DWARF_COMPILE_UNIT_HEADER_SIZE
> +                      DWARF_COMPILE_UNIT_SKELETON_HEADER_SIZE
>                         - DWARF_INITIAL_LENGTH_SIZE
>                         + size_of_die (comp_unit),
>                        "Length of Compilation Unit Info");
> @@ -10449,12 +10450,8 @@ output_skeleton_debug_sections (dw_die_r
>    if (dwarf_version < 5)
>      dw2_asm_output_data (1, DWARF2_ADDR_SIZE, "Pointer Size (in bytes)");
>    else
> -    {
> -      for (int i = 0; i < 8; i++)
> -       dw2_asm_output_data (1, dwo_id[i], i == 0 ? "DWO id" : NULL);
> -
> -      dw2_asm_output_data (DWARF_OFFSET_SIZE, 0, "Padding 2");
> -    }
> +    for (int i = 0; i < 8; i++)
> +      dw2_asm_output_data (1, dwo_id[i], i == 0 ? "DWO id" : NULL);
>
>    comp_unit->die_abbrev = SKELETON_COMP_DIE_ABBREV;
>    output_die (comp_unit);
>
>         Jakub

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-01-11 16:37 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-03 23:10 [PATCH] Remove padding from DWARF5 headers Jakub Jelinek
2017-01-04 12:58 ` Mark Wielaard
2017-01-04 13:47   ` Jakub Jelinek
2017-01-04 13:51     ` Jakub Jelinek
2017-01-04 14:04 ` Jan Kratochvil
2017-01-11 16:37 ` Jason Merrill

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).