[PATCH] readelf: Print DIE offset in attribute reading error messages.

[PATCH] readelf: Print DIE offset in attribute reading error messages.

[Mark Wielaard]

When processing large files it is useful to know the DIE offset if
printing of attributes fails (especially when redirecting the output).
With this change the error message looks like:

  eu-readelf: DIE [2aeb8ef1] cannot get attribute value: invalid DWARF

Signed-off-by: Mark Wielaard <mark@klomp.org>
---
 src/ChangeLog |  4 ++++
 src/readelf.c | 16 ++++++++++------
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/src/ChangeLog b/src/ChangeLog
index 0d3bfc1..ba7d46d 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,7 @@
+2017-10-03  Mark Wielaard  <mark@klomp.org>
+
+	* readelf.c (attr_callback): Print DIE offset in error messages.
+
 2017-09-10  Mark Wielaard  <mark@klomp.org>
 
 	* ar.c (do_oper_delete): Remove DEBUG conditional check.
diff --git a/src/readelf.c b/src/readelf.c
index 5e2f3fc..16d2033 100644
--- a/src/readelf.c
+++ b/src/readelf.c
@@ -5934,13 +5934,15 @@ attr_callback (Dwarf_Attribute *attrp, void *arg)
 {
   struct attrcb_args *cbargs = (struct attrcb_args *) arg;
   const int level = cbargs->level;
+  Dwarf_Die *die = cbargs->die;
 
   unsigned int attr = dwarf_whatattr (attrp);
   if (unlikely (attr == 0))
     {
       if (!cbargs->silent)
-	error (0, 0, gettext ("cannot get attribute code: %s"),
-	       dwarf_errmsg (-1));
+	error (0, 0, gettext ("DIE [%" PRIx64 "] "
+			      "cannot get attribute code: %s"),
+	       dwarf_dieoffset (die), dwarf_errmsg (-1));
       return DWARF_CB_ABORT;
     }
 
@@ -5948,8 +5950,9 @@ attr_callback (Dwarf_Attribute *attrp, void *arg)
   if (unlikely (form == 0))
     {
       if (!cbargs->silent)
-	error (0, 0, gettext ("cannot get attribute form: %s"),
-	       dwarf_errmsg (-1));
+	error (0, 0, gettext ("DIE [%" PRIx64 "] "
+			      "cannot get attribute form: %s"),
+	       dwarf_dieoffset (die), dwarf_errmsg (-1));
       return DWARF_CB_ABORT;
     }
 
@@ -5963,8 +5966,9 @@ attr_callback (Dwarf_Attribute *attrp, void *arg)
 	    {
 	    attrval_out:
 	      if (!cbargs->silent)
-		error (0, 0, gettext ("cannot get attribute value: %s"),
-		       dwarf_errmsg (-1));
+		error (0, 0, gettext ("DIE [%" PRIx64 "] "
+				      "cannot get attribute value: %s"),
+		       dwarf_dieoffset (die), dwarf_errmsg (-1));
 	      return DWARF_CB_ABORT;
 	    }
 	  char *a = format_dwarf_addr (cbargs->dwflmod, cbargs->addrsize,
-- 
1.8.3.1

Re: [PATCH] readelf: Print DIE offset in attribute reading error messages.

[Mark Wielaard]

On Tue, 2017-10-03 at 16:59 +0200, Mark Wielaard wrote:
> When processing large files it is useful to know the DIE offset if
> printing of attributes fails (especially when redirecting the output).
> With this change the error message looks like:
> 
>   eu-readelf: DIE [2aeb8ef1] cannot get attribute value: invalid DWARF

Got another report about a large debug file where this improved error
message would have been really handy. So I pushed it to master now.

Cheers,

Mark

Re: [PATCH] readelf: Print DIE offset in attribute reading error messages.

[Mark Wielaard]

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

On Thu, 2017-11-16 at 15:31 +0100, Mark Wielaard wrote:
> On Tue, 2017-10-03 at 16:59 +0200, Mark Wielaard wrote:
> > When processing large files it is useful to know the DIE offset if
> > printing of attributes fails (especially when redirecting the output).
> > With this change the error message looks like:
> > 
> >   eu-readelf: DIE [2aeb8ef1] cannot get attribute value: invalid DWARF
> 
> Got another report about a large debug file where this improved error
> message would have been really handy. So I pushed it to master now.

This has been really useful to debug some DWARF format issues, but it
could be even more useful. Since we know the attribute name and form
(or we would have generated an error earlier) add those to the error
message too.

[-- Attachment #2: 0001-readelf-Print-attribute-name-and-form-in-error-messa.patch --]
[-- Type: text/x-patch, Size: 1678 bytes --]

From ce0469a04bf3b8e438647bb66f468c16ac52a4af Mon Sep 17 00:00:00 2001
From: Mark Wielaard <mark@klomp.org>
Date: Tue, 21 Nov 2017 11:13:00 +0100
Subject: [PATCH] readelf: Print attribute name and form in error message.

Now an error getting the attribute value will not only print the DIE offset
and the reason, but also the attribute name and form. e.g.

  DIE [b] cannot get attribute 'ranges' (sec_offset) value: .debug_ranges
  section missing

Signed-off-by: Mark Wielaard <mark@klomp.org>
---
 src/ChangeLog | 5 +++++
 src/readelf.c | 8 ++++++--
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/ChangeLog b/src/ChangeLog
index d112cd2..d2b25de 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,8 @@
+2017-11-21  Mark Wielaard  <mark@klomp.org>
+
+	* readelf.c (attr_callback): Print attribute name and form in error
+	message.
+
 2017-10-03  Mark Wielaard  <mark@klomp.org>
 
 	* readelf.c (attr_callback): Print DIE offset in error messages.
diff --git a/src/readelf.c b/src/readelf.c
index e364583..229c036 100644
--- a/src/readelf.c
+++ b/src/readelf.c
@@ -5968,8 +5968,12 @@ attr_callback (Dwarf_Attribute *attrp, void *arg)
 	    attrval_out:
 	      if (!cbargs->silent)
 		error (0, 0, gettext ("DIE [%" PRIx64 "] "
-				      "cannot get attribute value: %s"),
-		       dwarf_dieoffset (die), dwarf_errmsg (-1));
+				      "cannot get attribute '%s' (%s) value: "
+				      "%s"),
+		       dwarf_dieoffset (die),
+		       dwarf_attr_name (attr),
+		       dwarf_form_name (form),
+		       dwarf_errmsg (-1));
 	      return DWARF_CB_ABORT;
 	    }
 	  char *a = format_dwarf_addr (cbargs->dwflmod, cbargs->addrsize,
-- 
1.8.3.1

Re: [PATCH] readelf: Print DIE offset in attribute reading error messages.

[Tom Tromey]

>>>>> "Mark" == Mark Wielaard <mark@klomp.org> writes:

Mark> This has been really useful to debug some DWARF format issues, but it
Mark> could be even more useful. Since we know the attribute name and form
Mark> (or we would have generated an error earlier) add those to the error
Mark> message too.

Thanks for doing this.

Does dwarflint also report the error that prompted this?
I tried dwarflint on my file, but there was a lot of output and I never
did track down the precise problem, so I wasn't sure...

Tom

Re: [PATCH] readelf: Print DIE offset in attribute reading error messages.

[Mark Wielaard]

On Tue, 2017-11-21 at 12:19 -0700, Tom Tromey wrote:
> > > > > > "Mark" == Mark Wielaard <mark@klomp.org> writes:
> 
> Mark> This has been really useful to debug some DWARF format issues, but it
> Mark> could be even more useful. Since we know the attribute name and form
> Mark> (or we would have generated an error earlier) add those to the error
> Mark> message too.
> 
> Thanks for doing this.
> 
> Does dwarflint also report the error that prompted this?

No, because I am actually testing my DWARF5 work and dwarflint doesn't
support DWARF5 yet. (Also dwarflint doesn't build with the GCC8 trunk
branch which I am using atm.) But also yes, in my tests I corrupted a
DWARF4 .debug_ranges section so it couldn't be read and then dwarflint
will yell and scream about attributes that references it. But also no,
because after that it seems to go into a loop and never finishes...
This might just be it finding too many issues with the missing ranges.
It does suppress warnings after finding too many duplicates, and I
haven't had the patience to let it finish.

> I tried dwarflint on my file, but there was a lot of output and I never
> did track down the precise problem, so I wasn't sure...

Yeah, dwarflint seems hyper sensitive :) Patches welcome!

Cheers,

Mark

Re: [PATCH] readelf: Print DIE offset in attribute reading error messages.

[Mark Wielaard]

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

On Tue, 2017-11-21 at 11:21 +0100, Mark Wielaard wrote:
> On Thu, 2017-11-16 at 15:31 +0100, Mark Wielaard wrote:
> > On Tue, 2017-10-03 at 16:59 +0200, Mark Wielaard wrote:
> > > When processing large files it is useful to know the DIE offset if
> > > printing of attributes fails (especially when redirecting the output).
> > > With this change the error message looks like:
> > > 
> > >   eu-readelf: DIE [2aeb8ef1] cannot get attribute value: invalid DWARF
> > 
> > Got another report about a large debug file where this improved error
> > message would have been really handy. So I pushed it to master now.
> 
> This has been really useful to debug some DWARF format issues, but it
> could be even more useful. Since we know the attribute name and form
> (or we would have generated an error earlier) add those to the error
> message too.

I made one other tweak to not abort, but keep trying to print the other
attributes of the DIE. It might just be this one attribute whose value
cannot be retrieved/resolved. Other errors (like not being able to
decode the attribute code or form still abort because then we really
don't know how to continue and the abbrev is probably bogus.

[-- Attachment #2: 0001-readelf-Print-attribute-name-and-form-in-error-messa.patch --]
[-- Type: text/x-patch, Size: 2012 bytes --]

From 62ac40bfb52df30048e2c3dc64a53296da5eb066 Mon Sep 17 00:00:00 2001
From: Mark Wielaard <mark@klomp.org>
Date: Tue, 21 Nov 2017 11:13:00 +0100
Subject: [PATCH] readelf: Print attribute name and form in error message.

Now an error getting the attribute value will not only print the DIE offset
and the reason, but also the attribute name and form. e.g.

  DIE [b] cannot get attribute 'ranges' (sec_offset) value: .debug_ranges
  section missing

Also we don't abort, but try to print the other attributes of the DIE
anyway. It might just be one attribute whose value cannot be resolved.

Signed-off-by: Mark Wielaard <mark@klomp.org>
---
 src/ChangeLog |  5 +++++
 src/readelf.c | 11 ++++++++---
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/src/ChangeLog b/src/ChangeLog
index d112cd2..c56c323 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,8 @@
+2017-11-21  Mark Wielaard  <mark@klomp.org>
+
+	* readelf.c (attr_callback): Print attribute name and form in error
+	message. If only the value cannot be retrieved/resolved don't abort.
+
 2017-10-03  Mark Wielaard  <mark@klomp.org>
 
 	* readelf.c (attr_callback): Print DIE offset in error messages.
diff --git a/src/readelf.c b/src/readelf.c
index 9a03af6..ca7a19d 100644
--- a/src/readelf.c
+++ b/src/readelf.c
@@ -6044,9 +6044,14 @@ attr_callback (Dwarf_Attribute *attrp, void *arg)
 	    attrval_out:
 	      if (!cbargs->silent)
 		error (0, 0, gettext ("DIE [%" PRIx64 "] "
-				      "cannot get attribute value: %s"),
-		       dwarf_dieoffset (die), dwarf_errmsg (-1));
-	      return DWARF_CB_ABORT;
+				      "cannot get attribute '%s' (%s) value: "
+				      "%s"),
+		       dwarf_dieoffset (die),
+		       dwarf_attr_name (attr),
+		       dwarf_form_name (form),
+		       dwarf_errmsg (-1));
+	      /* Don't ABORT, it might be other attributes can be resolved.  */
+	      return DWARF_CB_OK;
 	    }
 	  char *a = format_dwarf_addr (cbargs->dwflmod, cbargs->addrsize,
 				       addr, addr);
-- 
1.8.3.1

Re: [PATCH] readelf: Print DIE offset in attribute reading error messages.

[Dodji Seketeli]

Sorry for my late reply,

Tom Tromey <tom@tromey.com> a écrit:

>>>>>> "Mark" == Mark Wielaard <mark@klomp.org> writes:
>
> Mark> This has been really useful to debug some DWARF format issues, but it
> Mark> could be even more useful. Since we know the attribute name and form
> Mark> (or we would have generated an error earlier) add those to the error
> Mark> message too.
>
> Thanks for doing this.

Indeed, thank you or doing this.  I wish I had these facilities sooner
when I was tracking the cause of my issues.  So no doubt this work is
going to be useful.

Thanks again.

Cheers,

-- 
		Dodji

Re: [PATCH] readelf: Print DIE offset in attribute reading error messages.

[Mark Wielaard]

On Thu, 2017-11-23 at 09:39 +0100, Dodji Seketeli wrote:
> Indeed, thank you or doing this.  I wish I had these facilities sooner
> when I was tracking the cause of my issues.  So no doubt this work is
> going to be useful.

Thanks. I pushed my second variant to master now.

Cheers,

Mark