[v2,4/8] DWARF-5 basic functionality

Message ID m3wpcjl5df.fsf@oc1027705133.ibm.com
State New, archived
Headers

Commit Message

Andreas Arnez Feb. 21, 2017, 7:18 p.m. UTC
  On Sun, Feb 19 2017, Jan Kratochvil wrote:

[...]

> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> index a987e0e..063f463 100644
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c

[...]

> @@ -4699,8 +4775,8 @@ create_debug_type_hash_table (struct dwo_file *dwo_file,
>  	  dwo_tu = NULL;
>  	  sig_type = OBSTACK_ZALLOC (&objfile->objfile_obstack,
>  				     struct signatured_type);
> -	  sig_type->signature = signature;
> -	  sig_type->type_offset_in_tu = type_offset_in_tu;
> +	  sig_type->signature = header.signature;
> +	  sig_type->type_offset_in_tu = header.type_offset_in_tu;
>  	  sig_type->per_cu.objfile = objfile;
>  	  sig_type->per_cu.is_debug_types = 1;
>  	  sig_type->per_cu.section = section;

When I compile with "-O3", GCC now yields warnings for the two changed
lines above:

  [...] warning: ‘header.comp_unit_head::type_offset_in_tu.cu_offset::cu_off’
  may be used uninitialized in this function [-Wmaybe-uninitialized]
      sig_type->type_offset_in_tu = header.type_offset_in_tu;
      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~

  [...] warning: ‘header.comp_unit_head::signature’ may be used
  uninitialized in this function [-Wmaybe-uninitialized]
      sig_type->signature = header.signature;
      ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~

I *think* GCC is wrong.  From what I understand, we can only get here if
header.unit_type == DW_UT_type, and then these fields should have been
initialized in read_comp_unit_head before (right?).  But then I wonder
about the effect of the call to create_debug_type_hash_table in
create_all_type_units:

  create_debug_type_hash_table (NULL, &dwarf2_per_objfile->info, types_htab,
				rcuh_kind::COMPILE);

Is that needed?  If not, can we drop section_kind as a parameter to
create_debug_type_hash_table?  When doing so (see untested patch below),
the warnings go away.

--
Andreas

-- >8 --
  

Comments

Jan Kratochvil Feb. 22, 2017, 2:28 p.m. UTC | #1
On Tue, 21 Feb 2017 20:18:36 +0100, Andreas Arnez wrote:
> When I compile with "-O3", GCC now yields warnings for the two changed
> lines above:
> 
>   [...] warning: ‘header.comp_unit_head::type_offset_in_tu.cu_offset::cu_off’
>   may be used uninitialized in this function [-Wmaybe-uninitialized]

What compiler/version/options do you use?  I have tried now
	gcc-4.8.5-11.el7.x86_64
	CFLAGS=-O3 CXXFLAGS=-O3 ./configure;make

While I get some warnings/errors none of them is in dwarf2read.c.

I do not want to patch it without reproducibility of the warnings.


> I *think* GCC is wrong.  From what I understand, we can only get here if
> header.unit_type == DW_UT_type, and then these fields should have been
> initialized in read_comp_unit_head before (right?).

Yes, I hope so.


> But then I wonder
> about the effect of the call to create_debug_type_hash_table in
> create_all_type_units:
> 
>   create_debug_type_hash_table (NULL, &dwarf2_per_objfile->info, types_htab,
> 				rcuh_kind::COMPILE);
> 
> Is that needed?

Yes.  Otherwise GDB would not read definitions of classes during
	-gdwarf-5 -fdebug-types-section
as then DW_UT_type are located inside .debug_info.


> When doing so (see untested patch below),

No, that breaks: -gdwarf-5 -fdebug-types-section


Jan
  
Andreas Arnez Feb. 22, 2017, 5:38 p.m. UTC | #2
On Wed, Feb 22 2017, Jan Kratochvil wrote:

> What compiler/version/options do you use?  I have tried now
> 	gcc-4.8.5-11.el7.x86_64
> 	CFLAGS=-O3 CXXFLAGS=-O3 ./configure;make

I saw the warning with an upstream GCC I recently built myself for
s390x.  Today I retried with a fresh version from GCC git, with the same
result: gcc (GCC) 7.0.1 20170222 (experimental)

> While I get some warnings/errors none of them is in dwarf2read.c.
>
> I do not want to patch it without reproducibility of the warnings.

Sure.  Unfortunately the maybe-uninitialized warnings highly depend on
compiler version, platform, optimization level, etc.  Since this is a
false positive, I don't necessarily propose a change in this case.

[...]

>> But then I wonder
>> about the effect of the call to create_debug_type_hash_table in
>> create_all_type_units:
>> 
>>   create_debug_type_hash_table (NULL, &dwarf2_per_objfile->info, types_htab,
>> 				rcuh_kind::COMPILE);
>> 
>> Is that needed?
>
> Yes.  Otherwise GDB would not read definitions of classes during
> 	-gdwarf-5 -fdebug-types-section
> as then DW_UT_type are located inside .debug_info.

Ah, I see; thanks for explanation.  Seems I was misled by the function
description that mentions "all entries in the .debug_types section", but
fails to mention the .debug_info section.

I also stumbled upon the fact that read_comp_unit_head performs the
conditional initialization under "if (section_kind == rcuh_kind::TYPE)"
instead of checking for the appropriate unit type.  This works because
section_kind is adjusted accordingly above, but it might be clearer
otherwise.

--
Andreas
  
Jan Kratochvil Feb. 22, 2017, 6:32 p.m. UTC | #3
On Wed, 22 Feb 2017 18:38:11 +0100, Andreas Arnez wrote:
> On Wed, Feb 22 2017, Jan Kratochvil wrote:
> 
> > What compiler/version/options do you use?  I have tried now
> > 	gcc-4.8.5-11.el7.x86_64
> > 	CFLAGS=-O3 CXXFLAGS=-O3 ./configure;make
> 
> I saw the warning with an upstream GCC I recently built myself for
> s390x.  Today I retried with a fresh version from GCC git, with the same
> result: gcc (GCC) 7.0.1 20170222 (experimental)

g++ (GCC) 7.0.1 20170218 (experimental)
~/redhat/gcchead-root/bin/g++  -m64 -g3 -pipe -Wall -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -fno-diagnostics-show-caret    -I. -I. -I./common -I./config -DLOCALEDIR="\"/usr/local/share/locale\"" -DHAVE_CONFIG_H -I./../include/opcode -I./../opcodes/.. -I./../readline/.. -I./../zlib -I../bfd -I./../bfd -I./../include -I../libdecnumber -I./../libdecnumber  -I./gnulib/import -Ibuild-gnulib/import   -DTUI=1  -I/usr/include/python3.5m -I/usr/include/python3.5m -Wall -Wpointer-arith -Wno-unused -Wunused-value -Wunused-function -Wno-switch -Wno-char-subscripts -Wempty-body -Wunused-but-set-parameter -Wunused-but-set-variable -Wno-sign-compare -Wno-write-strings -Wno-narrowing -Wformat-nonliteral -Werror -c -o dwarf2read.o -MT dwarf2read.o -MMD -MP -MF .deps/dwarf2read.Tpo dwarf2read.c -O3

But on x86_64.  I can try building trunk GCC on s390x but do you really have
the warnings unreproducible on x86_64?


> Sure.  Unfortunately the maybe-uninitialized warnings highly depend on
> compiler version, platform, optimization level, etc.  Since this is a
> false positive, I don't necessarily propose a change in this case.

Also there are already many warnings just with RHEL-7.3 x86_64 GCC for
example.


Jan
  
Andreas Arnez Feb. 23, 2017, 4:59 p.m. UTC | #4
On Wed, Feb 22 2017, Jan Kratochvil wrote:

> On Wed, 22 Feb 2017 18:38:11 +0100, Andreas Arnez wrote:
>> On Wed, Feb 22 2017, Jan Kratochvil wrote:
>> 
>> > What compiler/version/options do you use?  I have tried now
>> > 	gcc-4.8.5-11.el7.x86_64
>> > 	CFLAGS=-O3 CXXFLAGS=-O3 ./configure;make
>> 
>> I saw the warning with an upstream GCC I recently built myself for
>> s390x.  Today I retried with a fresh version from GCC git, with the same
>> result: gcc (GCC) 7.0.1 20170222 (experimental)
>
> g++ (GCC) 7.0.1 20170218 (experimental)
> ~/redhat/gcchead-root/bin/g++ -m64 -g3 -pipe -Wall -fexceptions
> -fstack-protector-strong --param=ssp-buffer-size=4
> -fno-diagnostics-show-caret -I. -I. -I./common -I./config
> -DLOCALEDIR="\"/usr/local/share/locale\"" -DHAVE_CONFIG_H
> -I./../include/opcode -I./../opcodes/.. -I./../readline/.. -I./../zlib
> -I../bfd -I./../bfd -I./../include -I../libdecnumber -I./../libdecnumber
> -I./gnulib/import -Ibuild-gnulib/import -DTUI=1 -I/usr/include/python3.5m
> -I/usr/include/python3.5m -Wall -Wpointer-arith -Wno-unused -Wunused-value
> -Wunused-function -Wno-switch -Wno-char-subscripts -Wempty-body
> -Wunused-but-set-parameter -Wunused-but-set-variable -Wno-sign-compare
> -Wno-write-strings -Wno-narrowing -Wformat-nonliteral -Werror -c -o
> dwarf2read.o -MT dwarf2read.o -MMD -MP -MF .deps/dwarf2read.Tpo
> dwarf2read.c -O3
>
> But on x86_64.  I can try building trunk GCC on s390x but do you
> really have the warnings unreproducible on x86_64?

I don't know why you don't see the warnings.  I had not tested with
upstream GCC on x86_64 before.  Now I have, and the same warnings appear
there.  Both on s390x and x86_64 it basically looks the same:

g++ -O3 -g -I. -I. -I./common -I./config
-DLOCALEDIR="\"/home/arnez/install/share/locale\"" -DHAVE_CONFIG_H
-I./../include/opcode -I./../opcodes/.. -I./../readline/.. -I./../zlib
-I../bfd -I./../bfd -I./../include -I../libdecnumber -I./../libdecnumber
-I./gnulib/import -Ibuild-gnulib/import -DTUI=1 -I/usr/include/python2.7
-I/usr/include/python2.7 -Wall -Wpointer-arith -Wno-unused
-Wunused-value -Wunused-function -Wno-switch -Wno-char-subscripts
-Wempty-body -Wunused-but-set-parameter -Wunused-but-set-variable
-Wno-sign-compare -Wno-write-strings -Wno-narrowing -Wformat-nonliteral
-c -o dwarf2read.o -MT dwarf2read.o -MMD -MP -MF .deps/dwarf2read.Tpo
dwarf2read.c

dwarf2read.c: In function ‘void create_debug_type_hash_table(dwo_file*,
dwarf2_section_info*, htab*&, rcuh_kind)’: dwarf2read.c:4766:30:
warning: ‘header.comp_unit_head::type_offset_in_tu.cu_offset::cu_off’
may be used uninitialized in this function [-Wmaybe-uninitialized]
    dwo_tu->type_offset_in_tu = header.type_offset_in_tu;
    ~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~
dwarf2read.c:4819:21: warning: ‘header.comp_unit_head::signature’ may be
used uninitialized in this function [-Wmaybe-uninitialized]
  fprintf_unfiltered (gdb_stdlog, "  offset 0x%x, signature %s\n",
  ~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
        offset.sect_off,
        ~~~~~~~~~~~~~~~~
        hex_string (header.signature));
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

--
Andreas
  

Patch

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 8a6e1f3..94f5bac 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -4687,8 +4687,7 @@  add_signatured_type_cu_to_table (void **slot, void *datum)
 
 static void
 create_debug_type_hash_table (struct dwo_file *dwo_file,
-			      dwarf2_section_info *section, htab_t &types_htab,
-			      rcuh_kind section_kind)
+			      dwarf2_section_info *section, htab_t &types_htab)
 {
   struct objfile *objfile = dwarf2_per_objfile->objfile;
   struct dwarf2_section_info *abbrev_section;
@@ -4735,7 +4734,8 @@  create_debug_type_hash_table (struct dwo_file *dwo_file,
 	 table, but we don't need anything else just yet.  */
 
       ptr = read_and_check_comp_unit_head (&header, section,
-					   abbrev_section, ptr, section_kind);
+					   abbrev_section, ptr,
+					   rcuh_kind::TYPE);
 
       length = get_cu_length (&header);
 
@@ -4847,8 +4847,7 @@  create_debug_types_hash_table (struct dwo_file *dwo_file,
   for (ix = 0;
        VEC_iterate (dwarf2_section_info_def, types, ix, section);
        ++ix)
-    create_debug_type_hash_table (dwo_file, section, types_htab,
-				  rcuh_kind::TYPE);
+    create_debug_type_hash_table (dwo_file, section, types_htab);
 }
 
 /* Create the hash table of all entries in the .debug_types section,
@@ -4862,8 +4861,6 @@  create_all_type_units (struct objfile *objfile)
   htab_t types_htab = NULL;
   struct signatured_type **iter;
 
-  create_debug_type_hash_table (NULL, &dwarf2_per_objfile->info, types_htab,
-				rcuh_kind::COMPILE);
   create_debug_types_hash_table (NULL, dwarf2_per_objfile->types, types_htab);
   if (types_htab == NULL)
     {