[PATCH] D129941: [JITLink][COFF] Implement IMAGE_COMDAT_SELECT_LARGEST.

[Sunho Kim via Phabricator via llvm-commits]

sunho created this revision.
sunho added reviewers: lhames, sgraenitz.
Herald added subscribers: StephenFan, hiraditya.
Herald added a project: All.
sunho requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Implement IMAGE_COMDAT_SELECT_LARGEST partially. It's going to fail if larger symbol appears but this hasn't happened at least in vcruntime library.

We probably would not implement this properly as it requires complicated runtime patching which is not of nature of JIT. However, we'd like to validate if larger section appears and report to the user in the near future.


https://reviews.llvm.org/D129941

Files:
  llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.cpp
  llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_largest.test


Index: llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_largest.test
===================================================================
--- llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_largest.test
+++ llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_largest.test
@@ -1,10 +1,14 @@
 # REQUIRES: asserts
-# XFAIL: *
 # RUN: yaml2obj %s -o %t
 # RUN: llvm-jitlink -abs __ImageBase=0xdeadbeaf -noexec --debug-only=jitlink -noexec %t 2>&1
 # 
-# Check jitlink return an error when IMAGE_COMDAT_SELECT_LARGEST selection type is encountered.
+# Check jitlink handles largest selection type as plain weak symbol.
 #
+# CHECK: Creating graph symbols...
+# CHECK:      2: Creating defined graph symbol for COFF symbol ".text" in .text (index: 2)
+# CHECK-NEXT:   0x0 (block + 0x00000000): size: 0x00000001, linkage: strong, scope: local, dead  -   <anonymous symbol>
+# CHECK-NEXT: 4: Exporting COMDAT graph symbol for COFF symbol "func" in section 2
+# CHECK-NEXT:   0x0 (block + 0x00000000): size: 0x00000001, linkage: weak, scope: default, dead  -   func
 
 --- !COFF
 header:
Index: llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.cpp
===================================================================
--- llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.cpp
+++ llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.cpp
@@ -482,10 +482,16 @@
     break;
   }
   case COFF::IMAGE_COMDAT_SELECT_LARGEST: {
-    // FIXME: Support IMAGE_COMDAT_SELECT_LARGEST when LinkGraph is able to
-    // handle this.
-    return make_error<JITLinkError>(
-        "IMAGE_COMDAT_SELECT_LARGEST is not supported.");
+    // FIXME: Support IMAGE_COMDAT_SELECT_LARGEST properly when LinkGraph is
+    // able to handle this.
+    LLVM_DEBUG({
+      dbgs() << "    " << SymIndex
+             << ": Partially supported IMAGE_COMDAT_SELECT_LARGEST was used"
+                " in section "
+             << Symbol.getSectionNumber() << "\n";
+    });
+    L = Linkage::Weak;
+    break;
   }
   case COFF::IMAGE_COMDAT_SELECT_NEWEST: {
     // Even link.exe doesn't support this selection properly.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D129941.445253.patch
Type: text/x-patch
Size: 2095 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220716/4af7585d/attachment.bin>