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

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

[Andrew Burgess]

I encountered a binary compiled with Intel's C Compiler version
14.0.5.212, which seemed to contain some non-standard DWARF.

The DWARF spec (V5 3.3.2) says:

    Debugging information entries for C void functions should not have
    an attribute for the return type.

However, what I observed in the DWARF from this ICC compiled binary
was this:

    ...
    <0><857>: Abbrev Number: 1 (DW_TAG_compile_unit)
       <858>   DW_AT_comp_dir    : (indirect string, offset: 0x48d): /tmp/
       <85c>   DW_AT_language    : 1       (ANSI C)
       <85d>   DW_AT_name        : (indirect string, offset: 0x77c): filename.c
       <861>   DW_AT_producer    : (indirect string, offset: 0x520): Intel(R) C Intel(R) 64 Compiler ...
       <865>   DW_AT_low_pc      : 0x4378d0
       <86d>   DW_AT_high_pc     : 0x4378f0
       <875>   DW_AT_stmt_list   : 0xa37
    ...
    <1><7ea>: Abbrev Number: 2 (DW_TAG_base_type)
       <7eb>   DW_AT_byte_size   : 0
       <7ec>   DW_AT_encoding    : 5       (signed)
       <7ed>   DW_AT_name        : (indirect string, offset: 0x58f): void
    ...
    <1><7f1>: Abbrev Number: 3 (DW_TAG_subprogram)
       <7f2>   DW_AT_decl_line   : 268
       <7f4>   DW_AT_decl_column : 30
       <7f5>   DW_AT_decl_file   : 1
       <7f6>   DW_AT_type        : <0x7ea>
       <7fa>   DW_AT_prototyped  : 1
       <7fb>   DW_AT_name        : (indirect string, offset: 0x761): function_foo
       <7ff>   DW_AT_MIPS_linkage_name: (indirect string, offset: 0x761): function_foo
       <803>   DW_AT_low_pc      : 0x4378a0
       <80b>   DW_AT_high_pc     : 0x4378d0
       <813>   DW_AT_external    : 1
    ...

So function 'function_foo' has void return type, but still has a
DW_AT_type attribute for a 0 sized, type called void.

What was found was that when the 'finish' command was used to leave
'function_foo', GDB would crash.

The problem is that in infcmd.c:print_return_value GDB tries to filter
out void return types, by looking for the TYPE_CODE_VOID, this fails
for the 'void' type as it has code TYPE_CODE_INT and GDB then tries to
print the 'void' type.

This eventually ends in a call to valprint.c:maybe_negate_by_bytes,
however, the len (length) of the value being negated is 0, which is
not detected or expected by this code, and invalid memory accesses
occur, some of which might cause GDB to crash.

The above DWARF was seen on version 14.0.5.212 of Intel's C compliler.
I don't have access to any other versions in order to see when this
issue might have appeared, or if it has been fixed.

In this patch I have added an assertion to maybe_negate_by_bytes.
This is nothing to do with the actual fix, but should detect incorrect
use of this function in the future, without relying on undefined
behaviour to crash GDB.

We already have a predicate in the DWARF parser to detect versions of
ICC prior to 14, however, this issue was spotted on a later veresion.
As a result I've added a new predicate that is true for any version of
ICC.

Using the new predicate I convert synthetic void types into GDB's
precreated void type.

A test confirms that the issue is fixed.

gdb/ChangeLog:

	* dwarf2read.c (struct dwarf2_cu): Add producer_is_icc field.
	(producer_is_icc): New function.
	(check_producer): Set producer_is_icc field on dwarf2_cu.
	(read_base_type): Spot ICC's unexpected void type, and convert to
	GDB's expected void type.
	(dwarf2_cu::dwarf2_cu): Initialise producer_is_icc field.
	* valprint.c (maybe_negate_by_bytes): Add an assertion that the
	LEN is greater than 0.

gdb/testsuite/ChangeLog:

	* gdb.dwarf2/void-return-type.c: New file.
	* gdb.dwarf2/void-return-type.exp: New file.
---
 gdb/ChangeLog                                 |  11 +++
 gdb/dwarf2read.c                              |  26 ++++++-
 gdb/testsuite/ChangeLog                       |   5 ++
 gdb/testsuite/gdb.dwarf2/void-return-type.c   |  32 ++++++++
 gdb/testsuite/gdb.dwarf2/void-return-type.exp | 103 ++++++++++++++++++++++++++
 gdb/valprint.c                                |   1 +
 6 files changed, 176 insertions(+), 2 deletions(-)
 create mode 100644 gdb/testsuite/gdb.dwarf2/void-return-type.c
 create mode 100644 gdb/testsuite/gdb.dwarf2/void-return-type.exp

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 2a1b805c9a7..7d9026a59d7 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -551,6 +551,7 @@ struct dwarf2_cu
   unsigned int checked_producer : 1;
   unsigned int producer_is_gxx_lt_4_6 : 1;
   unsigned int producer_is_gcc_lt_4_3 : 1;
+  unsigned int producer_is_icc : 1;
   unsigned int producer_is_icc_lt_14 : 1;
   bool producer_is_codewarrior : 1;
 
@@ -11368,6 +11369,19 @@ producer_is_icc_lt_14 (struct dwarf2_cu *cu)
   return cu->producer_is_icc_lt_14;
 }
 
+/* ICC generates a DW_AT_type for C void functions.  This was observed on
+   ICC 14.0.5.212, and appears to be against the DWARF spec (V5 3.3.2)
+   which says that void functions should not have a DW_AT_type.  */
+
+static int
+producer_is_icc (struct dwarf2_cu *cu)
+{
+  if (!cu->checked_producer)
+    check_producer (cu);
+
+  return cu->producer_is_icc;
+}
+
 /* Check for possibly missing DW_AT_comp_dir with relative .debug_line
    directory paths.  GCC SVN r127613 (new option -fdebug-prefix-map) fixed
    this, it was first present in GCC release 4.3.0.  */
@@ -14902,7 +14916,10 @@ check_producer (struct dwarf2_cu *cu)
       cu->producer_is_gcc_lt_4_3 = major < 4 || (major == 4 && minor < 3);
     }
   else if (producer_is_icc (cu->producer, &major, &minor))
-    cu->producer_is_icc_lt_14 = major < 14;
+    {
+      cu->producer_is_icc = true;
+      cu->producer_is_icc_lt_14 = major < 14;
+    }
   else if (startswith (cu->producer, "CodeWarrior S12/L-ISA"))
     cu->producer_is_codewarrior = true;
   else
@@ -17548,7 +17565,11 @@ read_base_type (struct die_info *die, struct dwarf2_cu *cu)
 	type = dwarf2_init_float_type (objfile, bits, name, name);
 	break;
       case DW_ATE_signed:
-	type = init_integer_type (objfile, bits, 0, name);
+	if (bits == 0 && producer_is_icc (cu)
+	    && strcmp (name, "void") == 0)
+	  type = objfile_type (objfile)->builtin_void;
+	else
+	  type = init_integer_type (objfile, bits, 0, name);
 	break;
       case DW_ATE_unsigned:
 	if (cu->language == language_fortran
@@ -25134,6 +25155,7 @@ dwarf2_cu::dwarf2_cu (struct dwarf2_per_cu_data *per_cu_)
     checked_producer (0),
     producer_is_gxx_lt_4_6 (0),
     producer_is_gcc_lt_4_3 (0),
+    producer_is_icc (0),
     producer_is_icc_lt_14 (0),
     producer_is_codewarrior (false),
     processing_has_namespace_info (0)
diff --git a/gdb/testsuite/gdb.dwarf2/void-return-type.c b/gdb/testsuite/gdb.dwarf2/void-return-type.c
new file mode 100644
index 00000000000..42a6d4a1a46
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/void-return-type.c
@@ -0,0 +1,32 @@
+/* Copyright (C) 2018 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   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/>.  */
+
+__attribute__((noinline)) void
+func ()
+{
+  asm ("func_label: .globl func_label");
+  asm ("" ::: "memory");
+  return;
+}
+
+int
+main ()
+{
+  asm ("main_label: .globl main_label");
+  func ();
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.dwarf2/void-return-type.exp b/gdb/testsuite/gdb.dwarf2/void-return-type.exp
new file mode 100644
index 00000000000..ec93ebe764d
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/void-return-type.exp
@@ -0,0 +1,103 @@
+# 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 spotted in an Intel C Compiler
+# generated binary.
+#
+# The DWARF standard (V5 3.3.2) says that a void C function should not
+# have a DW_AT_type attribute, however, an ICC compiled binary was
+# found to have a DW_AT_type that referenced a signed integer type, of
+# size 0, with the name 'void'.
+#
+# This 'void' integer type would cause GDB to crash in some cases, one
+# that was seen was when using 'finish' to leave the void function.
+
+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-return-type.c void-return-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 "Intel(R) C Intel(R) 64 Compiler XE for applications running on Intel(R) 64, Version 14.0.5.212 Build 20150212"}
+                {DW_AT_language @DW_LANG_C}
+                {DW_AT_name     void-return-type.c}
+                {DW_AT_comp_dir /tmp}
+        } {
+	    declare_labels main_type func_type
+
+	    main_type: DW_TAG_base_type {
+		{DW_AT_byte_size 4 DW_FORM_sdata}
+		{DW_AT_encoding  @DW_ATE_signed}
+		{DW_AT_name      int}
+	    }
+
+	    func_type: DW_TAG_base_type {
+		{DW_AT_byte_size 0 DW_FORM_sdata}
+		{DW_AT_encoding  @DW_ATE_signed}
+		{DW_AT_name      void}
+	    }
+
+            DW_TAG_subprogram {
+                {name func}
+                {low_pc $func_start addr}
+                {high_pc "$func_start + $func_length" addr}
+                {type :$func_type}
+	    }
+            DW_TAG_subprogram {
+                {name main}
+                {low_pc $main_start addr}
+                {high_pc "$main_start + $main_length" addr}
+                {type :$main_type}
+	    }
+	}
+    }
+}
+
+if { [prepare_for_testing "failed to prepare" ${testfile} \
+	  [list $srcfile $asm_file] {nodebug}] } {
+    return -1
+}
+
+if ![runto_main] {
+    return -1
+}
+
+# Place a breakpoint in 'func' and continue to there.
+gdb_breakpoint func
+gdb_continue_to_breakpoint "func"
+
+# Now finish, returning to main.
+gdb_test "finish" [multi_line \
+		       "Run till exit from #0  $hex in func \\\(\\\)" \
+		       "$hex in main \\\(\\\)"] \
+    "check that finish completes"
diff --git a/gdb/valprint.c b/gdb/valprint.c
index 2b63a91e8df..b2236f89318 100644
--- a/gdb/valprint.c
+++ b/gdb/valprint.c
@@ -1604,6 +1604,7 @@ maybe_negate_by_bytes (const gdb_byte *bytes, unsigned len,
 		       gdb::byte_vector *out_vec)
 {
   gdb_byte sign_byte;
+  gdb_assert (len > 0);
   if (byte_order == BFD_ENDIAN_BIG)
     sign_byte = bytes[0];
   else
-- 
2.14.5

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

[Kevin Buettner]

Hi Andrew,

Mostly okay, just a two nits...

On Tue, 23 Oct 2018 22:28:43 +0100
Andrew Burgess <andrew.burgess@embecosm.com> wrote:

> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> index 2a1b805c9a7..7d9026a59d7 100644
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
> @@ -551,6 +551,7 @@ struct dwarf2_cu
>    unsigned int checked_producer : 1;
>    unsigned int producer_is_gxx_lt_4_6 : 1;
>    unsigned int producer_is_gcc_lt_4_3 : 1;
> +  unsigned int producer_is_icc : 1;

Use bool here like the recently added producer_is_codewarrior.

And...

> @@ -25134,6 +25155,7 @@ dwarf2_cu::dwarf2_cu (struct dwarf2_per_cu_data *per_cu_)
>      checked_producer (0),
>      producer_is_gxx_lt_4_6 (0),
>      producer_is_gcc_lt_4_3 (0),
> +    producer_is_icc (0),

Use false instead of 0.

Kevin

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

[Keith Seitz]

On 10/23/18 2:28 PM, Andrew Burgess wrote:
> So function 'function_foo' has void return type, but still has a
> DW_AT_type attribute for a 0 sized, type called void.

Not only that, but the C/C++ "void" type is explicitly called out in
the spec (section 5.2)...

> gdb/ChangeLog:
> 
> 	* dwarf2read.c (struct dwarf2_cu): Add producer_is_icc field.
> 	(producer_is_icc): New function.
> 	(check_producer): Set producer_is_icc field on dwarf2_cu.
> 	(read_base_type): Spot ICC's unexpected void type, and convert to
> 	GDB's expected void type.
> 	(dwarf2_cu::dwarf2_cu): Initialise producer_is_icc field.
> 	* valprint.c (maybe_negate_by_bytes): Add an assertion that the
> 	LEN is greater than 0.

While I think this is reasonable (and non-invasive/safe), I have some questions.

> @@ -17548,7 +17565,11 @@ read_base_type (struct die_info *die, struct dwarf2_cu *cu)
>  	type = dwarf2_init_float_type (objfile, bits, name, name);
>  	break;
>        case DW_ATE_signed:
> -	type = init_integer_type (objfile, bits, 0, name);
> +	if (bits == 0 && producer_is_icc (cu)
> +	    && strcmp (name, "void") == 0)
> +	  type = objfile_type (objfile)->builtin_void;
> +	else
> +	  type = init_integer_type (objfile, bits, 0, name);
>  	break;
>        case DW_ATE_unsigned:
>  	if (cu->language == language_fortran

Is this the appropriate place for this? The patch is attempting to deal specifically
with the void return of a function. I would have thought to catch this anomaly in
read_func_scope/add_dwarf2_member_fn. These sorts of exceptions are handled
relatively commonly in dwarf2read.c.

While the DWARF specifications specifically call out using DW_TAG_unspecified_type
for the void type (in C/C++), it doesn't actually say that this particular usage
is invalid AFAICT. [Although I think it is.] Nonetheless, changing this here would
preclude some language from doing something like this (as silly as that sounds).

Also, if it this is the appropriate place (or even if it is decided to move this
check elsewhere), why limit this to ICC? Is it simply because ICC only handles
C/C++? Would it hurt/be worth it to safe guard that gcc or clang or rustc or
who-knows-what wouldn't cause us similar harm?

As for the actual code as it is, why only check DW_ATE_signed? I realize that this
is specifically to address the particular ICC instance you are trying to fix, but
something tells me that this could potentially change. Regardless of the encoding,
this would be invalid.

I'm not really asking for any changes. After looking at all kinds of DWARF output
that I never dreamed possible, my thoughts tend to defensive programming in the
DWARF reader.
 
Thanks,
Keith (IANAM*)

* I Am Not A Maintainer

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

[Andrew Burgess]

Keith,

Thanks for your feedback.  Sorry for the delay in replying, I've
managed to get some more test binaries since my original patch, so I
have a better response now.

* Keith Seitz <keiths@redhat.com> [2018-10-24 14:00:16 -0700]:

> On 10/23/18 2:28 PM, Andrew Burgess wrote:
> > So function 'function_foo' has void return type, but still has a
> > DW_AT_type attribute for a 0 sized, type called void.
> 
> Not only that, but the C/C++ "void" type is explicitly called out in
> the spec (section 5.2)...
> 
> > gdb/ChangeLog:
> > 
> > 	* dwarf2read.c (struct dwarf2_cu): Add producer_is_icc field.
> > 	(producer_is_icc): New function.
> > 	(check_producer): Set producer_is_icc field on dwarf2_cu.
> > 	(read_base_type): Spot ICC's unexpected void type, and convert to
> > 	GDB's expected void type.
> > 	(dwarf2_cu::dwarf2_cu): Initialise producer_is_icc field.
> > 	* valprint.c (maybe_negate_by_bytes): Add an assertion that the
> > 	LEN is greater than 0.
> 
> While I think this is reasonable (and non-invasive/safe), I have some questions.
> 
> > @@ -17548,7 +17565,11 @@ read_base_type (struct die_info *die, struct dwarf2_cu *cu)
> >  	type = dwarf2_init_float_type (objfile, bits, name, name);
> >  	break;
> >        case DW_ATE_signed:
> > -	type = init_integer_type (objfile, bits, 0, name);
> > +	if (bits == 0 && producer_is_icc (cu)
> > +	    && strcmp (name, "void") == 0)
> > +	  type = objfile_type (objfile)->builtin_void;
> > +	else
> > +	  type = init_integer_type (objfile, bits, 0, name);
> >  	break;
> >        case DW_ATE_unsigned:
> >  	if (cu->language == language_fortran
> 
> Is this the appropriate place for this? The patch is attempting to deal specifically
> with the void return of a function. I would have thought to catch this anomaly in
> read_func_scope/add_dwarf2_member_fn. These sorts of exceptions are handled
> relatively commonly in dwarf2read.c.

In the new patch you'll see that after further testing the
non-standard DWARF is actually from more than just the return type.
Function arguments also pick up these strange integer void types.

As such, moving the fix into read_func_scope/add_dwarf2_member_fn
isn't the right solution - though given my original patch that was a
sensible suggestion.

> 
> While the DWARF specifications specifically call out using DW_TAG_unspecified_type
> for the void type (in C/C++), it doesn't actually say that this particular usage
> is invalid AFAICT. [Although I think it is.] Nonetheless, changing this here would
> preclude some language from doing something like this (as silly as
> that sounds).

I agree.  And I've tried very hard to avoid saying "incorrect" or
"wrong" when describing this case specifically to avoid getting into
discussion about what is a DWARF "requirement" and what is merely a
suggestions or standard practice.  I've tried to say "non-standard" as
this seems pretty neutral.

I agree, at some point a language might want to do the above.  One
problem with this is that GDB appears to make the assumption that
integer types are non-zero in length.  This patch includes an
assertion that guards one place where this assumption is made.  Maybe
this is the only place in GDB that makes such an assumption .... maybe
not.  I guess what I'm thinking is that if someone ever wanted to do
this, then we might find there are other things in GDB to fix.  Plus
with the ICC producer check, this only precludes other languages
within ICC from doing this....

> 
> Also, if it this is the appropriate place (or even if it is decided to move this
> check elsewhere), why limit this to ICC? Is it simply because ICC only handles
> C/C++? Would it hurt/be worth it to safe guard that gcc or clang or rustc or
> who-knows-what wouldn't cause us similar harm?

I guess I added the check for ICC specifically because of your concern
above.  I'm certainly not going to claim to know all the nasty details
for all the different DWARF producers.  And, as we say above this
isn't obviously "wrong" (though like you, I tend to think it is) it's
more just "non-standard", so, who am I to say that if some other
producer is creating this we should transform it....

I would certainly be happy to drop the ICC check if there was a
feeling that this is the right way to go.

> 
> As for the actual code as it is, why only check DW_ATE_signed? I realize that this
> is specifically to address the particular ICC instance you are trying to fix, but
> something tells me that this could potentially change. Regardless of the encoding,
> this would be invalid.

The new patch drops the signed check.  Like I said above I kept the
patch tight in order to try and limit the number of incorrect
conversions, however, I don't see dropping the signed check as being a
big deal, so it's gone.

> 
> I'm not really asking for any changes. After looking at all kinds of DWARF output
> that I never dreamed possible, my thoughts tend to defensive programming in the
> DWARF reader.

Me too!

The new patch moves the conversion from integer type to void type into
a new function which is now called from a couple of places, but all
from within dwarf2read.c:read_base_type, this catches the return type
and the variable type issues.  The test has been renamed to remove the
focus on return type.  Otherwise, everything is pretty similar.

Let me know what you think.

Thanks,
Andrew

--

gdb: Handle ICC's unexpected void return type

I encountered a binary compiled with Intel's C Compiler (ICC) version
14.0.5.212, which seemed to contain some non-standard DWARF.

The DWARF spec (V5 3.3.2) says:

    Debugging information entries for C void functions should not have
    an attribute for the return type.

However, what I observed in the DWARF from this ICC compiled binary
was this:

    ...
    <0><857>: Abbrev Number: 1 (DW_TAG_compile_unit)
       <858>   DW_AT_comp_dir    : (indirect string, offset: 0x48d): /tmp/
       <85c>   DW_AT_language    : 1       (ANSI C)
       <85d>   DW_AT_name        : (indirect string, offset: 0x77c): filename.c
       <861>   DW_AT_producer    : (indirect string, offset: 0x520): Intel(R) C Intel(R) 64 Compiler ...
       <865>   DW_AT_low_pc      : 0x4378d0
       <86d>   DW_AT_high_pc     : 0x4378f0
       <875>   DW_AT_stmt_list   : 0xa37
    ...
    <1><7ea>: Abbrev Number: 2 (DW_TAG_base_type)
       <7eb>   DW_AT_byte_size   : 0
       <7ec>   DW_AT_encoding    : 5       (signed)
       <7ed>   DW_AT_name        : (indirect string, offset: 0x58f): void
    ...
    <1><7f1>: Abbrev Number: 3 (DW_TAG_subprogram)
       <7f2>   DW_AT_decl_line   : 268
       <7f4>   DW_AT_decl_column : 30
       <7f5>   DW_AT_decl_file   : 1
       <7f6>   DW_AT_type        : <0x7ea>
       <7fa>   DW_AT_prototyped  : 1
       <7fb>   DW_AT_name        : (indirect string, offset: 0x761): function_foo
       <7ff>   DW_AT_MIPS_linkage_name: (indirect string, offset: 0x761): function_foo
       <803>   DW_AT_low_pc      : 0x4378a0
       <80b>   DW_AT_high_pc     : 0x4378d0
       <813>   DW_AT_external    : 1
    ...

So function 'function_foo' has void return type, but still has a
DW_AT_type attribute for a 0 sized type called void.

What was found was that when the 'finish' command was used to leave
'function_foo', GDB would crash.

The problem is that in infcmd.c:print_return_value GDB tries to filter
out void return types, by looking for the TYPE_CODE_VOID, this fails
for the 'void' type as it has code TYPE_CODE_INT and GDB then tries to
print the 'void' type.

This eventually ends in a call to valprint.c:maybe_negate_by_bytes,
however, the len (length) of the value being negated is 0, which is
not detected or expected by this code, and invalid memory accesses
occur, some of which might cause GDB to crash.

The above DWARF was seen on version 14.0.5.212 of ICC.

I have also tested ICC versions 18.0.2.199 and 17.0.7.259, on both of
these versions, the DW_AT_type on the DW_TAG_subprogram has been
removed, bringing ICC inline with the DWARF standard, and with the
DWARF produced by GCC.

I only have limited access to these specific versions of ICC so I am
unable to get more specific details for when the generated DWARF
became non-standard or when it was changed to be more inline with the
DWARF standard.

Further testing revealed additional places where ICC produced 'void'
related DWARF that GDB struggles with.  When I compiled code that
contained a function with this signature:

    void funcx (void *arg);

on ICC 17/18, I got the following DWARF (notice the void return type
is now gone):

    ...
    <1><32>: Abbrev Number: 2 (DW_TAG_subprogram)
       <33>   DW_AT_decl_line   : 2
       <34>   DW_AT_decl_file   : 1
       <35>   DW_AT_prototyped  : 1
       <36>   DW_AT_name        : (indirect string, offset: 0xc5): funcx
       <3a>   DW_AT_MIPS_linkage_name: (indirect string, offset: 0xc5): funcx
       <3e>   DW_AT_low_pc      : 0x6dc
       <46>   DW_AT_high_pc     : 0x703
       <4e>   DW_AT_external    : 1
    <2><4f>: Abbrev Number: 3 (DW_TAG_formal_parameter)
       <50>   DW_AT_decl_line   : 2
       <51>   DW_AT_decl_file   : 1
       <52>   DW_AT_type        : <0x6a>
       <56>   DW_AT_name        : arg
       <5a>   DW_AT_location    : 2 byte block: 76 70      (DW_OP_breg6 (rbp): -16)
    ...
    <1><6a>: Abbrev Number: 5 (DW_TAG_pointer_type)
       <6b>   DW_AT_type        : <0x6f>
    <1><6f>: Abbrev Number: 6 (DW_TAG_base_type)
       <70>   DW_AT_byte_size   : 0
       <71>   DW_AT_encoding    : 5        (signed)
       <72>   DW_AT_name        : (indirect string, offset: 0xcb): void
    ...

However, the function argument 'arg' does still reference a 'void'
type.  This case doesn't seem as obviously non-standard as the
previous one, but I think that the DWARF standard (V5 5.2) does
suggest that the above is not the recommended approach.  If we compare
to the DWARF generated by GCC 7.3.1:

    ...
    <1><68>: Abbrev Number: 5 (DW_TAG_subprogram)
       <69>   DW_AT_external    : 1
       <69>   DW_AT_name        : (indirect string, offset: 0x221): funcx
       <6d>   DW_AT_decl_file   : 1
       <6e>   DW_AT_decl_line   : 2
       <6f>   DW_AT_prototyped  : 1
       <6f>   DW_AT_low_pc      : 0x400487
       <77>   DW_AT_high_pc     : 0x22
       <7f>   DW_AT_frame_base  : 1 byte block: 9c         (DW_OP_call_frame_cfa)
       <81>   DW_AT_GNU_all_call_sites: 1
       <81>   DW_AT_sibling     : <0xa0>
    <2><85>: Abbrev Number: 6 (DW_TAG_formal_parameter)
       <86>   DW_AT_name        : arg
       <8a>   DW_AT_decl_file   : 1
       <8b>   DW_AT_decl_line   : 2
       <8c>   DW_AT_type        : <0xa0>
       <90>   DW_AT_location    : 2 byte block: 91 58      (DW_OP_fbreg: -40)
    ...
    <1><a0>: Abbrev Number: 7 (DW_TAG_pointer_type)
       <a1>   DW_AT_byte_size   : 8
    ...

Here we see that the DW_TAG_pointer_type doesn't reference any further
type.  This also seems out of line with the DWARF standard (which I
think recommends using a DW_TAG_unspecified_type entry), however GDB
does handle the GCC generated DWARF better.

If we look at how GDB handles the DWARF from GCC, then we see this:

    (gdb) print *arg
    Attempt to dereference a generic pointer.

While on the current HEAD of master dereferencing arg causes undefined
behaviour which will likely crash GDB (for the same reason as was
described above for the 'finish' case).  On earlier versions of GDB
the ICC DWARF would cause this:

    (gdb) print *arg
    $1 = 0

In this patch both the return type, and general variable/parameter
type handling is fixed by transforming the synthetic void entries in
the DWARF, the ones that look like this:

    <1><6f>: Abbrev Number: 6 (DW_TAG_base_type)
       <70>   DW_AT_byte_size   : 0
       <71>   DW_AT_encoding    : 5        (signed)
       <72>   DW_AT_name        : (indirect string, offset: 0xcb): void

into GDB's builtin void type.  My criteria for performing the fix are:

  1. Binary produced by any version of ICC,
  2. We're producing an integer type,
  3. The size is 0, and
  4. The name is "void".

I ignore the signed / unsigned nature of the integer.

Potentially we could drop the ICC detection too, this should be a
reasonably safe transformation to perform, however, I'm generally
pretty nervous when it comes to modifying how the DWARF is parsed so,
for now, I have restricted this to ICC only.

I also added an assertion to maybe_negate_by_bytes.  This is nothing
to do with the actual fix, but should detect incorrect use of this
function in the future, without relying on undefined behaviour to
crash GDB.

I added a new test that makes use the of the testsuite's DWARF
generator.  As it is tricky to create target independent tests that
pass function parameters using the DWARF generator (as specifying the
argument location is target specific) I have instead made use of a
global variable void*.  This still shows the issue.

We already have a predicate in the DWARF parser to detect versions of
ICC prior to 14, however, this issue was spotted on a later version.
As a result I've added a new predicate that is true for any version of
ICC.

gdb/ChangeLog:

	* dwarf2read.c (struct dwarf2_cu): Add producer_is_icc field.
	(producer_is_icc): New function.
	(check_producer): Set producer_is_icc field on dwarf2_cu.
	(dwarf2_init_integer_type): New function.
	(read_base_type): Call dwarf2_init_integer_type instead of
	init_integer_type in all cases.
	(dwarf2_cu::dwarf2_cu): Initialise producer_is_icc field.
	* valprint.c (maybe_negate_by_bytes): Add an assertion that the
	LEN is greater than 0.

gdb/testsuite/ChangeLog:

	* gdb.dwarf2/void-type.c: New file.
	* gdb.dwarf2/void-type.exp: New file.
---
 gdb/ChangeLog                          |  12 ++++
 gdb/dwarf2read.c                       |  49 +++++++++++--
 gdb/testsuite/ChangeLog                |   5 ++
 gdb/testsuite/gdb.dwarf2/void-type.c   |  36 ++++++++++
 gdb/testsuite/gdb.dwarf2/void-type.exp | 125 +++++++++++++++++++++++++++++++++
 gdb/valprint.c                         |   1 +
 6 files changed, 222 insertions(+), 6 deletions(-)
 create mode 100644 gdb/testsuite/gdb.dwarf2/void-type.c
 create mode 100644 gdb/testsuite/gdb.dwarf2/void-type.exp

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 1b4f966cc35..b237c81fe4f 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -551,6 +551,7 @@ struct dwarf2_cu
   unsigned int checked_producer : 1;
   unsigned int producer_is_gxx_lt_4_6 : 1;
   unsigned int producer_is_gcc_lt_4_3 : 1;
+  bool producer_is_icc : 1;
   unsigned int producer_is_icc_lt_14 : 1;
   bool producer_is_codewarrior : 1;
 
@@ -11363,6 +11364,19 @@ producer_is_icc_lt_14 (struct dwarf2_cu *cu)
   return cu->producer_is_icc_lt_14;
 }
 
+/* ICC generates a DW_AT_type for C void functions.  This was observed on
+   ICC 14.0.5.212, and appears to be against the DWARF spec (V5 3.3.2)
+   which says that void functions should not have a DW_AT_type.  */
+
+static bool
+producer_is_icc (struct dwarf2_cu *cu)
+{
+  if (!cu->checked_producer)
+    check_producer (cu);
+
+  return cu->producer_is_icc;
+}
+
 /* Check for possibly missing DW_AT_comp_dir with relative .debug_line
    directory paths.  GCC SVN r127613 (new option -fdebug-prefix-map) fixed
    this, it was first present in GCC release 4.3.0.  */
@@ -14897,7 +14911,10 @@ check_producer (struct dwarf2_cu *cu)
       cu->producer_is_gcc_lt_4_3 = major < 4 || (major == 4 && minor < 3);
     }
   else if (producer_is_icc (cu->producer, &major, &minor))
-    cu->producer_is_icc_lt_14 = major < 14;
+    {
+      cu->producer_is_icc = true;
+      cu->producer_is_icc_lt_14 = major < 14;
+    }
   else if (startswith (cu->producer, "CodeWarrior S12/L-ISA"))
     cu->producer_is_codewarrior = true;
   else
@@ -17494,6 +17511,25 @@ dwarf2_init_float_type (struct objfile *objfile, int bits, const char *name,
   return type;
 }
 
+/* Allocate an integer type of size BITS and name NAME.  */
+
+static struct type *
+dwarf2_init_integer_type (struct dwarf2_cu *cu, struct objfile *objfile,
+			  int bits, int unsigned_p, const char *name)
+{
+  struct type *type;
+
+  /* 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)
+    type = objfile_type (objfile)->builtin_void;
+  else
+    type = init_integer_type (objfile, bits, unsigned_p, name);
+
+  return type;
+}
+
 /* Find a representation of a given base type and install
    it in the TYPE field of the die.  */
 
@@ -17543,7 +17579,7 @@ read_base_type (struct die_info *die, struct dwarf2_cu *cu)
 	type = dwarf2_init_float_type (objfile, bits, name, name);
 	break;
       case DW_ATE_signed:
-	type = init_integer_type (objfile, bits, 0, name);
+	type = dwarf2_init_integer_type (cu, objfile, bits, 0, name);
 	break;
       case DW_ATE_unsigned:
 	if (cu->language == language_fortran
@@ -17551,7 +17587,7 @@ read_base_type (struct die_info *die, struct dwarf2_cu *cu)
 	    && startswith (name, "character("))
 	  type = init_character_type (objfile, bits, 1, name);
 	else
-	  type = init_integer_type (objfile, bits, 1, name);
+	  type = dwarf2_init_integer_type (cu, objfile, bits, 1, name);
 	break;
       case DW_ATE_signed_char:
 	if (cu->language == language_ada || cu->language == language_m2
@@ -17559,7 +17595,7 @@ read_base_type (struct die_info *die, struct dwarf2_cu *cu)
 	    || cu->language == language_fortran)
 	  type = init_character_type (objfile, bits, 0, name);
 	else
-	  type = init_integer_type (objfile, bits, 0, name);
+	  type = dwarf2_init_integer_type (cu, objfile, bits, 0, name);
 	break;
       case DW_ATE_unsigned_char:
 	if (cu->language == language_ada || cu->language == language_m2
@@ -17568,7 +17604,7 @@ read_base_type (struct die_info *die, struct dwarf2_cu *cu)
 	    || cu->language == language_rust)
 	  type = init_character_type (objfile, bits, 1, name);
 	else
-	  type = init_integer_type (objfile, bits, 1, name);
+	  type = dwarf2_init_integer_type (cu, objfile, bits, 1, name);
 	break;
       case DW_ATE_UTF:
 	{
@@ -17582,7 +17618,7 @@ read_base_type (struct die_info *die, struct dwarf2_cu *cu)
 	    {
 	      complaint (_("unsupported DW_ATE_UTF bit size: '%d'"),
 			 bits);
-	      type = init_integer_type (objfile, bits, 1, name);
+	      type = dwarf2_init_integer_type (cu, objfile, bits, 1, name);
 	    }
 	  return set_die_type (die, type, cu);
 	}
@@ -25129,6 +25165,7 @@ dwarf2_cu::dwarf2_cu (struct dwarf2_per_cu_data *per_cu_)
     checked_producer (0),
     producer_is_gxx_lt_4_6 (0),
     producer_is_gcc_lt_4_3 (0),
+    producer_is_icc (false),
     producer_is_icc_lt_14 (0),
     producer_is_codewarrior (false),
     processing_has_namespace_info (0)
diff --git a/gdb/testsuite/gdb.dwarf2/void-type.c b/gdb/testsuite/gdb.dwarf2/void-type.c
new file mode 100644
index 00000000000..8a88dc25df4
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/void-type.c
@@ -0,0 +1,36 @@
+/* Copyright (C) 2018 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   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/>.  */
+
+int var_a = 5;
+
+void *var_ptr = &var_a;
+
+__attribute__((noinline)) void
+func ()
+{
+  asm ("func_label: .globl func_label");
+  asm ("" ::: "memory");
+  return;
+}
+
+int
+main ()
+{
+  asm ("main_label: .globl main_label");
+  func ();
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.dwarf2/void-type.exp b/gdb/testsuite/gdb.dwarf2/void-type.exp
new file mode 100644
index 00000000000..3691bff7792
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/void-type.exp
@@ -0,0 +1,125 @@
+# 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 spotted in an Intel C Compiler
+# generated binary.
+#
+# The DWARF standard (V5 3.3.2) says that a void C function should not
+# have a DW_AT_type attribute, however, an ICC compiled binary was
+# found to have a DW_AT_type that referenced a signed integer type, of
+# size 0, with the name 'void'.
+#
+# This 'void' integer type would cause GDB to crash in some cases, one
+# that was seen was when using 'finish' to leave the void function.
+
+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 "Intel(R) C Intel(R) 64 Compiler XE for applications running on Intel(R) 64, Version 14.0.5.212 Build 20150212"}
+                {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}
+		{DW_AT_name      int}
+	    }
+
+	    int_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_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 :$int_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
+}
+
+# Place a breakpoint in 'func' and continue to there.
+gdb_breakpoint func
+gdb_continue_to_breakpoint "func"
+
+# Check how GDB handles the void* variable.
+gdb_test "p *var_ptr" "Attempt to dereference a generic pointer." \
+    "check that dereferencing a void* gives a suitable message"
+
+# Now finish, returning to main.
+gdb_test "finish" [multi_line \
+		       "Run till exit from #0  $hex in func \\\(\\\)" \
+		       "$hex in main \\\(\\\)"] \
+    "check that finish completes"
diff --git a/gdb/valprint.c b/gdb/valprint.c
index 2b63a91e8df..b2236f89318 100644
--- a/gdb/valprint.c
+++ b/gdb/valprint.c
@@ -1604,6 +1604,7 @@ maybe_negate_by_bytes (const gdb_byte *bytes, unsigned len,
 		       gdb::byte_vector *out_vec)
 {
   gdb_byte sign_byte;
+  gdb_assert (len > 0);
   if (byte_order == BFD_ENDIAN_BIG)
     sign_byte = bytes[0];
   else
-- 
2.14.5

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

[Keith Seitz]

On 10/30/18 5:22 PM, Andrew Burgess wrote:

> Thanks for your feedback.  Sorry for the delay in replying, I've
> managed to get some more test binaries since my original patch, so I
> have a better response now.

This is awesome. Thank you so much for humoring me!

> * Keith Seitz <keiths@redhat.com> [2018-10-24 14:00:16 -0700]:
> 
>> On 10/23/18 2:28 PM, Andrew Burgess wrote:
>> Is this the appropriate place for this? The patch is attempting to deal specifically
>> with the void return of a function. I would have thought to catch this anomaly in
>> read_func_scope/add_dwarf2_member_fn. These sorts of exceptions are handled
>> relatively commonly in dwarf2read.c.
> 
> In the new patch you'll see that after further testing the
> non-standard DWARF is actually from more than just the return type.
> Function arguments also pick up these strange integer void types.

Yikes. That certainly changes the landscape.

> As such, moving the fix into read_func_scope/add_dwarf2_member_fn
> isn't the right solution - though given my original patch that was a
> sensible suggestion.

I agree.

>> Also, if it this is the appropriate place (or even if it is decided to move this
>> check elsewhere), why limit this to ICC? Is it simply because ICC only handles
>> C/C++? Would it hurt/be worth it to safe guard that gcc or clang or rustc or
>> who-knows-what wouldn't cause us similar harm?
> 
> I guess I added the check for ICC specifically because of your concern
> above.  I'm certainly not going to claim to know all the nasty details
> for all the different DWARF producers.  And, as we say above this
> isn't obviously "wrong" (though like you, I tend to think it is) it's
> more just "non-standard", so, who am I to say that if some other
> producer is creating this we should transform it....
> 
> I would certainly be happy to drop the ICC check if there was a
> feeling that this is the right way to go.

I'm okay with the check, especially given your expanded findings. [And
I'm all for minimizing the impact of a patch.]

> The new patch moves the conversion from integer type to void type into
> a new function which is now called from a couple of places, but all
> from within dwarf2read.c:read_base_type, this catches the return type
> and the variable type issues.  The test has been renamed to remove the
> focus on return type.  Otherwise, everything is pretty similar.
> 
> Let me know what you think.

That answers all the questions I had. Thank you very much.

Kevin has already approved the patch, so unless he has anything further,
I would say this patch is good to go in.

Keith

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

[Kevin Buettner]

On Tue, 30 Oct 2018 17:34:00 -0700
Keith Seitz <keiths@redhat.com> wrote:

> > The new patch moves the conversion from integer type to void type into
> > a new function which is now called from a couple of places, but all
> > from within dwarf2read.c:read_base_type, this catches the return type
> > and the variable type issues.  The test has been renamed to remove the
> > focus on return type.  Otherwise, everything is pretty similar.
> > 
> > Let me know what you think.  
> 
> That answers all the questions I had. Thank you very much.
> 
> Kevin has already approved the patch, so unless he has anything further,
> I would say this patch is good to go in.

I read through the new patch.  It looks good to me also.

Kevin

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

[Tom Tromey]

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

Tom

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

[Andrew Burgess]

* 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

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

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:

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

Andrew> Is this OK to apply?

Yes, thank you.

Tom