Wrong name mangling in inline assembly on Windows x64

[kinke]

Environment: MSVC x64, LLVM head, LDC head

When trying to compile a hello-world program, the linker complains about not finding _fiber_entryPoint referenced in core.thread.Fiber.initStack().trampoline() (a naked assembly function).

Win64's C calling convention doesn't prepend symbol names with an underscore, so this issue can be fixed with the following patch:

diff --git a/gen/asm-x86.h b/gen/asm-x86.h
index 1918153..eec750d 100644
--- a/gen/asm-x86.h
+++ b/gen/asm-x86.h
@@ -2178,6 +2178,17 @@ namespace AsmParserx8664
             return false;
         }

+        static bool prependExtraUnderscore()
+        {
+            // osx and windows x86 need an extra underscore when mangling a symbol name
+            return global.params.targetTriple.getOS() == llvm::Triple::MacOSX
+                || global.params.targetTriple.getOS() == llvm::Triple::Darwin
+#ifndef ASM_X86_64
+                || global.params.targetTriple.isOSWindows() // 32-bit only
+#endif
+                ;
+        }
+
         void addOperand ( const char * fmt, AsmArgType type, Expression * e, AsmCode * asmcode, AsmArgMode mode = Mode_Input )
         {
             if ( sc->func->naked )
@@ -2217,15 +2228,11 @@ namespace AsmParserx8664
                                     break;
                                 }

-                                // osx needs an extra underscore
-                                if ( global.params.targetTriple.getOS() == llvm::Triple::MacOSX ||
-                                    global.params.targetTriple.getOS() == llvm::Triple::Darwin ||
-                                    global.params.targetTriple.isOSWindows() )
+                                // print out the mangle
+                                if ( prependExtraUnderscore() )
                                 {
                                     insnTemplate << "_";
                                 }
-
-                                // print out the mangle
                                 insnTemplate << mangle(vd);
                                 vd->nakedUse = true;
                                 break;
@@ -2897,10 +2904,7 @@ namespace AsmParserx8664
                                 {
                                     use_star = false;
                                     // simply write out the mangle
-                                    // on osx and windows, prepend extra _
-                                    if ( global.params.targetTriple.getOS() == llvm::Triple::MacOSX ||
-                                        global.params.targetTriple.getOS() == llvm::Triple::Darwin ||
-                                        global.params.targetTriple.isOSWindows() )
+                                    if ( prependExtraUnderscore() )
                                     {
                                         insnTemplate << "_";
                                     }

[Trass3r]

Confirmed.

[dnadlinger]

Seems reasonable, but I'm a bit surprised that it should only pop up now. Was naked inline asm simply not used on Win64 before?

[Trass3r]

No clue.
Not sure if the ASM_X86_64 check is ok either or if it should be a runtime targetTriple.isArch32Bit().

[kinke]

Phobos does use naked asm for Win64, e.g., in std.math (atan2 etc). I guess the naked functions just haven't called another function so far.
Trass3r is right, I'd prefer the runtime check too.

[dnadlinger]

Nope, the compile time conditional isn't per se problematic here. It's just a hack from when the two different files from x86/x86_64 were merged together (note that asm-x86.h is header-only without an include guard). The actual asm parser selection is still done at runtime. Making the bitness a runtime parameter would indeed be a bit cleaner and save on code size, though.

[kinke]

I know, as it was me who merged the two asm parsers a while ago. ;) I'd just prefer the runtime check because it's more intuitive and seems less hacky.

[dnadlinger]

Oh, yes – sorry, I forgot. ;)

[dnadlinger]

Could somebody who can actually test this on Win64/MSVC please create a pull request?