Page MenuHomePhabricator

[COFF]Correct .eh_frame name, let llvm tools such as llvm-dwarfdump can dump .eh_frame info
ClosedPublic

Authored by SquallATF on Dec 6 2018, 7:57 PM.

Diff Detail

Event Timeline

SquallATF created this revision.Dec 6 2018, 7:57 PM

For backstory/reference: In lld, we choose to store the name of the section .eh_frame truncated as .eh_fram instead of storing the full name in the string table, to allow libunwind to find the section introspectively at runtime. (The string table, where long section names are stored, isn't loaded at runtime.)

I'm a little undecided about this. For one, I think it might be better to keep COFFObjectFile truthful and actually return the real section name, and have code that looks for the .eh_frame section (in llvm-dwarfdump?) accept anything like .eh_fram* (at least for COFF files). But I imagine that llvm-dwardump is using the higher level abstracted object API that abstracts away the object file format specific details - and adding coff specific code there isn't very neat either.

Oh, and also, please add a test case. That should probably be easy to do with yaml2obj.

SquallATF updated this revision to Diff 177138.Dec 7 2018, 12:48 AM

a better way map .eh_frame name

Oh, and also, please add a test case. That should probably be easy to do with yaml2obj.

Writing this test case is too difficult for me. I have update patch by override mapDebugSectionName like MachOObjectFile done.

Oh, if there's such an existing function that can be implemented, that's excellent. Then it's just missing a testcase.

Writing this test case is too difficult for me.

I'd suggest something like this: Link an absolute minimal executable with the properties you want (a section named .eh_fram). Dump this to yaml with the obj2yaml tool. Then create a test e.g. like test/tools/llvm-dwarfdump/uuid.yaml, which consists of the dumped yaml and a RUN line at the start, which runs yaml2obj to convert it back to an executable, and runs llvm-dwarfdump on it, and a CHECK line which checks for some piece of information. In this case it's enough to just check for any information that dwarfdump shows when it found the .eh_fram but didn't show before (e.g. an example of what value you get from fixing this issue).

SquallATF updated this revision to Diff 177149.Dec 7 2018, 2:04 AM
  • add test ccase

Is the binary object file superfluous and identical to the yaml based test?

rnk added inline comments.Dec 7 2018, 12:40 PM
lib/Object/COFFObjectFile.cpp
1291

Thanks, this seems better.

SquallATF updated this revision to Diff 177370.EditedDec 7 2018, 7:18 PM
  • remove llvm-dwarfdump duplicate test
mstorsjo accepted this revision.Dec 7 2018, 10:58 PM

LGTM

The test input could also be kept in the same file as the test itself, as you had it previously, but it's probably fine either way. This at least allows reusing the input for another test.

Will commit for you a bit later.

This revision is now accepted and ready to land.Dec 7 2018, 10:58 PM
This revision was automatically updated to reflect the committed changes.