Fix segmentation fault when compiling va_arg intrinsic

[zyedidia]

The following code currently causes a segmentation fault in LDC when compiled:

alias va_list = void*;

pragma(LDC_va_arg) T va_arg(T)(va_list ap);

int foo(va_list ap) {
    return va_arg!(int)(ap);
}

This PR fixes that.

Note: the following code still causes a segfault during compilation, but I think this is a bug in LLVM, not LDC.

alias va_list = void*;

pragma(LDC_va_arg) T va_arg(T)(va_list ap);

string foo(va_list ap) {
    return va_arg!(string)(ap);
}

I will report that separately to the LLVM project.

Also, it seems like the D runtime that LDC uses does not make use of the va_arg intrinsic, and instead uses custom code for each architecture (from DMD I assume). What is the reason for this? Using some custom implementation instead of relying on the LLVM intrinsic seems much more prone to bugs. Perhaps some followup work to this PR would be to convert D runtime over to using the va_arg intrinsic for LDC instead of the (hacky?) one from DMD.

[zyedidia]

Looks like this needs a little more work on aarch64 hosts? I'm not really able to debug that at the moment.

[kinke]

From what I've seen, that intrinsic is basically useless, and we need to do it manually in druntime. Under examples, https://llvm.org/docs/LangRef.html#va-arg-instruction states:

Note that the code generator does not yet fully support va_arg on many targets. Also, it does not currently support va_arg with aggregate types on any target.

A slice like string is an aggregate.

[dnadlinger]

Yep, LLVM doesn't implement the target C ABI (the lowering is done in Clang); consequently, the intrinsic is also useless, as it would be missing the requisite translation of types according to the respective ABI details.

[zyedidia]

That is unfortunate. It would still be good for LDC to support emitting the intrinsic, even if LLVM doesn't handle it well. Either that or the LDC intrinsic for it should not exist, and shouldn't be listed on the wiki here: https://wiki.dlang.org/LDC-specific_language_changes#LDC_va_.2A_for_variadic_argument_handling_intrinsics. For my target platforms (riscv64-unknown-elf and aarch64-none-elf), the LLVM va_arg instruction appears to work, but maybe I should just use the D runtime implementation because LLVM va_arg is half-baked.

[kinke]

Well years ago when I last looked at varargs stuff, the va_arg intrinsic was supported just fine by LDC, but as said, not usable. I'd have to check why it's now apparently segfaulting.

[kinke]

Ah, so the other 3 va_* intrinsics are intrinsic functions, but va_arg is an IR instruction (but a D function declaration). So we're nowadays e.g. hitting an assertion for https://github.com/ldc-developers/ldc/blob/981c58e4983f69fe3fb03f3f7cf11def57dd45be/gen/llvmhelpers.cpp#L1571-L1573

Besides skipping the IR declaration of the D function, we should probably handle va_arg (emitting the instruction instead of a call) in https://github.com/ldc-developers/ldc/blob/981c58e4983f69fe3fb03f3f7cf11def57dd45be/gen/toir.cpp#L713-L723

[kinke]

This seemingly does the job:

diff --git a/gen/toir.cpp b/gen/toir.cpp
index 6a2c00052e..421b7d0e1a 100644
--- a/gen/toir.cpp
+++ b/gen/toir.cpp
@@ -719,6 +719,12 @@ public:
         if (fd->llvmInternal == LLVMinline_ir) {
           return DtoInlineIRExpr(e->loc, fd, e->arguments, sretPointer);
         }
+        if (fd->llvmInternal == LLVMva_arg) {
+          DValue *result = nullptr;
+          const auto success = DtoLowerMagicIntrinsic(p, fd, e, result);
+          assert(success);
+          return result;
+        }
       }
     }
 

Little test file:

import core.stdc.stdarg : va_list;

pragma(LDC_va_arg) T va_arg(T)(ref va_list ap);

int foo(...) {
    return va_arg!int(_argptr);
}

void main() {
    assert(foo(123) == 123);
}

Compiling with -mtriple=i686-linux-gnu is fine (haven't tried running it), and for riscv64-unknown-elf too. With an LLVM with enabled assertions, the LLVM limitations become obvious quite quickly:

• -mtriple=x86_64-linux-gnu:

  ldc2: /home/mkinkelin/dev/llvm-project/llvm/lib/CodeGen/RegAllocFast.cpp:961: void {anonymous}::RegAllocFast::useVirtReg(llvm::MachineInstr&, unsigned int, llvm::Register): Assertion `(!MO.isKill() || LRI->LastUse == &MI) && "Invalid kill flag"' failed.
  

• -mtriple=aarch64-none-elf:

  ldc2: /home/mkinkelin/dev/llvm-project/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:8212: llvm::SDValue llvm::AArch64TargetLowering::LowerVAARG(llvm::SDValue, llvm::SelectionDAG&) const: Assertion `Subtarget->isTargetDarwin() && "automatic va_arg instruction only works on Darwin"' failed.
  

[zyedidia]

Ah I see that is unfortunate for aarch64-none-elf. I'll just use the D runtime version I think. I can close this PR if you want to commit your fix separately.