Check ABI target compatibility for function pointers

[tdittr]

Tracking issue: #130260
Related tracking issue: #87678

Compatibility of an ABI for a target was previously only performed on function definitions and extern blocks. This PR adds it also to function pointers to be consistent.

This might have broken some of the tests/ui/ depending on the platform, so a try run seems like a good idea.

Also this might break existing code, because we now emit extra errors. Does this require a crater run?

Example

// build with: --target=x86_64-unknown-linux-gnu 

// These raise E0570
extern "thiscall" fn foo() {}
extern "thiscall" { fn bar() }

// This did not raise any error
fn baz(f: extern "thiscall" fn()) { f() }

Open Questions

• Should this report a future incompatibility warning like Tracking issue for future-incompatibility lint unsupported_calling_conventions #87678 ?
• Is this the best place to perform the check?

[rustbot]

r? @compiler-errors

rustbot has assigned @compiler-errors.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[rustbot]

HIR ty lowering was modified

cc @fmease

[tdittr]

Some notes we had while writing this PR.

[tdittr]

Thinking about it and seeing how many tests already broke by this and this only showing up on specific targets, maybe this should only raise a future incompatibility warning?

[tdittr]

@compiler-errors could you trigger a try run for this PR? Just to check if there are more tests that fail when run on a different target

[jieyouxu]

could you trigger a try run for this PR? Just to check if there are more tests that fail when run on a different target

Do you have any particular targets in mind? I added some common try-jobs to your PR description. We can run a first battery of try-jobs on the common targets.

[jieyouxu]

@bors try

[bors]

⌛ Trying commit 0cef57e with merge adb3631...

[bors]

💔 Test failed - checks-actions

[tdittr]

could you trigger a try run for this PR? Just to check if there are more tests that fail when run on a different target

Do you have any particular targets in mind? I added some common try-jobs to your PR description. We can run a first battery of try-jobs on the common targets.

Thanks you! Sorry I missed that I could have filtered out specific jobs. I'll try to fix this soon.

[jieyouxu]

Thanks you! Sorry I missed that I could have filtered out specific jobs. I'll try to fix this soon.

Feel free to ping me if you need another run.

[tdittr]

Just saw the t-compiler/meeting discussion that mentioned this PR. And this comment by @compiler-errors

At least AFAICT #128784 is motivated primarily by "C-cmse-nonsecure-call"

I think this PR actually fixes a Bug, we came across this while working on CMSE, but the problem seems to be bigger. ABIs are not checked at all for target compatibility when used with function pointers.

Or did we miss something and not checking ABIs on function pointers is on purpose? Is there a valid use-case for using a function pointer with an ABI for a different target somewhere?

[tdittr]

Just so I don't forget to check them. The list of all usages of function pointers in rust-lang/rust:

with ABIs listed in the reference

$ rg --sort path --iglob \*.rs 'extern\s+"(cdecl|stdcall|win64|sysv64|aapcs|fastcall|vectorcall|thiscall|efiapi)"\s+fn\s*\('

✔️ This is gated by #[cfg(target_os = "uefi")] so should be fine.

library/std/src/sys/pal/uefi/helpers.rs
27:    unsafe extern "efiapi" fn(_: *mut r_efi::efi::Handle, _: ...) -> r_efi::efi::Status;
30:    unsafe extern "efiapi" fn(_: r_efi::efi::Handle, _: ...) -> r_efi::efi::Status;

• this needs to be checked, maybe we can use another ABI here?

tests/debuginfo/type-names.rs
20:// gdb-check:type = type_names::GenericStruct<type_names::Struct1, extern "fastcall" fn(isize) -> usize>
375:    let generic_struct2: GenericStruct<Struct1, extern "fastcall" fn(isize) -> usize> =

✔️ Rustdoc seems fine with this even on aarch64

tests/rustdoc-json/fn_pointer/abi.rs
21:pub type AbiVecorcall = extern "vectorcall" fn();

✔️ This is gated by #[cfg(target_arch = "x86_64")]

tests/ui/abi/abi-sysv64-arg-passing.rs
272:    fn indirect_call(func: unsafe extern "sysv64" fn(s: S) -> u64, s: S) -> u64 {

✔️ These were added by this PR

tests/ui/abi/unsupported.rs
47:fn aapcs_ptr(f: extern "aapcs" fn()) {
122:fn thiscall_ptr(f: extern "thiscall" fn()) {
148:fn stdcall_ptr(f: extern "stdcall" fn()) {

✔️ gated by //@ only-x86_64

tests/ui/asm/issue-97490.rs
5:pub type Yes = extern "sysv64" fn(&'static u8) -> !;

✔️ gated by //@ only-x86_64

tests/ui/c-variadic/feature-gate-extended_varargs_abi_support.rs
3:fn efiapi(f: extern "efiapi" fn(usize, ...)) {
8:fn sysv(f: extern "sysv64" fn(usize, ...)) {
13:fn win(f: extern "win64" fn(usize, ...)) {

✔️ gated by //@ only-arm

tests/ui/c-variadic/variadic-ffi-2-arm.rs
5:fn aapcs(f: extern "aapcs" fn(usize, ...)) {

✔️ gated by //@ ignore-arm and various #[cfg(target_arch = "...")]

tests/ui/c-variadic/variadic-ffi-2.rs
5:fn baz(f: extern "stdcall" fn(usize, ...)) {
15:fn sysv(f: extern "sysv64" fn(usize, ...)) {
19:fn win(f: extern "win64" fn(usize, ...)) {
22:fn efiapi(f: extern "efiapi" fn(usize, ...)) {

✔️ has //@ compile-flags: --target=i686-pc-windows-msvc

tests/ui/feature-gates/feature-gate-vectorcall.rs
29:type TA = extern "vectorcall" fn(); //~ ERROR vectorcall is experimental

with other ABIs

$ rg --sort path --iglob \*.rs 'extern\s+"(x86-interrupt|aapcs-unwind|C-cmse-nonsecure-call|ptx-kernel|msp430-interrupt|riscv-interrupt-m|riscv-interrupt-s|avr-interrupt|avr
-non-blocking-interrupt)"\s+fn\s*\('

✔️ only in comments

compiler/rustc_feature/src/unstable.rs
333:    /// Allows `extern "avr-interrupt" fn()` and `extern "avr-non-blocking-interrupt" fn()`.
335:    /// Allows `extern "C-cmse-nonsecure-call" fn()`.
337:    /// Allows `extern "msp430-interrupt" fn()`.
341:    /// Allows `extern "riscv-interrupt-m" fn()` and `extern "riscv-interrupt-s" fn()`.
343:    /// Allows `extern "x86-interrupt" fn()`.

✔️ only in text

src/tools/rust-analyzer/crates/ide-db/src/generated/lints.rs
1091:        unsafe { core::mem::transmute::<usize, extern "C-cmse-nonsecure-call" fn() -> u32>(addr) };
1163:pub static TIM0_VECTOR: extern "msp430-interrupt" fn() = tim0;

✔️ introduced by this PR

tests/ui/abi/unsupported.rs
34:fn ptx_ptr(f: extern "ptx-kernel" fn()) {
64:fn msp430_ptr(f: extern "msp430-interrupt" fn()) {
73:fn avr_ptr(f: extern "avr-interrupt" fn()) {
85:fn riscv_ptr(f: extern "riscv-interrupt-m" fn()) {
103:fn x86_ptr(f: extern "x86-interrupt" fn()) {
173:fn cmse_ptr(f: extern "C-cmse-nonsecure-call" fn()) {

✔️ Checks for this error and ignores supported targets

tests/ui/cmse-nonsecure/cmse-nonsecure-call/gate_test.rs
9:        core::mem::transmute::<usize, extern "C-cmse-nonsecure-call" fn(i32, i32, i32, i32) -> i32>(

✔️ Has //@ compile-flags: --target thumbv8m.main-none-eabi

tests/ui/cmse-nonsecure/cmse-nonsecure-call/generics.rs
17:    f2: extern "C-cmse-nonsecure-call" fn(impl Copy, u32, u32, u32) -> u64,
19:    f3: extern "C-cmse-nonsecure-call" fn(T, u32, u32, u32) -> u64, //~ ERROR [E0798]
20:    f4: extern "C-cmse-nonsecure-call" fn(Wrapper<T>, u32, u32, u32) -> u64, //~ ERROR [E0798]

✔️ Has //@ compile-flags: --target thumbv8m.main-none-eabi

tests/ui/cmse-nonsecure/cmse-nonsecure-call/params-via-stack.rs
17:    f1: extern "C-cmse-nonsecure-call" fn(u32, u32, u32, u32, x: u32, y: u32), //~ ERROR [E0798]
18:    f2: extern "C-cmse-nonsecure-call" fn(u32, u32, u32, u16, u16),            //~ ERROR [E0798]
19:    f3: extern "C-cmse-nonsecure-call" fn(u32, u64, u32),                      //~ ERROR [E0798]
20:    f4: extern "C-cmse-nonsecure-call" fn(AlignRelevant, u32),                 //~ ERROR [E0798]
21:    f5: extern "C-cmse-nonsecure-call" fn([u32; 5]),                           //~ ERROR [E0798]

✔️ Has //@ compile-flags: --target thumbv8m.main-none-eabi

tests/ui/cmse-nonsecure/cmse-nonsecure-call/return-via-stack.rs
25:    f1: extern "C-cmse-nonsecure-call" fn() -> ReprCU64, //~ ERROR [E0798]
26:    f2: extern "C-cmse-nonsecure-call" fn() -> ReprCBytes, //~ ERROR [E0798]
27:    f3: extern "C-cmse-nonsecure-call" fn() -> U64Compound, //~ ERROR [E0798]
28:    f4: extern "C-cmse-nonsecure-call" fn() -> ReprCAlign16, //~ ERROR [E0798]
29:    f5: extern "C-cmse-nonsecure-call" fn() -> [u8; 5],  //~ ERROR [E0798]
35:    u128: extern "C-cmse-nonsecure-call" fn() -> u128, //~ ERROR [E0798]
36:    i128: extern "C-cmse-nonsecure-call" fn() -> i128, //~ ERROR [E0798]
51:    f1: extern "C-cmse-nonsecure-call" fn() -> ReprRustUnionU64, //~ ERROR [E0798]
52:    f2: extern "C-cmse-nonsecure-call" fn() -> ReprCUnionU64,    //~ ERROR [E0798]

✔️ Has //@ compile-flags: --target thumbv8m.main-none-eabi

tests/ui/cmse-nonsecure/cmse-nonsecure-call/via-registers.rs
31:    f1: extern "C-cmse-nonsecure-call" fn(),
32:    f2: extern "C-cmse-nonsecure-call" fn(u32, u32, u32, u32),
33:    f3: extern "C-cmse-nonsecure-call" fn(u64, u64),
34:    f4: extern "C-cmse-nonsecure-call" fn(u128),
35:    f5: extern "C-cmse-nonsecure-call" fn(f64, f32, f32),
36:    f6: extern "C-cmse-nonsecure-call" fn(ReprTransparentStruct<u64>, U32Compound),
37:    f7: extern "C-cmse-nonsecure-call" fn([u32; 4]),
43:    f1: extern "C-cmse-nonsecure-call" fn() -> u32,
44:    f2: extern "C-cmse-nonsecure-call" fn() -> u64,
45:    f3: extern "C-cmse-nonsecure-call" fn() -> i64,
46:    f4: extern "C-cmse-nonsecure-call" fn() -> f64,
47:    f5: extern "C-cmse-nonsecure-call" fn() -> [u8; 4],
48:    f6: extern "C-cmse-nonsecure-call" fn() -> ReprTransparentStruct<u64>,
49:    f7: extern "C-cmse-nonsecure-call" fn() -> ReprTransparentStruct<ReprTransparentStruct<u64>>,
50:    f8: extern "C-cmse-nonsecure-call" fn() -> ReprTransparentEnumU64,
51:    f9: extern "C-cmse-nonsecure-call" fn() -> U32Compound,

✔️ Has //@ compile-flags: --target=avr-unknown-gnu-atmega328

tests/ui/feature-gates/feature-gate-abi-avr-interrupt.rs
43:type TA = extern "avr-interrupt" fn();
45:type TAU = extern "avr-non-blocking-interrupt" fn();

✔️ Has //@ compile-flags: --target=...

tests/ui/feature-gates/feature-gate-abi-msp430-interrupt.rs
30:type TA = extern "msp430-interrupt" fn();

✔️ Has //@ compile-flags: --target=...

tests/ui/feature-gates/feature-gate-abi-riscv-interrupt.rs
32:type TA = extern "riscv-interrupt-m" fn();

✔️ Has //@ compile-flags: --target=...

tests/ui/feature-gates/feature-gate-abi-x86-interrupt.rs
26:type A7 = extern "x86-interrupt" fn(); //~ ERROR x86-interrupt ABI is experimental

✔️ Has //@ compile-flags: --target=...

tests/ui/feature-gates/feature-gate-abi_ptx.rs
24:type TAU = extern "ptx-kernel" fn(); //~ ERROR PTX ABIs are experimental

If there is a better way then greping these I would be happy to learn about it 😉

[tdittr]

I'm not sure if it is ok to change this test from fastcall (only allowed on x86) to system.

From what I understand this test only wants to check that the ABI actually show up in the debug output, and I would guess that system works just as nice for this.

[tdittr]

I think I found all the places that use extern "..." fn() now. Can you give it another try @jieyouxu ? I think the task selection looks good the way it is right now.

[jieyouxu]

I think I found all the places that use extern "..." fn() now. Can you give it another try @jieyouxu ? I think the task selection looks good the way it is right now.

Sure. @bors try

[bors]

⌛ Trying commit b2be5a8 with merge d7da183...

[bors]

💔 Test failed - checks-actions

[tdittr]

Oh I missed that one, I think it is fixed now, tested locally with s390x-unknown-linux-gnu which triggered the same case. Can we give it one more try @jieyouxu ? I also reduced the try jobs a bit, since msvc or mingw should not make a difference.

[jieyouxu]

Oh I missed that one, I think it is fixed now, tested locally with s390x-unknown-linux-gnu which triggered the same case. Can we give it one more try @jieyouxu ? I also reduced the try jobs a bit, since msvc or mingw should not make a difference.

Sure. @bors try

[RalfJung]

@rustbot ready

[RalfJung]

@compiler-errors wrote

LGTM, but T-lang nominating as a final lint sign-off.

But I'm going to let them do the r+ since this is quite the big PR and it did require some rebasing since the review.

[compiler-errors]

LGTM. One diagnostic tweak, but I can review that as a follow-up PR.

[compiler-errors]

This diagnostic is a bit awkward. Perhaps change it to:

"the calling convention {} is not supported on this target"

I think mentioning fn ptrs is also unnecessary, since it's also a warning for items (and soon an error).

[compiler-errors]

@bors r+

[bors]

📌 Commit 867e776 has been approved by compiler-errors

It is now in the queue for this repository.

[compiler-errors]

@tdittr would you like to tweak this message as a follow-up, or should I?

[tdittr]

@tdittr would you like to tweak this message as a follow-up, or should I?

I will do it :)