[llvm] c7ea209 - [ORC][COFF] Properly set weak flag to COMDAT symbols.

Sunho Kim via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 25 07:25:47 PDT 2022


Author: Sunho Kim
Date: 2022-07-25T23:24:25+09:00
New Revision: c7ea209068a7f0bcc84629ece24274db06c61878

URL: https://github.com/llvm/llvm-project/commit/c7ea209068a7f0bcc84629ece24274db06c61878
DIFF: https://github.com/llvm/llvm-project/commit/c7ea209068a7f0bcc84629ece24274db06c61878.diff

LOG: [ORC][COFF] Properly set weak flag to COMDAT symbols.

Properly set weak flag to COMDAT symbols so that no duplicate definition error will be generated. There is an inaccuracy in setting plain weak for largest selection type, which will be dealt with soon when largest type is properly implemented.

Reviewed By: lhames

Differential Revision: https://reviews.llvm.org/D129764

Added: 
    llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_weak_duplicate.s
    llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_weak_plus_strong.s
    llvm/test/ExecutionEngine/JITLink/X86/COFF_strong_duplicate.s
    llvm/test/ExecutionEngine/JITLink/X86/Inputs/COFF_comdat_weak_def.yaml
    llvm/test/ExecutionEngine/JITLink/X86/Inputs/COFF_strong_def.yaml

Modified: 
    llvm/lib/ExecutionEngine/Orc/ObjectFileInterface.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/ExecutionEngine/Orc/ObjectFileInterface.cpp b/llvm/lib/ExecutionEngine/Orc/ObjectFileInterface.cpp
index 356b81b4f1c56..3de15db3f1c66 100644
--- a/llvm/lib/ExecutionEngine/Orc/ObjectFileInterface.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/ObjectFileInterface.cpp
@@ -150,16 +150,39 @@ static Expected<MaterializationUnit::Interface>
 getCOFFObjectFileSymbolInfo(ExecutionSession &ES,
                             const object::COFFObjectFile &Obj) {
   MaterializationUnit::Interface I;
-
+  std::vector<Optional<object::coff_aux_section_definition>> ComdatDefs(
+      Obj.getNumberOfSections() + 1);
   for (auto &Sym : Obj.symbols()) {
     Expected<uint32_t> SymFlagsOrErr = Sym.getFlags();
     if (!SymFlagsOrErr)
       // TODO: Test this error.
       return SymFlagsOrErr.takeError();
 
-    // Skip symbols not defined in this object file.
-    if (*SymFlagsOrErr & object::BasicSymbolRef::SF_Undefined)
-      continue;
+    // Handle comdat symbols
+    auto COFFSym = Obj.getCOFFSymbol(Sym);
+    bool IsWeak = false;
+    if (auto *Def = COFFSym.getSectionDefinition()) {
+      auto Sec = Obj.getSection(COFFSym.getSectionNumber());
+      if (!Sec)
+        return Sec.takeError();
+      if (((*Sec)->Characteristics & COFF::IMAGE_SCN_LNK_COMDAT) &&
+          Def->Selection != COFF::IMAGE_COMDAT_SELECT_ASSOCIATIVE) {
+        ComdatDefs[COFFSym.getSectionNumber()] = *Def;
+        continue;
+      }
+    }
+    if (!COFF::isReservedSectionNumber(COFFSym.getSectionNumber()) &&
+        ComdatDefs[COFFSym.getSectionNumber()]) {
+      auto Def = ComdatDefs[COFFSym.getSectionNumber()];
+      if (Def->Selection != COFF::IMAGE_COMDAT_SELECT_NODUPLICATES) {
+        IsWeak = true;
+      }
+      ComdatDefs[COFFSym.getSectionNumber()] = None;
+    } else {
+      // Skip symbols not defined in this object file.
+      if (*SymFlagsOrErr & object::BasicSymbolRef::SF_Undefined)
+        continue;
+    }
 
     // Skip symbols that are not global.
     if (!(*SymFlagsOrErr & object::BasicSymbolRef::SF_Global))
@@ -180,12 +203,13 @@ getCOFFObjectFileSymbolInfo(ExecutionSession &ES,
     if (!SymFlags)
       return SymFlags.takeError();
     *SymFlags |= JITSymbolFlags::Exported;
-    auto COFFSym = Obj.getCOFFSymbol(Sym);
 
     // Weak external is always a function
-    if (COFFSym.isWeakExternal()) {
+    if (COFFSym.isWeakExternal())
       *SymFlags |= JITSymbolFlags::Callable;
-    }
+
+    if (IsWeak)
+      *SymFlags |= JITSymbolFlags::Weak;
 
     I.SymbolFlags[ES.intern(*Name)] = std::move(*SymFlags);
   }

diff  --git a/llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_weak_duplicate.s b/llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_weak_duplicate.s
new file mode 100644
index 0000000000000..6a0c0015ca729
--- /dev/null
+++ b/llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_weak_duplicate.s
@@ -0,0 +1,29 @@
+# RUN: rm -rf %t && mkdir -p %t
+# RUN: yaml2obj %S/Inputs/COFF_comdat_weak_def.yaml -o %t/COFF_weak_1.o
+# RUN: yaml2obj %S/Inputs/COFF_comdat_weak_def.yaml -o %t/COFF_weak_2.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-windows-msvc %s -o %t/COFF_main.o
+# RUN: 
+# RUN: llvm-jitlink -noexec %t/COFF_main.o %t/COFF_weak_1.o %t/COFF_weak_2.o \
+# RUN: -slab-allocate 100Kb -slab-address 0xfff00000 -slab-page-size 4096 \
+# RUN: -show-graph -noexec 2>&1 | FileCheck %s
+#
+# Check that duplicate comdat any definitions don't generate duplicate definition error.
+#
+# CHECK: section weakfunc:
+# CHECK-EMPTY:
+# CHECK-NEXT:  block 0xfff02000 size = 0x00000001, align = 16, alignment-offset = 0
+# CHECK-NEXT:    symbols:
+# CHECK-NEXT:      0xfff02000 (block + 0x00000000): size: 0x00000001, linkage: weak, scope: default, live  -   func
+# CHECK-NEXT:    no edges
+
+	.text
+
+	.def	main;
+	.scl	2;
+	.type	32;
+	.endef
+	.globl	main
+	.p2align	4, 0x90
+main:
+    callq func
+	retq
\ No newline at end of file

diff  --git a/llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_weak_plus_strong.s b/llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_weak_plus_strong.s
new file mode 100644
index 0000000000000..30e0193b11a65
--- /dev/null
+++ b/llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_weak_plus_strong.s
@@ -0,0 +1,32 @@
+# FIXME: Comdat any + ordinary strong symbol should generate duplicate section error
+# XFAIL: *
+# RUN: rm -rf %t && mkdir -p %t
+# RUN: yaml2obj %S/Inputs/COFF_comdat_weak_def.yaml -o %t/COFF_weak_1.o
+# RUN: yaml2obj %S/Inputs/COFF_strong_def.yaml -o %t/COFF_strong.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-windows-msvc %s -o %t/COFF_main.o
+# RUN: 
+# RUN: not llvm-jitlink -noexec %t/COFF_main.o %t/COFF_weak_1.o %t/COFF_strong.o \
+# RUN: -slab-allocate 100Kb -slab-address 0xfff00000 -slab-page-size 4096 \
+# RUN: -show-graph -noexec 2>&1 | FileCheck %s
+#
+# Check that a combination of comdat any definition and strong definition generate 
+# duplicate definition error.
+#
+# CHECK: section strongfunc:
+# CHECK-EMPTY:
+# CHECK-NEXT:  block 0xfff02000 size = 0x00000001, align = 16, alignment-offset = 0
+# CHECK-NEXT:    symbols:
+# CHECK-NEXT:      0xfff02000 (block + 0x00000000): size: 0x00000001, linkage: strong, scope: default, live  -   func
+# CHECK-NEXT:    no edges
+
+	.text
+
+	.def	main;
+	.scl	2;
+	.type	32;
+	.endef
+	.globl	main
+	.p2align	4, 0x90
+main:
+    callq func
+	retq
\ No newline at end of file

diff  --git a/llvm/test/ExecutionEngine/JITLink/X86/COFF_strong_duplicate.s b/llvm/test/ExecutionEngine/JITLink/X86/COFF_strong_duplicate.s
new file mode 100644
index 0000000000000..5ad9c68766617
--- /dev/null
+++ b/llvm/test/ExecutionEngine/JITLink/X86/COFF_strong_duplicate.s
@@ -0,0 +1,23 @@
+# RUN: rm -rf %t && mkdir -p %t
+# RUN: yaml2obj %S/Inputs/COFF_strong_def.yaml -o %t/COFF_strong_1.o
+# RUN: yaml2obj %S/Inputs/COFF_strong_def.yaml -o %t/COFF_strong_2.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-windows-msvc %s -o %t/COFF_main.o
+# RUN: 
+# RUN: not llvm-jitlink -noexec %t/COFF_main.o %t/COFF_strong_1.o %t/COFF_strong_2.o \
+# RUN: -slab-allocate 100Kb -slab-address 0xfff00000 -slab-page-size 4096 \
+# RUN: -show-graph
+#
+# Check that duplicate strong definitions cause llvm-jitlink to terminate with error.
+#
+
+	.text
+
+	.def	main;
+	.scl	2;
+	.type	32;
+	.endef
+	.globl	main
+	.p2align	4, 0x90
+main:
+    callq func
+	retq
\ No newline at end of file

diff  --git a/llvm/test/ExecutionEngine/JITLink/X86/Inputs/COFF_comdat_weak_def.yaml b/llvm/test/ExecutionEngine/JITLink/X86/Inputs/COFF_comdat_weak_def.yaml
new file mode 100644
index 0000000000000..c734cb2bf8d72
--- /dev/null
+++ b/llvm/test/ExecutionEngine/JITLink/X86/Inputs/COFF_comdat_weak_def.yaml
@@ -0,0 +1,30 @@
+--- !COFF
+header:
+  Machine:         IMAGE_FILE_MACHINE_AMD64
+  Characteristics: [  ]
+sections:
+  - Name:            weakfunc
+    Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_LNK_COMDAT, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+    Alignment:       16
+    SectionData:     C3
+symbols:
+  - Name:            weakfunc
+    Value:           0
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition:
+      Length:          1
+      NumberOfRelocations: 0
+      NumberOfLinenumbers: 0
+      CheckSum:        40735498
+      Number:          1
+      Selection:       IMAGE_COMDAT_SELECT_ANY
+  - Name:            func
+    Value:           0
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_FUNCTION
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+...

diff  --git a/llvm/test/ExecutionEngine/JITLink/X86/Inputs/COFF_strong_def.yaml b/llvm/test/ExecutionEngine/JITLink/X86/Inputs/COFF_strong_def.yaml
new file mode 100644
index 0000000000000..3ba49cd4a6f4f
--- /dev/null
+++ b/llvm/test/ExecutionEngine/JITLink/X86/Inputs/COFF_strong_def.yaml
@@ -0,0 +1,29 @@
+--- !COFF
+header:
+  Machine:         IMAGE_FILE_MACHINE_AMD64
+  Characteristics: [  ]
+sections:
+  - Name:            strongfunc
+    Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+    Alignment:       16
+    SectionData:     C3
+symbols:
+  - Name:            strongfunc
+    Value:           0
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition:
+      Length:          1
+      NumberOfRelocations: 0
+      NumberOfLinenumbers: 0
+      CheckSum:        40735498
+      Number:          1
+  - Name:            func
+    Value:           0
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_FUNCTION
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+...


        


More information about the llvm-commits mailing list