Re: [PATCH v2 1/7] DWARF: Don't add nameless modules to partial symbol table.

Paweł Wódkowski
2018-12-02T21:01:54Z

Thanks for review. See comments.

On 27.11.2018 22:41, Andrew Burgess wrote:
> * Pawel Wodkowski <pwodkowski@pl.sii.eu> [2018-11-19 22:38:44 +0100]:
>
>> From: Bernhard Heckel <bernhard.heckel@intel.com>
>>
>> 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.
>
> Thanks for this.
>
> Locally I have:
>
> GNU Fortran (GCC) 7.3.1 20180712 (Red Hat 7.3.1-6)
>
> For me at least this test passes with and without the change to GDB.
>
> Can you confirm which Fortran compiler you see the issue with? It
> probably should be documented in the commit message and in comment
> around the check in dwarf2read.c. It could also be useful to include
> examples of the problem DWARF in the commit message in cases like
> this.
>

Indeed, I checked this for GNU fortran it there is no issue. But for
Intel fortran compiler ifort I'm getting crash in add_partial_symbol
function:

$ ifort block-data.f -g -o block-data
$ gdb --args ../../gdb ./block-data
<< ... >>
GNU gdb (GDB) 8.2.50.20181114-git
Copyright (C) 2018 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later
<http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
<< ... >>
Program received signal SIGSEGV, Segmentation fault.
__strlen_sse2 () at ../sysdeps/x86_64/multiarch/../strlen.S:120
120 ../sysdeps/x86_64/multiarch/../strlen.S: No such file or directory.
(gdb) bt
#0 __strlen_sse2 () at ../sysdeps/x86_64/multiarch/../strlen.S:120
#1 0x000055555575dee9 in add_partial_symbol
(pdi=pdi@entry=0x5555562ca490, cu=cu@entry=0x5555562c34b0) at
dwarf2read.c:8998
#2 0x000055555575e59b in add_partial_namespace (cu=0x5555562c34b0,
set_addrmap=0, highpc=0x7fffffffda90, lowpc=0x7fffffffda88,
pdi=0x5555562ca490) at dwarf2read.c:9058
#3 scan_partial_symbols (first_die=<optimized out>,
lowpc=lowpc@entry=0x7fffffffda88, highpc=highpc@entry=0x7fffffffda90,
set_addrmap=0, cu=cu@entry=0x5555562c34b0) at dwarf2read.c:8682
#4 0x000055555575ee1e in process_psymtab_comp_unit_reader
(reader=reader@entry=0x7fffffffdb50, info_ptr=0x55555617ddb3 "\002\025",
<incomplete sequence \310>, comp_unit_die=0x5555562ca0f0,
has_children=<optimized out>, data=data@entry=0x7fffffffdbf0) at
dwarf2read.c:8033
<< ... >>

>>
>> 2016-06-15 Bernhard Heckel <bernhard.heckel@intel.com>
>>
>> gdb/Changelog:
>> * dwarf2read.c (add_partial_symbol): Skip nameless modules.
>>
>> gdb/Testsuite/Changelog:
>
> It's 'testsuite' all the patches in this series :)
>
>> * gdb.fortran/block-data.f: New.
>> * gdb.fortran/block-data.exp: New.
>> ---
>> gdb/dwarf2read.c | 13 ++++---
>> gdb/testsuite/gdb.fortran/block-data.exp | 51 ++++++++++++++++++++++++++++
>> gdb/testsuite/gdb.fortran/block-data.f | 58 ++++++++++++++++++++++++++++++++
>> 3 files changed, 117 insertions(+), 5 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 d2a8cd44f9a5..89fd4ae15e80 100644
>> --- a/gdb/dwarf2read.c
>> +++ b/gdb/dwarf2read.c
>> @@ -8995,11 +8995,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,
>> - &objfile->global_psymbols,
>> - 0, cu->language, objfile);
>> + /* In Fortran 77 there might be a "BLOCK DATA" module available wihout
>> + any name. If so, we skip the module as it doesn't bring any value */
>
> You need a full-stop and two spaces at the end, like "...any value. */".
>

Ok, will fix.

>> + if (actual_name != nullptr)
>> + add_psymbol_to_list (actual_name, strlen (actual_name),
>> + built_actual_name != nullptr,
>> + MODULE_DOMAIN, LOC_TYPEDEF, -1,
>> + &objfile->global_psymbols,
>> + 0, cu->language, objfile);
>> break;
>> case DW_TAG_class_type:
>> case DW_TAG_interface_type:
>> diff --git a/gdb/testsuite/gdb.fortran/block-data.exp b/gdb/testsuite/gdb.fortran/block-data.exp
>> new file mode 100644
>> index 000000000000..2af250ad3886
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.fortran/block-data.exp
>> @@ -0,0 +1,51 @@
>> +# Copyright 2018 Free Software Foundation, Inc.
>> +#
>> +# Contributed by Intel Corp. <bernhard.heckel@intel.com>
>
> Someone else might correct me, but I think "Contributed by..." lines
> are not supposed to be added to GDB source..
>

Will remove it.

>> +#
>> +# 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 test is supposed to test anonymous block-data statement.
>> +
>> +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
>> +}
>> +
>> +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"
>> +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"
>> diff --git a/gdb/testsuite/gdb.fortran/block-data.f b/gdb/testsuite/gdb.fortran/block-data.f
>> new file mode 100644
>> index 000000000000..a28e687ec885
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.fortran/block-data.f
>> @@ -0,0 +1,58 @@
>> +! Copyright 2018 Free Software Foundation, Inc.
>> +!
>> +! Contributed by Intel Corp. <bernhard.heckel@intel.com>
>> +!
>> +! 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/>.
>> +!
>> +! Test if GDB can handle block data without 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
>> --
>> 2.7.4
>>
>