[lld-macho] Support common symbols in bitcode (but differently from ld64)

[int3]

Summary

ld64 seems to handle common symbols in bitcode rather
bizarrely. They follow entirely different precedence rules from their
non-bitcode counterparts. I initially tried to emulate ld64 in D106597,
but I'm not sure the extra complexity is worth it, especially given that
common symbols are not, well, very common.

This diff accords common bitcode symbols the same precedence as regular
common symbols, just as we treat all other pairs of bitcode and
non-bitcode symbol types. The tests document ld64's behavior in detail,
just in case we want to revisit this.

[thakis]

Makes sense to me. Nice tests!

lld/MachO/InputFiles.cpp

 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;
   }
 
+  if (objSym.isCommon())
+    return symtab->addCommon(name, &file, objSym.getCommonSize(),
+                             objSym.getCommonAlignment(), isPrivateExtern);
+
   return symtab->addDefined(name, &file, /*isec=*/nullptr, /*value=*/0,
                             /*size=*/0, objSym.isWeak(), isPrivateExtern,
                             /*isThumb=*/false,
                             /*isReferencedDynamically=*/false,
                             /*noDeadStrip=*/false);
 }
 
 BitcodeFile::BitcodeFile(MemoryBufferRef mb, StringRef archiveName,

lld/test/MachO/lto-common-symbol-coalescing.ll

+; REQUIRES: x86
+
+;; NOTE: We deviate significantly from ld64's behavior here. We treat common
+;; bitcode symbols like regular common symbols, but ld64 gives them different
+;; (and IMO very strange) precedence. This test documents the differences.
+
+; RUN: rm -rf %t; split-file %s %t
+; RUN: opt -module-summary %t/test.ll -o %t/test.o
+; RUN: opt -module-summary %t/same-size.ll -o %t/same-size.o
+; RUN: opt -module-summary %t/smaller-size.ll -o %t/smaller-size.o
+; RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/same-size.s -o %t/same-size-asm.o
+; RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/smaller-size.s -o %t/smaller-size-asm.o
+
+;; ld64: Common bitcode symbols all have equal precedence, regardless of size or
+;; alignment.
+;; lld: We pick the symbol with the larger size, regardless of alignment.
+; RUN: %lld -dylib %t/test.o %t/smaller-size.o -order_file %t/order -o %t/test
+; RUN: llvm-objdump --section-headers --syms %t/test | FileCheck %s -D#SIZE=2 -D#ALIGN=8
+; RUN: %lld -dylib %t/smaller-size.o %t/test.o -order_file %t/order -o %t/test
+; RUN: llvm-objdump --section-headers --syms %t/test | FileCheck %s -D#SIZE=2 -D#ALIGN=8
+; COM (ld64): llvm-objdump --section-headers --syms %t/test | FileCheck %s -D#SIZE=1 -D#ALIGN=16
+
+;; ld64: Common bitcode symbols all have equal precedence, regardless of size or
+;; alignment.
+;; lld: When the sizes are equal, we pick the symbol whose file occurs later in
+;; the command-line argument list.
+; RUN: %lld -dylib %t/test.o %t/same-size.o -order_file %t/order -o %t/test
+; RUN: llvm-objdump --section-headers --syms %t/test | FileCheck %s -D#SIZE=2 -D#ALIGN=16
+; COM (ld64): llvm-objdump --section-headers --syms %t/test | FileCheck %s -D#SIZE=2 -D#ALIGN=8
+; RUN: %lld -dylib %t/same-size.o %t/test.o -order_file %t/order -o %t/test
+; RUN: llvm-objdump --section-headers --syms %t/test | FileCheck %s -D#SIZE=2 -D#ALIGN=8
+
+;; ld64: Non-bitcode common symbols take precedence.
+;; lld: We pick the symbol with the larger size, regardless of alignment.
+; RUN: %lld -dylib %t/test.o %t/smaller-size-asm.o -order_file %t/order -o %t/test
+; RUN: llvm-objdump --section-headers --syms %t/test | FileCheck %s -D#SIZE=2 -D#ALIGN=8
+; COM (ld64): llvm-objdump --section-headers --syms %t/test | FileCheck %s -D#SIZE=1 -D#ALIGN=16
+; RUN: %lld -dylib %t/smaller-size-asm.o %t/test.o -order_file %t/order -o %t/test
+; RUN: llvm-objdump --section-headers --syms %t/test | FileCheck %s -D#SIZE=2 -D#ALIGN=8
+; COM (ld64): llvm-objdump --section-headers --syms %t/test | FileCheck %s -D#SIZE=1 -D#ALIGN=16
+
+; RUN: %lld -dylib %t/test.o %t/same-size-asm.o -order_file %t/order -o %t/test
+; RUN: llvm-objdump --section-headers --syms %t/test | FileCheck %s -D#SIZE=2 -D#ALIGN=16
+; RUN: %lld -dylib %t/same-size-asm.o %t/test.o -order_file %t/order -o %t/test
+; RUN: llvm-objdump --section-headers --syms %t/test | FileCheck %s -D#SIZE=2 -D#ALIGN=8
+; COM (ld64): llvm-objdump --section-headers --syms %t/test | FileCheck %s -D#SIZE=2 -D#ALIGN=16
+
+; CHECK-LABEL: Sections:
+; CHECK:       __common      {{[0-9a-f]+}} [[#%x, COMMON_START:]]  BSS
+;
+; CHECK-LABEL: SYMBOL TABLE:
+; CHECK-DAG:   [[#%.16x, COMMON_START]]         g     O __DATA,__common _check_size
+; CHECK-DAG:   [[#%.16x, COMMON_START + SIZE]]  g     O __DATA,__common _end_marker
+; CHECK-DAG:   [[#%.16x, COMMON_START + ALIGN]] g     O __DATA,__common _check_alignment
+
+;--- order
+;; Order is important as we determine the size of a given symbol via the
+;; address of the next symbol.
+_check_size
+_end_marker
+_check_alignment
+
+;--- smaller-size.ll
+target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-apple-macosx10.15.0"
+
+@check_size = common global i8 0, align 1
+@check_alignment = common global i8 0, align 16
+
+;--- same-size.ll
+target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-apple-macosx10.15.0"
+
+@check_size = common global i16 0, align 1
+@check_alignment = common global i16 0, align 16
+
+;--- smaller-size.s
+.comm _check_size, 1, 1
+.comm _check_alignment, 1, 4
+
+;--- same-size.s
+.comm _check_size, 2, 1
+.comm _check_alignment, 2, 4
+
+;--- test.ll
+target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-apple-macosx10.15.0"
+@check_size = common global i16 0, align 1
+@end_marker = common global i8 0
+@check_alignment = common global i16 0, align 8

lld/test/MachO/lto-common-symbol-resolution.ll

+; REQUIRES: x86
+
+;; NOTE: We deviate significantly from ld64's behavior here. We treat common
+;; bitcode symbols like regular common symbols, but ld64 gives them different
+;; (and IMO very strange) precedence. This test documents the differences.
+
+; RUN: rm -rf %t; split-file %s %t
+; RUN: opt -module-summary %t/common.ll -o %t/common.o
+; RUN: opt -module-summary %t/defined.ll -o %t/defined.o
+; RUN: opt -module-summary %t/weak-defined.ll -o %t/weak-defined.o
+; RUN: opt -module-summary %t/libfoo.ll -o %t/libfoo.o
+; RUN: opt -module-summary %t/refs-foo.ll -o %t/refs-foo.o
+; RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/weak-defined.s -o %t/weak-defined-asm.o
+
+; RUN: %lld -dylib -dylib %t/libfoo.o -o %t/libfoo.dylib
+
+; RUN: llvm-ar rcs %t/defined.a %t/defined.o
+; RUN: llvm-ar rcs %t/defined-and-common.a %t/defined.o %t/common.o
+; RUN: llvm-ar rcs %t/common-and-defined.a %t/common.o %t/defined.o
+; RUN: llvm-ar rcs %t/weak-defined-and-common.a %t/weak-defined.o %t/common.o
+; RUN: llvm-ar rcs %t/common-and-weak-defined.a %t/common.o %t/weak-defined.o
+
+;; Defined symbols take precedence over common bitcode symbols.
+; RUN: %lld -dylib %t/defined.o %t/common.o -o %t/test
+; RUN: llvm-objdump --syms %t/test | FileCheck %s --check-prefix=DEFINED
+; RUN: %lld -dylib %t/common.o %t/defined.o -o %t/test
+; RUN: llvm-objdump --syms %t/test | FileCheck %s --check-prefix=DEFINED
+
+;; Defined symbols have the same precedence as common bitcode symbols within
+;; an archive.
+; RUN: %lld -dylib %t/defined-and-common.a %t/refs-foo.o -o %t/refs-foo
+; RUN: llvm-objdump --syms %t/refs-foo | FileCheck %s --check-prefix=DEFINED
+; RUN: %lld -dylib %t/common-and-defined.a %t/refs-foo.o -o %t/refs-foo
+; RUN: llvm-objdump --syms %t/refs-foo | FileCheck %s --check-prefix=COMMON
+
+;; ld64: Weak bitcode symbols have the same precedence as common bitcode symbols.
+;; lld: Weak bitcode symbols take precedence over common bitcode symbols.
+; RUN: %lld -dylib %t/weak-defined.o %t/common.o -o %t/test
+; RUN: llvm-objdump --syms %t/test | FileCheck %s --check-prefix=WEAK-DEFINED
+; RUN: %lld -dylib %t/common.o %t/weak-defined.o -o %t/test
+; RUN: llvm-objdump --syms %t/test | FileCheck %s --check-prefix=WEAK-DEFINED
+
+;; Weak non-bitcode symbols take precedence over common bitcode symbols.
+; RUN: %lld -dylib %t/weak-defined-asm.o %t/common.o -o %t/test
+; RUN: llvm-objdump --syms %t/test | FileCheck %s --check-prefix=WEAK-DEFINED
+; RUN: %lld -dylib %t/common.o %t/weak-defined-asm.o -o %t/test
+; RUN: llvm-objdump --syms %t/test | FileCheck %s --check-prefix=WEAK-DEFINED
+
+;; ld64: Archive symbols take precedence over common bitcode symbols.
+;; lld: Common bitcode symbols take precedence over archive symbols.
+; RUN: %lld -dylib %t/defined.a %t/common.o -o %t/test
+; RUN: llvm-objdump --syms %t/test | FileCheck %s --check-prefix=COMMON
+; COM (ld64): llvm-objdump --syms %t/test | FileCheck %s --check-prefix=DEFINED
+; RUN: %lld -dylib %t/common.o %t/defined.a -o %t/test
+; RUN: llvm-objdump --syms %t/test | FileCheck %s --check-prefix=COMMON
+; COM (ld64): llvm-objdump --syms %t/test | FileCheck %s --check-prefix=DEFINED
+
+;; ld64: Dylib symbols take precedence over common bitcode symbols.
+;; lld: Common bitcode symbols take precedence over dylib symbols.
+; RUN: %lld -dylib %t/libfoo.dylib %t/common.o %t/refs-foo.o -o %t/test
+; RUN: llvm-objdump --syms %t/test | FileCheck %s --check-prefix=COMMON
+; COM (ld64): llvm-objdump --syms %t/test | FileCheck %s --check-prefix=DYLIB
+; RUN: %lld -dylib %t/common.o %t/libfoo.dylib %t/refs-foo.o -o %t/test
+; RUN: llvm-objdump --syms %t/test | FileCheck %s --check-prefix=COMMON
+; COM (ld64): llvm-objdump --syms %t/test | FileCheck %s --check-prefix=DYLIB
+
+; COMMON:       g     O __DATA,__common _foo
+; DEFINED:      g     O __DATA,__data _foo
+; WEAK-DEFINED: w     O __DATA,__data _foo
+; DYLIB:        *UND* _foo
+
+;--- common.ll
+target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-apple-macosx10.15.0"
+
+@foo = common global i8 0
+
+;--- defined.ll
+target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-apple-macosx10.15.0"
+
+@foo = global i8 12
+
+;--- weak-defined.ll
+target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-apple-macosx10.15.0"
+
+@foo = weak global i8 12
+
+;--- weak-defined.s
+.globl _foo
+.weak_definition _foo
+.data
+_foo:
+
+;--- libfoo.ll
+target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-apple-macosx10.15.0"
+
+@foo = common global i8 0
+
+;--- refs-foo.ll
+target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-apple-macosx10.15.0"
+
+@foo = external global i8
+
+define void @f() {
+  %1 = load i8, i8* @foo
+  ret void
+}