Bug 284124

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.

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.

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.

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.

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

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

Committed, r12491.  Thanks!