This is an archive of the discontinued LLVM Phabricator instance.

[llvm-objdump] Teach -d about AArch64 mapping symbols
ClosedPublic

Authored by davide on Oct 1 2015, 12:22 PM.

Details

Summary

AArch64 uses $d* and $x* to interleave between text and data.
llvm-objdump didn't know about this so it ended up printing garbage. This is the initial support to to teach llvm-objdump how to behave correctly in this situation.
I think llvm-objdump needs a lot of love. For now, this is a missing functionality that blocks my lld AArch64 testing, so I implemented it in the most straightforward way. Comments are welcome, and appreciated.

Diff Detail

Repository
rL LLVM

Event Timeline

davide updated this revision to Diff 36280.Oct 1 2015, 12:22 PM
davide retitled this revision from to [llvm-objdump] Teach -d about AArch64 mapping symbols.
davide updated this object.
davide added reviewers: rafael, Bigcheese.
davide added a subscriber: llvm-commits.
emaste added a subscriber: emaste.Oct 1 2015, 12:31 PM
Bigcheese requested changes to this revision.Oct 1 2015, 1:56 PM
Bigcheese edited edge metadata.

I'd rather we not loop over all the mapping symbols each time. Or at least do a binary search. Other than that it looks fine.

This revision now requires changes to proceed.Oct 1 2015, 1:56 PM
davide updated this revision to Diff 36642.Oct 6 2015, 11:55 AM
davide edited edge metadata.
davide removed rL LLVM as the repository for this revision.

Reworked as per Rafael request.

rafael edited edge metadata.Oct 6 2015, 12:08 PM

Much better, but I think it can still be improved.

auto DAI =
          std::lower_bound(Symbols.begin(), Symbols.end(), Index, SymCmp);

Index is >= Start, so you don't need to search past Symbols[si], no?

In fact, why do you need to search at all? When you are about to print a symbol from Start to End, you can check if the range for that symbol is data or not and you can do that by just maintaining a single "InCode" variable that is updated every time you pass a $x or $d.

davide updated this revision to Diff 36657.Oct 6 2015, 2:17 PM
davide edited edge metadata.

Updated. Thank you for your help, Rafael!
This also now matches better with GNU objdump does, not showing mapping symbols.

davide updated this revision to Diff 36659.Oct 6 2015, 2:18 PM

git-clang-format.

jmolloy added a subscriber: jmolloy.Oct 7 2015, 8:25 AM
jmolloy added inline comments.
tools/llvm-objdump/llvm-objdump.cpp
988 ↗(On Diff #36659)

As this is a target-independent file, shouldn't we also make sure $a and $t (for AArch32) are covered here? Are you going to do this in another patch?

davide added inline comments.Oct 7 2015, 8:39 AM
tools/llvm-objdump/llvm-objdump.cpp
988 ↗(On Diff #36659)

Hi James, I think I can take of this separately. Please file a PR and assign to me so I won't forget.

Thanks,

Davide