Re: [PATCH] gdb: Handle ICC's unexpected void return type

[Andrew Burgess <andrew dot burgess at embecosm dot com>]

* Tom Tromey <tom@tromey.com> [2018-11-06 11:10:57 -0700]:

> >>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:
> 
> Andrew> I encountered a binary compiled with Intel's C Compiler version
> Andrew> 14.0.5.212, which seemed to contain some non-standard DWARF.
> [...]
> 
> Andrew> A test confirms that the issue is fixed.
> 
> Thanks for this test.  It's very good to have these; in the past I think
> some compiler workarounds have gone in without such tests, and then
> later it is difficult to figure out whether changes keep gdb working.
> 
> Andrew> +	if (bits == 0 && producer_is_icc (cu)
> Andrew> +	    && strcmp (name, "void") == 0)
> 
> I think NAME can be null here, so an additional check is needed to avoid
> a potential crash.

Thanks for the feedback.  You're absolutely right (of course).

Below is the obvious patch to fix the bug, I also add a test to cover
this case, and I've also fixed a small non-bug in the test I added in
the original patch.

Is this OK to apply?

Thanks,
Andrew

--

[PATCH] gdb: Guard against NULL dereference in dwarf2_init_integer_type

In this commit:

    commit eb77c9df9f6d2f7aa644a170280fe31ce080f887
    Date:   Thu Oct 18 14:04:27 2018 +0100

        gdb: Handle ICC's unexpected void return type

A potential dereference of a NULL pointer was introduced if a
DW_TAG_base_type is missing a DW_AT_name attribute.

I have taken this opportunity to fix a slight confusion that existed
in the test also added in the above commit, the test had two C
variables, declared like this:

    int var_a = 5;

    void *var_ptr = &var_a;

However, the fake DWARF in the test script declared them like this:

    void var_a = 5;

    void *var_ptr = &var_a;

This wasn't a problem as the test never uses 'var_a' directly, this
only exists so 'var_ptr' can be initialised.  However, it seemed worth
fixing.

I've also added a test for a DW_TAG_base_type with a missing
DW_AT_name, as clearly there's not test currently that covers this
(the original patch tested cleanly).  I can confirm that the new test
causes GDB to crash before this patch, and passes with this patch.

gdb/ChangeLog:

	* dwarf2read.c (dwarf2_init_integer_type): Check for name being
	NULL before dereferencing it.

gdb/testsuite/ChangeLog:

	* gdb.dwarf2/void-type.exp: Rename types, and make var_a an 'int'.
	* gdb.dwarf2/missing-type-name.exp: New file.
---
 gdb/ChangeLog                                  |   5 +
 gdb/dwarf2read.c                               |   3 +-
 gdb/testsuite/ChangeLog                        |   5 +
 gdb/testsuite/gdb.dwarf2/missing-type-name.exp | 123 +++++++++++++++++++++++++
 gdb/testsuite/gdb.dwarf2/void-type.exp         |  12 +--
 5 files changed, 141 insertions(+), 7 deletions(-)
 create mode 100644 gdb/testsuite/gdb.dwarf2/missing-type-name.exp

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index b237c81fe4f..d2a8cd44f9a 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -17522,7 +17522,8 @@ dwarf2_init_integer_type (struct dwarf2_cu *cu, struct objfile *objfile,
   /* Versions of Intel's C Compiler generate an integer type called "void"
      instead of using DW_TAG_unspecified_type.  This has been seen on
      at least versions 14, 17, and 18.  */
-  if (bits == 0 && producer_is_icc (cu) && strcmp (name, "void") == 0)
+  if (bits == 0 && producer_is_icc (cu) && name != nullptr
+      && strcmp (name, "void") == 0)
     type = objfile_type (objfile)->builtin_void;
   else
     type = init_integer_type (objfile, bits, unsigned_p, name);
diff --git a/gdb/testsuite/gdb.dwarf2/missing-type-name.exp b/gdb/testsuite/gdb.dwarf2/missing-type-name.exp
new file mode 100644
index 00000000000..62d46e4dd02
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/missing-type-name.exp
@@ -0,0 +1,123 @@
+# Copyright 2018 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This tests some non-standard DWARF that we still expect GDB to be
+# able to handle.
+#
+# The DWARF standard (v5 5.1) says this:
+#
+#     A base type is represented by a debugging information entry with
+#     the tag DW_TAG_base_type.
+#
+#     A base type entry may have a DW_AT_name attribute whose value is
+#     a null-terminated string containing the name of the base type as
+#     recognized by the programming language of the compilation unit
+#     containing the base type entry.
+#
+# So the DW_AT_name field for a DW_TAG_base_type is optional.  This
+# test provides some basic checking that GDB doesn't crash when
+# presented with this situation.
+
+load_lib dwarf.exp
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+if {![dwarf2_support]} {
+    return 0
+}
+
+standard_testfile void-type.c void-type.S
+
+# Make some DWARF for the test.
+set asm_file [standard_output_file $srcfile2]
+Dwarf::assemble $asm_file {
+    global srcdir subdir srcfile
+
+    set func_result [function_range func ${srcdir}/${subdir}/${srcfile}]
+    set func_start [lindex $func_result 0]
+    set func_length [lindex $func_result 1]
+
+    set main_result [function_range main ${srcdir}/${subdir}/${srcfile}]
+    set main_start [lindex $main_result 0]
+    set main_length [lindex $main_result 1]
+
+    cu {} {
+	DW_TAG_compile_unit {
+	        {DW_AT_producer "GNU C 8.1"}
+                {DW_AT_language @DW_LANG_C}
+                {DW_AT_name     void-type.c}
+                {DW_AT_comp_dir /tmp}
+        } {
+	    declare_labels main_type int_type ptr_type
+
+	    main_type: DW_TAG_base_type {
+		{DW_AT_byte_size 4 DW_FORM_sdata}
+		{DW_AT_encoding  @DW_ATE_signed}
+	    }
+
+	    int_type: DW_TAG_base_type {
+		{DW_AT_byte_size 0 DW_FORM_sdata}
+		{DW_AT_encoding  @DW_ATE_signed}
+	    }
+
+	    ptr_type: DW_TAG_pointer_type {
+		{DW_AT_type :$int_type}
+	    }
+
+            DW_TAG_subprogram {
+                {name func}
+                {low_pc $func_start addr}
+                {high_pc "$func_start + $func_length" addr}
+                {type :$int_type}
+	    }
+            DW_TAG_subprogram {
+                {name main}
+                {low_pc $main_start addr}
+                {high_pc "$main_start + $main_length" addr}
+                {type :$main_type}
+	    }
+
+	    DW_TAG_variable {
+		{DW_AT_name "var_a"}
+		{DW_AT_type :$main_type}
+		{DW_AT_external 1 DW_FORM_flag}
+		{DW_AT_location {DW_OP_addr [gdb_target_symbol "var_a"]} SPECIAL_expr}
+	    }
+
+	    DW_TAG_variable {
+		{DW_AT_name "var_ptr"}
+		{DW_AT_type :$ptr_type}
+		{DW_AT_external 1 DW_FORM_flag}
+		{DW_AT_location {DW_OP_addr [gdb_target_symbol "var_ptr"]} SPECIAL_expr}
+	    }
+	}
+    }
+}
+
+if { [prepare_for_testing "failed to prepare" ${testfile} \
+	  [list $srcfile $asm_file] {nodebug}] } {
+    return -1
+}
+
+if ![runto_main] {
+    return -1
+}
+
+# Use 'ptype' on two variables that are using DW_TAG_base_type types
+# with missing DW_AT_name attributes.
+gdb_test "ptype var_ptr" "type = <invalid type code $decimal> \\*" \
+    "ptype of a pointer to a basic type with missing name"
+
+gdb_test "ptype var_a" "type = <invalid type code $decimal>" \
+    "ptype of a variable that is a basic type with a missing name"
diff --git a/gdb/testsuite/gdb.dwarf2/void-type.exp b/gdb/testsuite/gdb.dwarf2/void-type.exp
index 3691bff7792..4367eee8c51 100644
--- a/gdb/testsuite/gdb.dwarf2/void-type.exp
+++ b/gdb/testsuite/gdb.dwarf2/void-type.exp
@@ -53,35 +53,35 @@ Dwarf::assemble $asm_file {
                 {DW_AT_name     void-type.c}
                 {DW_AT_comp_dir /tmp}
         } {
-	    declare_labels main_type int_type ptr_type
+	    declare_labels int_type void_type ptr_type
 
-	    main_type: DW_TAG_base_type {
+	    int_type: DW_TAG_base_type {
 		{DW_AT_byte_size 4 DW_FORM_sdata}
 		{DW_AT_encoding  @DW_ATE_signed}
 		{DW_AT_name      int}
 	    }
 
-	    int_type: DW_TAG_base_type {
+	    void_type: DW_TAG_base_type {
 		{DW_AT_byte_size 0 DW_FORM_sdata}
 		{DW_AT_encoding  @DW_ATE_signed}
 		{DW_AT_name      void}
 	    }
 
 	    ptr_type: DW_TAG_pointer_type {
-		{DW_AT_type :$int_type}
+		{DW_AT_type :$void_type}
 	    }
 
             DW_TAG_subprogram {
                 {name func}
                 {low_pc $func_start addr}
                 {high_pc "$func_start + $func_length" addr}
-                {type :$int_type}
+                {type :$void_type}
 	    }
             DW_TAG_subprogram {
                 {name main}
                 {low_pc $main_start addr}
                 {high_pc "$main_start + $main_length" addr}
-                {type :$main_type}
+                {type :$int_type}
 	    }
 
 	    DW_TAG_variable {
-- 
2.14.5