Don't crash is dwarf_decode_macro_bytes's 'body' is NULL, even when '!is_define'

Message ID 20190529144343.20201-1-sergiodj@redhat.com
State New, archived
Headers

Commit Message

Sergio Durigan Junior May 29, 2019, 2:43 p.m. UTC
  Ref.: https://bugzilla.redhat.com/show_bug.cgi?id=1715008

On commit 7bede82892a06e6c26989803e70f53697392dcf9 ("Don't crash if
dwarf_decode_macro_bytes's 'body' is NULL"), I was too strict when
checking if 'body' is NULL: the check only comprised the case when
'is_define' is true.  However, the corruption of .debug_macro by
rpmbuild's "debugedit" also affects the case when 'is_define' is
false, i.e., when the macro is being undefined.

This commit improves the check and covers both cases now.  This has
been tested on Fedora 30 with a problematic debuginfo, and I don't see
a segfault anymore.

OK to push?

gdb/ChangeLog:
2019-05-29  Sergio Durigan Junior  <sergiodj@redhat.com>

	Ref.: https://bugzilla.redhat.com/show_bug.cgi?id=1708192
	Ref.: https://bugzilla.redhat.com/show_bug.cgi?id=1715008
	* dwarf2read.c (dwarf_decode_macro_bytes): Move check to see if
	'body' is NULL to the outter 'if', protecting the '!is_define'
	situation as well.
---
 gdb/ChangeLog    |  8 ++++++++
 gdb/dwarf2read.c | 31 ++++++++++++++-----------------
 2 files changed, 22 insertions(+), 17 deletions(-)
  

Comments

Tom Tromey May 29, 2019, 3:55 p.m. UTC | #1
>>>>> "Sergio" == Sergio Durigan Junior <sergiodj@redhat.com> writes:

Sergio> Ref.: https://bugzilla.redhat.com/show_bug.cgi?id=1715008
Sergio> On commit 7bede82892a06e6c26989803e70f53697392dcf9 ("Don't crash if
Sergio> dwarf_decode_macro_bytes's 'body' is NULL"), I was too strict when
Sergio> checking if 'body' is NULL: the check only comprised the case when
Sergio> 'is_define' is true.  However, the corruption of .debug_macro by
Sergio> rpmbuild's "debugedit" also affects the case when 'is_define' is
Sergio> false, i.e., when the macro is being undefined.

Sergio> This commit improves the check and covers both cases now.  This has
Sergio> been tested on Fedora 30 with a problematic debuginfo, and I don't see
Sergio> a segfault anymore.

Sergio> OK to push?

Thanks for doing this.  This is ok.

Tom
  
Sergio Durigan Junior May 29, 2019, 8:15 p.m. UTC | #2
On Wednesday, May 29 2019, Tom Tromey wrote:

>>>>>> "Sergio" == Sergio Durigan Junior <sergiodj@redhat.com> writes:
>
> Sergio> Ref.: https://bugzilla.redhat.com/show_bug.cgi?id=1715008
> Sergio> On commit 7bede82892a06e6c26989803e70f53697392dcf9 ("Don't crash if
> Sergio> dwarf_decode_macro_bytes's 'body' is NULL"), I was too strict when
> Sergio> checking if 'body' is NULL: the check only comprised the case when
> Sergio> 'is_define' is true.  However, the corruption of .debug_macro by
> Sergio> rpmbuild's "debugedit" also affects the case when 'is_define' is
> Sergio> false, i.e., when the macro is being undefined.
>
> Sergio> This commit improves the check and covers both cases now.  This has
> Sergio> been tested on Fedora 30 with a problematic debuginfo, and I don't see
> Sergio> a segfault anymore.
>
> Sergio> OK to push?
>
> Thanks for doing this.  This is ok.

Thanks for the review, Tom.

Pushed: 955b06fa576df1a6954263043ea3f3a5b9ad5940
  

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index f122f5b21f..48e5847b13 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,11 @@ 
+2019-05-29  Sergio Durigan Junior  <sergiodj@redhat.com>
+
+	Ref.: https://bugzilla.redhat.com/show_bug.cgi?id=1708192
+	Ref.: https://bugzilla.redhat.com/show_bug.cgi?id=1715008
+	* dwarf2read.c (dwarf_decode_macro_bytes): Move check to see if
+	'body' is NULL to the outter 'if', protecting the '!is_define'
+	situation as well.
+
 2019-05-28  Tom Tromey  <tromey@adacore.com>
 
 	* ada-lang.c (ada_remove_Xbn_suffix)
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index f48b931a3f..d1c7a8e67c 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -24635,25 +24635,22 @@  dwarf_decode_macro_bytes (struct dwarf2_cu *cu,
 			 is_define ? _("definition") : _("undefinition"),
 			 line == 0 ? _("zero") : _("non-zero"), line, body);
 
-	    if (is_define)
+	    if (body == NULL)
 	      {
-		if (body != NULL)
-		  parse_macro_definition (current_file, line, body);
-		else
-		  {
-		    /* Fedora's rpm-build's "debugedit" binary
-		       corrupted .debug_macro sections.
-
-		       For more info, see
-		       https://bugzilla.redhat.com/show_bug.cgi?id=1708786 */
-		    complaint (_("debug info gives %s invalid macro definition "
-				 "without body (corrupted?) at line %d"
-				 "on file %s"),
-			       at_commandline ? _("command-line")
-			       : _("in-file"),
-			       line, current_file->filename);
-		  }
+		/* Fedora's rpm-build's "debugedit" binary
+		   corrupted .debug_macro sections.
+
+		   For more info, see
+		   https://bugzilla.redhat.com/show_bug.cgi?id=1708786 */
+		complaint (_("debug info gives %s invalid macro %s "
+			     "without body (corrupted?) at line %d "
+			     "on file %s"),
+			   at_commandline ? _("command-line") : _("in-file"),
+			   is_define ? _("definition") : _("undefinition"),
+			   line, current_file->filename);
 	      }
+	    else if (is_define)
+	      parse_macro_definition (current_file, line, body);
 	    else
 	      {
 		gdb_assert (macinfo_type == DW_MACRO_undef