This is an archive of the discontinued LLVM Phabricator instance.

Add support for headerpad_max_install_names cmdline option
Needs ReviewPublic

Authored by pete on Jan 6 2016, 5:18 PM.

Details

Reviewers
lhames
Summary

Hi Lang

This adds support for -headerpad_max_install_names which pads the header by enough bytes so that the dylib ID and loaded dylib paths can all be extended to MAXPATHLEN.

Thanks,
Pete

Diff Detail

Event Timeline

pete updated this revision to Diff 44183.Jan 6 2016, 5:18 PM
pete retitled this revision from to Add support for headerpad_max_install_names cmdline option.
pete updated this object.
pete added a reviewer: lhames.
pete added a project: lld.
pete added subscribers: kledzik, llvm-commits.
kledzik added inline comments.Jan 7 2016, 11:16 AM
lib/ReaderWriter/MachO/MachONormalizedFile.h
289–290

This is a design change. writeBinary() has only depended on the NormalizedFile object - not the LinkingContext. That is, for extra info like this, we add a new field to NormalizedFile.

pete added inline comments.Jan 7 2016, 11:19 AM
lib/ReaderWriter/MachO/MachONormalizedFile.h
289–290

Yeah, i was wondering whether the right thing was to add a new field or pass in the context. Thanks for clarifying the intended dependencies of writeBinary. I'll make this change now.

pete updated this revision to Diff 44238.Jan 7 2016, 11:29 AM

Updated to no longer pass the context to functions, but instead put a padHeaderPaths bool inside NormalizedFile.

lhames added inline comments.Jan 20 2016, 6:51 PM
lib/ReaderWriter/MachO/MachONormalizedFileBinaryWriter.cpp
462

We may want to add a command line option (with a default of 1024) for this eventually, but we can deal with that separately.

466

Is this pointerAlign necessary?

I don't think it matters much either way, but consider a 1 character path name on a 64-bit platform: With this pointerAlign that original path length will be "aligned" out to 8, then padded with 1016 bytes. Then, at write-out time the 1017 characters (1 char path plus padding) will be aligned back up to 1024.

Alternatively, I think we can ditch this and just add 1023 bytes of padding up front, for 1024 bytes total, at which point the second pointerAlign call is a no-op.

That said I haven't actually *tried* this, and you might have. If you did and it exploded, feel free to disregard this suggestion. ;)

473

Likewise for this pointerAlign.

pete added inline comments.Feb 11 2016, 3:52 PM
lib/ReaderWriter/MachO/MachONormalizedFileBinaryWriter.cpp
462

Sounds good. Will do that in a follow up if we need it.

466

You're right. Its not needed.

I think I got this by copying the cmdsize from the macho writer which is aligning the string up to pointer align so that the load command is the correct length. But thats not needed here when we are just padding them out. I'll remove it and the other pointerAlign call