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]

This comment was removed by belleyb.

[belleyb]

I will recreate a new review for this change. It got mixed up with an "arc diff" that was wrongly uploaded. Sorry for the noise. Beginner error...

lib/MC/ELFObjectWriter.cpp

 
     // The @@@ in symbol version is replaced with @ in undefined symbols and
     // @@ in defined ones.
+    //
+    // We have to be careful though:
+    //
+    // 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 sub-string. The
+    // EFLObjectWriter should not interpret the "@@@" sub-string as specifying
+    // GNU-style symbol versioning. The ELFObjectWriter therefore check for
+    // the MSVC C++ name mangling prefix which is either "?", "@?", "__imp_?"
+    // or "__imp_@?".
+    //
+    // It would have been interesting to be able to perform this 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();
-    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;
+    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

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 symbol are not affected by the
+// ELF 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:  ]