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

["Paweł Wódkowski" <pwodkowski@pl.sii.eu>]

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