Fix VaList and disable va_arg for AArch64 (#9422)

va_arg is broken on most platforms, including AArch64

Author: jhass

spec/compiler/codegen/primitives_spec.cr

@@ -239,8 +239,8 @@ describe "Code gen: primitives" do
   end
 
   describe "va_arg" do
-    # On Windows llvm's va_arg instruction works incorrectly.
-    {% unless flag?(:win32) %}
+    # On Windows and AArch64 llvm's va_arg instruction works incorrectly.
+    {% unless flag?(:win32) || flag?(:aarch64) %}
       it "uses llvm's va_arg instruction" do
         mod = codegen(%(
           struct VaList
@@ -255,33 +255,33 @@ describe "Code gen: primitives" do
         str = mod.to_s
         str.should contain("va_arg %VaList* %list")
       end
-    {% end %}
 
-    pending_win32 "works with C code" do
-      test_c(
-        %(
-          extern int foo_f(int,...);
-          int foo() {
-            return foo_f(3,1,2,3);
-          }
-        ),
-        %(
-          lib LibFoo
-            fun foo() : LibC::Int
-          end
+      it "works with C code" do
+        test_c(
+          %(
+            extern int foo_f(int,...);
+            int foo() {
+              return foo_f(3,1,2,3);
+            }
+          ),
+          %(
+            lib LibFoo
+              fun foo() : LibC::Int
+            end
 
-          fun foo_f(count : Int32, ...) : LibC::Int
-            sum = 0
-            VaList.open do |list|
-              count.times do |i|
-                sum += list.next(Int32)
+            fun foo_f(count : Int32, ...) : LibC::Int
+              sum = 0
+              VaList.open do |list|
+                count.times do |i|
+                  sum += list.next(Int32)
+                end
               end
+              sum
             end
-            sum
-          end
 
-          LibFoo.foo
-        ), &.to_i.should eq(6))
-    end
+            LibFoo.foo
+          ), &.to_i.should eq(6))
+      end
+    {% end %}
   end
 end

spec/spec_helper.cr

@@ -257,11 +257,7 @@ def run(code, filename = nil, inject_primitives = true, debug = Crystal::Debug::
 end
 
 def test_c(c_code, crystal_code, *, file = __FILE__)
-  with_tempfile("temp_abi.c", "temp_abi.o", file: file) do |c_filename, o_filename|
-    File.write(c_filename, c_code)
-
-    `#{Crystal::Compiler::CC} #{Process.quote(c_filename)} -c -o #{Process.quote(o_filename)}`.should be_truthy
-
+  with_temp_c_object_file(c_code, file: file) do |o_filename|
     yield run(%(
     require "prelude"
 

spec/std/spec_helper.cr

@@ -108,3 +108,14 @@ def compile_and_run_source(source, flags = %w(), file = __FILE__)
     compile_and_run_file(source_file, flags, file: file)
   end
 end
+
+def compile_and_run_source_with_c(c_code, crystal_code, flags = %w(--debug), file = __FILE__)
+  with_temp_c_object_file(c_code, file: file) do |o_filename|
+    yield compile_and_run_source(%(
+    require "prelude"
+
+    @[Link(ldflags: #{o_filename.inspect})]
+    #{crystal_code}
+    ))
+  end
+end

spec/std/va_list_spec.cr

@@ -0,0 +1,40 @@
+require "./spec_helper"
+
+describe VaList do
+  it "works with C code" do
+    compile_and_run_source_with_c(
+      %(
+          #include <stdarg.h>
+          extern int foo_f(int,...);
+          int foo() {
+            return foo_f(3,1,2,3);
+          }
+
+          int read_arg(va_list *ap) {
+            return va_arg(*ap, int);
+          }
+        ),
+      %(
+        lib LibFoo
+          fun foo : LibC::Int
+          fun read_arg(ap : LibC::VaList*) : LibC::Int
+        end
+
+        fun foo_f(count : LibC::Int, ...) : LibC::Int
+          sum = 0
+          VaList.open do |list|
+            ap = list.to_unsafe
+            count.times do |i|
+              sum += LibFoo.read_arg(pointerof(ap))
+            end
+          end
+          sum
+        end
+
+        puts LibFoo.foo
+      )) do |status, output|
+      status.success?.should be_true
+      output.to_i.should eq(6)
+    end
+  end
+end

spec/support/tempfile.cr

@@ -43,6 +43,16 @@ def with_temp_executable(name, file = __FILE__)
   end
 end
 
+def with_temp_c_object_file(c_code, file = __FILE__)
+  with_tempfile("temp_c.c", "temp_c.o", file: file) do |c_filename, o_filename|
+    File.write(c_filename, c_code)
+
+    `#{ENV["CC"]? || "cc"} #{Process.quote(c_filename)} -c -o #{Process.quote(o_filename)}`.should be_truthy
+
+    yield o_filename
+  end
+end
+
 if SPEC_TEMPFILE_CLEANUP
   at_exit do
     FileUtils.rm_r(SPEC_TEMPFILE_PATH) if Dir.exists?(SPEC_TEMPFILE_PATH)

spec/win32_std_spec.cr

@@ -221,6 +221,7 @@ require "./std/uint_spec.cr"
 require "./std/uri/punycode_spec.cr"
 require "./std/uri_spec.cr"
 require "./std/uuid_spec.cr"
+# require "./std/va_list_spec.cr"
 require "./std/weak_ref_spec.cr"
 require "./std/xml/builder_spec.cr"
 require "./std/xml/html_spec.cr"

src/compiler/crystal/semantic/main_visitor.cr

@@ -2431,9 +2431,6 @@ module Crystal
     end
 
     def visit_va_arg(node)
-      if program.has_flag? "windows"
-        node.raise "va_arg is not yet supported on Windows"
-      end
       arg = call.not_nil!.args[0]? || node.raise("requires type argument")
       node.type = arg.type.instance_type
     end

src/lib_c/aarch64-linux-gnu/c/stdarg.cr

@@ -1,3 +1,10 @@
 lib LibC
-  type VaList = Void*
+  # based on https://github.com/llvm/llvm-project/blob/bf1cdc2c6c0460b7121ac653c796ef4995b1dfa9/clang/lib/AST/ASTContext.cpp#L7678-L7739
+  struct VaList
+    __stack : Void*
+    __gr_top : Void*
+    __vr_top : Void*
+    __gr_offs : Int32
+    __vr_offs : Int32
+  end
 end

src/lib_c/aarch64-linux-musl/c/stdarg.cr

@@ -1,10 +1,10 @@
 lib LibC
-  struct VaListTag
-    gp_offset : UInt
-    fp_offset : UInt
-    overflow_arg_area : Void*
-    reg_save_area : Void*
+  # based on https://github.com/llvm/llvm-project/blob/bf1cdc2c6c0460b7121ac653c796ef4995b1dfa9/clang/lib/AST/ASTContext.cpp#L7678-L7739
+  struct VaList
+    __stack : Void*
+    __gr_top : Void*
+    __vr_top : Void*
+    __gr_offs : Int32
+    __vr_offs : Int32
   end
-
-  type VaList = VaListTag[1]
 end

src/va_list.cr

@@ -18,7 +18,17 @@ struct VaList
     end
   end
 
-  {% if compare_versions(Crystal::VERSION, "0.33.0-0") > 0 %}
+  {% if flag?(:aarch64) || flag?(:win32) %}
+    {% platform = flag?(:aarch64) ? "AArch64" : "Windows" %}
+    {% clang_impl = flag?(:aarch64) ? "https://github.com/llvm/llvm-project/blob/a574edbba2b24fcfb733aa2d82308131f5b7d2d6/clang/lib/CodeGen/TargetInfo.cpp#L5677-L5921" : "https://github.com/llvm/llvm-project/blob/a574edbba2b24fcfb733aa2d82308131f5b7d2d6/clang/lib/CodeGen/TargetInfo.cpp#L5958-L5964" %}
+    # Do not call this, instead use C wrappers calling the va_arg macro for the types you need.
+    #
+    # Clang implements va_arg on {{platform.id}} like this: {{clang_impl.id}}
+    # If somebody wants to fix the LLVM IR va_arg instruction on {{platform}} upstream, or port the above here, that would be welcome.
+    def next(type)
+      \{% raise "Cannot get variadic argument on {{platform.id}}. As a workaround implement wrappers in C calling the va_arg macro for the types you need and bind to those." %}
+    end
+  {% else %}
     @[Primitive(:va_arg)]
     def next(type)
     end