public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Ignore DWARF debug information for -gsplit-dwarf with dwarf-5.
@ 2022-09-29 12:43 Potharla, Rupesh
  2022-09-29 15:00 ` Jan Beulich
  0 siblings, 1 reply; 6+ messages in thread
From: Potharla, Rupesh @ 2022-09-29 12:43 UTC (permalink / raw)
  To: Potharla, Rupesh via Binutils
  Cc: Parasuraman, Hariharan, George, Jini Susan, Kumar N, Bhuvanendra


[-- Attachment #1.1: Type: text/plain, Size: 951 bytes --]

[AMD Official Use Only - General]

Hi,

Bfd is throwing errors for programs compiled with -gsplit-dwarf with clang and dwarf-5.  Made the code changes skipping DWO_id while reading debug_info header. Can you review the code changes and send in your comments/suggestions?

Inline Patch:


Skip dwo_id for split dwarf.
* dwarf2.c (parse_comp_unit): Skip DWO_id for DW_UT_skeleton.
---
bfd/dwarf2.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/bfd/dwarf2.c b/bfd/dwarf2.c
index 4a6a1e40185..011bf886e16 100644
--- a/bfd/dwarf2.c
+++ b/bfd/dwarf2.c
@@ -4441,6 +4441,10 @@ parse_comp_unit (struct dwarf2_debug *stash,
       return NULL;
     }

+  /* Skip DWO_id field.  */
+  if (unit_type == DW_UT_skeleton)
+    info_ptr += 8;
+
   /* Read the abbrevs for this compilation unit into a table.  */
   abbrevs = read_abbrevs (abfd, abbrev_offset, stash, file);
   if (! abbrevs)
--
2.25.1

Regards,
Rupesh P


[-- Attachment #2: 0001-Ignore-DWARF-debug-information-for-gsplit-dwarf-with.patch --]
[-- Type: application/octet-stream, Size: 852 bytes --]

From f197ae03c3b0ae83334e1c3a799dcc094faa84f8 Mon Sep 17 00:00:00 2001
From: rupesh potharla <rupesh.potharla@amd.com>
Date: Thu, 29 Sep 2022 17:49:07 +0530
Subject: [PATCH] Ignore DWARF debug information for -gsplit-dwarf with
 dwarf-5.

Skip dwo_id for split dwarf.
 * dwarf2.c (parse_comp_unit): Skip DWO_id for DW_UT_skeleton.
---
 bfd/dwarf2.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/bfd/dwarf2.c b/bfd/dwarf2.c
index 4a6a1e40185..011bf886e16 100644
--- a/bfd/dwarf2.c
+++ b/bfd/dwarf2.c
@@ -4441,6 +4441,10 @@ parse_comp_unit (struct dwarf2_debug *stash,
       return NULL;
     }
 
+  /* Skip DWO_id field.  */
+  if (unit_type == DW_UT_skeleton)
+    info_ptr += 8;
+
   /* Read the abbrevs for this compilation unit into a table.  */
   abbrevs = read_abbrevs (abfd, abbrev_offset, stash, file);
   if (! abbrevs)
-- 
2.25.1


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

* Re: [PATCH] Ignore DWARF debug information for -gsplit-dwarf with dwarf-5.
  2022-09-29 12:43 [PATCH] Ignore DWARF debug information for -gsplit-dwarf with dwarf-5 Potharla, Rupesh
@ 2022-09-29 15:00 ` Jan Beulich
  2022-09-29 15:54   ` Potharla, Rupesh
  0 siblings, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2022-09-29 15:00 UTC (permalink / raw)
  To: Potharla, Rupesh
  Cc: George, Jini Susan, Parasuraman, Hariharan, Kumar N, Bhuvanendra,
	Potharla, Rupesh via Binutils

On 29.09.2022 14:43, Potharla, Rupesh via Binutils wrote:
> --- a/bfd/dwarf2.c
> +++ b/bfd/dwarf2.c
> @@ -4441,6 +4441,10 @@ parse_comp_unit (struct dwarf2_debug *stash,
>        return NULL;
>      }
> 
> +  /* Skip DWO_id field.  */
> +  if (unit_type == DW_UT_skeleton)
> +    info_ptr += 8;
> +
>    /* Read the abbrevs for this compilation unit into a table.  */
>    abbrevs = read_abbrevs (abfd, abbrev_offset, stash, file);
>    if (! abbrevs)

A few lines up from here there's already a check of unit_type against
DW_UT_type. May I suggest to convert that to switch() and add your new
code there in form of a separate case block?

Jan

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

* RE: [PATCH] Ignore DWARF debug information for -gsplit-dwarf with dwarf-5.
  2022-09-29 15:00 ` Jan Beulich
@ 2022-09-29 15:54   ` Potharla, Rupesh
  2022-09-29 16:03     ` Jan Beulich
  0 siblings, 1 reply; 6+ messages in thread
From: Potharla, Rupesh @ 2022-09-29 15:54 UTC (permalink / raw)
  To: Jan Beulich
  Cc: George, Jini Susan, Parasuraman, Hariharan, Kumar N, Bhuvanendra,
	Potharla, Rupesh via Binutils

[-- Attachment #1: Type: text/plain, Size: 1894 bytes --]

[Public]

Jan,


.debug_info contents:
0x00000000: Compile Unit: length = 0x00000031, format = DWARF32, version = 0x0005, unit_type = DW_UT_skeleton, abbr_offset = 0x0000, addr_size = 0x08, DWO_id = 0xcd97775ca070a83c (next unit at 0x00000035)

The issue is seen with both clang and gcc.  Above is the Debug_info header of a program compiled with gcc using -gsplit-dwarf and dwarf-5.
There are two other fields(abbr_offset and addr_Size) after unit_type in the header.  Skipping the dwo_id field in the previous check of unit_type will not fix the issue.

Regards,
Rupesh P

>-----Original Message-----
>From: Jan Beulich <jbeulich@suse.com>
>Sent: Thursday, September 29, 2022 8:31 PM
>To: Potharla, Rupesh <Rupesh.Potharla@amd.com>
>Cc: George, Jini Susan <JiniSusan.George@amd.com>; Parasuraman,
>Hariharan <Hariharan.Parasuraman@amd.com>; Kumar N, Bhuvanendra
><Bhuvanendra.KumarN@amd.com>; Potharla, Rupesh via Binutils
><binutils@sourceware.org>
>Subject: Re: [PATCH] Ignore DWARF debug information for -gsplit-dwarf with
>dwarf-5.
>
>Caution: This message originated from an External Source. Use proper caution
>when opening attachments, clicking links, or responding.
>
>
>On 29.09.2022 14:43, Potharla, Rupesh via Binutils wrote:
>> --- a/bfd/dwarf2.c
>> +++ b/bfd/dwarf2.c
>> @@ -4441,6 +4441,10 @@ parse_comp_unit (struct dwarf2_debug *stash,
>>        return NULL;
>>      }
>>
>> +  /* Skip DWO_id field.  */
>> +  if (unit_type == DW_UT_skeleton)
>> +    info_ptr += 8;
>> +
>>    /* Read the abbrevs for this compilation unit into a table.  */
>>    abbrevs = read_abbrevs (abfd, abbrev_offset, stash, file);
>>    if (! abbrevs)
>
>A few lines up from here there's already a check of unit_type against
>DW_UT_type. May I suggest to convert that to switch() and add your new
>code there in form of a separate case block?
>
>Jan


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

* Re: [PATCH] Ignore DWARF debug information for -gsplit-dwarf with dwarf-5.
  2022-09-29 15:54   ` Potharla, Rupesh
@ 2022-09-29 16:03     ` Jan Beulich
  2022-09-29 16:39       ` Potharla, Rupesh
  0 siblings, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2022-09-29 16:03 UTC (permalink / raw)
  To: Potharla, Rupesh
  Cc: George, Jini Susan, Parasuraman, Hariharan, Kumar N, Bhuvanendra,
	Potharla, Rupesh via Binutils

On 29.09.2022 17:54, Potharla, Rupesh wrote:
> .debug_info contents:
> 0x00000000: Compile Unit: length = 0x00000031, format = DWARF32, version = 0x0005, unit_type = DW_UT_skeleton, abbr_offset = 0x0000, addr_size = 0x08, DWO_id = 0xcd97775ca070a83c (next unit at 0x00000035)
> 
> The issue is seen with both clang and gcc.  Above is the Debug_info header of a program compiled with gcc using -gsplit-dwarf and dwarf-5.
> There are two other fields(abbr_offset and addr_Size) after unit_type in the header.  Skipping the dwo_id field in the previous check of unit_type will not fix the issue.

According to the doc I'm looking at the headers for DW_UT_type and
DW_UT_skeleton agree up to the debug_abbrev_offset field. This
matches the code, which reads addr_size and abbrev_offset ahead of
the DW_UT_type conditional I was talking about. What's between that
if() and the one you're adding in your present patch is _precessing_
of addr_size, but not fetching of it from the header. This is
supported by the intermediate code also not further touching
info_ptr.

Jan

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

* RE: [PATCH] Ignore DWARF debug information for -gsplit-dwarf with dwarf-5.
  2022-09-29 16:03     ` Jan Beulich
@ 2022-09-29 16:39       ` Potharla, Rupesh
  2022-09-30  5:53         ` Jan Beulich
  0 siblings, 1 reply; 6+ messages in thread
From: Potharla, Rupesh @ 2022-09-29 16:39 UTC (permalink / raw)
  To: Jan Beulich
  Cc: George, Jini Susan, Parasuraman, Hariharan, Kumar N, Bhuvanendra,
	Potharla, Rupesh via Binutils

[-- Attachment #1: Type: text/plain, Size: 2568 bytes --]

[Public]

Thanks Jan, 

I made the suggested changes. Can you review the latest code changes and send in your comments ?

Inline Patch:

Skip dwo_id for split dwarf.
 * dwarf2.c (parse_comp_unit): Skip DWO_id for DW_UT_skeleton.
---
 bfd/dwarf2.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/bfd/dwarf2.c b/bfd/dwarf2.c
index 4a6a1e40185..028df77c4c2 100644
--- a/bfd/dwarf2.c
+++ b/bfd/dwarf2.c
@@ -4411,13 +4411,23 @@ parse_comp_unit (struct dwarf2_debug *stash,
   if (version < 5)
     addr_size = read_1_byte (abfd, &info_ptr, end_ptr);

-  if (unit_type == DW_UT_type)
+  switch (unit_type)
     {
-      /* Skip type signature.  */
-      info_ptr += 8;
+      case DW_UT_type:
+       /* Skip type signature.  */
+       info_ptr += 8;

-      /* Skip type offset.  */
-      info_ptr += offset_size;
+       /* Skip type offset.  */
+       info_ptr += offset_size;
+       break;
+
+      case DW_UT_skeleton:
+       /* Skip DWO_id field.  */
+       info_ptr += 8;
+       break;
+
+      default:
+       break;
     }

   if (addr_size > sizeof (bfd_vma))
@@ -4441,6 +4451,7 @@ parse_comp_unit (struct dwarf2_debug *stash,
       return NULL;
     }

+
   /* Read the abbrevs for this compilation unit into a table.  */
   abbrevs = read_abbrevs (abfd, abbrev_offset, stash, file);
   if (! abbrevs)
--
2.25.1


Regards,
Rupesh P

>On 29.09.2022 17:54, Potharla, Rupesh wrote:
>> .debug_info contents:
>> 0x00000000: Compile Unit: length = 0x00000031, format = DWARF32,
>> version = 0x0005, unit_type = DW_UT_skeleton, abbr_offset = 0x0000,
>> addr_size = 0x08, DWO_id = 0xcd97775ca070a83c (next unit at
>> 0x00000035)
>>
>> The issue is seen with both clang and gcc.  Above is the Debug_info header
>of a program compiled with gcc using -gsplit-dwarf and dwarf-5.
>> There are two other fields(abbr_offset and addr_Size) after unit_type in the
>header.  Skipping the dwo_id field in the previous check of unit_type will not
>fix the issue.
>
>According to the doc I'm looking at the headers for DW_UT_type and
>DW_UT_skeleton agree up to the debug_abbrev_offset field. This matches the
>code, which reads addr_size and abbrev_offset ahead of the DW_UT_type
>conditional I was talking about. What's between that
>if() and the one you're adding in your present patch is _precessing_ of
>addr_size, but not fetching of it from the header. This is supported by the
>intermediate code also not further touching info_ptr.
>
>Jan

[-- Attachment #2: 0001-Ignore-DWARF-debug-information-for-gsplit-dwarf-with.patch --]
[-- Type: application/octet-stream, Size: 1424 bytes --]

From 0007ca4559090da986c19dbd71bc5fe6bc48cd1e Mon Sep 17 00:00:00 2001
From: rupesh potharla <rupesh.potharla@amd.com>
Date: Thu, 29 Sep 2022 17:49:07 +0530
Subject: [PATCH] Ignore DWARF debug information for -gsplit-dwarf with
 dwarf-5.

Skip dwo_id for split dwarf.
 * dwarf2.c (parse_comp_unit): Skip DWO_id for DW_UT_skeleton.
---
 bfd/dwarf2.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/bfd/dwarf2.c b/bfd/dwarf2.c
index 4a6a1e40185..028df77c4c2 100644
--- a/bfd/dwarf2.c
+++ b/bfd/dwarf2.c
@@ -4411,13 +4411,23 @@ parse_comp_unit (struct dwarf2_debug *stash,
   if (version < 5)
     addr_size = read_1_byte (abfd, &info_ptr, end_ptr);
 
-  if (unit_type == DW_UT_type)
+  switch (unit_type)
     {
-      /* Skip type signature.  */
-      info_ptr += 8;
+      case DW_UT_type:
+	/* Skip type signature.  */
+	info_ptr += 8;
 
-      /* Skip type offset.  */
-      info_ptr += offset_size;
+	/* Skip type offset.  */
+	info_ptr += offset_size;
+	break;
+
+      case DW_UT_skeleton:
+	/* Skip DWO_id field.  */
+	info_ptr += 8;
+	break;
+
+      default:
+	break;
     }
 
   if (addr_size > sizeof (bfd_vma))
@@ -4441,6 +4451,7 @@ parse_comp_unit (struct dwarf2_debug *stash,
       return NULL;
     }
 
+
   /* Read the abbrevs for this compilation unit into a table.  */
   abbrevs = read_abbrevs (abfd, abbrev_offset, stash, file);
   if (! abbrevs)
-- 
2.25.1


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

* Re: [PATCH] Ignore DWARF debug information for -gsplit-dwarf with dwarf-5.
  2022-09-29 16:39       ` Potharla, Rupesh
@ 2022-09-30  5:53         ` Jan Beulich
  0 siblings, 0 replies; 6+ messages in thread
From: Jan Beulich @ 2022-09-30  5:53 UTC (permalink / raw)
  To: Potharla, Rupesh
  Cc: George, Jini Susan, Parasuraman, Hariharan, Kumar N, Bhuvanendra,
	Potharla, Rupesh via Binutils

On 29.09.2022 18:39, Potharla, Rupesh wrote:
> I made the suggested changes. Can you review the latest code changes and send in your comments ?

As a general request - please send new versions as independent new
mails, with the version number bumped in the subject (e.g. here:
[PATCH v2]). This makes it easier to follow revisions, especially
when later looking at the list archives.

> --- a/bfd/dwarf2.c
> +++ b/bfd/dwarf2.c
> @@ -4411,13 +4411,23 @@ parse_comp_unit (struct dwarf2_debug *stash,
>    if (version < 5)
>      addr_size = read_1_byte (abfd, &info_ptr, end_ptr);
> 
> -  if (unit_type == DW_UT_type)
> +  switch (unit_type)
>      {
> -      /* Skip type signature.  */
> -      info_ptr += 8;
> +      case DW_UT_type:
> +       /* Skip type signature.  */
> +       info_ptr += 8;

Please follow existing style, indenting the case labels the same as the
opening and closing braces. You'll note that this way the diff will be
smaller as well (which, besides patch size, also improves the results
of e.g. "git blame").

> -      /* Skip type offset.  */
> -      info_ptr += offset_size;
> +       /* Skip type offset.  */
> +       info_ptr += offset_size;
> +       break;
> +
> +      case DW_UT_skeleton:
> +       /* Skip DWO_id field.  */
> +       info_ptr += 8;
> +       break;
> +
> +      default:
> +       break;
>      }
> 
>    if (addr_size > sizeof (bfd_vma))
> @@ -4441,6 +4451,7 @@ parse_comp_unit (struct dwarf2_debug *stash,
>        return NULL;
>      }
> 
> +
>    /* Read the abbrevs for this compilation unit into a table.  */
>    abbrevs = read_abbrevs (abfd, abbrev_offset, stash, file);
>    if (! abbrevs)

Please don't introduce such a stray blank line.

Jan

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

end of thread, other threads:[~2022-09-30  5:53 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-29 12:43 [PATCH] Ignore DWARF debug information for -gsplit-dwarf with dwarf-5 Potharla, Rupesh
2022-09-29 15:00 ` Jan Beulich
2022-09-29 15:54   ` Potharla, Rupesh
2022-09-29 16:03     ` Jan Beulich
2022-09-29 16:39       ` Potharla, Rupesh
2022-09-30  5:53         ` Jan Beulich

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).