Vararg functions does not compile on x86_64

[redstar]

The following module

import core.vararg;
import core.stdc.stdio;

void log(const(char)* str, ...)
{
    va_list args;
    version(LDC)
        va_start(args, str);
    else
        va_start(args, __va_argsave);
    vprintf(str, args);
    va_end(args);
}

void main()
{
    log("%d %d %d\n", 1, 2, 3);
}

compiles with dmd but gives an error with ldc:

ldc_github.d(11): Error: function core.stdc.stdio.vprintf (const(char*) format, __va_list* arg) is not callable using argument types (const(char)*, void*)

Only on x86_64. The implementation of core.vararg is wrong.

[AlexeyProkhin]

I believe it is the same bug as #73

[redstar]

Yes, it is indeed the same error. Closing this as duplicate.

[dnadlinger]

I don't think it is. #73 is about the implicit _argptr parameter for D variadics. This issue (__va_list* vs. void*) should be much easier to fix.

[Trass3r]

_d_arraycatnT also contains varargs and crashes on Win64.

[kinke]

On Win64, Kai's test case works when importing core.stdc.stdarg instead of core.vararg.
So at least for Win64, the LDC-specific code in core.vararg should be ignored and core.stdc.stdarg be publicly imported just like DMD does.

The joy vanishes as soon as you put extern(C) in front of log():

//import core.vararg;
import core.stdc.stdarg;
import core.stdc.stdio;

void log(const(char)* str, ...)
{
    va_list args;
    va_start(args, str);
    vprintf(str, args);
    va_end(args);
}

extern(C) void logC(const(char)* str, ...)
{
    va_list args;
    va_start(args, str);
    vprintf(str, args);
    va_end(args);
}

void main()
{
    log   ("log:    %d %lf %f %d %d %s\n", 1, 2.0, 3.0f, 4, 5, "lala".ptr);
    logC  ("logC:   %d %lf %f %d %d %s\n", 1, 2.0, 3.0f, 4, 5, "lala".ptr);
    printf("printf: %d %lf %f %d %d %s\n", 1, 2.0, 3.0f, 4, 5, "lala".ptr);
}

yields something like:

log:    1 2.000000 0.000000 4 5 lala
logC:   -546243576 0.000000 0.000000 -546243632 -112462288 Hì♣☼Õ♥
printf: 1 2.000000 3.000000 4 5 lala

Also note that the float parameter 3.0f isn't passed correctly to log with D calling convention.

/edit: Well I hope extern(C) isn't a real issue here, since core.stdc.stdio.printf() is declared as extern(C) as well and seems to work fine. And D functions should use the D calling convention anyway. But there's still that float parameter...
/edit2: Too bad rt.lifetime._d_arraycatnT() and other array functions are declared as extern(C), causing multiple segfaults when running the tests on Win64!! 😜

[kinke]

Regarding core.vararg:
I tried to simply publicly import core.stdc.stdarg in core.vararg and then rebuild druntime/Phobos. That fails because va_list is defined as __va_list_tag* in core.stdc.stdarg whereas it's char* in core.vararg (and there are other big discrepancies). LDC translates it to char* in gen/target.cpp:81, so "disabling" core.vararg alone doesn't work unless one did a hack such as this one:

diff --git a/src/core/stdc/stdarg.d b/src/core/stdc/stdarg.d
index e53ec4f..c8c02ba 100644
--- a/src/core/stdc/stdarg.d
+++ b/src/core/stdc/stdarg.d
@@ -435,7 +435,7 @@ else version ( LDC_X86_64 )
     // magic built-in type that on x86_64 decays to a reference like the actual
     // definition in C (one-element array), or by giving va_copy a different
     // signature.
-    alias va_list = __va_list*;
+    alias va_list = char*;//__va_list*;

     pragma(LDC_va_start)
         void va_start(T)(out va_list ap, ref T);
@@ -449,12 +449,12 @@ else version ( LDC_X86_64 )

     void va_arg(T)(va_list apx, ref T parmn)
     {
-        va_arg_x86_64(apx, parmn);
+        va_arg_x86_64(cast(__va_list*) apx, parmn);
     }

     void va_arg()(va_list apx, TypeInfo ti, void* parmn)
     {
-        va_arg_x86_64(apx, ti, parmn);
+        va_arg_x86_64(cast(__va_list*) apx, ti, parmn);
     }

     pragma(LDC_va_end)

I also tried to use extern(D) for the originally extern(C) vararg functions in druntime (1 function in ldc.eh2 & 4 in rt.lifetime) but then had to realize those use the C conv because of the (un)mangled name as their names are hardcoded in gen/arrays.cpp etc...

[kinke]

So from the ABI side, we seem to be doing the right thing when calling extern(C) vararg functions, otherwise printf() wouldn't work. This means that core.stdc.stdarg needs to be modified so that the extern(C) vararg implementations in druntime (such as these crucial array functions) work properly.

In order not to have to differentiate between C and D linkage (I guess that's why core.vararg and core.stdc.stdarg currently differ), we would also need to modify the ABI when calling extern(D) vararg functions (analog to the extern(C) case, with the exception of the additional TypeInfo[] argument before the optional ones).

Is that correct? If so, I'll try to work on it.

The current state on Win64 is that core.vararg doesn't work for extern(C) as well as extern(D). core.stdc.stdarg seems to almost work with extern(D) (i.e., the current ABI for D linkage) but doesn't with extern(C).

[dnadlinger]

@kinke: There is one more issue, see #172.

You are right, on all 64 bit x86 platforms the extern(C) and extern(D) ABIs should be identical, expect of course for the implicitly added TypeInfo array for D varargs. LDC was first regarding proper Linux x86_64 support, and back then they just chose a solution for varargs that doesn't match up with what DMD does. Ever since the initial D2 port, everything varargs is a convoluted mess. The __va_list type definition you linked to above is specific to the System V AMD64 ABI (i.e. most Unix-like systems). It should definitely not be used on Win64.

It would be great if you could finally resolve the situation. Feel free to ping me with questions, as I looked into this a while ago (but don't have the time to fix this myself right now.

[kinke]

Thanks David for clarifying. I'll only focus on Win64 here. So what I gathered is that varargs are passed just like normal args, i.e., the first 4 ones are passed in registers (integer or XMM for float/double), the rest on the stack. The caller always allocates 4x8 extra bytes just before pushing the return address. This space has been specifically designed for vararg functions to dump the 4 register args into, right below the remaining args. For vararg callees, MS chose to pass a float/double register arg in both corresponding integer and XMM registers. This means that the vararg callee would first need to dump rcx, rdx, r8 and r9 into rbp+16, rbp+24, rbp+32 and rbp+40:

extern(C) long simpleTest(long a, long b, long c, long d, long e, long f)
{
    asm
    {
        mov [RBP+16], RCX; // a
        mov [RBP+24], RDX;
        mov [RBP+32], R8;
        mov [RBP+40], R9;  // d
        mov RAX, [RBP+48]; // return e
    }
}

void main()
{
    assert(simpleTest(1, 2, 3, 4, 5, 6) == 5);
}

A va_list (MS defines it as char*) starting with the 2nd argument would therefore be initialized to rbp+24 by va_start(). The problem is that I cannot take the address of the last regular argument to deduce the offset of the first optional argument -- if there are no more than 4 regular arguments before the optional ones, the last one is passed in a register and not on the stack.
In core.stdc.stdarg, there's this va_start() implementation for DMD x86_64:

void va_start(T)(out va_list ap, ref T parmn) { ap = &parmn.va; }

@redstar @klickverbot @Trass3r What's that va property all about? Any other ideas?

And btw, the argument order is reversed when using the extern(D) ABI. I guess that should be fixed as well then.

[dnadlinger]

@kinke: That's from DMD's implementation of System V AMD64-style varargs. Walter couldn't be bothered to (or couldn't mange to) properly implement them in his backend, so the user is expected to pass __va_argsave to va_start. It's an implicitly generated object of type __va_argsave_t, so the line in question just accesses its va member.

[dnadlinger]

Also, as far as the vararg implementation goes, can't you use the LLVM vararg intrinsics for figuring out the address? As always, the TypeInfo overload might be a bit tricky, though.

[redstar]

Use of the LLVM vararg intrinsics should be possible. I did the same for PPC. The situation is the same: the first parameters are passed in registers with backing store allocated on the stack.

[kinke]

I've already tried the LLVM intrinsic, which returned garbage iirc. LLVM has a test/CodeGen/X86/win64_vararg.ll test file though, so it seems like it should be working. Will investigate further.

[AlexeyProkhin]

@kinke, if every va argument is on the stack, X86 vaarg implementation should work (or something very similar). But you must be aware about special handling of va_start and va_copy intrinsics in the backend. See 0d5f084

[kinke]

Okay the LLVM intrinsics work fine -- I previously tested the LDC-specific LDC_va_start intrinsic instead of the actual LLVM one. This works now:

import core.stdc.stdio;

pragma(LDC_intrinsic, "llvm.va_start")
void llvm_va_start(void* pArglist);

pragma(LDC_intrinsic, "llvm.va_end")
void llvm_va_end(void* pArglist);

extern(C) void log(const(char)* str, ...)
{
    char* args;
    llvm_va_start(&args);
    vprintf(str, args);
    llvm_va_end(&args);
}

void main()
{
    log   ("log:    %d %f %f %d %d %s\n", 1, 2.0, 3.0f, 4, 5, "lala".ptr);
    printf("printf: %d %f %f %d %d %s\n", 1, 2.0, 3.0f, 4, 5, "lala".ptr);
}

Unfortunately, one must pass the address of the va_list object to va_start() and va_end() because the parameters cannot be declared ref char* (assertion error in LLVM due to incompatible types - char** vs. char* expected by the intrinsic). I guess this means we'll still have to use an LDC-specific LDC_va_start intrinsic and just give it a different meaning for Win64 (=> cast address & call LLVM intrinsic).

/edit: Ah wait, if I understand this correctly, this seems to be done normally anyway in gen/toir.cpp:859, but isn't atm for x86_64. So varargs for extern(C) will be easy to fix. :)

[kinke]

Fixed by #768 and ldc-developers/druntime#14.