ziglang / zig

General-purpose programming language and toolchain for maintaining robust, optimal, and reusable software.
https://ziglang.org
MIT License
34.84k stars 2.55k forks source link

Incorrect behavior on C function that returns struct which contains array of one or two doubles #21245

Open tatjam opened 2 months ago

tatjam commented 2 months ago

Zig Version

0.14.0-dev.1349+6a21875dd

Steps to Reproduce and Observed Behavior

Consider the following C struct and function:

// test.h
typedef struct {
  double dat[2];
} complex_number;

complex_number test_function();

// test.c
complex_number test_function() {
  complex_number out;

  out.dat[0] = 3;
  out.dat[1] = -6;

  return out;
}

Equivalently, the test_function can be implemented in Zig as follows:

// main.zig
const c = @cImport(@cInclude("test.h"));

pub fn zig_equiv() c.complex_number {
    var out: c.complex_number = undefined;
    out.dat[0] = 3;
    out.dat[1] = -6;
    return out;
}

In either case, the functions may be invoked as follows:

// main.zig
const std = @import("std");

pub fn main() !void {
  const res = c.test_function();
  //const res = zig_equiv();

  std.log.info("{}, {}", .{res.dat[0], res.dat[1]});
}

The code is built on an Ubuntu 22.04 without additional flags via zig build. The behavior is as follows:

I attach a minimum reproducible case (minimum_case.zip) that contains the code I've used. Should be as straightforward as downloading the zip and running zig build run.

For context, the error has come up while trying to wrap a C library that uses complex values, stored as this struct of an array of two elements.

Expected Behavior

For the struct to be correctly returned.

rohlem commented 2 months ago

Sounds like a C ABI issue, which is based on the CPU architecture of your system (probably either x86_64 or aarch64?), but also specific CPU features. You could also try adding -mcpu baseline as a compile flag and see whether the issue persists.

For x86_64 there used to be an old issue https://github.com/ziglang/zig/issues/1481 , which has since been closed. For aarch64 there is https://github.com/ziglang/zig/issues/14908 , although it seems primarily Windows is affected. Either way, it working in Zig 0.13.0 means this is a regression, which should be tagged (and I believe may be prioritized before the next release, 0.14.0). (Should also be tagged miscompilation fwict.)

tatjam commented 2 months ago

I'm indeed running x86_64, forgot to mention. The case appears to fall within "x86_64: struct & union return values <= 16 bytes"

tatjam commented 2 months ago

Speaking of which, I checked again and this case is NOT working on Zig 0.13, I accidentally ran the code with the struct containing more than 2 doubles. Going to update the main post.

vesim987 commented 1 month ago

Just from quick glance, on x86_64: zig:

declare { i64, i64 } @test_function() #5

c:

define dso_local { double, double } @test_function() #0 {  

this is causing zig to read the return value from rdx and rax instead of xmm0 and xmm1.

https://github.com/ziglang/zig/blob/2111f4c38b4c91a2406da3a5cf578162c1cafc4d/src/arch/x86_64/abi.zig#L308-L319 I think this piece of code need special handling of floats