Page MenuHomePhabricator

[lld-macho] Add -static
AcceptedPublic

Authored by Link1J on Feb 13 2022, 8:39 AM.

Details

Reviewers
int3
Group Reviewers
Restricted Project
Summary

Adds -static (and -add_load_version)

-static drives will probably drive me (or anyone) insane if it is to be completely and correctly implemented. It seems to touch everything, and I don't even know if everything below is all that it does.

Implemented behavior:

  • Entry is defined by LC_UNIXTHREAD command
  • LC_BUILD_VERSION and LC_VERSION_MIN_* are no longer emitted by default
  • LC_FUNCTION_STARTS is no longer emitted by default
  • LC_DATA_IN_CODE is no longer emitted by default
  • LC_ENCRYPTION_INFO is no longer emitted by default
  • LC_LOAD_DYLINKER is no longer emitted
  • Default entry becomes start
  • Is not PIC by default.

Will be implemented:

  • -image_base effecting the __PAGEZERO and not the __TEXT

Not implemented behavior:

  • -arch is not needed
  • -platform_version is not needed
  • Forcing -keep_dwarf_unwind, even if -no_keep_dwarf_unwind is provided (I don't if they are implemented)

Notes within notes:

  • The tests for -static will need fixing if LC_SOURCE_VERSION is added

Diff Detail

Event Timeline

Link1J created this revision.Feb 13 2022, 8:39 AM
Link1J requested review of this revision.Feb 13 2022, 8:39 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 13 2022, 8:39 AM
Link1J updated this revision to Diff 408273.Feb 13 2022, 9:11 AM
int3 added a comment.Feb 15 2022, 6:54 AM

-static drives will probably drive me (or anyone) insane if it is to be completely and correctly implemented. It seems to touch everything, and I don't even know if everything below is all that it does.

I guess my question is... why are we implementing this, if it's too much work to implement correctly? Do you foresee a real use case?

lld/MachO/Driver.cpp
1249
lld/MachO/Writer.cpp
361

if you did buf = reinterpret_cast<uint8_t *>(hdr + 1); here, you could replace the calls to get_thread_state with just a reinterpret_cast.

just a matter of opinion though, I'm fine if you want to keep things as-is :)

lld/test/MachO/static.s
51

we should add some checks (via CHECK-NOT) to make sure we aren't emitting the function starts and other unneeded commands

-static drives will probably drive me (or anyone) insane if it is to be completely and correctly implemented. It seems to touch everything, and I don't even know if everything below is all that it does.

I guess my question is... why are we implementing this, if it's too much work to implement correctly? Do you foresee a real use case?

It isn't making the changes that are hard, as most of the stuff is already in lld. The part that was driving me insane is tracking down what it changes. I was doing very basic code searches in ld64 code, and it was hard to track what it was doing (There are about three different variables to control most flags it has). So it really seems like -static is just making ld64 output older style mach-o binaries, with most of the dynamic linker stuff missing.

In the end, I think that I at least got everything noted down, with most that I implemented being found from linking using ld64. So everything should be listed, so that others could make the changes.
I do plan to make the changes for -image_base, as I already have a patch in the works to add it.

I am going to use it, and it does have uses it OS development. So I thought it would be a good idea to add it upstream, as lld is a lot easier to get working on other platforms then ld64 is.
Also some of the items added (like LC_UNIXTHREAD) do have uses for linking binaries for older macOS versions. So I don't expect this flag to get a lot of use directly, there is a lot that can be used for improvement for linking older macOS binaries, or for building projects like darling.

int3 added a comment.Mar 10 2022, 8:37 PM

Oops, I realized I'd never replied to this. Ok, if you're going to use it then I'm good with including this. If you can make the requested changes, I'll accept this.

Herald added a project: Restricted Project. · View Herald TranscriptMar 10 2022, 8:37 PM
int3 requested changes to this revision.Mar 16 2022, 2:34 PM

clearing queue

This revision now requires changes to proceed.Mar 16 2022, 2:34 PM
Link1J updated this revision to Diff 436458.Jun 13 2022, 10:12 AM
Link1J set the repository for this revision to rG LLVM Github Monorepo.
int3 accepted this revision.Jun 14 2022, 3:01 AM

lgtm

lld/test/MachO/static.s
2–4

can you elaborate here? Are you saying that LC_SOURCE_VERSION should really be emitted for both -static and regular links?

This revision is now accepted and ready to land.Jun 14 2022, 3:01 AM
Link1J added inline comments.Jun 14 2022, 5:39 AM
lld/test/MachO/static.s
2–4

That is correct. But it doesn't appear to be implemented by lld. I don't know what LC_SOURCE_VERSION does, or if it is even useful. But by default ld64 does emit LC_SOURCE_VERSION load command.