[lld-macho] Basic support for linkage and visibility attributes in LTO

[int3]

Summary

When parsing bitcode, convert LTO Symbols to LLD Symbols in order to perform
resolution. The "winning" symbol will then be marked as Prevailing at LTO
compilation time. This is similar to what the other LLD ports do.

This change allows us to handle linkonce symbols correctly, and to deal with
duplicate bitcode symbols gracefully. Previously, both scenarios would result in
an assertion failure inside the LTO code, complaining that multiple Prevailing
definitions are not allowed.

While at it, I also added basic logic around visibility. We don't do anything
useful with it yet, but we do check that its value is valid. LLD-ELF appears to
use it only to set FinalDefinitionInLinkageUnit for LTO, which I think is just a
performance optimization.

From my local experimentation, the linker itself doesn't seem to do anything
differently when encountering linkonce / linkonce_odr / weak / weak_odr. So I've
only written a test for one of them. LLD-ELF has more, but they seem to mostly
be testing the intermediate bitcode output of their LTO backend...? I'm far from
an expert here though, so I might very well be missing things.

[int3]

rebase with stacked diff

[tejohnson]

The change to select the prevailing seems reasonable to me, but I'm not terribly familiar with some of the lld internals that this touches. Probably best reviewed by @pcc who implemented some of the ELF lld/ELF support, or @MaskRay.

[int3]

@pcc, @MaskRay, would you like to have a look before I land this?

[MaskRay]

While at it, I also added basic logic around visibility. We don't do anything useful with it yet, but we do check that its value is valid. LLD-ELF appears to use it only to set FinalDefinitionInLinkageUnit for LTO, which I think is just a performance optimization.

D92900 may have some interesting tests.

[smeenai]

LGTM

[int3]

@MaskRay thanks for the pointer! I'll try and crib from those tests when I address FinalDefinitionInLinkageUnit in a future diff.

lld/MachO/InputFiles.cpp

void ArchiveFile::fetch(const object::Archive::Symbol &sym) {
   }
   inputFiles.insert(file);
 
   // ld64 doesn't demangle sym here even with -demangle. Match that, so
   // intentionally no call to toMachOString() here.
   printArchiveMemberLoad(sym_copy.getName(), file);
 }
 
+static macho::Symbol *createBitcodeSymbol(const lto::InputFile::Symbol &objSym,
+                                          BitcodeFile &file) {
+  StringRef name = saver.save(objSym.getName());
+
+  // TODO: support weak references
+  if (objSym.isUndefined())
+    return symtab->addUndefined(name, &file, /*isWeakRef=*/false);
+
+  assert(!objSym.isCommon() && "TODO: support common symbols in LTO");
+
+  // TODO: Write a test demonstrating why computing isPrivateExtern before
+  // LTO compilation is important.
+  bool isPrivateExtern = false;
+  switch (objSym.getVisibility()) {
+  case GlobalValue::HiddenVisibility:
+    isPrivateExtern = true;
+    break;
+  case GlobalValue::ProtectedVisibility:
+    error(name + " has protected visibility, which is not supported by Mach-O");
+    break;
+  case GlobalValue::DefaultVisibility:
+    break;
+  }
+
+  return symtab->addDefined(name, &file, /*isec=*/nullptr, /*value=*/0,
+                            objSym.isWeak(), isPrivateExtern);
+}
+
 BitcodeFile::BitcodeFile(MemoryBufferRef mbref)
     : InputFile(BitcodeKind, mbref) {
   obj = check(lto::InputFile::create(mbref));
+
+  // Convert LTO Symbols to LLD Symbols in order to perform resolution. The
+  // "winning" symbol will then be marked as Prevailing at LTO compilation
+  // time.
+  for (const lto::InputFile::Symbol &objSym : obj->symbols())
+    symbols.push_back(createBitcodeSymbol(objSym, *this));
 }

lld/MachO/LTO.cpp

 //===- LTO.cpp ------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "LTO.h"
 #include "Config.h"
 #include "Driver.h"
 #include "InputFiles.h"
+#include "Symbols.h"
 
 #include "lld/Common/ErrorHandler.h"
 #include "lld/Common/Strings.h"
 #include "lld/Common/TargetOptionsCommandFlags.h"
 #include "llvm/LTO/LTO.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"

 }
 
 void BitcodeCompiler::add(BitcodeFile &f) {
   ArrayRef<lto::InputFile::Symbol> objSyms = f.obj->symbols();
   std::vector<lto::SymbolResolution> resols;
   resols.reserve(objSyms.size());
 
   // Provide a resolution to the LTO API for each symbol.
+  auto symIt = f.symbols.begin();
   for (const lto::InputFile::Symbol &objSym : objSyms) {
     resols.emplace_back();
     lto::SymbolResolution &r = resols.back();
+    Symbol *sym = *symIt++;
 
     // Ideally we shouldn't check for SF_Undefined but currently IRObjectFile
     // 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();
+    r.Prevailing = !objSym.isUndefined() && sym->getFile() == &f;
+
+    // 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(),
+                               RefState::Strong);
 
     // TODO: set the other resolution configs properly
     r.VisibleToRegularObj = true;
   }
   checkError(ltoObj->add(std::move(f.obj), resols));
 }
 
 // Merge all the bitcode files we have seen, codegen the result

lld/test/MachO/invalid/duplicate-symbol.ll

+; REQUIRES: x86
+; RUN: rm -rf %t; mkdir -p %t
+; RUN: opt -module-summary %s -o %t/first.o
+; RUN: opt -module-summary %s -o %t/second.o
+; RUN: not %lld -dylib -lSystem %t/first.o %t/second.o -o /dev/null 2>&1 | FileCheck %s
+; CHECK:      error: duplicate symbol: _foo
+; CHECK-NEXT: >>> defined in {{.*}}/first.o
+; CHECK-NEXT: >>> defined in {{.*}}/second.o
+
+target triple = "x86_64-apple-macosx10.15.0"
+target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+
+define void @foo() {
+  ret void
+}

lld/test/MachO/invalid/protected.ll

+; REQUIRES: x86
+; RUN: opt -module-summary %s -o %t.o
+; RUN: not %lld -dylib -lSystem %t.o -o /dev/null 2>&1 | FileCheck %s
+; CHECK: error: _foo has protected visibility, which is not supported by Mach-O
+
+target triple = "x86_64-apple-macosx10.15.0"
+target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+
+define protected void @foo() {
+  ret void
+}

lld/test/MachO/linkonce.ll

+; REQUIRES: x86
+; RUN: rm -rf %t; split-file %s %t
+; RUN: opt -module-summary %t/first.ll -o %t/first.o
+; RUN: opt -module-summary %t/second.ll -o %t/second.o
+; RUN: %lld -dylib -lSystem %t/first.o %t/second.o -o %t/12
+; RUN: llvm-objdump --syms %t/12 | FileCheck %s --check-prefix=FIRST
+; RUN: %lld -dylib -lSystem %t/second.o %t/first.o -o %t/21
+; RUN: llvm-objdump --syms %t/21 | FileCheck %s --check-prefix=SECOND
+
+; FIRST:  w    O __TEXT,first  _foo
+; SECOND: w    O __TEXT,second _foo
+
+#--- first.ll
+
+target triple = "x86_64-apple-macosx10.15.0"
+target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+
+define linkonce void @foo() section "__TEXT,first" {
+  ret void
+}
+
+#--- second.ll
+
+target triple = "x86_64-apple-macosx10.15.0"
+target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+
+define linkonce void @foo() section "__TEXT,second" {
+  ret void
+}