Page MenuHomePhabricator

[ObjectYAML][ELF] Add support for emitting the .debug_gnu_pubnames/pubtypes sections.
ClosedPublic

Authored by Higuoxing on Jun 23 2020, 4:36 AM.

Details

Summary

This patch helps add support for emitting the .debug_gnu_pubnames and .debug_gnu_pubtypes sections.

The .debug_gnu_pub* sections is verified by llvm-dwarfdump.

Known issues:

  • Doesn't support emitting multiple pub-tables.

Diff Detail

Event Timeline

Higuoxing created this revision.Jun 23 2020, 4:36 AM
Herald added a project: Restricted Project. · View Herald Transcript

My understanding of the .debug_gun_pubnames table is that it is a slightly different format to the .debug_pubnames table. The output of yaml2obj here is not consistent with the parsing code for GNU style, as far as I can tell, because of this and the bug you mentioned. I don't think we should add support for the section unless it is actually formatted correctly, as it is likely to cause confusion, should anybody attempt to use it in its seemingly-working-but-broken state.

Higuoxing planned changes to this revision.Jun 23 2020, 6:16 AM

My understanding of the .debug_gun_pubnames table is that it is a slightly different format to the .debug_pubnames table. The output of yaml2obj here is not consistent with the parsing code for GNU style, as far as I can tell, because of this and the bug you mentioned. I don't think we should add support for the section unless it is actually formatted correctly, as it is likely to cause confusion, should anybody attempt to use it in its seemingly-working-but-broken state.

Got it! I will try to fix it later.

Higuoxing edited the summary of this revision. (Show Details)Jun 30 2020, 1:15 AM
jhenderson accepted this revision.Jun 30 2020, 1:55 AM

LGTM, with one suggestion.

llvm/test/tools/yaml2obj/ELF/DWARF/debug-gnu-pubtypes.yaml
110

Probably missed this elsewhere, but can't this be SIZE-NEXT?

This revision is now accepted and ready to land.Jun 30 2020, 1:55 AM
This revision was automatically updated to reflect the committed changes.
dblaikie added inline comments.
llvm/test/tools/yaml2obj/ELF/DWARF/debug-gnu-pubnames.yaml
8–9

Should this be tested via llvm-dwarfdump instead? (perhaps there's already lots of precedent/reasons that yaml2obj is being tested via readobj?)

Higuoxing marked an inline comment as done.Mon, Jul 6, 6:58 PM
Higuoxing added inline comments.
llvm/test/tools/yaml2obj/ELF/DWARF/debug-gnu-pubnames.yaml
8–9

Because some tests in llvm-dwarfdump are using yaml2obj to generate DWARF sections, e.g., llvm-dwarfdump/X86/verify_overlapping_cu_ranges.yaml, llvm-dwarfdump/X86/Inputs/i386_macho_with_debug.yaml, etc. We don't want to create a circular dependency. Does it make sense?

dblaikie added inline comments.Mon, Jul 6, 8:45 PM
llvm/test/tools/yaml2obj/ELF/DWARF/debug-gnu-pubnames.yaml
8–9

Hmm, fair enough. Not sure what the right call is there - I would've thought assembly would be easier to read than hex object dumps? Case in point with these hex dumps and multiline ASCII art comments, compared to assembly with comments & appropriate-width values, symbolic expressions, etc.

(so using assembly tests for llvm-dwarfdump and then llvm-dwarfdump for tests of obj2yaml, rather than obj2yaml tests of llvm-dwarfdump and objdump tests of obj2yaml)

jhenderson added inline comments.Tue, Jul 7, 2:38 AM
llvm/test/tools/yaml2obj/ELF/DWARF/debug-gnu-pubnames.yaml
8–9

(just in case you missed it, this is a yaml2obj test). The intent longer term with @Higuoxing's project is to get yaml2obj DWARF support to a good enough state that it makes it much easier to craft tests for llvm-dwarfdump etc without needing to specify all the fine details that assembly currently requires (just consider how much assembly some of the exisiting llvm-dwarfdump tests require for example). Assembly would probably still work well for creating broken inputs, but yaml2obj would be better for the higher-level testing.

The problem of course with using yaml2obj to test llvm-dwarfdump is that we can't use the reverse. Somewhere, we have to test either hex output or use assembly (or YAML + raw content hex) input. Whilst I agree assembly input would be easier to read than this hex output, it rather defeats the point of the project, and it doesn't scale well (in theory, the testing here can be kept fairly small, so the costs of having hex aren't too great).

Once we have basic testing in place for all the DWARF sections, it should be possible to use llvm-dwarfdump to verify the higher level auto-generation of things by yaml2obj that is intended for later in the project.

Higuoxing marked an inline comment as done.Tue, Jul 7, 2:48 AM
Higuoxing added inline comments.
llvm/test/tools/yaml2obj/ELF/DWARF/debug-gnu-pubnames.yaml
8–9

Oops, I missed @dblaikie 's previous comments. Thank you @jhenderson for clarifying this for me!

dblaikie added inline comments.Tue, Jul 7, 5:29 PM
llvm/test/tools/yaml2obj/ELF/DWARF/debug-gnu-pubnames.yaml
8–9

Whilst I agree assembly input would be easier to read than this hex output, it rather defeats the point of the project, and it doesn't scale well (in theory, the testing here can be kept fairly small, so the costs of having hex aren't too great).

Not sure - why is it likely that the yaml2obj+hexdump tests scale better than the assembly+llvm-dwarfdump tests directly? Seems like we'd have to test maybe as many weird cases of DWARF emission to get a nice legible format for writing dwarfdump tests as we would for the dwarfdump tests themselves? It's starting to feel a bit "turtles all the way down" to me.

Something like yaml2obj could be handy for testing lldb, for instance - constructing arbitrarily interesting inputs. But for the yaml2obj<>llvm-dwarfdump circularity, I'm not so sure.

jhenderson added inline comments.Wed, Jul 8, 12:08 AM
llvm/test/tools/yaml2obj/ELF/DWARF/debug-gnu-pubnames.yaml
8–9

By "scale" I meant the auto-generation aspects probably don't need to be tested using hex dumps, so can be tested using llvm-dwarfdump, but honestly I'm not sure either way too.

dblaikie added inline comments.Wed, Jul 8, 10:53 AM
llvm/test/tools/yaml2obj/ELF/DWARF/debug-gnu-pubnames.yaml
8–9

By "scale" I meant the auto-generation aspects probably don't need to be tested using hex dumps, so can be tested using llvm-dwarfdump, but honestly I'm not sure either way too.

What do you mean by "auto-generation aspects"?

But, yeah, I'm not holding this patch up over this direction that's already got precedent, etc - but raising the question at least for consideration/thinking about over time.

jhenderson added inline comments.Thu, Jul 9, 12:46 AM
llvm/test/tools/yaml2obj/ELF/DWARF/debug-gnu-pubnames.yaml
8–9

At the moment, to use yaml2obj to generate DWARF, you have to specify pretty much every detail of the DWARF, including the details of the abbrev table and the string table for example. Ideally, we should be able to describe the DWARF in a higher level manner (e.g. by just specifying the attributes and values in the .debug_info description, letting yaml2obj do all the leg work of selecting a form, populating the abbrev and string tables etc). You'll see details of this in @Higuoxing's mailing list posts about his GSOC project.

We can use the basic-level testing for "bootstrapping". yaml2obj can generate valid raw sections, tested via hex -> allows testing of llvm-dwarfdump section dumping -> allows testing of yaml2obj higher-level functionality (because we know that llvm-dwarfdump section dumping now works).

dblaikie added inline comments.Sat, Jul 11, 11:18 AM
llvm/test/tools/yaml2obj/ELF/DWARF/debug-gnu-pubnames.yaml
8–9

That seems like it's going to be fairly subtle/hard to maintain the separation here - if some yaml2obj tests use hex dumping but others can use llvm-dwarfdump - if/when/that's happening, might be worth separate directories for the two kinds of tests and some fairly specific documentation about how to determine which tests go where.

Higuoxing marked an inline comment as done.Sun, Jul 12, 1:56 AM
Higuoxing added inline comments.
llvm/test/tools/yaml2obj/ELF/DWARF/debug-gnu-pubnames.yaml
8–9

What do you think of making elf2yaml support dumping DWARF sections? In the future, we can use raw assembly to test elf2yaml and use elf2yaml to test yaml2elf.

dblaikie added inline comments.Sun, Jul 12, 9:35 AM
llvm/test/tools/yaml2obj/ELF/DWARF/debug-gnu-pubnames.yaml
8–9

Probably useful that elf2yaml and yaml2elf roundtrip/support the same features (would make it easier to create yaml files to work with/pare down, etc).

But as for testing - not sure - seems like it adds another layer of indirection (then we'd use raw assembly+llvm-mc to test elf2yaml, to test yaml2elf, to test llvm-dwarfdump - when we could've been using raw assembly to test llvm-dwarfdump) & not sure how much it improves/streamlines the testing matrix.

All that said, we did used to test llvm-dwarfdump with checked in object files - then we accepted that assembly + llvm-mc didn't especially reduce the test quality despite increasing the surface area of the test by using llvm-mc. Though I think the more DWARF-specific the functionality gets the less that sort of line of reasoning applies (ie: Once we're generating all of DWARF - we're reaching the same complexity as the parsing logic and have now written a whole other DWARF representation with all the risk of bugs, etc).

But really - I don't have any particular action/takeaway from these thoughts right now, but I think they're worth keeping in mind/thinking about as this work continues.