[1/6] DWARF: Don't add nameless modules to partial symbol table.

Message ID 1500629040-12972-2-git-send-email-tim.wiederhake@intel.com
State New, archived
Headers

Commit Message

Wiederhake, Tim July 21, 2017, 9:23 a.m. UTC
  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.

xxxx-yy-zz  Bernhard Heckel  <bernhard.heckel@intel.com>

gdb/ChangeLog:
	* dwarf2read.c (add_partial_symbol): Skip nameless modules.

gdb/testsuite/ChangeLog:
	* gdb.fortran/block-data.f: New file.
	* gdb.fortran/block-data.exp: New file.


---
 gdb/dwarf2read.c                         | 13 +++++---
 gdb/testsuite/gdb.fortran/block-data.exp | 49 ++++++++++++++++++++++++++++
 gdb/testsuite/gdb.fortran/block-data.f   | 56 ++++++++++++++++++++++++++++++++
 3 files changed, 113 insertions(+), 5 deletions(-)
 create mode 100644 gdb/testsuite/gdb.fortran/block-data.exp
 create mode 100644 gdb/testsuite/gdb.fortran/block-data.f
  

Comments

Yao Qi July 31, 2017, 10:09 p.m. UTC | #1
On 17-07-21 11:23:55, Tim Wiederhake wrote:
> 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.

What is your Fortran compiler?  I used gfortran 4.9.4, it doesn't generate
DW_TAG_module in debug information.  So, I run block-data.exp with
unpatched GDB, all tests pass.

> 
> BLOCK DATA is used for one-time initialization of non-pointer variables in
> named common blocks.
> 
> xxxx-yy-zz  Bernhard Heckel  <bernhard.heckel@intel.com>
> 
> gdb/ChangeLog:
> 	* dwarf2read.c (add_partial_symbol): Skip nameless modules.
> 
> gdb/testsuite/ChangeLog:
> 	* gdb.fortran/block-data.f: New file.
> 	* gdb.fortran/block-data.exp: New file.
>
  
Wiederhake, Tim Aug. 1, 2017, 12:47 p.m. UTC | #2
Hi Yao,

Thank you for your response.

I'm using the Intel? Fortran Compiler:

$ ifort --version
ifort (IFORT) 15.0.0 20140716
Copyright (C) 1985-2014 Intel Corporation.  All rights reserved.

$ ifort -g -o block-data testsuite/gdb.fortran/block-data.f
$ readelf -w block-data | grep DW_TAG_module
 <1><d7>: Abbrev Number: 7 (DW_TAG_module)
   7      DW_TAG_module    [has children]

Regards,
Tim


> -----Original Message-----

> From: Yao Qi [mailto:qiyaoltc@gmail.com]

> Sent: Tuesday, August 1, 2017 12:10 AM

> To: Wiederhake, Tim <tim.wiederhake@intel.com>

> Cc: gdb-patches@sourceware.org; Bernhard Heckel

> <bernhard.heckel@intel.com>

> Subject: Re: [PATCH 1/6] DWARF: Don't add nameless modules to partial

> symbol table.

> 

> On 17-07-21 11:23:55, Tim Wiederhake wrote:

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

> 

> What is your Fortran compiler?  I used gfortran 4.9.4, it doesn't generate

> DW_TAG_module in debug information.  So, I run block-data.exp with

> unpatched GDB, all tests pass.

> 

> >

> > BLOCK DATA is used for one-time initialization of non-pointer variables

> in

> > named common blocks.

> >

> > xxxx-yy-zz  Bernhard Heckel  <bernhard.heckel@intel.com>

> >

> > gdb/ChangeLog:

> > 	* dwarf2read.c (add_partial_symbol): Skip nameless modules.

> >

> > gdb/testsuite/ChangeLog:

> > 	* gdb.fortran/block-data.f: New file.

> > 	* gdb.fortran/block-data.exp: New file.

> >

> 

> 

> --

> Yao (齐尧)

Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
  
Yao Qi Aug. 2, 2017, 11:16 a.m. UTC | #3
"Wiederhake, Tim" <tim.wiederhake@intel.com> writes:

> I'm using the Intel? Fortran Compiler:
>
> $ ifort --version
> ifort (IFORT) 15.0.0 20140716
> Copyright (C) 1985-2014 Intel Corporation.  All rights reserved.
>
> $ ifort -g -o block-data testsuite/gdb.fortran/block-data.f
> $ readelf -w block-data | grep DW_TAG_module
>  <1><d7>: Abbrev Number: 7 (DW_TAG_module)
>    7      DW_TAG_module    [has children]

Hi Tim,
I checked both gfortran and armflang, neither generate DW_TAG_module for
nameless BLOCK DATA.

When I google "block data", I find I reviewed this patch before :)
https://sourceware.org/ml/gdb-patches/2016-11/msg00014.html I didn't see
the point Intel fortran compiler generates DW_TAG_module for a nameless
BLOCK DATA, but GDB shouldn't crash in any case.  Could you adjust the
commit log and comments to reflect that Intel fortran compiler
generates DW_TAG_module without DW_AT_name for a nameless BLOCK DATA,
and GDB will crash.  This patch is to fix the crash.
  
Wiederhake, Tim Aug. 4, 2017, 11:03 a.m. UTC | #4
Hi Yao,

> -----Original Message-----

> From: Yao Qi [mailto:qiyaoltc@gmail.com]

> Sent: Wednesday, August 2, 2017 1:17 PM

> To: Wiederhake, Tim <tim.wiederhake@intel.com>

> Cc: gdb-patches@sourceware.org

> Subject: Re: [PATCH 1/6] DWARF: Don't add nameless modules to partial

> symbol table.

> 

> "Wiederhake, Tim" <tim.wiederhake@intel.com> writes:

> 

> > I'm using the Intel? Fortran Compiler:

> >

> > $ ifort --version

> > ifort (IFORT) 15.0.0 20140716

> > Copyright (C) 1985-2014 Intel Corporation.  All rights reserved.

> >

> > $ ifort -g -o block-data testsuite/gdb.fortran/block-data.f

> > $ readelf -w block-data | grep DW_TAG_module

> >  <1><d7>: Abbrev Number: 7 (DW_TAG_module)

> >    7      DW_TAG_module    [has children]

> 

> Hi Tim,

> I checked both gfortran and armflang, neither generate DW_TAG_module for

> nameless BLOCK DATA.

> 

> When I google "block data", I find I reviewed this patch before :)

> https://sourceware.org/ml/gdb-patches/2016-11/msg00014.html I didn't see

> the point Intel fortran compiler generates DW_TAG_module for a nameless

> BLOCK DATA, but GDB shouldn't crash in any case.  Could you adjust the

> commit log and comments to reflect that Intel fortran compiler

> generates DW_TAG_module without DW_AT_name for a nameless BLOCK DATA,

> and GDB will crash.  This patch is to fix the crash.


Changed locally.

> 

> --

> Yao (齐尧)


Regards,
Tim
Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
  

Patch

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 2c2ecda..6d38d70 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -7245,11 +7245,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,
-			   &objfile->global_psymbols,
-			   0, cu->language, objfile);
+      /* In 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 != NULL)
+	add_psymbol_to_list (actual_name, strlen (actual_name),
+			     built_actual_name != NULL,
+			     MODULE_DOMAIN, LOC_TYPEDEF,
+			     &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 0000000..9e1f6bd
--- /dev/null
+++ b/gdb/testsuite/gdb.fortran/block-data.exp
@@ -0,0 +1,49 @@ 
+# Copyright 2017 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 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 0000000..3bc5eb6
--- /dev/null
+++ b/gdb/testsuite/gdb.fortran/block-data.f
@@ -0,0 +1,56 @@ 
+c Copyright 2017 Free Software Foundation, Inc.
+c
+c This program is free software; you can redistribute it and/or modify
+c it under the terms of the GNU General Public License as published by
+c the Free Software Foundation; either version 3 of the License, or
+c (at your option) any later version.
+c
+c This program is distributed in the hope that it will be useful,
+c but WITHOUT ANY WARRANTY; without even the implied warranty of
+c MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+c GNU General Public License for more details.
+c
+c You should have received a copy of the GNU General Public License
+c along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+c Test if GDB can handle block data without global name
+
+c 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
+
+c 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
+
+c 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