Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
tatjam opened this issue Aug 29, 2024 · 4 comments
Labels
arch-aarch64 64-bit ARM arch-riscv 32-bit and 64-bit RISC-V arch-wasm 32-bit and 64-bit WebAssembly arch-x86_64 64-bit x86 bug Observed behavior contradicts documented or intended behavior miscompilation The compiler reports success but produces semantically incorrect code.
Milestone

Comments

@tatjam
Copy link

tatjam commented Aug 29, 2024

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:

  • If the C code is called, the log will return nonsense values (that vary each run).
  • If the Zig code is called, the expected values are printed
  • If the struct's array is adjusted to contain more values (those are simply left uninitialized), the code starts working correctly on either C or Zig implementation.
  • If compiling with ReleaseFast, incorrect behavior still happens, but in this case, instead of random values, 0 is printed.

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.

@tatjam tatjam added the bug Observed behavior contradicts documented or intended behavior label Aug 29, 2024
@tatjam tatjam changed the title Incorrect behavior on C function that returns struct which contains array of one or two elements Incorrect behavior on C function that returns struct which contains array of one or two doubles Aug 29, 2024
@rohlem
Copy link
Contributor

rohlem commented Aug 29, 2024

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 #1481 , which has since been closed.
For aarch64 there is #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
Copy link
Author

tatjam commented Aug 29, 2024

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

@tatjam
Copy link
Author

tatjam commented Aug 29, 2024

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.

@Vexu Vexu added this to the 0.14.0 milestone Sep 1, 2024
@Vexu Vexu added miscompilation The compiler reports success but produces semantically incorrect code. arch-x86_64 64-bit x86 arch-aarch64 64-bit ARM arch-wasm 32-bit and 64-bit WebAssembly arch-riscv 32-bit and 64-bit RISC-V labels Sep 1, 2024
@vesim987
Copy link
Contributor

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.

zig/src/arch/x86_64/abi.zig

Lines 308 to 319 in 2111f4c

.array => {
const ty_size = ty.abiSize(zcu);
if (ty_size <= 8) {
result[0] = .integer;
return result;
}
if (ty_size <= 16) {
result[0] = .integer;
result[1] = .integer;
return result;
}
return memory_class;

I think this piece of code need special handling of floats

@andrewrk andrewrk modified the milestones: 0.14.0, 0.15.0 Feb 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arch-aarch64 64-bit ARM arch-riscv 32-bit and 64-bit RISC-V arch-wasm 32-bit and 64-bit WebAssembly arch-x86_64 64-bit x86 bug Observed behavior contradicts documented or intended behavior miscompilation The compiler reports success but produces semantically incorrect code.
Projects
None yet
Development

No branches or pull requests

5 participants