Skip to content
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

C ABI tests not passing for 32-bit x86 #10852

Open
Tracked by #1929 ...
andrewrk opened this issue Feb 10, 2022 · 1 comment
Open
Tracked by #1929 ...

C ABI tests not passing for 32-bit x86 #10852

andrewrk opened this issue Feb 10, 2022 · 1 comment
Labels
arch-x86 32-bit x86 backend-llvm The LLVM backend outputs an LLVM IR Module. bug Observed behavior contradicts documented or intended behavior
Milestone

Comments

@andrewrk
Copy link
Member

andrewrk commented Feb 10, 2022

Umbrella issue: #1481

Zig Version

0.10.0-dev.625+e42b5e76b

Steps to Reproduce

Apply this diff:

diff --git a/test/stage1/c_abi/cfuncs.c b/test/stage1/c_abi/cfuncs.c
index f5c90adba..a2a5895ab 100644
--- a/test/stage1/c_abi/cfuncs.c
+++ b/test/stage1/c_abi/cfuncs.c
@@ -11,24 +11,14 @@ static void assert_or_panic(bool ok) {
     }
 }
 
-struct i128 {
-    __int128 value;
-};
-
-struct u128 {
-    unsigned __int128 value;
-};
-
 void zig_u8(uint8_t);
 void zig_u16(uint16_t);
 void zig_u32(uint32_t);
 void zig_u64(uint64_t);
-void zig_struct_u128(struct u128);
 void zig_i8(int8_t);
 void zig_i16(int16_t);
 void zig_i32(int32_t);
 void zig_i64(int64_t);
-void zig_struct_i128(struct i128);
 void zig_five_integers(int32_t, int32_t, int32_t, int32_t, int32_t);
 
 void zig_f32(float);
@@ -140,19 +130,11 @@ void run_c_tests(void) {
     zig_u16(0xfffe);
     zig_u32(0xfffffffd);
     zig_u64(0xfffffffffffffffc);
-    {
-        struct u128 s = {0xfffffffffffffffc};
-        zig_struct_u128(s);
-    }
 
     zig_i8(-1);
     zig_i16(-2);
     zig_i32(-3);
     zig_i64(-4);
-    {
-        struct i128 s = {-6};
-        zig_struct_i128(s);
-    }
     zig_five_integers(12, 34, 56, 78, 90);
 
     zig_f32(12.34f);
@@ -239,10 +221,6 @@ void c_u64(uint64_t x) {
     assert_or_panic(x == 0xfffffffffffffffcULL);
 }
 
-void c_struct_u128(struct u128 x) {
-    assert_or_panic(x.value == 0xfffffffffffffffcULL);
-}
-
 void c_i8(int8_t x) {
     assert_or_panic(x == -1);
 }
@@ -259,10 +237,6 @@ void c_i64(int64_t x) {
     assert_or_panic(x == -4);
 }
 
-void c_struct_i128(struct i128 x) {
-    assert_or_panic(x.value == -6);
-}
-
 void c_f32(float x) {
     assert_or_panic(x == 12.34f);
 }
diff --git a/test/stage1/c_abi/main.zig b/test/stage1/c_abi/main.zig
index 71a53bede..9043af76e 100644
--- a/test/stage1/c_abi/main.zig
+++ b/test/stage1/c_abi/main.zig
@@ -16,12 +16,10 @@ extern fn c_u8(u8) void;
 extern fn c_u16(u16) void;
 extern fn c_u32(u32) void;
 extern fn c_u64(u64) void;
-extern fn c_struct_u128(U128) void;
 extern fn c_i8(i8) void;
 extern fn c_i16(i16) void;
 extern fn c_i32(i32) void;
 extern fn c_i64(i64) void;
-extern fn c_struct_i128(I128) void;
 
 // On windows x64, the first 4 are passed via registers, others on the stack.
 extern fn c_five_integers(i32, i32, i32, i32, i32) void;
@@ -39,13 +37,11 @@ test "C ABI integers" {
     c_u16(0xfffe);
     c_u32(0xfffffffd);
     c_u64(0xfffffffffffffffc);
-    c_struct_u128(.{ .value = 0xfffffffffffffffc });
 
     c_i8(-1);
     c_i16(-2);
     c_i32(-3);
     c_i64(-4);
-    c_struct_i128(.{ .value = -6 });
     c_five_integers(12, 34, 56, 78, 90);
 }
 
@@ -74,19 +70,6 @@ export fn zig_i64(x: i64) void {
     expect(x == -4) catch @panic("test failure: zig_i64");
 }
 
-const I128 = extern struct {
-    value: i128,
-};
-const U128 = extern struct {
-    value: u128,
-};
-export fn zig_struct_i128(a: I128) void {
-    expect(a.value == -6) catch @panic("test failure: zig_struct_i128");
-}
-export fn zig_struct_u128(a: U128) void {
-    expect(a.value == 0xfffffffffffffffc) catch @panic("test failure: zig_struct_u128");
-}
-
 extern fn c_f32(f32) void;
 extern fn c_f64(f64) void;

Then run the C ABI tests with the i386-linux target:

cd build # from zig source tree
make
cd ../test/stage1/c_abi
../../../build/zig build test -Dtarget=i386-linux

Expected Behavior

All 16 tests passed.

Actual Behavior

Test [8/16] test "C ABI small struct of ints"... thread error: the following test command crashed:
/home/andy/misc/zig/test/stage1/c_abi/zig-cache/o/e925b1bf7cfa036c875e7cbb17f07e9c/test /home/andy/misc/zig/build/zig
test...The following command exited with error code 1:
/home/andy/misc/zig/build/zig test /home/andy/misc/zig/test/stage1/c_abi/main.zig /home/andy/misc/zig/test/stage1/c_abi/zig-cache/o/3859a4e0a46391fd840bbfb6aeb58624/cfuncs.o -lc --cache-dir /home/andy/misc/zig/test/stage1/c_abi/zig-cache --global-cache-dir /home/andy/.cache/zig --name test -target i386-linux -mcpu pentium4 --enable-cache 
error: the following build command failed with exit code 1:
/home/andy/misc/zig/test/stage1/c_abi/zig-cache/o/261ff3a2e64da5421bf429002cc46776/build /home/andy/misc/zig/build/zig /home/andy/misc/zig/test/stage1/c_abi /home/andy/misc/zig/test/stage1/c_abi/zig-cache /home/andy/.cache/zig test -Dtarget=i386-linux
@andrewrk andrewrk added bug Observed behavior contradicts documented or intended behavior stage1 The process of building from source via WebAssembly and the C backend. arch-x86 32-bit x86 labels Feb 10, 2022
@andrewrk andrewrk added this to the 0.10.0 milestone Feb 10, 2022
@andrewrk andrewrk added the backend-llvm The LLVM backend outputs an LLVM IR Module. label Feb 10, 2022
@andrewrk
Copy link
Member Author

Here's a clue to get started.

Make this file named test.zig

const Stage2SemVer = extern struct {
    major: u32,
    minor: u32,
    patch: u32,
};

export fn stage2_version() Stage2SemVer {
    return .{
        .major = 0,
        .minor = 9,
        .patch = 0,
    };
}

pub fn panic(msg: []const u8, stack_trace: ?*@import("std").builtin.StackTrace) noreturn {
    _ = msg;
    _ = stack_trace;
    @breakpoint();
    unreachable;
}

Build this for i386-linux and look at the LLVM IR:

./zig build-obj test.zig --verbose-llvm-ir -target i386-linux

The relevant function is

; Function Attrs: nobuiltin nounwind
define { i64, i32 } @stage2_version() #3 !dbg !61 {
Entry:
  %result = alloca %Stage2SemVer, align 4
  %0 = alloca { i64, i32 }, align 8
  %1 = getelementptr inbounds %Stage2SemVer, %Stage2SemVer* %result, i32 0, i32 0, !dbg !71
  store i32 0, i32* %1, align 4, !dbg !73
  %2 = getelementptr inbounds %Stage2SemVer, %Stage2SemVer* %result, i32 0, i32 1, !dbg !74
  store i32 9, i32* %2, align 4, !dbg !75
  %3 = getelementptr inbounds %Stage2SemVer, %Stage2SemVer* %result, i32 0, i32 2, !dbg !76
  store i32 0, i32* %3, align 4, !dbg !77
  %4 = bitcast { i64, i32 }* %0 to i8*, !dbg !78
  %5 = bitcast %Stage2SemVer* %result to i8*, !dbg !78
  call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 8 %4, i8* align 4 %5, i64 12, i1 false), !dbg !78
  %6 = load { i64, i32 }, { i64, i32 }* %0, align 4, !dbg !78
  ret { i64, i32 } %6, !dbg !78
}

{i64, i32} is probably the wrong way to lower that. Should probably be {i32, i32, i32}. We can verify by checking clang output:

#include <stdint.h>

struct Stage2SemVer {
    uint32_t major;
    uint32_t minor;
    uint32_t patch;
};

struct Stage2SemVer stage2_version(void) {
    struct Stage2SemVer result = {0, 9, 0};
    return result;
}
zig cc -emit-llvm -S test.c -target i386-linux
; Function Attrs: mustprogress nofree norecurse nosync nounwind readnone sspstrong willreturn
define [2 x i64] @stage2_version() local_unnamed_addr #0 {
  ret [2 x i64] [i64 38654705664, i64 0]
}

OK so I was completely wrong, apparently the way to lower this is [2 x i64]. That kinda seems compatible with {i64, i32}. So maybe this function isn't actually the issue. Should probably try to follow the repro steps in the original post and figure out which of the C ABI cases are failing. Or idk maybe [2 x i64] lowers to something weird in llvm.

Anyway that's the clue. good luck and good night

@andrewrk andrewrk modified the milestones: 0.10.0, 0.11.0 Aug 26, 2022
@Vexu Vexu removed the stage1 The process of building from source via WebAssembly and the C backend. label Jan 13, 2023
@Vexu Vexu modified the milestones: 0.11.0, 0.12.0 Apr 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arch-x86 32-bit x86 backend-llvm The LLVM backend outputs an LLVM IR Module. bug Observed behavior contradicts documented or intended behavior
Projects
None yet
Development

No branches or pull requests

2 participants