From patchwork Tue Jun 11 19:23:58 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 33088 Received: (qmail 30515 invoked by alias); 11 Jun 2019 19:24:16 -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 30494 invoked by uid 89); 11 Jun 2019 19:24:14 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 spammy=revealed, HTo:D*pl X-HELO: mail-wm1-f66.google.com Received: from mail-wm1-f66.google.com (HELO mail-wm1-f66.google.com) (209.85.128.66) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 11 Jun 2019 19:24:02 +0000 Received: by mail-wm1-f66.google.com with SMTP id c66so4135953wmf.0 for ; Tue, 11 Jun 2019 12:24:02 -0700 (PDT) Return-Path: Received: from ?IPv6:2001:8a0:f913:f700:4eeb:42ff:feef:f164? ([2001:8a0:f913:f700:4eeb:42ff:feef:f164]) by smtp.gmail.com with ESMTPSA id y184sm4040161wmg.14.2019.06.11.12.23.59 (version=TLS1_3 cipher=AEAD-AES128-GCM-SHA256 bits=128/128); Tue, 11 Jun 2019 12:23:59 -0700 (PDT) Subject: Re: [PATCH v4] Dwarf: Don't add nameless modules to partial symbol table. To: Tomasz Kulasek , gdb-patches@sourceware.org References: <1546688626-18391-1-git-send-email-pwodkowski@pl.sii.eu> <1551277272-79935-1-git-send-email-tkulasek@sii.pl> Cc: Bernhard Heckel , Joel Brobecker From: Pedro Alves Message-ID: <71d2a071-1c2a-23bf-c8cf-a3e48431e194@redhat.com> Date: Tue, 11 Jun 2019 20:23:58 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: <1551277272-79935-1-git-send-email-tkulasek@sii.pl> Hi, Looks like this has been reviewed a number of times over the years but never managed to get merged. I was asked internally at Red Hat to take a look, and I've merged it now. I confirmed that this version addresses Joel's comments to v3. See more below. On 2/27/19 2:21 PM, Tomasz Kulasek wrote: > From: Bernhard Heckel > > A name for BLOCK DATA in Fortran is optional. If no > name has been assigned, GDB will crash during read-in of DWARF > when BLOCK DATA is represented via DW_TAG_module. > BLOCK DATA is used for one-time initialization of > non-pointer variables in named common blocks. > > As of now there is no issue when gfortran is used as > DW_TAG_module is not emited. For Intel ifort the nameless > DW_TAG_module is present and have following form: > > ... > <1>
: Abbrev Number: 7 (DW_TAG_module) > DW_AT_decl_line : 46 > DW_AT_decl_file : 1 > DW_AT_description : (indirect string, offset: 0x110): block > data > DW_AT_high_pc : 0x402bb7 > DW_AT_low_pc : 0x402bb7 > ... > > Missing name lead to crash in add_partial_symbol function > during length calculation. > > 2016-06-15 Bernhard Heckel > > gdb/Changelog: > * dwarf2read.c (add_partial_symbol): Skip nameless modules. > > gdb/testsuite/Changelog: > * gdb.fortran/block-data.f: New. > * gdb.fortran/block-data.exp: New. > > Changes from V1 to V2: > * No changes - just sent with other patches in series. > > Changes from V2 to V3: > * Decouple from patch series. > * Provide example DW_TAG_module in commit message. > * Fix mistakes in comments. > > Changes from V3 to V4: > * Dont complain if module have no name in read_module_type. > * Fix period excapeing in block-data.exp. > --- > gdb/dwarf2read.c | 16 ++++----- > gdb/testsuite/gdb.fortran/block-data.exp | 49 ++++++++++++++++++++++++++++ > gdb/testsuite/gdb.fortran/block-data.f | 56 ++++++++++++++++++++++++++++++++ > 3 files changed, 113 insertions(+), 8 deletions(-) > create mode 100644 gdb/testsuite/gdb.fortran/block-data.exp > create mode 100644 gdb/testsuite/gdb.fortran/block-data.f > > diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c > index 98f46e0..3b0274d 100644 > --- a/gdb/dwarf2read.c > +++ b/gdb/dwarf2read.c > @@ -9018,11 +9018,14 @@ add_partial_symbol (struct partial_die_info *pdi, struct dwarf2_cu *cu) > 0, cu->language, objfile); > break; > case DW_TAG_module: > - add_psymbol_to_list (actual_name, strlen (actual_name), > - built_actual_name != NULL, > - MODULE_DOMAIN, LOC_TYPEDEF, -1, > - psymbol_placement::GLOBAL, > - 0, cu->language, objfile); > + /* In Fortran 77 there might be a "BLOCK DATA" module available wihout Typo "wihout". > + any name. If so, we skip the module as it doesn't bring any value. */ > + if (actual_name != nullptr) > + add_psymbol_to_list (actual_name, strlen (actual_name), > + built_actual_name != NULL, > + MODULE_DOMAIN, LOC_TYPEDEF, -1, > + psymbol_placement::GLOBAL, > + 0, cu->language, objfile); > break; > case DW_TAG_class_type: > case DW_TAG_interface_type: > @@ -16929,9 +16932,6 @@ read_module_type (struct die_info *die, struct dwarf2_cu *cu) > struct type *type; > > module_name = dwarf2_name (die, cu); > - if (!module_name) > - complaint (_("DW_TAG_module has no name, offset %s"), > - sect_offset_str (die->sect_off)); > type = init_type (objfile, TYPE_CODE_MODULE, 0, module_name); > > return set_die_type (die, type, cu); > diff --git a/gdb/testsuite/gdb.fortran/block-data.exp b/gdb/testsuite/gdb.fortran/block-data.exp > new file mode 100644 > index 0000000..389ae15 > --- /dev/null > +++ b/gdb/testsuite/gdb.fortran/block-data.exp > @@ -0,0 +1,49 @@ > +# Copyright 2019 Free Software Foundation, Inc. This file was first posted in 2016, so the copyright years should include that. > + > +gdb_test "print doub1" "= 1\.11\\d+" "print doub1, default values" > +gdb_test "print doub2" "= 2\.22\\d+" "print doub2, default values" > +gdb_test "print char1" "= 'abcdef'" "print char1, default values" > +gdb_test "print char2" "= 'ghijkl'" "print char2, default values" > + > +gdb_breakpoint [gdb_get_line_number "! BP_BEFORE_SUB"] > +gdb_continue_to_breakpoint "! BP_BEFORE_SUB" ".*! BP_BEFORE_SUB.*" > +gdb_test "print doub1" "= 11\.11\\d+" "print doub1, before sub" > +gdb_test "print doub2" "= 22\.22\\d+" "print doub2, before sub" > +gdb_test "print char1" "= 'ABCDEF'" "print char1, before sub" > +gdb_test "print char2" "= 'GHIJKL'" "print char2, before sub" > + > +gdb_breakpoint [gdb_get_line_number "! BP_SUB"] > +gdb_continue_to_breakpoint "! BP_SUB" ".*! BP_SUB.*" > +gdb_test "print doub1" "= 11\.11\\d+" "print char1, in sub" ^^^^^ I checked for duplicate test names, and that revealed the typo above. Should be "doub1" instead of "char1". But better is to use with_test_prefix instead. > +gdb_test "print doub2" "= 22\.22\\d+" "print doub2, in sub" > +gdb_test "print char1" "= 'ABCDEF'" "print char1, in sub" > +gdb_test "print char2" "= 'GHIJKL'" "print char2, in sub" > +! > +! Test if GDB can handle block data without global name Missing period. I've now applied the patch, with the issues above fixed, along with a few other editorial tweaks to commit log and comments. Note that I've extended the intro comment in the .exp file. Here's what I merged. From a5fd13a91534b1c79a4b61995894a5bb4f08d3b0 Mon Sep 17 00:00:00 2001 From: Bernhard Heckel Date: Wed, 27 Feb 2019 15:21:12 +0100 Subject: [PATCH] Dwarf: Don't add nameless modules to partial symbol table A name for BLOCK DATA in Fortran is optional. If no name has been assigned, GDB crashes during read-in of DWARF when BLOCK DATA is represented via DW_TAG_module. BLOCK DATA is used for one-time initialization of non-pointer variables in named common blocks. As of now there is no issue when gfortran is used as DW_TAG_module is not emitted. However, with Intel ifort the nameless DW_TAG_module is present and has the following form: ... <1>
: Abbrev Number: 7 (DW_TAG_module) DW_AT_decl_line : 46 DW_AT_decl_file : 1 DW_AT_description : (indirect string, offset: 0x110): block data DW_AT_high_pc : 0x402bb7 DW_AT_low_pc : 0x402bb7 ... The missing name leads to a crash in add_partial_symbol, during length calculation. gdb/ChangeLog: 2019-06-11 Bernhard Heckel * dwarf2read.c (add_partial_symbol): Skip nameless modules. gdb/testsuite/Changelog: 2019-06-11 Bernhard Heckel * gdb.fortran/block-data.f: New. * gdb.fortran/block-data.exp: New. --- gdb/dwarf2read.c | 17 +++++---- gdb/testsuite/gdb.fortran/block-data.exp | 63 ++++++++++++++++++++++++++++++++ gdb/testsuite/gdb.fortran/block-data.f | 56 ++++++++++++++++++++++++++++ 3 files changed, 128 insertions(+), 8 deletions(-) create mode 100644 gdb/testsuite/gdb.fortran/block-data.exp create mode 100644 gdb/testsuite/gdb.fortran/block-data.f diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c index 06c319dac02..4cf9fcfa218 100644 --- a/gdb/dwarf2read.c +++ b/gdb/dwarf2read.c @@ -9018,11 +9018,15 @@ add_partial_symbol (struct partial_die_info *pdi, struct dwarf2_cu *cu) 0, cu->language, objfile); break; case DW_TAG_module: - add_psymbol_to_list (actual_name, strlen (actual_name), - built_actual_name != NULL, - MODULE_DOMAIN, LOC_TYPEDEF, -1, - psymbol_placement::GLOBAL, - 0, cu->language, objfile); + /* With Fortran 77 there might be a "BLOCK DATA" module + available without any name. If so, we skip the module as it + doesn't bring any value. */ + if (actual_name != nullptr) + add_psymbol_to_list (actual_name, strlen (actual_name), + built_actual_name != NULL, + MODULE_DOMAIN, LOC_TYPEDEF, -1, + psymbol_placement::GLOBAL, + 0, cu->language, objfile); break; case DW_TAG_class_type: case DW_TAG_interface_type: @@ -16955,9 +16959,6 @@ read_module_type (struct die_info *die, struct dwarf2_cu *cu) struct type *type; module_name = dwarf2_name (die, cu); - if (!module_name) - complaint (_("DW_TAG_module has no name, offset %s"), - sect_offset_str (die->sect_off)); type = init_type (objfile, TYPE_CODE_MODULE, 0, module_name); return set_die_type (die, type, cu); diff --git a/gdb/testsuite/gdb.fortran/block-data.exp b/gdb/testsuite/gdb.fortran/block-data.exp new file mode 100644 index 00000000000..8da8c8824c8 --- /dev/null +++ b/gdb/testsuite/gdb.fortran/block-data.exp @@ -0,0 +1,63 @@ +# Copyright 2016-2019 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 . + +# Test anonymous block-data statements. + +# A name for BLOCK DATA in Fortran is optional. BLOCK DATA is used +# for one-time initialization of non-pointer variables in named common +# blocks. GDB used to crash with 'Intel ifort'-generated code, which +# outputs nameless DW_TAG_module, unlike with gfortran which just +# doesn't emit DW_TAG_module in this case. + +if { [skip_fortran_tests] } { return -1 } + +standard_testfile .f +load_lib "fortran.exp" + +if {[prepare_for_testing $testfile.exp $testfile $srcfile {debug f90}]} { + return -1 +} + +if ![runto MAIN__] then { + untested "couldn't run to breakpoint MAIN__" + return -1 +} + +with_test_prefix "default values" { + gdb_test "print doub1" "= 1\.11\\d+" + gdb_test "print doub2" "= 2\.22\\d+" + gdb_test "print char1" "= 'abcdef'" + gdb_test "print char2" "= 'ghijkl'" +} + +gdb_breakpoint [gdb_get_line_number "! BP_BEFORE_SUB"] +gdb_continue_to_breakpoint "! BP_BEFORE_SUB" ".*! BP_BEFORE_SUB.*" + +with_test_prefix "before sub" { + gdb_test "print doub1" "= 11\.11\\d+" + gdb_test "print doub2" "= 22\.22\\d+" + gdb_test "print char1" "= 'ABCDEF'" + gdb_test "print char2" "= 'GHIJKL'" +} + +gdb_breakpoint [gdb_get_line_number "! BP_SUB"] +gdb_continue_to_breakpoint "! BP_SUB" ".*! BP_SUB.*" + +with_test_prefix "in sub" { + gdb_test "print doub1" "= 11\.11\\d+" + gdb_test "print doub2" "= 22\.22\\d+" + gdb_test "print char1" "= 'ABCDEF'" + gdb_test "print char2" "= 'GHIJKL'" +} diff --git a/gdb/testsuite/gdb.fortran/block-data.f b/gdb/testsuite/gdb.fortran/block-data.f new file mode 100644 index 00000000000..acb2c5a8591 --- /dev/null +++ b/gdb/testsuite/gdb.fortran/block-data.f @@ -0,0 +1,56 @@ +! Copyright 2016-2019 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 . +! +! Check that GDB can handle block data with no global name. +! +! MAIN + PROGRAM bdata + DOUBLE PRECISION doub1, doub2 + CHARACTER*6 char1, char2 + + COMMON /BLK1/ doub1, char1 + COMMON /BLK2/ doub2, char2 + + doub1 = 11.111 + doub2 = 22.222 + char1 = 'ABCDEF' + char2 = 'GHIJKL' + CALL sub_block_data ! BP_BEFORE_SUB + STOP + END + +! BLOCK DATA + BLOCK DATA + + DOUBLE PRECISION doub1, doub2 + CHARACTER*6 char1, char2 + + COMMON /BLK1/ doub1, char1 + COMMON /BLK2/ doub2, char2 + DATA doub1, doub2 /1.111, 2.222/ + DATA char1, char2 /'abcdef', 'ghijkl'/ + END + +! SUBROUTINE + SUBROUTINE sub_block_data + + DOUBLE PRECISION doub1, doub2 + CHARACTER*6 char1, char2 + + COMMON /BLK1/ doub1, char1 + COMMON /BLK2/ doub2, char2 + + char1 = char2; ! BP_SUB + END