gdb: Handle ICC's unexpected void return type
Commit Message
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
Comments
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
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
>>>>> "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
@@ -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)
new file mode 100644
@@ -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;
+}
new file mode 100644
@@ -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"
@@ -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