Page MenuHomePhabricator

[llvm-objcopy][MachO] Handle relocations where r_extern is 0
ClosedPublic

Authored by alexshap on Apr 27 2020, 10:05 AM.

Details

Summary

Fix handling of relocations with r_extern == 0.
If r_extern == 0 then r_symbolnum is an index of a section rather than a symbol index.

This diff is essentially an updated version of https://reviews.llvm.org/D71647 by @seiya,
the code has been slightly refactored, the test has been converted to YAML.
This change depends on https://reviews.llvm.org/D78898.

Test plan: make check-all

Diff Detail

Event Timeline

alexshap created this revision.Apr 27 2020, 10:05 AM
Herald added a project: Restricted Project. · View Herald Transcript
Herald added a subscriber: abrachet. · View Herald Transcript

LGTM from the Mach-O side.

llvm/tools/llvm-objcopy/MachO/MachOReader.cpp
221

Should this be a user-visible error instead? It can happen for a malformed input file, for example.

alexshap marked an inline comment as done.Apr 27 2020, 2:53 PM
alexshap added inline comments.
llvm/tools/llvm-objcopy/MachO/MachOReader.cpp
221

yup, it should, i looked into this yesterday, so the interface of Reader needs some adjustments (to improve error handling). I'd prefer to add FIXME or TODO, this diff (I mean @seiya's) has been around for so long and it's really blocking me now in many ways, so would be good to commit it and refactor error handling on a separate diff.

MaskRay accepted this revision.Apr 27 2020, 3:01 PM
MaskRay added inline comments.
llvm/tools/llvm-objcopy/MachO/MachOReader.cpp
107

!R.Scattered && MachOObj.getPlainRelocationExternal(R.Info)

llvm/tools/llvm-objcopy/MachO/Object.cpp
63

*R.Symbol

This revision is now accepted and ready to land.Apr 27 2020, 3:01 PM
smeenai added inline comments.Apr 27 2020, 3:08 PM
llvm/tools/llvm-objcopy/MachO/MachOReader.cpp
221

Sure, adding a TODO or FIXME and looking into this in a follow-up is good with me.

alexshap updated this revision to Diff 260485.Apr 27 2020, 3:53 PM

minor cleanup

alexshap updated this revision to Diff 260508.Apr 27 2020, 5:44 PM

Fix typo.

This revision was automatically updated to reflect the committed changes.
seiya added a comment.Apr 27 2020, 9:32 PM

Thank you for the fix, @alexshap!