[lld/mac] reject -undefined warning and -undefined suppress with -twolevel_namespace

[thakis]

Summary

-flat_namespace isn't implemented yet, and neither is -undefined dynamic,
so this makes -undefined pretty pointless in lld/MachO for now. But once
we implement -flat_namespace (which we need to do anyways to get check-llvm
to pass with lld as host linker), the code's already there.

Follow-up to https://reviews.llvm.org/D93263#2491865

[int3]

Thanks for the cleanup!

lld/test/MachO/treat-undef-sym.s

25

nit: can we use hyphens instead of underscores so we don't have mixed separators?

[thakis]

Thanks!

lld/test/MachO/treat-undef-sym.s

25

Sure, done.

(I like having them mixed since it makes it clearer that "-NEXT" is some magic lit suffix while "INVAL_WARNING" is just some arbitrary string.)

lld/MachO/Config.h

 struct SymbolPriorityEntry;
 
 struct PlatformInfo {
   llvm::MachO::PlatformKind kind;
   llvm::VersionTuple minimum;
   llvm::VersionTuple sdk;
 };
 
+enum class NamespaceKind {
+  twolevel,
+  flat,
+};
+
 enum class UndefinedSymbolTreatment {
   unknown,
   error,
   warning,
   suppress,
   dynamic_lookup,
 };
 
struct Configuration {
   uint32_t dylibCompatibilityVersion = 0;
   uint32_t dylibCurrentVersion = 0;
   llvm::StringRef installName;
   llvm::StringRef outputFile;
   llvm::StringRef ltoObjPath;
   bool demangle = false;
   llvm::MachO::Architecture arch;
   PlatformInfo platform;
+  NamespaceKind namespaceKind = NamespaceKind::twolevel;
   UndefinedSymbolTreatment undefinedSymbolTreatment =
       UndefinedSymbolTreatment::error;
   llvm::MachO::HeaderFileType outputType;
   std::vector<llvm::StringRef> systemLibraryRoots;
   std::vector<llvm::StringRef> librarySearchPaths;
   std::vector<llvm::StringRef> frameworkSearchPaths;
   std::vector<llvm::StringRef> runtimePaths;
   std::vector<Symbol *> explicitUndefineds;

lld/MachO/Driver.cpp

static void handlePlatformVersion(const opt::Arg *arg) {
   if (config->platform.minimum.tryParse(minVersionStr))
     error(Twine("malformed minimum version: ") + minVersionStr);
   if (config->platform.sdk.tryParse(sdkVersionStr))
     error(Twine("malformed sdk version: ") + sdkVersionStr);
 }
 
 static void handleUndefined(const opt::Arg *arg) {
   StringRef treatmentStr = arg->getValue(0);
-  config->undefinedSymbolTreatment =
+  auto treatment =
       StringSwitch<UndefinedSymbolTreatment>(treatmentStr)
           .Case("error", UndefinedSymbolTreatment::error)
           .Case("warning", UndefinedSymbolTreatment::warning)
           .Case("suppress", UndefinedSymbolTreatment::suppress)
           .Case("dynamic_lookup", UndefinedSymbolTreatment::dynamic_lookup)
           .Default(UndefinedSymbolTreatment::unknown);
-  if (config->undefinedSymbolTreatment == UndefinedSymbolTreatment::unknown) {
+  if (treatment == UndefinedSymbolTreatment::unknown) {
     warn(Twine("unknown -undefined TREATMENT '") + treatmentStr +
          "', defaulting to 'error'");
-    config->undefinedSymbolTreatment = UndefinedSymbolTreatment::error;
+    treatment = UndefinedSymbolTreatment::error;
+  } else if (config->namespaceKind == NamespaceKind::twolevel &&
+             (treatment == UndefinedSymbolTreatment::warning ||
+              treatment == UndefinedSymbolTreatment::suppress)) {
+    if (treatment == UndefinedSymbolTreatment::warning)
+      error("'-undefined warning' only valid with '-flat_namespace'");
+    else
+      error("'-undefined suppress' only valid with '-flat_namespace'");
+    treatment = UndefinedSymbolTreatment::error;
   }
+  config->undefinedSymbolTreatment = treatment;
 }
 
 static void warnIfDeprecatedOption(const opt::Option &opt) {
   if (!opt.getGroup().isValid())
     return;
   if (opt.getGroup().getID() == OPT_grp_deprecated) {
     warn("Option `" + opt.getPrefixedName() + "' is deprecated in ld64:");
     warn(opt.getHelpText());
bool macho::link(ArrayRef<const char *> argsArr, bool canExitEarly,
   config->allLoad = args.hasArg(OPT_all_load);
   config->forceLoadObjC = args.hasArg(OPT_ObjC);
   config->demangle = args.hasArg(OPT_demangle);
   config->implicitDylibs = !args.hasArg(OPT_no_implicit_dylibs);
 
   if (const opt::Arg *arg = args.getLastArg(OPT_static, OPT_dynamic))
     config->staticLink = (arg->getOption().getID() == OPT_static);
 
+  if (const opt::Arg *arg =
+          args.getLastArg(OPT_flat_namespace, OPT_twolevel_namespace)) {
+    config->namespaceKind = arg->getOption().getID() == OPT_twolevel_namespace
+                                ? NamespaceKind::twolevel
+                                : NamespaceKind::flat;
+    if (config->namespaceKind == NamespaceKind::flat) {
+      warn("Option '" + arg->getOption().getPrefixedName() +
+           "' is not yet implemented. Stay tuned...");
+      config->namespaceKind = NamespaceKind::twolevel;
+    }
+  }
+
   config->systemLibraryRoots = getSystemLibraryRoots(args);
   config->librarySearchPaths =
       getLibrarySearchPaths(args, config->systemLibraryRoots);
   config->frameworkSearchPaths =
       getFrameworkSearchPaths(args, config->systemLibraryRoots);
   if (const opt::Arg *arg =
           args.getLastArg(OPT_search_paths_first, OPT_search_dylibs_first))
     config->searchDylibsFirst =

lld/MachO/Options.td

def alias : MultiArg<["-"], "alias", 2>,
      Flags<[HelpHidden]>,
      Group<grp_resolve>;
 def alias_list : Separate<["-"], "alias_list">,
      MetaVarName<"<file>">,
      HelpText<"Create symbol aliases specified in <file>">,
      Flags<[HelpHidden]>,
      Group<grp_resolve>;
 def flat_namespace : Flag<["-"], "flat_namespace">,
-     HelpText<"Resolve symbols from all dylibs, both direct & transitive. Do not record source libraries: dyld must re-search at runtime and use the first definition found">,
+     HelpText<"Resolve symbols from all dylibs, both direct and transitive. Do not record source libraries: dyld must re-search at runtime and use the first definition found">,
      Flags<[HelpHidden]>,
      Group<grp_resolve>;
+def twolevel_namespace : Flag<["-"], "twolevel_namespace">,
+     HelpText<"Make dyld look up symbols by (dylib,name) pairs (default)">,
+     Group<grp_resolve>;
 def u : Separate<["-"], "u">,
      MetaVarName<"<symbol>">,
      HelpText<"Require that <symbol> be defined for the link to succeed">,
      Group<grp_resolve>;
 def U : Separate<["-"], "U">,
      MetaVarName<"<symbol>">,
      HelpText<"Allow <symbol> to have no definition">,
      Flags<[HelpHidden]>,

 def text_exec : Flag<["-"], "text_exec">,
      HelpText<"This option is undocumented in ld64">,
      Flags<[HelpHidden]>,
      Group<grp_undocumented>;
 def threaded_starts_section : Flag<["-"], "threaded_starts_section">,
      HelpText<"This option is undocumented in ld64">,
      Flags<[HelpHidden]>,
      Group<grp_undocumented>;
-def twolevel_namespace : Flag<["-"], "twolevel_namespace">,
-     HelpText<"This option is undocumented in ld64">,
-     Flags<[HelpHidden]>,
-     Group<grp_undocumented>;
 def verbose_optimization_hints : Flag<["-"], "verbose_optimization_hints">,
      HelpText<"This option is undocumented in ld64">,
      Flags<[HelpHidden]>,
      Group<grp_undocumented>;
 def version_load_command : Flag<["-"], "version_load_command">,
      HelpText<"This option is undocumented in ld64">,
      Flags<[HelpHidden]>,
      Group<grp_undocumented>;

lld/test/MachO/treat-undef-sym.s

 # REQUIRES: x86
 
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-macos %s -o %t.o
 # RUN: not %lld -undefined bogus -o /dev/null %t.o 2>&1 | \
 # RUN:     FileCheck %s -check-prefix=UNKNOWN
 # RUN: not %lld -undefined error -o /dev/null %t.o 2>&1 | \
 # RUN:     FileCheck %s -check-prefix=ERROR
-# RUN:     %no_fatal_warnings_lld -undefined warning -o /dev/null %t.o 2>&1 | \
-# RUN:     FileCheck %s -check-prefix=WARNING
-# RUN:     %lld -undefined suppress -o /dev/null %t.o 2>&1 | \
-# RUN:     FileCheck %s -check-prefix=SUPPRESS --allow-empty
+
+# RUN: not %lld -undefined warning -o /dev/null %t.o 2>&1 | \
+# RUN:     FileCheck %s -check-prefix=INVAL-WARNING
+# RUN: not %lld -undefined suppress -o /dev/null %t.o 2>&1 | \
+# RUN:     FileCheck %s -check-prefix=INVAL-SUPPRESS
+
+# FIXME: Enable these -undefined checks once -flat_namespace is implemented.
+# RN: %no_fatal_warnings_lld -flat_namespace -undefined warning \
+# RN:     -o /dev/null %t.o 2>&1 | \
+# RN:     FileCheck %s -check-prefix=WARNING
+# RN: %lld -flat_namespace -undefined suppress -o /dev/null %t.o 2>&1 | \
+# RN:     FileCheck %s -check-prefix=SUPPRESS --allow-empty
 
 # ERROR: error: undefined symbol: _bar
 # ERROR-NEXT: >>> referenced by
 
+# INVAL-WARNING: error: '-undefined warning' only valid with '-flat_namespace'
+# INVAL-WARNING-NEXT: error: undefined symbol: _bar
+
+# INVAL-SUPPRESS: error: '-undefined suppress' only valid with '-flat_namespace'
+# INVAL-SUPPRESS-NEXT: error: undefined symbol: _bar
+
+
 # WARNING: warning: undefined symbol: _bar
 # WARNING-NEXT: >>> referenced by
 
 # SUPPRESS-NOT: undefined symbol: _bar
 
 # UNKNOWN: unknown -undefined TREATMENT 'bogus'
 # UNKNOWN-NEXT: error: undefined symbol: _bar
 # UNKNOWN-NEXT: >>> referenced by
 
 .globl _main
 _main:
   callq _bar
   ret