Page MenuHomePhabricator

[LLDB] - Support the single file split DWARF.
ClosedPublic

Authored by grimar on Sep 23 2018, 3:30 AM.

Details

Summary

DWARF5 spec describes single file split dwarf case (when .dwo sections are in the .o files).

Problem is that LLDB does not work correctly in that case. The issue is that, for example, both .debug_info and .debug_info.dwo
has the same type: eSectionTypeDWARFDebugInfo. And when code searches section by type it might find the regular debug section
and not the .dwo one. It seems can be easily fixed though, patch shows the fix I am suggesting.

Note that currently, clang is unable to produce such output, the patch is on review atm: D52296.
It is still would be possible to use assembler to produce such files though.
But the test case refers to the flag from the review mentioned.

Diff Detail

Event Timeline

grimar created this revision.Sep 23 2018, 3:30 AM

So the main questions is: do we need a new section enum called eSectionTypeDWARFDebugInfoDWO? If so, then this patch changes. I think I would prefer adding a new enum.

source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
68

If dwo has a unique section name, just name a new enum for the .debug_info.dwo.

labath added a subscriber: labath.Sep 24 2018, 5:02 AM

I believe you should be able to use lldb-test to write this kind of a test. I am thinking of a sequence like:

yaml2obj %p/Inputs/test.yaml > %t/test.yaml
yaml2obj %p/Inputs/test.o.yaml > %t/test.o.yaml
lldb-test breakpoint %t/test.yaml %s | FileCheck %s
b main
CHECK-LABEL: b main
CHECK: Address: {{.*}}main + 13 at test.cpp:2:3

b foo
CHECK-LABEL: b foo
CHECK: Address: {{.*}}foo() + 4 at test.cpp:6:1

You can look at existing tests in lit/Breakpoints for details.

Thanks for all the comments!

So the main questions is: do we need a new section enum called eSectionTypeDWARFDebugInfoDWO? If so, then this patch changes. I think I would prefer adding a new enum.

Yeah, that is what I thought at the first place too. I found that when code looks for some section, it seems sometimes assumes it can find .dwo section by regular section enum value.
I thought that time that much more changes might be required (for example, introducing enum values for each .dwo section and perhaps fixing callers)
and so selected the simplest solution (this patch), because original code already used filtering by section name "dwo" suffix.

I'll try to investigate it again, want to check a few ideas I have in mind.

grimar updated this revision to Diff 167287.Sep 27 2018, 5:16 AM
  • Addressed review comments.
clayborg requested changes to this revision.Sep 27 2018, 9:24 AM

Just a few fixes. Looking good.

include/lldb/lldb-enumerations.h
643–660

Add all of these to the end of this enum for API stability since this is a public header used in the API. If an older binary runs against a newer liblldb.so, all of these enums will be off.

source/Symbol/ObjectFile.cpp
347

Check for other ObjectFile subclasses that override this function. I believe ObjectFileMachO does.

This revision now requires changes to proceed.Sep 27 2018, 9:24 AM
grimar updated this revision to Diff 167440.Sep 28 2018, 2:10 AM
grimar marked an inline comment as done.
  • Addressed review comments.
include/lldb/lldb-enumerations.h
643–660

Done.

source/Symbol/ObjectFile.cpp
347

Yes, and my patch already has the change for ObjectFileMachO :)
All other classes are fine too I think.

clayborg accepted this revision.Sep 28 2018, 10:05 AM

Looks good!

This revision is now accepted and ready to land.Sep 28 2018, 10:05 AM
grimar added a comment.EditedSep 28 2018, 10:20 AM

Thanks for the review! It still depends on -gsingle-file-split-dwarf (D52403), I'll wait for it some time.
It should be possible to rewrite the comment in the test case to avoid mentioning the flag, but I would prefer to use it.

grimar closed this revision.Nov 14 2018, 2:40 AM

Committed as rLLDB346848.
The commit message has a wrong review link by mistake.