Page MenuHomePhabricator

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

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



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 by @seiya,
the code has been slightly refactored, the test has been converted to YAML.
This change depends on

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.


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.

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.

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



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

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!