[ORC][JITLink] Treat common symbols as weak definitions.

Duplicate common definitions should be coaleseced, rather than being treated as
duplicate definitions. Strong definitions should override common definitions.

rdar://132314264

Author: lhames

llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.cpp

@@ -467,7 +467,7 @@ Expected<Symbol *> COFFLinkGraphBuilder::createDefinedSymbol(
     return &G->addDefinedSymbol(
         G->createZeroFillBlock(getCommonSection(), Symbol.getValue(),
                                orc::ExecutorAddr(), Symbol.getValue(), 0),
-        0, SymbolName, Symbol.getValue(), Linkage::Strong, Scope::Default,
+        0, SymbolName, Symbol.getValue(), Linkage::Weak, Scope::Default,
         false, false);
   }
   if (Symbol.isAbsolute())

llvm/lib/ExecutionEngine/JITLink/ELFLinkGraphBuilder.h

@@ -472,7 +472,7 @@ template <typename ELFT> Error ELFLinkGraphBuilder<ELFT>::graphifySymbols() {
       Symbol &GSym = G->addDefinedSymbol(
           G->createZeroFillBlock(getCommonSection(), Sym.st_size,
                                  orc::ExecutorAddr(), Sym.getValue(), 0),
-          0, *Name, Sym.st_size, Linkage::Strong, Scope::Default, false, false);
+          0, *Name, Sym.st_size, Linkage::Weak, Scope::Default, false, false);
       setGraphSymbol(SymIndex, GSym);
       continue;
     }

llvm/lib/ExecutionEngine/JITLink/MachOLinkGraphBuilder.cpp

@@ -366,7 +366,7 @@ Error MachOLinkGraphBuilder::graphifyRegularSymbols() {
                                    orc::ExecutorAddrDiff(NSym.Value),
                                    orc::ExecutorAddr(),
                                    1ull << MachO::GET_COMM_ALIGN(NSym.Desc), 0),
-            0, *NSym.Name, orc::ExecutorAddrDiff(NSym.Value), Linkage::Strong,
+            0, *NSym.Name, orc::ExecutorAddrDiff(NSym.Value), Linkage::Weak,
             NSym.S, false, NSym.Desc & MachO::N_NO_DEAD_STRIP);
       } else {
         if (!NSym.Name)

llvm/lib/ExecutionEngine/Orc/Core.cpp

@@ -932,13 +932,17 @@ Error JITDylib::resolve(MaterializationResponsibility &MR,
           if (SymI->second.getFlags().hasError())
             SymbolsInErrorState.insert(KV.first);
           else {
-            auto Flags = KV.second.getFlags();
-            Flags &= ~JITSymbolFlags::Common;
-            assert(Flags ==
-                       (SymI->second.getFlags() & ~JITSymbolFlags::Common) &&
+            auto ExpectedFlags = SymI->second.getFlags();
+            if (ExpectedFlags & JITSymbolFlags::Common) {
+              ExpectedFlags &= ~JITSymbolFlags::Common;
+              ExpectedFlags |= JITSymbolFlags::Weak;
+            }
+
+            assert(KV.second.getFlags() == ExpectedFlags &&
                    "Resolved flags should match the declared flags");
 
-            Worklist.push_back({SymI, {KV.second.getAddress(), Flags}});
+            Worklist.push_back(
+                {SymI, {KV.second.getAddress(), SymI->second.getFlags()}});
           }
         }
 
@@ -2899,8 +2903,12 @@ Error ExecutionSession::OL_notifyResolved(MaterializationResponsibility &MR,
            "Resolving symbol outside this responsibility set");
     assert(!I->second.hasMaterializationSideEffectsOnly() &&
            "Can't resolve materialization-side-effects-only symbol");
-    assert((KV.second.getFlags() & ~JITSymbolFlags::Common) ==
-               (I->second & ~JITSymbolFlags::Common) &&
+    auto ExpectedFlags = I->second;
+    if (ExpectedFlags & JITSymbolFlags::Common) {
+      ExpectedFlags &= ~JITSymbolFlags::Common;
+      ExpectedFlags |= JITSymbolFlags::Weak;
+    }
+    assert(KV.second.getFlags() == ExpectedFlags &&
            "Resolving symbol with incorrect flags");
   }
 #endif

llvm/test/ExecutionEngine/JITLink/AArch64/Inputs/MachO_common_x_and_addr_getter.s

@@ -0,0 +1,10 @@
+	.section	__TEXT,__text,regular,pure_instructions
+	.globl	_getXAddr
+	.p2align	2
+_getXAddr:
+	adrp	x0, _x@GOTPAGE
+	ldr	x0, [x0, _x@GOTPAGEOFF]
+	ret
+
+	.comm	_x,4,2
+.subsections_via_symbols

llvm/test/ExecutionEngine/JITLink/AArch64/Inputs/MachO_strong_x_and_addr_getter.s

@@ -0,0 +1,15 @@
+	.section	__TEXT,__text,regular,pure_instructions
+	.globl	_getXAddr
+	.p2align	2
+_getXAddr:
+	adrp	x0, _x@PAGE
+	add	x0, x0, _x@PAGEOFF
+	ret
+
+	.section	__DATA,__data
+	.globl	_x
+	.p2align	2, 0x0
+_x:
+	.long	42
+
+.subsections_via_symbols

llvm/test/ExecutionEngine/JITLink/AArch64/MachO_common_symbol_x_multiple_defs.s

@@ -0,0 +1,33 @@
+# RUN: rm -rf %t && mkdir -p %t
+# RUN: llvm-mc -triple=arm64-apple-darwin19 -filetype=obj -o %t/main.o %s
+# RUN: llvm-mc -triple=arm64-apple-darwin19 -filetype=obj -o %t/aux_common.o \
+# RUN:     %S/Inputs/MachO_common_x_and_addr_getter.s
+# RUN: llvm-mc -triple=arm64-apple-darwin19 -filetype=obj -o %t/aux_strong.o \
+# RUN:     %S/Inputs/MachO_strong_x_and_addr_getter.s
+# RUN: llvm-jitlink -noexec %t/main.o %t/aux_common.o
+# RUN: llvm-jitlink -noexec -check=%s %t/main.o %t/aux_strong.o
+#
+# Check that linking multiple common definitions of the same symbol (in this
+# case _x) doesn't lead to an "Unexpected definitions" error.
+#
+# rdar://132314264
+#
+# Check that strong defs override:
+# jitlink-check: *{8}_x = 42
+
+	.section	__TEXT,__text,regular,pure_instructions
+	.globl	_main
+	.p2align	2
+_main:
+	stp	x29, x30, [sp, #-16]!
+	mov	x29, sp
+	bl	_getXAddr
+	adrp	x8, _x@GOTPAGE
+	ldr	x8, [x8, _x@GOTPAGEOFF]
+	cmp	x0, x8
+	cset	w0, eq
+	ldp	x29, x30, [sp], #16
+	ret
+
+	.comm	_x,4,2
+.subsections_via_symbols

llvm/test/ExecutionEngine/JITLink/x86-64/COFF_common_symbol.s

@@ -6,7 +6,7 @@
 #
 # CHECK: Creating graph symbols...
 # CHECK:      7: Creating defined graph symbol for COFF symbol "var" in (common) (index: 0)
-# CHECK-NEXT:   0x0 (block + 0x00000000): size: 0x00000004, linkage: strong, scope: default, dead  -   var
+# CHECK-NEXT:   0x0 (block + 0x00000000): size: 0x00000004, linkage: weak, scope: default, dead  -   var
 
 	.text