Page MenuHomePhabricator

[dsymutil] Emit valid debug locations when no symbol flags are set
ClosedPublic

Authored by fjricci on Oct 5 2017, 7:54 AM.

Details

Summary

swiftc emits symbols without flags set, which led dsymutil to ignore
them when searching for global symbols, causing dwarf location data
to be omitted. Xcode's dsymutil handles this case correctly, and emits
valid location data. Add this functionality to llvm-dsymutil by
allowing parsing of symbols with no flags set.

Diff Detail

Repository
rL LLVM

Event Timeline

fjricci created this revision.Oct 5 2017, 7:54 AM
fjricci planned changes to this revision.Oct 5 2017, 8:34 AM

I'm going to try to clean up the IR a bit

fjricci updated this revision to Diff 117830.Oct 5 2017, 8:48 AM

Prune some unneeded IR and debug info

aprantl added inline comments.Oct 5 2017, 8:52 AM
tools/dsymutil/MachODebugMapParser.cpp
485 ↗(On Diff #117830)
fjricci added inline comments.Oct 5 2017, 9:31 AM
tools/dsymutil/MachODebugMapParser.cpp
485 ↗(On Diff #117830)

Ah, I didn't realize that the apple fork was open source as well. Are there plans to merge it back into llvm? (probably applies to https://reviews.llvm.org/D38504 as well)

fjricci added inline comments.Oct 5 2017, 9:33 AM
tools/dsymutil/MachODebugMapParser.cpp
485 ↗(On Diff #117830)

If there are plans to eventually merge the two, these two diffs are just going to cause the forks to diverge and make merging harder (unless I merge them in as copies from what's in the apple version)

aprantl edited edge metadata.Oct 5 2017, 10:06 AM

Yes, we are in the process of contributing all changes where this makes sense for back into the llvm.org repository. I wouldn't worry too much about introducing merge conflicts (we can handle that), but we shouldn't change the behavior of the tool (unless you have found a bug in the shipping dsymutil, of course).

JDevlieghere accepted this revision.Oct 5 2017, 10:09 AM

Looks good with Adrian's comments addressed.

This revision is now accepted and ready to land.Oct 5 2017, 10:09 AM
fjricci updated this revision to Diff 117861.Oct 5 2017, 12:05 PM

Maintain same behavior as apple's dsymutil

fjricci updated this revision to Diff 118003.Oct 6 2017, 8:05 AM

Move to x86 test dir

This revision was automatically updated to reflect the committed changes.
fjricci reopened this revision.Oct 6 2017, 11:30 AM
This revision is now accepted and ready to land.Oct 6 2017, 11:30 AM
fjricci updated this revision to Diff 118050.Oct 6 2017, 11:31 AM

Use precompiled binary with IR as a comment to allow testing on linux

I'm running into some issues with using a precompiled binary + object file here. For some reason, the debug info contains an absolute path to the object file instead of a relative path, so dsymutil will fail because it's looking for a path that doesn't exist (ie a path on my local machine). This is true even if I use -fdebug-compilation-dir (it updates the debug info in the IR, but doesn't change where dsymutil looks for the object file). I also tried -add_ast_path, which works as a hacky workaround because it will search the relative path in addition to the absolute path, but that seems like a bad solution.

Interestingly, I tried to reproduce the test object files created in one of the other tests (X86/alias.test), and when I regenerate the object files locally, they switch to absolute paths as well and the test fails.

One way around this would be to go back to compiling/linking the IR file as part of the test, but this requires adding REQUIRES: darwin, for linking to work.

Any thoughts?

Let me know if I misunderstood your question. When creating binary testcases for dsymutil, I usually create them directly in / so the absolute paths don't contain any local directories.

Oh interesting. I tried using PWD=/, but I didn't try actually cd-ing into that directory. I'll try that - thanks!

This revision was automatically updated to reflect the committed changes.