Bug 284124 - parse_type_DIE: confused by: DWARF 4
Summary: parse_type_DIE: confused by: DWARF 4
Status: RESOLVED FIXED
Alias: None
Product: valgrind
Classification: Developer tools
Component: general (show other bugs)
Version: 3.6.0
Platform: Debian testing Linux
: NOR normal
Target Milestone: ---
Assignee: Julian Seward
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-10-15 19:25 UTC by Samuel Bronson
Modified: 2012-06-30 10:19 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
initial patch implementing .debug_types reading (34.77 KB, patch)
2012-04-02 21:15 UTC, Tom Tromey
Details
updated patch (41.92 KB, patch)
2012-04-04 16:26 UTC, Tom Tromey
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Samuel Bronson 2011-10-15 19:25:57 UTC
I get stuff like the following when using --read-var-info=yes on C++ programs compiled with -gdwarf-4 (using "gcc (Debian 4.6.1-4) 4.6.1"):

==25851== Memcheck, a memory error detector
==25851== Copyright (C) 2002-2010, and GNU GPL'd, by Julian Seward et al.
==25851== Using Valgrind-3.6.1 and LibVEX; rerun with -h for copyright info
==25851== Command: ./crawl -test
==25851== Parent PID: 24869
==25851==

parse_type_DIE: confused by:
 <1><2747>: DW_TAG_array_type
     DW_AT_type        : 8 byte signature: f2 15 ea 72 79 f2 f1 f9
     DW_AT_sibling     : <275d>

--25851-- WARNING: Serious error when reading debug info
--25851-- When reading debug info from /home/naesten/hacking/crawl/crawl-ref/source/crawl:
--25851-- parse_type_DIE: confused by the above DIE

I'm not really too sure what this means; I tried finding the relevant DIEs in the output of "readelf" and "dwarfdump", but the latter appears to use decimal and I think the former must be counting differently.

Anyway, I'm not really sure what other data I could give you that wouldn't be excessively large, but I expect there will be problems like this with just about any non-trivial C++ program, and maybe with C programs too.

If you really would like exact steps to reproduce or binaries or something, though, I would be happy to oblige.
Comment 1 Samuel Bronson 2011-10-29 23:38:30 UTC
It looks like the problem here is that valgrind doesn't know about the third type of "reference", which was added in DWARF 4 (Section 7.5.4):

> The third type of reference can identify any debugging information type entry that
> has been placed in its own type unit. This type of reference (DW_FORM_ref_sig8) is
> the 64-bit type signature (see Section 7.27) that was computed for the type.

This type of reference is used by GCC 4.6 (with -gdwarf-4) to avoid massive duplication of debug information between compilation units. If valgrind supported it, this would probably improve symbol load times a great deal for programs with large types shared between many compilation units.
Comment 2 Tom Tromey 2012-04-02 21:15:50 UTC
Created attachment 70092 [details]
initial patch implementing .debug_types reading

Here is an initial patch that adds .debug_types reading.

Much of the patch is straightforward, so I will just note the odd and/or hacky bits.

Possibly the most hacky thing is the notion of a "cooked DIE offset".  The idea here is
to pretend that .debug_info and .debug_types form a contiguous whole; DIEs occurring
in .debug_types are given an additional offset of the size of .debug_info.  This lets all
the existing DIE-lookup code in readdwarf3.c work without further modification.

The DWARF reader now does an initial pass over the .debug_types section to build
a hash table that maps type signatures to DIE offsets.  This approach is simple and also
lets us handle signature->offset lookup in get_Form_contents.

new_dwarf2_reader_wrk now has two subsequent DIE-reading passes: one for .debug_info
and a second for .debug_types.  Unfortunately this part of the patch is hard to read due to
the need for reindentation.

I've added a test case and verified that it fails without the patch.
The test case relies on having a new-enough gcc.

I've tried to follow the prevailing coding style; but feel free to point out errors and I
will try to fix them.
Comment 3 Tom Tromey 2012-04-02 22:18:39 UTC
I forgot to mention a couple things.

I looked at factoring out the 2-pass loop from new_dwarf3_reader_wrk into
a separate function.  I decided against it since it would require a lot of arguments.
However, this is easily revisited if you want.

Also, lookup_signatured_type maybe knows too much about the hash table
implementation.  One way to clean this up would be to add a way to make a hash
iterator that is restricted to just items matching a given key.
Comment 4 Julian Seward 2012-04-03 16:35:00 UTC
Seems plausible.

none/tests/dw4.c: seems a pretty weak test.  Can you redo it in
the style of memcheck/tests/varinfo[1-8].c so as to force V to
print out text that depends on the read type/location info.

you preserved my nasty icc9 hackarounds, yes?

Please add comments to the source as per comment 2, 3 in the bug.
Comment 5 Tom Tromey 2012-04-03 17:02:08 UTC
(In reply to comment #4)
> Seems plausible.
> 
> none/tests/dw4.c: seems a pretty weak test.  Can you redo it in
> the style of memcheck/tests/varinfo[1-8].c so as to force V to
> print out text that depends on the read type/location info.

Sure, no problem.

> you preserved my nasty icc9 hackarounds, yes?

Yes, I believe so.  I just realized now that the code to delete filenameTable (at the
end of new_dwarf3_reader_wrk) is now dead, so I will remove that in the next revision.

> Please add comments to the source as per comment 2, 3 in the bug.

No problem.

Thanks for looking at this.  I'll upload a new patch once I've made the changes you
want and also (as discussed on irc) smoke-tested it on a large project.
Comment 6 Tom Tromey 2012-04-04 16:26:30 UTC
Created attachment 70152 [details]
updated patch

Here is an updated patch.  Relative to the previous patch:

* I added more comments, based on comments I made in the bug.
* I found that I missed fixing the bad_DIE cases in parse_var_DIE
  and parse_type_DIE.  The new patch updates these to uncook
  the DIE offset and note if the DIE came from .debug_types.
* Testing on a gdb built with .debug_types showed a strange DW_TAG_enumeration_type
  emitted by GCC into .debug_types.  This patch adds a workaround.
  In this particular case it seems to me that the resulting type isn't essential,
  so erroring out because of it seemed wrong.
* I removed the now-dead code to destroy varparser.filenameTable at the end
  of new_dwarf3_reader_wrk.  This is now always destroyed in the CU-parsing loop.
* I rewrote the dw4 test per your instructions and moved it to memcheck.
  I verified that it fails before my patch and passes after.
* I added configury support to check to -gdwarf-4 -fdebug-types-section and disable
  the dw4 test if not available
* I ran the test suite (make regtest) before and after the patch and verified that
  I had the same list of failures in both cases.
* I built LibreOffice with -gdwarf-4 -fdebug-types-section and ran a smoke test on
  swriter (valgrind --trace-children=yes --tool=none --read-var-info=yes ./swriter)
  with both the system valgrind (verifying that it failed) and the patched valgrind
  (which worked).
Comment 7 Julian Seward 2012-04-05 07:56:34 UTC
Committed, r12491.  Thanks!