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

Sun Jul 17 11:36:02 PDT 2022

sgraenitz added a comment.

Thanks!

================
Comment at: llvm/lib/ExecutionEngine/Orc/ObjectFileInterface.cpp:165
+    if (auto *Def = COFFSym.getSectionDefinition()) {
+      auto Sec = cantFail(Obj.getSection(COFFSym.getSectionNumber()));
+      if ((Sec->Characteristics & COFF::IMAGE_SCN_LNK_COMDAT) &&
----------------
I guess this could indeed fail for malformed objects. The referred section might not exist, right?

================
Comment at: llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_weak_plus_strong.s:11
+# Check that a combination of comdat any definition and strong definition don't generate 
+# duplicate definition error.
+#
----------------
I never double-checked this with the Mircosoft toolchain. Did you?

================
Comment at: llvm/test/ExecutionEngine/JITLink/X86/COFF_strong_duplicate.s:9
+# RUN: -slab-allocate 100Kb -slab-address 0xfff00000 -slab-page-size 4096 \
+# RUN: -show-graph -noexec 2>&1 | FileCheck %s
+#
----------------
Can we use `# RUN: not llvm-jitlink ...` to check that we get a non-zero exit code instead of XFAILing the test? My impression was that XFAIL should be used specifically to mark bugs and todos.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129764/new/

https://reviews.llvm.org/D129764