From patchwork Tue Nov 6 20:43:28 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 30047 Received: (qmail 119935 invoked by alias); 6 Nov 2018 20:43:43 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 119837 invoked by uid 89); 6 Nov 2018 20:43:36 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.4 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=recognized X-HELO: mail-wr1-f65.google.com Received: from mail-wr1-f65.google.com (HELO mail-wr1-f65.google.com) (209.85.221.65) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 06 Nov 2018 20:43:33 +0000 Received: by mail-wr1-f65.google.com with SMTP id j17-v6so9929074wrq.11 for ; Tue, 06 Nov 2018 12:43:32 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=yj/aq7QRpPXvLI4/i5+/CvXmm8HXwJUfqCgYdHUjPy8=; b=KJtTStTTq9iBI7x2iR/NW2LbyloPYwadK3tcOkfuQH16zvm+4dY6SahxZWaUq2zjl0 2XH/Gq/2HQKB9aqEfHKlF8aifKT2/zs+xdbht/PACLIWY130C+/imWCh3e+jkmq4a8Qe kEpXPEs6/zMFb1jEl/xBPHmnsviN5kAawOYomKrQKRLeRKYkETJRoRlxV5034VYmLk5u rkkaRj1Bwftex8fTbvSDxe8eAVU/eWzGPZ6mHOR+kFEKi2jmosvCbHAZgk6yGIExv5u8 dGr4+LaHH6ChZvLux1KvkbTzQshSArcZg0hIAWRpyjnZvt6ZFOExOjblDtPjBphs9GvV Fjfw== Return-Path: Received: from localhost (host81-148-252-35.range81-148.btcentralplus.com. [81.148.252.35]) by smtp.gmail.com with ESMTPSA id k4sm18741135wrx.91.2018.11.06.12.43.29 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 06 Nov 2018 12:43:29 -0800 (PST) Date: Tue, 6 Nov 2018 20:43:28 +0000 From: Andrew Burgess To: Tom Tromey Cc: gdb-patches@sourceware.org Subject: Re: [PATCH] gdb: Handle ICC's unexpected void return type Message-ID: <20181106204328.GJ16539@embecosm.com> References: <20181023212843.4774-1-andrew.burgess@embecosm.com> <878t26rs9a.fsf@tromey.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <878t26rs9a.fsf@tromey.com> X-Fortune: A log may float in a river, but that does not make it a crocodile. X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] User-Agent: Mutt/1.9.2 (2017-12-15) X-IsSubscribed: yes * Tom Tromey [2018-11-06 11:10:57 -0700]: > >>>>> "Andrew" == Andrew Burgess 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 . + +# 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 = \\*" \ + "ptype of a pointer to a basic type with missing name" + +gdb_test "ptype var_a" "type = " \ + "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 {