[lld-macho] Implement -bundle_loader

[oontvoo]

Summary

Usage: -bundle_loader <executable>
This option specifies the executable that will load the build output file being linked.
When building a bundle, users can use the --bundle_loader to specify an executable that contains symbols referenced, but not implemented in the bundle.

[oontvoo]

updated diff

[oontvoo]

rebase

[oontvoo]

resolve conflicts

[jyknight]

Please prefix the title of the commit with "[lld-macho]" so that it's easier to understand what bundle_loader is being implemented in.

And, I'm not an expert on this codebase, so while I have some comments, I'm also adding a code-owner of lld macho to the review.

lld/MachO/Driver.cpp

805

Why is this check before opening needed?

lld/MachO/InputFiles.cpp

627

As discussed via chat, you'll need to make sure that the proper "EXECUTABLE_ORDINAL" ordinal gets used, rather than emitting a LOAD_DYLIB with an empty dylib name.

lld/test/MachO/bundle_loader-darwin.test

2

The tests for LLD shouldn't invoke clang; this will need to use assembly, and invoke llvm-mc and lld, instead. Also, for this sort of multi-input test, the "split-file" util will be better than a bunch of "echo" commands.

See e.g. lld/test/MachO/archive.s for examples of how you might structure it.

[int3]

Some context (in the commit message and/or comments) on what semantics -bundle_loader is supposed to achieve would be helpful :)

[oontvoo]

(added leftover file, which sets the executable_ordinal)

[oontvoo]

lld/MachO/Driver.cpp

805

to fail early... should it not check that?

lld/MachO/InputFiles.cpp

627

Resolved. Sorry, I'd forgot to add the SyntheticSections.cpp to the commit.

[int3]

Could you add more detail to the commit message? Assume the reader (like me) has general knowledge of linkers and Mach-O but no knowledge of what specifying a bundle loader does. What kind of restrictions does this impose on the binary / what kind of optimizations does it allow / how is the current implementation falling short? The other commits in lld-macho should give you a flavor of how detailed things should be.

If there are things in the diff that you are uncertain about (e.g. I see the test is not quite done yet) do add it to the message as well (or in a comment, that works too).

[oontvoo]

Updated test. Also added more comments to the test to explain what bundle_loader is used for

[oontvoo]

updated commit message

[oontvoo]

In D95913#2541041, @int3 wrote:

Could you add more detail to the commit message? Assume the reader (like me) has general knowledge of linkers and Mach-O but no knowledge of what specifying a bundle loader does. What kind of restrictions does this impose on the binary / what kind of optimizations does it allow / how is the current implementation falling short? The other commits in lld-macho should give you a flavor of how detailed things should be.

If there are things in the diff that you are uncertain about (e.g. I see the test is not quite done yet) do add it to the message as well (or in a comment, that works too).

Done! PTAL. Thanks

[jyknight]

lld/MachO/Driver.cpp

328–329

That an error message is emitted here but not in the above call to loadDylib confuses me. I don't know if that's because it's extraneous here or missing above, but it seems like one or the other must be wrong.

805

Usually failing on actual open is sufficient. Also, the call to addFile right below this doesn't do so -- so I'd expect that either that call is missing such a check, or this one is extraneous.

lld/MachO/SyntheticSections.cpp

798

I'd expect to see the code that sets ->ordinal modified to set it to EXECUTABLE_ORDINAL, rather than the code that uses ->ordinal modified to override it.

lld/test/MachO/bundle_loader-darwin.test

20–21

Is the dylib necessary in this test? It's weird to link the same "2.o" file into both a dylib and the main binary.

And, assuming not, I think my_func definition can just be moved into main.s, eliminating 2.s.

37

There's no need for the test to be a correctly-executing function -- you can delete all the asm except the call (in order to have the needed symbol reference) or ret (just so there's at least an instruction in the function).

[oontvoo]

updated diff to address comments

lld/MachO/Driver.cpp

328–329

I've removed it to be consistent with the others...

lld/MachO/SyntheticSections.cpp

798

Lookin at the code, I think DylibFile::ordinal means a different thing than "ordinal" in EXECUTABLE_ORDINAL.
So we can't set DylibFile::ordinal, which is a uint64, to EXECUTABLE_ORDINAL (which is 0xff).

Also, the line right above, which sets the WEAK_REF flag also didn't set it earlier.

lld/test/MachO/bundle_loader-darwin.test

20–21

Yes, I think the point of the test to show that even though both (the dylib and the executable) have the symbol, the output bundle gets the symbol from the executable

[oontvoo]

lld/MachO/SyntheticSections.cpp

798

pls ignore the comment above(should've been discarded) .

[int3]

lld/MachO/Driver.cpp

323

nit

324

ultra nit: here you refer to it as bundle_loader, in Writer.cpp you refer to it as bundle-loader. Personally I think it reads more naturally with a space in between, but anything is fine as long as it's consistent :P

325–326

this should be an assert() -- error()s are things that can arise from user input

802

OPT_bundle_loader should be handled using getLastArg (like the other options in lines 735-773 above) instead of in this loop. This loop should be reserved for options whose values can be specified multiple times.

803–804

should test this

lld/MachO/InputFiles.cpp

629

No need for llvm::. Also this should probably live in Writer.cpp (see comment there)

634–636

I don't think this change is necessary, unless bundle loaders can somehow be sub-libraries

lld/MachO/InputFiles.h

144

this needs a detailed comment. I think it's pretty surprising that we're making DylibFiles out of bundles and executables, which are clearly not dylibs, but I get why -- they serve the same role here as binary interfaces whose contents do not appear in the final output. Still, surprising behavior should always be explained. (Would also be good to preface it with some explanation of what a bundle loader is, i.e. the stuff you have written in the commit message.)

lld/MachO/SyntheticSections.cpp

269–273

since EXECUTABLE_ORDINAL is larger than BIND_IMMEDIATE_MASK, this seems unnecessary

lld/MachO/Writer.cpp

506–507

I think we should assign ordinal = EXECUTABLE_ORDINAL here instead of in InputFiles.cpp. It's kind of weird to have ordinal set to its final value for bundle loaders at the parsing phase while all other ordinal values get set at the writing phase.

lld/test/MachO/bundle_loader-darwin.test

9–18

ah I guess you were following archive.s, but most of our other tests don't do these smoke checks, I think they make things unnecessarily verbose. We're not testing llvm-mc here, we can just assume that the object files are correct

20

the three "Produce" comments in this file aren't really necessary -- it's easy enough to see what the commands are doing

27

-dynamic isn't necessary (it's currently a no-op in LLD, and even if it weren't, it's implied by -bundle)

I don't think -demangle is necessary either

27

Also, I guess if we created bundle.bundle from 3.o and 2.o (instead of mylib.dylib) then my_func would come from 2.o, right? Could we have a test for that too?

30

doesn't seem like the grep is necessary

30

I would prefer we use llvm-objdump --macho --bind instead of llvm-nm here -- it should provide a more "raw" view of the file data since it's Mach-O-specific and isn't trying to shoehorn multiple file formats into one output format

37

nit: most test files use two-space indents

43

53

[int3]

lld/MachO/InputFiles.cpp

668–674

it would be ideal if this TBD code path were tested as well. We have some example TBD files e.g. at test/MachO/Inputs/MacOSX.sdk/usr/lib/libSystem.tbd. But I'm fine with having this added later so long as you leave a TODO.

lld/MachO/InputFiles.h

128–130

would be good to assert(!isBundleLoader || !umbrella). Or just create a makeBundleLoader function that doesn't take an umbrella.

[oontvoo]

addressed comments

lld/MachO/Driver.cpp

325–326

This was because some tests was expecting this error upon seeing executable file being passed here. (It was a correct error, except for the case where the executable being the bundle-loader)

lld/MachO/InputFiles.cpp

668–674

Added a FIXME for this case.

lld/test/MachO/bundle_loader-darwin.test

30

llvm-objdump didn't seem to produce anything useful to assert on for me:

llvm-objdump-9 --macho --bind /mnt/ssd/repo/llvm-project/build/tools/lld/test/MachO/Output/bundle_loader-darwin.test.tmp/bundle.bundle
/mnt/ssd/repo/llvm-project/build/tools/lld/test/MachO/Output/bundle_loader-darwin.test.tmp/bundle.bundle:
Bind table:
segment  section            address    type       addend dylib            symbol
__DATA_CONST __got              0x00002000 pointer         0 libSystem        dyld_stub_binder

[int3]

lld/MachO/Driver.cpp

325–326

aha I see.

lld/MachO/InputFiles.h

144

Thanks for adding the comment. However, while it explains what a bundle loader is, it does not talk about why we are creating a DylibFile from things that are not dylibs, which was the main point of my comment :)

145

this line is unnecessary (that much is clear from the variable name)

lld/test/MachO/bundle_loader-darwin.test

2

to follow the other tests

also can you name this test bundle-loader.s? I.e. don't use underscores, no need to mention darwin (all the tests here are for darwin), and since the test inputs are assembly, the extension should match.

15

1. use hyphen to follow other tests
2. missing colon

30

Sorry, it should have been --lazy-bind, since _main is a function and all functions are lazily bound.

Anyway I tried it locally and ran into this error:

(base) test/MachO: llvm-objdump --macho --lazy-bind /Users/jezng/src/llvm-project/build/release/tools/lld/test/MachO/Output/bundle_loader-darwin.test.tmp/bundle2.bundle
/Users/jezng/src/llvm-project/build/release/tools/lld/test/MachO/Output/bundle_loader-darwin.test.tmp/bundle2.bundle:
Lazy bind table:
segment  section            address     dylib            symbol
llvm-objdump: error: '/Users/jezng/src/llvm-project/build/release/tools/lld/test/MachO/Output/bundle_loader-darwin.test.tmp/bundle2.bundle': truncated or malformed object (for BIND_OPCODE_SET_DYLIB_ORDINAL_ULEB bad library ordinal: 255 (max 1) for opcode at: 0x2)

I wasn't sure if this was a problem in our output or in llvm-objdump, so I tried replacing %lld in the test with ld to see what ld64 was emitting. But it appears that ld64 isn't even emitting any references to _main in bundle2.bundle:

(base) test/MachO: llvm-nm -m /Users/jezng/src/llvm-project/build/release/tools/lld/test/MachO/Output/bundle_loader-darwin.test.tmp/bundle2.bundle
                 (undefined) external dyld_stub_binder (from libSystem)
0000000000003fb7 (__TEXT,__text) external my_func
0000000000003fb1 (__TEXT,__text) non-external my_user

I was testing this on Catalina 10.15.7 with ld64-609.7. Does the behavior differ on your system?

[oontvoo]

updated diff

[oontvoo]

lld/MachO/InputFiles.cpp

634–636

No, it's not technically needed. I just thought it should be made a bit clearer (Otherwise, it was calling isImplicitlyLinked() on an empty dylibName). WDYT?

lld/MachO/Writer.cpp

485

@int3 I think this needs to emit the __mh_bundle_header here too?

(not because of this patch, but it seems it's currently lacking that)

lld/test/MachO/bundle_loader-darwin.test

30

Ah, I think the error was because the EXECUTABLE_ORDINAL should not have been emitted. (really, the point of setting that EXECUTABLE_ORDINAL was for llvm-nm to know to emit "from executable" when it prints the symbol).

Anyways, I've fixed that and updated the tests to also use llvm-objdump.

About` _main`, yeah, maybe there needs to be a special case where _main doesn't get emitted?

[jyknight]

lld/MachO/SyntheticSections.cpp

275

I think this needs to emit BIND_OPCODE_SET_DYLIB_SPECIAL_IMM when ordinal < 0, instead of checking isBundleLoader and not outputting any command.

I guess the default is executable, perhaps? So the test works if it's the first symbol, but if there was a symbol refering to another lib first, then this would incorrectly remain pointing to that other library since no command is emitted.

792–795

Probably should explicitly convert back to EXECUTABLE_ORDINAL here -- it's working anyways because BIND_SPECIAL_DYLIB_MAIN_EXECUTABLE is -1, which converts to 0xff, but that seems kinda sketchy.

[oontvoo]

changed ordinal types to signed type

[oontvoo]

rebase

[int3]

lld/MachO/InputFiles.cpp

634–636

I think it's confusing either way...

I'm actually wondering if it we should have separate DylibFile and BundleLoader classes that inherit from some common base class. Not asking you to do it in this diff though. But for now I would prefer we just remove the isBundleLoader check here

lld/MachO/Writer.cpp

485

yup I'm aware it's a missing feature

506

hm why did this change from EXECUTABLE_ORDINAL to BIND_SPECIAL_DYLIB_MAIN_EXECUTABLE?

Also, shouldn't there be a continue; here since I assume we don't want to emit an LC_LOAD_DYLIB for this? (Can you add a test that would catch this issue?)

also, no need for llvm::

[int3]

lld/test/MachO/bundle_loader-darwin.test

16

I think this comment is kinda redundant

18

Since the address is not relevant to the test, let's not match on it (to make future changes to this file easier)

I was also confused for a bit about how this was testing for EXECUTABLE_ORDINAL. Then I realized that the lack of dylib name in the output indicates that it's not from a dylib. I think it would be clearer if you included the segment/section/address etc header before this line.

[oontvoo]

updated diff

lld/MachO/InputFiles.cpp

634–636

Done. That makes sense.

lld/MachO/Writer.cpp

506

hm why did this change from EXECUTABLE_ORDINAL to BIND_SPECIAL_DYLIB_MAIN_EXECUTABLE?

These two are kind of the same thing (The former being 0xff and the latter -1). BIND_SPECIAL_DYLIB_MAIN_EXECUTABLE is the right thing to use when encoding because it's consistent with others BIND_* things.
Also (from looking at what ld64 does), it's more convenient to set BIND_SPECIAL_DYLIB_MAIN_EXECUTABLE here so that you can have the encodeDylibOrdinal function do the right thing for the special cases where ordinal <=0. ( 0 is for self, and -1 is for dylib-main-exec)

Also, shouldn't there be a continue; here since I assume we don't want to emit an LC_LOAD_DYLIB for this? (Can you add a test that would catch this issue?)

No, I think it doesn't "reexport" the symbols but it still needs the load_dylib for the symbols that are referenced.
(otherwise, we're missing those. And the test is that when you do llvm-objdump, you'd get an error for "truncated or malformed object")

also, no need for llvm::

Done

lld/test/MachO/bundle_loader-darwin.test

18

I was also confused for a bit about how this was testing for EXECUTABLE_ORDINAL.

It's a bit clearer to use llvm-nm to test for this. (because it'd say "from executable")

Then I realized that the lack of dylib name in the output indicates that it's not from a dylib.

Right.

I think it would be clearer if you included the segment/section/address etc header before this line.

done

[int3]

Okay, this seems like it's almost there. Thanks for putting up with all the nitty comments and explaining the details, I know it's taken a while :) Once the remaining comments are addressed I'll stamp this. Thank you!

lld/MachO/SyntheticSections.cpp

270

hmm doesn't BIND_OPCODE_SET_DYLIB_SPECIAL_IMM encode its argument as an immediate rather than in a trailing ULEB?

lld/MachO/Writer.cpp

506

These two are kind of the same thing (The former being 0xff and the latter -1).

Ah, I see... the encoded ordinal value isn't actually a signed integer, but an unsigned byte with a few special values.

No, I think it doesn't "reexport" the symbols but it still needs the load_dylib for the symbols that are referenced.

Got it, thanks!

lld/test/MachO/bundle_loader-darwin.test

18

It's a bit clearer to use llvm-nm to test for this. (because it'd say "from executable")

Fair. Though I'm actually not sure if llvm-nm is getting that information from the bind table or from the symbol table. (I think it's actually the symbol table.)

[oontvoo]

Updated diff:

• fixed BIND_OPCODE_SET_DYLIB_SPECIAL_IMM encoding
• rename test to bundle-loader.s

[oontvoo]

In D95913#2563998, @int3 wrote:

Okay, this seems like it's almost there. Thanks for putting up with all the nitty comments and explaining the details, I know it's taken a while :) Once the remaining comments are addressed I'll stamp this. Thank you!

No worries. Thanks for taking the time to review this!

One more thing, some tests started to fail after the signed => unsigned types change ... I must've missed something obvious ... thoughts?

lld/MachO/SyntheticSections.cpp

270

hmm doesn't BIND_OPCODE_SET_DYLIB_SPECIAL_IMM encode its argument as an immediate rather than in a trailing ULEB?

Whoops, yes - it should be. (fixed now).

lld/test/MachO/bundle_loader-darwin.test

18

Fair. Though I'm actually not sure if llvm-nm is getting that information from the bind table or from the symbol table. (I think it's actually the symbol table.)

Yeah, it gets it from the symbol table[0] - Specifically, it looks at the nlist_64::n_desc field and prints from executable for ordinal-exec [1]

[0] https://github.com/llvm/llvm-project/blob/6c05005238a805a699d9dec39a61971affd1cab4/llvm/tools/llvm-nm/llvm-nm.cpp#L414
[1] https://github.com/llvm/llvm-project/blob/6c05005238a805a699d9dec39a61971affd1cab4/llvm/tools/llvm-nm/llvm-nm.cpp#L613

[oontvoo]

updated diff:

• removed accidental incr on the dylibOrdinal (twice)
• added missing handling of the BIND_OPCODE_SET_DYLIB_SPECIAL_IMM

[oontvoo]

updated diff: refactored code to reuse encodeDylibOrdinal

[oontvoo]

In D95913#2566101, @oontvoo wrote:

One more thing, some tests started to fail after the signed => unsigned types change ... I must've missed something obvious ... thoughts?

P.S: never mind - I'd forgot to also handle the special_imm case in LazyBindingSection::encode().
(resolved by refactoring the code to use encodeDylibOrdinal)

[int3]

lld/test/MachO/bundle-loader.s

1 ↗

(On Diff #324062)

This currently works because split-file is being used, which means the uncommented line doesn't get passed to llvm-mc, but we should still comment it out for uniformity

13 ↗

(On Diff #324062)

missing colon + consider not adding the _CHECK suffix which is kinda redundant. Also I would suggest using hyphens instead of underscores as separators, since that's what llvm-lit uses for its native prefixes (e.g. CHECK-NEXT).

lld/test/MachO/bundle_loader-darwin.test

18

Great, we're testing both outputs then :)

[oontvoo]

updated all the check prefixes

[oontvoo]

rebase

[vitalybuka]

http://lab.llvm.org:8011/#/builders/5/builds/4761

/b/sanitizer-x86_64-linux-fast/build/llvm-project/lld/MachO/Writer.cpp:269:17: runtime error: null pointer passed as argument 2, which is declared to never be null
/usr/include/string.h:43:28: note: nonnull attribute specified here
    #0 0x50b3cf2 in (anonymous namespace)::LCDylib::writeTo(unsigned char*) const /b/sanitizer-x86_64-linux-fast/build/llvm-project/lld/MachO/Writer.cpp:269:5
    #1 0x5081da7 in lld::macho::MachHeaderSection::writeTo(unsigned char*) const /b/sanitizer-x86_64-linux-fast/build/llvm-project/lld/MachO/SyntheticSections.cpp:89:9
    #2 0x509fea5 in writeSections /b/sanitizer-x86_64-linux-fast/build/llvm-project/lld/MachO/Writer.cpp:761:13
    #3 0x509fea5 in (anonymous namespace)::Writer::run() /b/sanitizer-x86_64-linux-fast/build/llvm-project/lld/MachO/Writer.cpp:815:3
    #4 0x509dab0 in lld::macho::writeResult() /b/sanitizer-x86_64-linux-fast/build/llvm-project/lld/MachO/Writer.cpp:822:38
    #5 0x505a851 in lld::macho::link(llvm::ArrayRef<char const*>, bool, llvm::raw_ostream&, llvm::raw_ostream&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/lld/MachO/Driver.cpp:921:3
    #6 0x4b35e25 in lldMain(int, char const**, llvm::raw_ostream&, llvm::raw_ostream&, bool) /b/sanitizer-x86_64-linux-fast/build/llvm-project/lld/tools/lld/lld.cpp:159:13
    #7 0x4b385b3 in operator() /b/sanitizer-x86_64-linux-fast/build/llvm-project/lld/tools/lld/lld.cpp:180:15
    #8 0x4b385b3 in void llvm::function_ref<void ()>::callback_fn<lld::safeLldMain(int, char const**, llvm::raw_ostream&, llvm::raw_ostream&)::$_0>(long) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/include/llvm/ADT/STLExtras.h:185:12
    #9 0x4b5dc82 in operator() /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/include/llvm/ADT/STLExtras.h:209:12
    #10 0x4b5dc82 in llvm::CrashRecoveryContext::RunSafely(llvm::function_ref<void ()>) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/Support/CrashRecoveryContext.cpp:424:3
    #11 0x4b357df in lld::safeLldMain(int, char const**, llvm::raw_ostream&, llvm::raw_ostream&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/lld/tools/lld/lld.cpp:179:14
    #12 0x4b359b6 in main /b/sanitizer-x86_64-linux-fast/build/llvm-project/lld/tools/lld/lld.cpp:222:14
    #13 0x7f16b4e9d09a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)
    #14 0x4b121f9 in _start (/b/sanitizer-x86_64-linux-fast/build/llvm_build_ubsan/bin/lld+0x4b121f9)
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /b/sanitizer-x86_64-linux-fast/build/llvm-project/lld/MachO/Writer.cpp:269:17 in

[oontvoo]

@vitalybuka How do I rerun the test with uban enabled?
Thanks!

[vitalybuka]

In D95913#2576119, @oontvoo wrote:

@vitalybuka How do I rerun the test with uban enabled?
Thanks!

https://github.com/google/sanitizers/wiki/SanitizerBotReproduceBuild

For particular this case I'd just rerun check-lld with "assert()/if (is null) abort()" in the place of already known error.

[int3]

@oontvoo this is what I do using cmake:

llvm-project/build/asan: cmake -G Ninja -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_PROJECTS='lld' -DLLVM_USE_SANITIZER='Address;Undefined' ../../llvm
llvm-project/build/asan: ninja check-lld-macho

[oontvoo]

Updated diff: fix ubsan bug. The executable didn't have a dylib path/name.

lld/MachO/Writer.cpp

506

Actually, I think you were right about this. The bundle-loader also didn't need this extra load command. (I've jsut retried the test case with ld64, and it skipped this.)
This was also why we hit the ubsan one (the path name is going to be empty for executable).

[oontvoo]

lld/MachO/Writer.cpp

506

P.S: updated test was:

llvm-objdump  --macho --lazy-bind %t/bundle.bundle | FileCheck %s --check-prefix BUNDLE-OBJ
segment   section                    address        dylib                          symbol
 __DATA    __la_symbol_ptr     0x{{[0-9a-f]*}}    main-executable       my_fun

Will sleep on this reland it on Monday, just in case.

[int3]

Closed by rG5a856f5b44997dd45db08f434e5da1f85e4693f5.

lld/MachO/Driver.h

 void parseLCLinkerOption(InputFile*, unsigned argc, StringRef data);
 
 std::string createResponseFile(const llvm::opt::InputArgList &args);
 
 // Check for both libfoo.dylib and libfoo.tbd (in that order).
 llvm::Optional<std::string> resolveDylibPath(llvm::StringRef path);
 
 llvm::Optional<DylibFile *> loadDylib(llvm::MemoryBufferRef mbref,
-                                      DylibFile *umbrella = nullptr);
+                                      DylibFile *umbrella = nullptr,
+                                      bool isBundleLoader = false);
 
 uint32_t getModTime(llvm::StringRef path);
 
 void printArchiveMemberLoad(StringRef reason, const InputFile *);
 
 } // namespace macho
 } // namespace lld
 
 #endif

lld/MachO/Driver.cpp

static std::vector<ArchiveMember> getArchiveMembers(MemoryBufferRef mb) {
   }
   if (err)
     fatal(mb.getBufferIdentifier() +
           ": Archive::children failed: " + toString(std::move(err)));
 
   return v;
 }
 
-static InputFile *addFile(StringRef path, bool forceLoadArchive) {
+static InputFile *addFile(StringRef path, bool forceLoadArchive,
+                          bool isBundleLoader = false) {
   Optional<MemoryBufferRef> buffer = readFile(path);
   if (!buffer)
     return nullptr;
   MemoryBufferRef mbref = *buffer;
   InputFile *newFile = nullptr;
 
   auto magic = identify_magic(mbref.getBuffer());
   switch (magic) {
static InputFile *addFile(StringRef path, bool forceLoadArchive,
   case file_magic::macho_dynamically_linked_shared_lib_stub:
   case file_magic::tapi_file:
     if (Optional<DylibFile *> dylibFile = loadDylib(mbref))
       newFile = *dylibFile;
     break;
   case file_magic::bitcode:
     newFile = make<BitcodeFile>(mbref);
     break;
+  case file_magic::macho_executable:
+  case file_magic::macho_bundle:
+    // We only allow executable and bundle type here if it is used
+    // as a bundle loader.
+    if (!isBundleLoader)
+      error(path + ": unhandled file type");
+    if (Optional<DylibFile *> dylibFile =
+            loadDylib(mbref, nullptr, isBundleLoader))
+      newFile = *dylibFile;
+    break;
   default:
     error(path + ": unhandled file type");
   }
   if (newFile) {
     // printArchiveMemberLoad() prints both .a and .o names, so no need to
     // print the .a name here.
     if (config->printEachFile && magic != file_magic::archive)
       message(toString(newFile));
bool macho::link(ArrayRef<const char *> argsArr, bool canExitEarly,
   config->installName =
       args.getLastArgValue(OPT_install_name, config->outputFile);
   config->headerPad = args::getHex(args, OPT_headerpad, /*Default=*/32);
   config->headerPadMaxInstallNames =
       args.hasArg(OPT_headerpad_max_install_names);
   config->printEachFile = args.hasArg(OPT_t);
   config->printWhyLoad = args.hasArg(OPT_why_load);
   config->outputType = getOutputType(args);
+  if (const opt::Arg *arg = args.getLastArg(OPT_bundle_loader)) {
+    if (config->outputType != MH_BUNDLE)
+      error("-bundle_loader can only be used with MachO bundle output");
+    addFile(arg->getValue(), false, true);
+  }
   config->ltoObjPath = args.getLastArgValue(OPT_object_path_lto);
   config->ltoNewPassManager =
       args.hasFlag(OPT_no_lto_legacy_pass_manager, OPT_lto_legacy_pass_manager,
                    LLVM_ENABLE_NEW_PASS_MANAGER);
   config->runtimePaths = args::getStrings(args, OPT_rpath);
   config->allLoad = args.hasArg(OPT_all_load);
   config->forceLoadObjC = args.hasArg(OPT_ObjC);
   config->demangle = args.hasArg(OPT_demangle);
bool macho::link(ArrayRef<const char *> argsArr, bool canExitEarly,
   }
 
   initLLVM(); // must be run before any call to addFile()
 
   for (const auto &arg : args) {
     const auto &opt = arg->getOption();
     warnIfDeprecatedOption(opt);
     warnIfUnimplementedOption(opt);
+
     // TODO: are any of these better handled via filtered() or getLastArg()?
     switch (opt.getID()) {
     case OPT_INPUT:
       addFile(arg->getValue(), false);
       break;
     case OPT_weak_library:
       if (auto *dylibFile =
               dyn_cast_or_null<DylibFile>(addFile(arg->getValue(), false)))
         dylibFile->forceWeakImport = true;
       break;
     case OPT_filelist:
       addFileList(arg->getValue());
       break;
     case OPT_force_load:

lld/MachO/DriverUtils.cpp

Optional<std::string> macho::resolveDylibPath(StringRef path) {
   return {};
 }
 
 // It's not uncommon to have multiple attempts to load a single dylib,
 // especially if it's a commonly re-exported core library.
 static DenseMap<CachedHashStringRef, DylibFile *> loadedDylibs;
 
 Optional<DylibFile *> macho::loadDylib(MemoryBufferRef mbref,
-                                       DylibFile *umbrella) {
+                                       DylibFile *umbrella,
+                                       bool isBundleLoader) {
   StringRef path = mbref.getBufferIdentifier();
   DylibFile *&file = loadedDylibs[CachedHashStringRef(path)];
   if (file)
     return file;
 
   file_magic magic = identify_magic(mbref.getBuffer());
   if (magic == file_magic::tapi_file) {
     Expected<std::unique_ptr<InterfaceFile>> result = TextAPIReader::get(mbref);
     if (!result) {
       error("could not load TAPI file at " + mbref.getBufferIdentifier() +
             ": " + toString(result.takeError()));
       return {};
     }
-    file = make<DylibFile>(**result, umbrella);
+    file = make<DylibFile>(**result, umbrella, isBundleLoader);
   } else {
     assert(magic == file_magic::macho_dynamically_linked_shared_lib ||
-           magic == file_magic::macho_dynamically_linked_shared_lib_stub);
-    file = make<DylibFile>(mbref, umbrella);
+           magic == file_magic::macho_dynamically_linked_shared_lib_stub ||
+           magic == file_magic::macho_executable ||
+           magic == file_magic::macho_bundle);
+    file = make<DylibFile>(mbref, umbrella, isBundleLoader);
   }
   return file;
 }
 
 uint32_t macho::getModTime(StringRef path) {
   fs::file_status stat;
   if (!fs::status(path, stat))
     if (fs::exists(stat))

lld/MachO/InputFiles.h

 public:
   // Mach-O dylibs can re-export other dylibs as sub-libraries, meaning that the
   // symbols in those sub-libraries will be available under the umbrella
   // library's namespace. Those sub-libraries can also have their own
   // re-exports. When loading a re-exported dylib, `umbrella` should be set to
   // the root dylib to ensure symbols in the child library are correctly bound
   // to the root. On the other hand, if a dylib is being directly loaded
   // (through an -lfoo flag), then `umbrella` should be a nullptr.
-  explicit DylibFile(MemoryBufferRef mb, DylibFile *umbrella = nullptr);
+  explicit DylibFile(MemoryBufferRef mb, DylibFile *umbrella = nullptr,
+                     bool isBundleLoader = false);
 
   explicit DylibFile(const llvm::MachO::InterfaceFile &interface,
-                     DylibFile *umbrella = nullptr);
+                     DylibFile *umbrella = nullptr,
+                     bool isBundleLoader = false);
 
   static bool classof(const InputFile *f) { return f->kind() == DylibKind; }
 
   StringRef dylibName;
   uint32_t compatibilityVersion = 0;
   uint32_t currentVersion = 0;
-  uint64_t ordinal = 0; // Ordinal numbering starts from 1, so 0 is a sentinel
+  int64_t ordinal = 0; // Ordinal numbering starts from 1, so 0 is a sentinel
   RefState refState;
   bool reexport = false;
   bool forceWeakImport = false;
+
+  // An executable can be used as a bundle loader that will load the output
+  // file being linked, and that contains symbols referenced, but not
+  // implemented in the bundle. When used like this, it is very similar
+  // to a Dylib, so we re-used the same class to represent it.
+  bool isBundleLoader;
 };
 
 // .a file
 class ArchiveFile : public InputFile {
 public:
   explicit ArchiveFile(std::unique_ptr<llvm::object::Archive> &&file);
   static bool classof(const InputFile *f) { return f->kind() == ArchiveKind; }
   void fetch(const llvm::object::Archive::Symbol &sym);

lld/MachO/InputFiles.cpp

 }
 
 void loadReexport(StringRef path, DylibFile *umbrella) {
   Optional<DylibFile *> reexport = loadReexportHelper(path, umbrella);
   if (reexport && isImplicitlyLinked(path))
     inputFiles.insert(*reexport);
 }
 
-DylibFile::DylibFile(MemoryBufferRef mb, DylibFile *umbrella)
-    : InputFile(DylibKind, mb), refState(RefState::Unreferenced) {
+DylibFile::DylibFile(MemoryBufferRef mb, DylibFile *umbrella,
+                     bool isBundleLoader)
+    : InputFile(DylibKind, mb), refState(RefState::Unreferenced),
+      isBundleLoader(isBundleLoader) {
+  assert(!isBundleLoader || !umbrella);
   if (umbrella == nullptr)
     umbrella = this;
 
   auto *buf = reinterpret_cast<const uint8_t *>(mb.getBufferStart());
   auto *hdr = reinterpret_cast<const mach_header_64 *>(mb.getBufferStart());
 
   // Initialize dylibName.
   if (const load_command *cmd = findCommand(hdr, LC_ID_DYLIB)) {
     auto *c = reinterpret_cast<const dylib_command *>(cmd);
     currentVersion = read32le(&c->dylib.current_version);
     compatibilityVersion = read32le(&c->dylib.compatibility_version);
     dylibName = reinterpret_cast<const char *>(cmd) + read32le(&c->dylib.name);
-  } else {
+  } else if (!isBundleLoader) {
+    // macho_executable and macho_bundle don't have LC_ID_DYLIB,
+    // so it's OK.
     error("dylib " + toString(this) + " missing LC_ID_DYLIB load command");
     return;
   }
 
   // Initialize symbols.
-  DylibFile *exportingFile = isImplicitlyLinked(dylibName) ? this : umbrella;
+  DylibFile *exportingFile =
+      (isBundleLoader || isImplicitlyLinked(dylibName)) ? this : umbrella;
   if (const load_command *cmd = findCommand(hdr, LC_DYLD_INFO_ONLY)) {
     auto *c = reinterpret_cast<const dyld_info_command *>(cmd);
     parseTrie(buf + c->export_off, c->export_size,
               [&](const Twine &name, uint64_t flags) {
                 bool isWeakDef = flags & EXPORT_SYMBOL_FLAGS_WEAK_DEFINITION;
                 bool isTlv = flags & EXPORT_SYMBOL_FLAGS_KIND_THREAD_LOCAL;
                 symbols.push_back(symtab->addDylib(
                     saver.save(name), exportingFile, isWeakDef, isTlv));
               });
for (uint32_t i = 0, n = hdr->ncmds; i < n; ++i) {
 
     auto *c = reinterpret_cast<const dylib_command *>(cmd);
     StringRef reexportPath =
         reinterpret_cast<const char *>(c) + read32le(&c->dylib.name);
     loadReexport(reexportPath, umbrella);
   }
 }
 
-DylibFile::DylibFile(const InterfaceFile &interface, DylibFile *umbrella)
-    : InputFile(DylibKind, interface), refState(RefState::Unreferenced) {
+DylibFile::DylibFile(const InterfaceFile &interface, DylibFile *umbrella,
+                     bool isBundleLoader)
+    : InputFile(DylibKind, interface), refState(RefState::Unreferenced),
+      isBundleLoader(isBundleLoader) {
+  // FIXME: Add test for the missing TBD code path.
+
   if (umbrella == nullptr)
     umbrella = this;
 
   dylibName = saver.save(interface.getInstallName());
   compatibilityVersion = interface.getCompatibilityVersion().rawValue();
   currentVersion = interface.getCurrentVersion().rawValue();
   DylibFile *exportingFile = isImplicitlyLinked(dylibName) ? this : umbrella;
   auto addSymbol = [&](const Twine &name) -> void {
     symbols.push_back(symtab->addDylib(saver.save(name), exportingFile,

lld/MachO/Options.td

def export_dynamic : Flag<["-"], "export_dynamic">,
      Flags<[HelpHidden]>,
      Group<grp_main>;
 
 def grp_bundle : OptionGroup<"bundle">, HelpText<"CREATING A BUNDLE">;
 
 def bundle_loader : Separate<["-"], "bundle_loader">,
      MetaVarName<"<executable>">,
      HelpText<"Resolve undefined symbols from <executable>">,
-     Flags<[HelpHidden]>,
      Group<grp_bundle>;
 
 def grp_object : OptionGroup<"object">, HelpText<"CREATING AN OBJECT FILE">;
 
 def keep_private_externs : Flag<["-"], "keep_private_externs">,
      HelpText<"Do not convert private external symbols to static symbols (only valid with -r)">,
      Flags<[HelpHidden]>,
      Group<grp_object>;

lld/MachO/SyntheticSections.cpp

 BindingSection::BindingSection()
     : LinkEditSection(segment_names::linkEdit, section_names::binding) {}
 
 namespace {
 struct Binding {
   OutputSegment *segment = nullptr;
   uint64_t offset = 0;
   int64_t addend = 0;
-  uint8_t ordinal = 0;
+  int16_t ordinal = 0;
 };
 } // namespace
 
 // Encode a sequence of opcodes that tell dyld to write the address of symbol +
 // addend at osec->addr + outSecOff.
 //
 // The bind opcode "interpreter" remembers the values of each binding field, so
 // we only need to encode the differences between bindings. Hence the use of
static void encodeBinding(const Symbol *sym, const OutputSection *osec,
   lastBinding.offset += WordSize;
 }
 
 // Non-weak bindings need to have their dylib ordinal encoded as well.
 static void encodeDylibOrdinal(const DylibSymbol *dysym, Binding &lastBinding,
                                raw_svector_ostream &os) {
   using namespace llvm::MachO;
   if (lastBinding.ordinal != dysym->file->ordinal) {
-    if (dysym->file->ordinal <= BIND_IMMEDIATE_MASK) {
+    if (dysym->file->ordinal <= 0) {
+      os << static_cast<uint8_t>(BIND_OPCODE_SET_DYLIB_SPECIAL_IMM);
+      encodeULEB128(dysym->file->ordinal, os);
+    } else if (dysym->file->ordinal <= BIND_IMMEDIATE_MASK) {
       os << static_cast<uint8_t>(BIND_OPCODE_SET_DYLIB_ORDINAL_IMM |
                                  dysym->file->ordinal);
     } else {
       os << static_cast<uint8_t>(BIND_OPCODE_SET_DYLIB_ORDINAL_ULEB);
       encodeULEB128(dysym->file->ordinal, os);
     }
     lastBinding.ordinal = dysym->file->ordinal;
   }
 }
 
 static void encodeWeakOverride(const Defined *defined,
if (auto *defined = dyn_cast<Defined>(entry.sym)) {
         nList->n_type = scope | MachO::N_SECT;
         nList->n_sect = defined->isec->parent->index;
         // For the N_SECT symbol type, n_value is the address of the symbol
         nList->n_value = defined->getVA();
       }
       nList->n_desc |= defined->isExternalWeakDef() ? MachO::N_WEAK_DEF : 0;
     } else if (auto *dysym = dyn_cast<DylibSymbol>(entry.sym)) {
       uint16_t n_desc = nList->n_desc;
+      if (dysym->file->isBundleLoader)
+        MachO::SET_LIBRARY_ORDINAL(n_desc, MachO::EXECUTABLE_ORDINAL);
+      else
         MachO::SET_LIBRARY_ORDINAL(n_desc, dysym->file->ordinal);
       nList->n_type = MachO::N_EXT;
       n_desc |= dysym->isWeakRef() ? MachO::N_WEAK_REF : 0;
       nList->n_desc = n_desc;
     }
     ++nList;
   }
 }
 
 IndirectSymtabSection::IndirectSymtabSection()
     : LinkEditSection(segment_names::linkEdit,
                       section_names::indirectSymbolTable) {}

lld/MachO/Writer.cpp

case MH_EXECUTE:
     in.header->addLoadCommand(make<LCMain>());
     in.header->addLoadCommand(make<LCLoadDylinker>());
     break;
   case MH_DYLIB:
     in.header->addLoadCommand(make<LCDylib>(LC_ID_DYLIB, config->installName,
                                             config->dylibCompatibilityVersion,
                                             config->dylibCurrentVersion));
     break;
   case MH_BUNDLE:
     break;
   default:
     llvm_unreachable("unhandled output file type");
   }
 
   in.header->addLoadCommand(make<LCBuildVersion>(config->platform));
 
   uuidCommand = make<LCUuid>();
   in.header->addLoadCommand(uuidCommand);
 
   uint8_t segIndex = 0;
   for (OutputSegment *seg : outputSegments) {
     in.header->addLoadCommand(make<LCSegment>(seg->name, seg));
     seg->index = segIndex++;
   }
 
   uint64_t dylibOrdinal = 1;
   for (InputFile *file : inputFiles) {
     if (auto *dylibFile = dyn_cast<DylibFile>(file)) {
+      if (dylibFile->isBundleLoader)
+        dylibFile->ordinal = llvm::MachO::BIND_SPECIAL_DYLIB_MAIN_EXECUTABLE;
+
       LoadCommandType lcType =
           dylibFile->forceWeakImport || dylibFile->refState == RefState::Weak
               ? LC_LOAD_WEAK_DYLIB
               : LC_LOAD_DYLIB;
       in.header->addLoadCommand(make<LCDylib>(lcType, dylibFile->dylibName,
                                               dylibFile->compatibilityVersion,
                                               dylibFile->currentVersion));
       dylibFile->ordinal = dylibOrdinal++;

lld/test/MachO/bundle_loader-darwin.test

+REQUIRES: x86
+
+# RUN: rm -rf %t; split-file %s %t
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/2.s -o %t/2.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/3.s -o %t/3.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/main.s -o %t/main.o
+
+# RUN: %lld  -lSystem -dylib -install_name %t/my_lib.dylib -o %t/mylib.dylib %t/2.o
+# RUN: %lld %t/2.o %t/main.o -o %t/main
+# RUN: %lld -lSystem -bundle -bundle_loader %t/main -o %t/bundle.bundle %t/3.o %t/mylib.dylib
+# Check bundle.bundle to ensure the `my_func` symbol is from executable
+# RUN: llvm-nm -m %t/bundle.bundle |  FileCheck %s --check-prefix BUNDLE_CHECK
+# BUNDLE_CHECK (undefined) external _main (from executable)
+# BUNDLE_CHECK: (undefined) external my_func (from executable)
+# Check with llvm-objdump:
+# RUN: llvm-objdump  --macho --lazy-bind %t/bundle.bundle | FileCheck %s --check-prefix BUNDLE_OBJ_CHECK
+# BUNDLE_OBJ_CHECK: __DATA   __la_symbol_ptr    0x00001010     my_fun
+
+
+# RUN: %lld -lSystem -bundle -bundle_loader %t/main -o %t/bundle2.bundle %t/3.o %t/2.o
+# Check bundle2.bundle to ensure that _main is still from executable
+# but my_func is not.
+# RUN: llvm-nm -m %t/bundle2.bundle | FileCheck %s --check-prefix BUNDLE2_CHECK
+# BUNDLE2_CHECK: (undefined) external _main (from executable)
+# BUNDLE2_CHECK: (__TEXT,__text) external my_func
+
+# Test that bundle_loader can only be used with MachO bundle output.
+# RUN: not %lld -lSystem -bundle_loader %t/main -o %t/bundle3.bundle 2>&1 | FileCheck %s --check-prefix ERROR_CHECK
+# ERROR_CHECK: -bundle_loader can only be used with MachO bundle output
+
+#--- 2.s
+# my_lib: This contains the exported function
+.globl my_func
+my_func:           
+  retq
+
+#--- 3.s
+# my_user.s: This is the user/caller of the
+#            exported function
+.text
+my_user:
+  callq my_func()
+  retq
+
+#--- main.s
+# main.s: dummy exec/main loads the exported function.
+# This is basically a way to say `my_user` should get
+# `my_func` from this executable.
+.globl _main
+.text
+ _main:
+  retq
 No newline at end of file