Bug 21886 - MCJIT/ELF now support MSVC C++ mangled symbol

[belleyb]

Summary

http://llvm.org/bugs/show_bug.cgi?id=21886

The ELF format is used on Windows by the MCJIT engine. Thus, on Windows, the ELFObjectWriter can encounter symbols mangled
using the MS Visual Studio C++ name mangling. Symbols mangled using the MSVC C++ name mangling can legally have "@@@" as a substring. The EFLObjectWriter should not interpret the "@@@" substring as specifying GNU-style symbol versioning. The ELFObjectWriter therefore check for the MSVC C++ name mangling prefix which is either "?", "@?", "imp_?" or "imp_?@".

[belleyb]

Let me know if your agree with the proposed solution or if an other approach would be more appropriate. I am willing to implement any suggested changes,

[belleyb]

ping

[• rafael]

The ELF format itself only requires names to be null terminated.

The GNU as syntax is what gives special meaning to @@@.

The fact that both issues are mixed in ELFObjectWriter is my fault and I am sorry. This should really have been handled outside of the ELF writer.

Changing it will not be simple, so this is fine for now. Please just add a FIXME saying that all name handling should be done before we get to the writer.

[• rafael]

Do you have commit access?

[belleyb]

Added a FIXME notice as suggested by Rafael.

[belleyb]

No, I don't have direct commit access to the llvm repository. Would "arc land" work once the revision is approved ? (I'll be watching the build bots afterward...)

Thanks for the review! :-)

If you give me a few hints regarding the proper place to put the GNU-style symbol versioning replacement, I might take a stab at it as a follow-up to this patch.

[• rafael]

clang-formatted and committed as r226830.

[belleyb]

Committed as http://llvm.org/viewvc/llvm-project?rev=226830&view=rev

[eugenis]

I think you broke this:
https://llvm.org/bugs/show_bug.cgi?id=23914

lib/MC/ELFObjectWriter.cpp

if (!BaseSymbol) {
         MCELF::SetBinding(SD, ELF::STB_WEAK);
     } else {
       const MCSectionELF &Section =
         static_cast<const MCSectionELF&>(BaseSymbol->getSection());
       MSD.SectionIndex = SectionIndexMap.lookup(&Section);
       assert(MSD.SectionIndex && "Invalid section index!");
     }
 
-    // The @@@ in symbol version is replaced with @ in undefined symbols and
-    // @@ in defined ones.
+    // The @@@ in symbol version is replaced with @ in undefined symbols and @@
+    // in defined ones.
+    //
+    // FIXME: All name handling should be done before we get to the writer,
+    // including dealing with GNU-style version suffixes.  Fixing this isn’t
+    // trivial.
+    //
+    // We thus have to be careful to not perform the symbol version replacement
+    // blindly:
+    //
+    // The ELF format is used on Windows by the MCJIT engine.  Thus, on
+    // Windows, the ELFObjectWriter can encounter symbols mangled using the MS
+    // Visual Studio C++ name mangling scheme. Symbols mangled using the MSVC
+    // C++ name mangling can legally have "@@@" as a sub-string. In that case,
+    // the EFLObjectWriter should not interpret the "@@@" sub-string as
+    // specifying GNU-style symbol versioning. The ELFObjectWriter therefore
+    // checks for the MSVC C++ name mangling prefix which is either "?", "@?",
+    // "__imp_?" or "__imp_@?".
+    //
+    // It would have been interesting to perform the MS mangling prefix check
+    // only when the target triple is of the form *-pc-windows-elf. But, it
+    // seems that this information is not easily accessible from the
+    // ELFObjectWriter.
     StringRef Name = Symbol.getName();
+    if (   !Name.startswith("?") 
+        && !Name.startswith("@?")
+        && !Name.startswith("__imp_?")
+        && !Name.startswith("__imp_@?")) {
+      // This symbol isn't following the MSVC C++ name mangling convention. We
+      // can thus safely interpret the @@@ in symbol names as specifying symbol
+      // versioning.
       SmallString<32> Buf;
       size_t Pos = Name.find("@@@");
       if (Pos != StringRef::npos) {
         Buf += Name.substr(0, Pos);
         unsigned Skip = MSD.SectionIndex == ELF::SHN_UNDEF ? 2 : 1;
         Buf += Name.substr(Pos + Skip);
         Name = Buf;
       }
+    }
 
     // Sections have their own string table
     if (MCELF::GetType(SD) != ELF::STT_SECTION)
       MSD.Name = StrTabBuilder.add(Name);
 
     if (MSD.SectionIndex == ELF::SHN_UNDEF)
       UndefinedSymbolData.push_back(MSD);
     else if (Local)

test/MC/ELF/symver-msvc.s

+// RUN: llvm-mc -filetype=obj -triple x86_64-pc-windows-elf %s -o - | llvm-readobj -r -t | FileCheck %s
+
+// Verify that MSVC C++ mangled symbols are not affected by the ELF
+// GNU-style symbol versioning. The ELF format is used on Windows by
+// the MCJIT execution engine.
+
+        .long "??_R0?AVexception@std@@@8"
+        .long "@??_R0?AVinvalid_argument@std@@@8"
+        .long "__imp_??_R0?AVlogic_error@std@@@8"
+        .long "__imp_@??_R0PAVexception@std@@@8"
+
+
+// CHECK:       Relocations [
+// CHECK-NEXT:    Section (2) .rela.text {
+// CHECK-NEXT:      0x0 R_X86_64_32 ??_R0?AVexception@std@@@8 0x0
+// CHECK-NEXT:      0x4 R_X86_64_32 @??_R0?AVinvalid_argument@std@@@8 0x0
+// CHECK-NEXT:      0x8 R_X86_64_32 __imp_??_R0?AVlogic_error@std@@@8 0x0
+// CHECK-NEXT:      0xC R_X86_64_32 __imp_@??_R0PAVexception@std@@@8 0x0
+// CHECK-NEXT:    }
+// CHECK-NEXT:  ]
+        
+// CHECK:       Symbols [
+// CHECK:         Symbol {
+// CHECK:           Name: ??_R0?AVexception@std@@@8 (102)
+// CHECK-NEXT:      Value: 0x0
+// CHECK-NEXT:      Size: 0
+// CHECK-NEXT:      Binding: Global (0x1)
+// CHECK-NEXT:      Type: None (0x0)
+// CHECK-NEXT:      Other: 0
+// CHECK-NEXT:      Section: Undefined (0x0)
+// CHECK-NEXT:    }
+// CHECK-NEXT:    Symbol {
+// CHECK-NEXT:      Name: @??_R0?AVinvalid_argument@std@@@8 (1)
+// CHECK-NEXT:      Value: 0x0
+// CHECK-NEXT:      Size: 0
+// CHECK-NEXT:      Binding: Global (0x1)
+// CHECK-NEXT:      Type: None (0x0)
+// CHECK-NEXT:      Other: 0
+// CHECK-NEXT:      Section: Undefined (0x0)
+// CHECK-NEXT:    }
+// CHECK-NEXT:    Symbol {
+// CHECK-NEXT:      Name: __imp_??_R0?AVlogic_error@std@@@8 (35)
+// CHECK-NEXT:      Value: 0x0
+// CHECK-NEXT:      Size: 0
+// CHECK-NEXT:      Binding: Global (0x1)
+// CHECK-NEXT:      Type: None (0x0)
+// CHECK-NEXT:      Other: 0
+// CHECK-NEXT:      Section: Undefined (0x0)
+// CHECK-NEXT:    }
+// CHECK-NEXT:    Symbol {
+// CHECK-NEXT:      Name: __imp_@??_R0PAVexception@std@@@8 (69)
+// CHECK-NEXT:      Value: 0x0
+// CHECK-NEXT:      Size: 0
+// CHECK-NEXT:      Binding: Global (0x1)
+// CHECK-NEXT:      Type: None (0x0)
+// CHECK-NEXT:      Other: 0
+// CHECK-NEXT:      Section: Undefined (0x0)
+// CHECK-NEXT:    }
+// CHECK-NEXT:  ]