[lld/mac] Partially implement -export_dynamic

[thakis]

Summary

This implements the part of -export_dynamic that adds external
symbols as dead strip roots even for executables.

It does not yet implement the effect -export_dynamic has for LTO.
I tried just replacing config->outputType != MH_EXECUTE with
(config->outputType != MH_EXECUTE || config->exportDynamic) in
LTO.cpp, but then local symbols make it into the symbol table too,
which is too much (and also doesn't match ld64). So punt on this
for now until I understand it better.
(D91583 may or may not be related too).

[thakis]

make test call %lld instead of ld :)

[int3]

lgtm assuming the test still passes with lld

lld/test/MachO/dead-strip.s

69

this seems to be invoking ld64

[thakis]

Thanks!

It _only_ passes with %lld :) (But only because ld uses a different symbol order.) I ran it with ld locally to verify that our list of exported symbols matches ld64 (…except for ordering). And then I forgot to undo it before uploading, but luckily the precommit bots caught it.

[tschuett]

In D105482#2859935, @thakis wrote:

Thanks!

It _only_ passes with %lld :) (But only because ld uses a different symbol order.) I ran it with ld locally to verify that our list of exported symbols matches ld64 (…except for ordering). And then I forgot to undo it before uploading, but luckily the precommit bots caught it.

Could you sort the symbols to make it independent of the order, i.e. it works with ld and lld?

[thakis]

In D105482#2859951, @tschuett wrote:

In D105482#2859935, @thakis wrote:

Thanks!

It _only_ passes with %lld :) (But only because ld uses a different symbol order.) I ran it with ld locally to verify that our list of exported symbols matches ld64 (…except for ordering). And then I forgot to undo it before uploading, but luckily the precommit bots caught it.

Could you sort the symbols to make it independent of the order, i.e. it works with ld and lld?

There's no good way (I know of) to tell FileCheck "Do CHECK-DAG: for all these lines, but don't allow any other lines to interleave". The motivation for all these -NEXTs is to check that nothing else is in the symbol table other than what's listed in the test.

If there's a way to achieve this with CHECK-DAG, I'd be glad to hear about it.

Many of our tests don't pass as-is with ld though. (Some do, and I often swap out %lld for ld either manually or by editing lld/test/MachO/lit.local.cfg when I'm working on a test, so I think having more tests pass with both linkers is definitely useful when it's easy to do.)

[tschuett]

In D105482#2859987, @thakis wrote:

In D105482#2859951, @tschuett wrote:

In D105482#2859935, @thakis wrote:

Thanks!

It _only_ passes with %lld :) (But only because ld uses a different symbol order.) I ran it with ld locally to verify that our list of exported symbols matches ld64 (…except for ordering). And then I forgot to undo it before uploading, but luckily the precommit bots caught it.

Could you sort the symbols to make it independent of the order, i.e. it works with ld and lld?

I though of something like llvm-objdump ... | grep ... | sort and then do FileCheck on the result.

There's no good way (I know of) to tell FileCheck "Do CHECK-DAG: for all these lines, but don't allow any other lines to interleave". The motivation for all these -NEXTs is to check that nothing else is in the symbol table other than what's listed in the test.

If there's a way to achieve this with CHECK-DAG, I'd be glad to hear about it.

Many of our tests don't pass as-is with ld though. (Some do, and I often swap out %lld for ld either manually or by editing lld/test/MachO/lit.local.cfg when I'm working on a test, so I think having more tests pass with both linkers is definitely useful when it's easy to do.)

[thakis]

In D105482#2859995, @tschuett wrote:

In D105482#2859987, @thakis wrote:

In D105482#2859951, @tschuett wrote:

In D105482#2859935, @thakis wrote:

Thanks!

It _only_ passes with %lld :) (But only because ld uses a different symbol order.) I ran it with ld locally to verify that our list of exported symbols matches ld64 (…except for ordering). And then I forgot to undo it before uploading, but luckily the precommit bots caught it.

Could you sort the symbols to make it independent of the order, i.e. it works with ld and lld?

I though of something like llvm-objdump ... | grep ... | sort and then do FileCheck on the result.

That feels like a kludge :) I don't think having the test work with ld is important enough to resort to stuff like that. But maybe someone is motivated to add real support for this use-case to FileCheck.

[tschuett]

Thats fine. But it is odd that you are not testing that you get the right symbols. You are testing that you get the right symbols and in a random order.

The order should be an implementation detail.

[thakis]

In D105482#2860080, @tschuett wrote:

Thats fine. But it is odd that you are not testing that you get the right symbols. You are testing that you get the right symbols and in a random order.

The order should be an implementation detail.

Yes, but as said above there's no good way I know of to do an order-independent test that also makes sure that a given list of matches is exhaustive. If I used CHECK-DAG then FileCheck would allow interleaving non-matched lines silently as far as I know.

[tschuett]

Then llvm-objdump should canonalize the symbols. llvm-objdump --syms --sorted-syms

lld/MachO/Config.h

public:
   bool match(llvm::StringRef symbolName) const;
 };
 
 struct Configuration {
   Symbol *entry = nullptr;
   bool hasReexports = false;
   bool allLoad = false;
   bool archMultiple = false;
+  bool exportDynamic = false;
   bool forceLoadObjC = false;
   bool forceLoadSwift = false;
   bool staticLink = false;
   bool implicitDylibs = false;
   bool isPic = false;
   bool headerPadMaxInstallNames = false;
   bool ltoNewPassManager = LLVM_ENABLE_NEW_PASS_MANAGER;
   bool markDeadStrippableDylib = false;

lld/MachO/Driver.cpp

 static void initLLVM() {
   InitializeAllTargets();
   InitializeAllTargetMCs();
   InitializeAllAsmPrinters();
   InitializeAllAsmParsers();
 }
 
 static void compileBitcodeFiles() {
+  // FIXME: Remove this once LTO.cpp honors config->exportDynamic.
+  if (config->exportDynamic)
+    for (InputFile *file : inputFiles)
+      if (auto *bitcodeFile = dyn_cast<BitcodeFile>(file)) {
+        warn("the effect of -export_dynamic on LTO is not yet implemented");
+        break;
+      }
+
   TimeTraceScope timeScope("LTO");
   auto *lto = make<BitcodeCompiler>();
   for (InputFile *file : inputFiles)
     if (auto *bitcodeFile = dyn_cast<BitcodeFile>(file))
       lto->add(*bitcodeFile);
 
   for (ObjFile *file : lto->compile())
     inputFiles.insert(file);
config->ltoNewPassManager =
       args.hasFlag(OPT_no_lto_legacy_pass_manager, OPT_lto_legacy_pass_manager,
                    LLVM_ENABLE_NEW_PASS_MANAGER);
   config->ltoo = args::getInteger(args, OPT_lto_O, 2);
   if (config->ltoo > 3)
     error("--lto-O: invalid optimization level: " + Twine(config->ltoo));
   config->runtimePaths = args::getStrings(args, OPT_rpath);
   config->allLoad = args.hasArg(OPT_all_load);
   config->archMultiple = args.hasArg(OPT_arch_multiple);
+  config->exportDynamic = args.hasArg(OPT_export_dynamic);
   config->forceLoadObjC = args.hasArg(OPT_ObjC);
   config->forceLoadSwift = args.hasArg(OPT_force_load_swift_libs);
   config->deadStripDylibs = args.hasArg(OPT_dead_strip_dylibs);
   config->demangle = args.hasArg(OPT_demangle);
   config->implicitDylibs = !args.hasArg(OPT_no_implicit_dylibs);
   config->emitFunctionStarts =
       args.hasFlag(OPT_function_starts, OPT_no_function_starts, true);
   config->emitBitcodeBundle = args.hasArg(OPT_bitcode_bundle);

lld/MachO/LTO.cpp

for (const lto::InputFile::Symbol &objSym : objSyms) {
     // reports two symbols for module ASM defined. Without this check, lld
     // flags an undefined in IR with a definition in ASM as prevailing.
     // Once IRObjectFile is fixed to report only one symbol this hack can
     // be removed.
     r.Prevailing = !objSym.isUndefined() && sym->getFile() == &f;
 
     // FIXME: What about other output types? And we can probably be less
     // restrictive with -flat_namespace, but it's an infrequent use case.
+    // FIXME: Honor config->exportDynamic.
     r.VisibleToRegularObj = config->outputType != MH_EXECUTE ||
                             config->namespaceKind == NamespaceKind::flat ||
                             sym->isUsedInRegularObj;
 
     // Un-define the symbol so that we don't get duplicate symbol errors when we
     // load the ObjFile emitted by LTO compilation.
     if (r.Prevailing)
       replaceSymbol<Undefined>(sym, sym->getName(), sym->getFile(),

lld/MachO/MarkLive.cpp

if (auto *defined = dyn_cast<Defined>(sym)) {
         continue;
       }
 
       // FIXME: When we implement these flags, make symbols from them GC roots:
       // * -reexported_symbol(s_list)
       // * -alias(-list)
       // * -init
 
-      // In dylibs and bundles, all external functions are GC roots.
-      // FIXME: -export_dynamic should enable this for executables too.
-      if (config->outputType != MH_EXECUTE && !defined->privateExtern) {
+      // In dylibs and bundles and in executables with -export_dynamic,
+      // all external functions are GC roots.
+      bool externsAreRoots =
+          config->outputType != MH_EXECUTE || config->exportDynamic;
+      if (externsAreRoots && !defined->privateExtern) {
         addSym(defined);
         continue;
       }
     }
   }
   // -u symbols
   for (Symbol *sym : config->explicitUndefineds)
     if (auto *defined = dyn_cast<Defined>(sym))

lld/MachO/Options.td

def stack_size : Separate<["-"], "stack_size">,
     HelpText<"Maximum hex stack size for the main thread in a program. (default is 8MB)">,
     Flags<[HelpHidden]>,
     Group<grp_main>;
 def allow_stack_execute : Flag<["-"], "allow_stack_execute">,
     HelpText<"Mark stack segment as executable">,
     Flags<[HelpHidden]>,
     Group<grp_main>;
 def export_dynamic : Flag<["-"], "export_dynamic">,
-    HelpText<"Preserve all global symbols during LTO">,
-    Flags<[HelpHidden]>,
+    HelpText<"Preserve all global symbols during LTO and when dead-stripping executables">,
     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>">,
     Group<grp_bundle>;

lld/test/MachO/dead-strip.s

 # DYLIB-NEXT:   l {{.*}} _ref_from_no_dead_strip_local
 # DYLIB-NEXT:   l {{.*}} _ref_private_extern_u
 # DYLIB-NEXT:   l {{.*}} _ref_private_extern
 # DYLIB-NEXT:   g {{.*}} _ref_com
 # DYLIB-NEXT:   g {{.*}} _unref_com
 # DYLIB-NEXT:   g {{.*}} _unref_extern
 # DYLIB-NEXT:   g {{.*}} _no_dead_strip_globl
 
+## Extern symbols aren't stripped from executables with -export_dynamic
+# RUN: %lld -lSystem -dead_strip -export_dynamic -u _ref_private_extern_u \
+# RUN:     %t/basics.o -o %t/basics
+# RUN: llvm-objdump --syms --section-headers %t/basics | \
+# RUN:     FileCheck --check-prefix=EXECDYN %s
+# EXECDYN-LABEL: Sections:
+# EXECDYN-LABEL: Name
+# EXECDYN-NEXT:  __text
+# EXECDYN-NEXT:  __got
+# EXECDYN-NEXT:  __ref_section
+# EXECDYN-NEXT:  __common
+# EXECDYN-LABEL: SYMBOL TABLE:
+# EXECDYN-NEXT:   l {{.*}} _ref_data
+# EXECDYN-NEXT:   l {{.*}} _ref_local
+# EXECDYN-NEXT:   l {{.*}} _ref_from_no_dead_strip_globl
+# EXECDYN-NEXT:   l {{.*}} _no_dead_strip_local
+# EXECDYN-NEXT:   l {{.*}} _ref_from_no_dead_strip_local
+# EXECDYN-NEXT:   l {{.*}} _ref_private_extern_u
+# EXECDYN-NEXT:   l {{.*}} _main
+# EXECDYN-NEXT:   l {{.*}} _ref_private_extern
+# EXECDYN-NEXT:   g {{.*}} _ref_com
+# EXECDYN-NEXT:   g {{.*}} _unref_com
+# EXECDYN-NEXT:   g {{.*}} _unref_extern
+# EXECDYN-NEXT:   g {{.*}} _no_dead_strip_globl
+# EXECDYN-NEXT:   g {{.*}} __mh_execute_header
+
 ## Absolute symbol handling.
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-macos \
 # RUN:     %t/abs.s -o %t/abs.o
 # RUN: %lld -lSystem -dead_strip %t/abs.o -o %t/abs
 # RUN: llvm-objdump --macho --syms --exports-trie %t/abs | \
 # RUN:     FileCheck --check-prefix=ABS %s
 #ABS-LABEL: SYMBOL TABLE:
 #ABS-NEXT:   g {{.*}} _main