rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.78k stars 12.77k forks source link

ABI mismatch between rustc and clang for passing ZSTs using the win64 ABI. #132893

Open programmerjake opened 1 week ago

programmerjake commented 1 week ago

I tried this code: https://clang.godbolt.org/z/eT5xEz757 rustc -O -C debuginfo=0 --target=x86_64-pc-windows-msvc

use std::ffi::c_int;

#[repr(C)]
pub struct Z {
    v: [c_int; 0],
}

const _: () = assert!(std::mem::size_of::<Z>() == 0);

#[no_mangle]
extern "win64" fn f(a: Z, b: c_int) -> c_int {
    let _ = a;
    b + 1
}

#[no_mangle]
extern "win64" fn g(a: c_int) -> c_int {
    a
}

clang -O -g0 --target=x86_64-pc-windows-gnu

struct Z {
    int v[0];
};

_Static_assert(sizeof(struct Z) == 0, "Z should be a ZST");

int __attribute__((ms_abi)) f(struct Z a, int b) {
    return b + 1;
}

int __attribute__((ms_abi)) g(int a) {
    return a;
}

I expected to see this happen: the generated assembly reads from the same register no matter if using clang or rustc.

Instead, this happened: rustc's f reads from rcx whereas clang's f reads from rdx.

Meta

rustc --version --verbose:

rustc 1.82.0 (f6e511eec 2024-10-15)
binary: rustc
commit-hash: f6e511eec7342f59a25f7c0534f1dbea00d01b14
commit-date: 2024-10-15
host: x86_64-unknown-linux-gnu
release: 1.82.0
LLVM version: 19.1.1

It also does the exact same thing on:

rustc 1.84.0-nightly (b91a3a056 2024-11-07)
binary: rustc
commit-hash: b91a3a05609a46f73d23e0995ae7ebb4a4f429a5
commit-date: 2024-11-07
host: x86_64-unknown-linux-gnu
release: 1.84.0-nightly
LLVM version: 19.1.3

clang version: 19.1.0

Generated Assembly

clang's assembly: ```asm f: lea eax, [rdx + 1] ret g: mov eax, ecx ret ``` rustc's assembly ```asm f: lea eax, [rcx + 1] ret g: mov eax, ecx ret ```

workingjubilee commented 1 week ago

@rustbot label: +A-ABI +A-FFI +O-windows-msvc

Fulgen301 commented 1 week ago

This code fails to compile if you target x86_64-pc-windows-msvc in Clang instead of x86_64-pc-windows-gnu due to sizeof(struct Z) no longer being 0. This is therefore a Clang bug between the interaction of [[gnu::ms_abi]] and its GNU target and not related to Rust.

programmerjake commented 1 week ago

clang matches gcc here, so I think this is a rustc bug rather than a clang bug. the reason the assert fails is instead because the msvc target has different struct layout rules so you don't get a ZST by default, there may be some attribute or pragma that lets you get a ZST on the msvc target, but I haven't found it (though didn't particularly look).

Fulgen301 commented 1 week ago

ZSTs aren't a thing in the MSVC ABI (nor are they in standard C or standard C++, they're a GCC extension). The only reason you can have a ZST in a MS ABI C function is because you're targeting the GNU ABI, but as the MSVC ABI doesn't have ZSTs, Clang doesn't elide the parameter. It would have made more sense for GCC and Clang to recognize that ZSTs aren't a thing in the ABI and completely elide the parameter, since you can never call that function with a non-ZST struct anyway due an to ABI mismatch, and you can't define that struct as a ZST in MSVC, but they didn't do that.

(How much sense it makes for GCC and Clang to allow that construct without even a single diagnostic is a different question.)

It would be a rustc bug if rustc promised to have exactly the same ABI for its ZST as a compiler extension to standard C, which to my knowledge it doesn't. And even if it did and therefore were to pessimize Rust for an obscure usecase, you're also comparing Clang targeting the GNU target to Rust targeting the MSVC target, which is rather pointless - why should rustc targeting MSVC care what Clang does in the GNU target?

bjorn3 commented 1 week ago

We only preserve ZST arguments for the win64 call conv on windows gnu targets, not on msvc targets or non-windows targets: https://github.com/rust-lang/rust/blob/3a258d1cf9d52831820c223018fd74b208b5c65a/compiler/rustc_target/src/callconv/x86_win64.rs#L44-L50

programmerjake commented 1 week ago

why should rustc targeting MSVC care what Clang does in the GNU target?

because they're both using the win64 ABI here, which is supposed to interoperate between those two targets, since that's how you access windows APIs using the MSVC calling convention from the GNU target

ChrisDenton commented 1 week ago

ZSTs will be a non-issue for Windows APIs. Surely it's only a subset of the ABI that GNU declares compatible with system APIs?

programmerjake commented 1 week ago

well, the way I see it, if you can pass a ZST through the win64 ABI from both GCC and clang and they agree on an ABI (which they do), then rustc should also support that and have the same ABI for any targets that can access the x86-64 win64 ABI (so, windows msvc, windows gnu, linux (for Wine-style things or assembly or JIT-compiled code), etc.).

ChrisDenton commented 1 week ago

I'm not sure that we can promise that because it's outside our control if it depends on everyone else agreeing on an Windows x86-64 ABI beyond what's needed to interop with the system.

If clang with a windows-gnu target has an ABI that differs from clang with a windows-msvc target then what can we do about it?

programmerjake commented 1 week ago

If clang with a windows-gnu target has an ABI that differs from clang with a windows-msvc target then what can we do about it?

afaict they don't differ for passing ZSTs, the issue is more that that is vacuously true on the windows-msvc target since I don't think you can make a ZST (if you can figure out how, please share!).

RalfJung commented 6 days ago

If clang with a windows-gnu target has an ABI that differs from clang with a windows-msvc target then what can we do about it?

We can discuss that case when it comes up. These are all case-by-case decisions anyway.

But that's not the case we have here. What we have here is two older compilers implementing this ABI in a certain way, and us doing something else. Seems fairly clear to me that rustc should change here. Maybe we should change to "stop compilation if a ZST is passed via that ABI", but that's generally not what we do for any ABI, so "skip ZST like clang and gcc do" seems best.