Crystal: C ABI: passing a struct after numerous other args works wrong

[oprypin]

The following function definition generates wrong calling code in x86_64 ABI:

lib Lib
  struct Vec
    x : Int64
    y : Int64
  end

  fun foo(Int64, Int64, Int64, Int64, Int64, Vec)
end

Lib.foo(1, 2, 3, 4, 5, Lib::Vec.new(x: 6, y: 7))

The C code sees this data:

void foo(long a, long b, long c, long d, long e, Vec v) {
    printf("%ld %ld %ld %ld %ld %ld %ld\n", a, b, c, d, e, v.x, v.y);
    // 1 2 3 4 5 7 139713085455776
}

The last two should be 6 7, not 7 <pointer to v>. And that's just the basic case, I'm sure you could get it to crash instead.

---

Compare the calling code generated by Crystal:

call void @foo(i64 1, i64 2, i64 3, i64 4, i64 5, { i64, i64 } %9)

to an analogous call generated from C by Clang (thanks, @jhass):

%struct.Vec = type { i64, i64 }
[...]
call void @foo(i64 1, i64 2, i64 3, i64 4, i64 5, %struct.Vec* byval(%struct.Vec) align 8 %1)

As I pointed out that this starts happening only beyond a certain number of arguments, @jhass found this very likely lead – that there can be "not enough registers".

---

Indeed, if we remove one of the integer args, the behaviors converge (without any change on Crystal's part):

call void @foo(i64 1, i64 2, i64 3, i64 4, { i64, i64 } %9)

vs Clang:

call void @foo(i64 1, i64 2, i64 3, i64 4, i64 %5, i64 %7)

This leads us to the conclusion that Crystal probably just doesn't have any code handling the "overflow" arguments, but it should...

[asterite]

After I ported the code from Rust, Rust kept changing and tweaking things. We need to update the code to reflect the latest Rust (or any other) language. If anyone is up for a challenge, then please do so!

[asterite]

There's another issue I already reported where I mention the ABI isn't working well in some cases, so this is almost a duplicate.

[oprypin]

I found this issue while fixing something for Win64 ABI (#9520). I added a spec for the previously failing case, and it turned out that it fails on x86_64 too! Presumably for completely different reasons.

So if someone wants to try fixing this, they can enable that spec and see.
Also some pointers in chat.

[oprypin]

@asterite I see, that would probably be https://github.com/crystal-lang/crystal/issues/4890.

Well, at least I think my report is better isolated and with some further digging on the root cause.

[jhass]

An interesting tidbit is how cgo deals with handling the C ABI: https://github.com/golang/go/blob/master/src/runtime/cgocall.go#L5

[asterite]

@jhass tl;dr? :-)

Or what's the relevant part?

[jhass]

No relevant part and no TL;DR, just interesting that Google's solution to the whole C ABI problem is "yeah nope, let's avoid re-implementing that and reuse the C compiler's implementation if possible in any way". Also interesting read on how they did so.