Vexu / arocc

A C compiler written in Zig.
http://aro.vexu.eu/
MIT License
1.09k stars 55 forks source link

Record layout tests failing for `sparc-v9-solaris-eabi:Clang` #747

Closed ehaas closed 1 month ago

ehaas commented 1 month ago

It looks like Sparc v9 is now treated as a 32-bit system, which is causing the layout tests to fail. Unclear if this is because the layout tests themselves are wrong or if the problem is in aro or std.Target

alexrp commented 1 month ago

It's most likely because of https://github.com/ziglang/zig/pull/20960/commits/db8f00e277016e12495bcc5217090052eea69eb3 (part of https://github.com/ziglang/zig/pull/20960) potentially in combination with https://github.com/ziglang/zig/pull/20934. What that code was doing before was just super wrong. The way you determine bitness is from the arch component (sparc vs sparc64). It's perfectly valid to run sparc32 code on a v9 CPU.

ehaas commented 1 month ago

Ok I dug into things a bit more. In 0001_test.c we have the following, for target sparc-solaris-eabi (with v9 cpu model)

typedef struct {
    int a;
    long b;
    void* c;
} X;
_Static_assert(sizeof(X) == 24, "");

Previously it worked because that was treated as a 64 bit target, now it fails because it's a 32 bit target. The test ultimately comes from here: https://github.com/mahkoh/repr-c/blob/master/repc/tests/testfiles/0001/input.txt which produces https://github.com/mahkoh/repr-c/blob/master/repc/tests/testfiles/0001/output/sparcv9-sun-solaris.expected.txt for sparcv9-sun-solaris (note 64 bit longs and pointers).

That C program roughly corresponds to this Rust program:

// rustc test.rs --target=sparcv9-sun-solaris
#![no_main]
use std::mem;

#[repr(C)]
struct X {
    first: std::os::raw::c_int,
    second: std::os::raw::c_long,
    third: *mut std::os::raw::c_void,
}

const _: () = assert!(24 == mem::size_of::<X>(), "size of X");

Which fails the assert if you change the 24 to something else.

I'm not sure what the conclusion is here - one answer is "the test target name is wrong" and we should change instances of sparc-v9-solaris-eabi to sparc64-v9-solaris-eabi in the layout tests. The other possible answer is "the layout tests themselves are wrong", but given how the test data was generated in the first place (https://github.com/mahkoh/repr-c/tree/master/repc/tests), I don't see how that's possible - supposedly it matches what was generated by clang itself.

Vexu commented 1 month ago

one answer is "the test target name is wrong" and we should change instances of sparc-v9-solaris-eabi to sparc64-v9-solaris-eabi in the layout tests.

That's probably it .sparc64 => .sparcv9, // In LLVM, sparc64 == sparcv9.. https://github.com/ziglang/zig/commit/fb0692334ed64a995690c21525fc3e729e63f424

alexrp commented 1 month ago

Note that sparcv9 and sparc64 should mean the same thing in a GNU target triple. Zig doesn't have sparcv9 (to avoid the very confusion we're seeing here), only sparc and sparc64.