This is an archive of the discontinued LLVM Phabricator instance.

[lld-macho] handle option -headerpad_max_install_names
ClosedPublic

Authored by gkm on Sep 21 2020, 7:01 PM.

Details

Reviewers
int3
Group Reviewers
Restricted Project
Commits
rG145ce86dba6e: [lld-macho] handle option -headerpad_max_install_names

Diff Detail

Event Timeline

gkm created this revision.Sep 21 2020, 7:01 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 21 2020, 7:01 PM
gkm requested review of this revision.Sep 21 2020, 7:01 PM
int3 added a subscriber: int3.Sep 21 2020, 7:34 PM
int3 added inline comments.
lld/MachO/Writer.cpp
371

nit: make 1024 a named constant

nit 2: I assume we are checking for MH_DYLIB here because the LC_DYLIB_ID command also contains an install name. Would be good to have a comment.

Also -- don't we need to allocate space for the LC_REEXPORT_DYLIB commands as well?

372–373

would be nice to have a test where the -headerpad argument is higher than the max install names size

lld/test/MachO/headerpad.s
56

ultra nit: delete one space for alignment

gkm added inline comments.Sep 21 2020, 8:44 PM
lld/MachO/Writer.cpp
371

Yes to all.

gkm updated this revision to Diff 293508.Sep 22 2020, 11:02 AM
  • Count instances of LCDylib inside constructor rather than at construction sites
  • Elide uninteresting columns from llvm-objdump --all-headers output to reduce length and noise of the lines
  • Add tests for all types of LCDylib: self, load, reexport
  • Add tests for null-effect of -headerpad_max_install_names when no LCDylibs are present
  • Add test for -headerpad N exceeding size of -headerpad_max_install_names
gkm updated this revision to Diff 293509.Sep 22 2020, 11:05 AM
  • revert unnecessary parens in MachHeaderSection::getSize()
gkm marked 2 inline comments as done.Sep 22 2020, 11:07 AM
gkm marked an inline comment as done.
int3 accepted this revision.Sep 22 2020, 4:17 PM

it would be nice to have some comments in the test indicating what behaviors are being tested... PADOVR and PADx aren't the most descriptive :)

This revision is now accepted and ready to land.Sep 22 2020, 4:17 PM
gkm updated this revision to Diff 293598.Sep 22 2020, 5:25 PM
  • Add overview comments to tests
This revision was landed with ongoing or failed builds.Sep 22 2020, 5:26 PM
This revision was automatically updated to reflect the committed changes.