Stabilize extern "custom"

[folkertdev]

View all comments

tracking issue: #140829
reference PR: rust-lang/reference#2300
closes #140829

Summary

An extern "custom" fn is a function with a custom ABI that is unknown to rust. Often these are low-level functions that pass arguments in different registers than any standard calling convention.

#[unsafe(naked)]
pub unsafe extern "custom" fn __aeabi_uidivmod() {
    core::arch::naked_asm!(
        "push {{lr}}",
        "sub sp, sp, #4",
        "mov r2, sp",
        "bl {trampoline}",
        "ldr r1, [sp]",
        "add sp, sp, #4",
        "pop {{pc}}",
        trampoline = sym crate::arm::__udivmodsi4
    );
}

unsafe extern "custom" {
	fn __fentry__();
}

Design

Because rust doesn't know what calling convention to use, an extern "custom" function can only be called via inline assembly or FFI.

error: functions with the "custom" ABI cannot be called
 --> <source>:5:5
  |
5 |     bar();
  |     ^^^^^
  |
note: an `extern "custom"` function can only be called using inline assembly

An extern "custom" function definition must be a naked function:

error: items with the "custom" ABI can only be declared externally or defined via naked functions
  --> <source>:10:1
   |
10 | unsafe extern "custom" fn bar() {
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
help: convert this to an `#[unsafe(naked)]` function
   |
10 + #[unsafe(naked)]
11 | unsafe extern "custom" fn bar() {
   |

An extern "custom" function definition must be unsafe. The intent here is that a safety comment is written on how this function may be called.

error: functions with the "custom" ABI must be unsafe
  --> <source>:10:1
   |
10 | extern "custom" fn bar() {
   | ^^^^^^^^^^^^^^^^^^^^^^^^
   |
help: add the `unsafe` keyword to this definition
   |
10 | unsafe extern "custom" fn bar() {
   | ++++++

In an extern "custom" block, functions cannot be marked as safe:

error: foreign functions with the "custom" ABI cannot be safe
  --> <source>:16:5
   |
16 |     safe fn foobar();
   |     ^^^^^^^^^^^^^^^^^
   |
help: remove the `safe` keyword from this definition
   |
16 -     safe fn foobar();
16 +     fn foobar();

An extern "custom" function cannot have any arguments or a return type:

error: invalid signature for `extern "custom"` function
 --> <source>:6:31
  |
6 | unsafe extern "custom" fn foo(a: i32) -> i32 {
  |                               ^^^^^^     ^^^
  |
  = note: functions with the "custom" ABI cannot have any parameters or return type
help: remove the parameters and return type
  |
6 - unsafe extern "custom" fn foo(a: i32) -> i32 {
6 + unsafe extern "custom" fn foo() {
  |

Tests

• tests/ui/abi/custom.rs tests that the feature works as expected, e.g. that functions can be defined, symbols are defined, and extern blocks can be used.
• tests/ui/abi/bad-custom.rs checks the restrictions: definitions must be unsafe and naked, attempting to call an extern "custom" function gives an error, etc.

History

• Lang proposal: extern "unspecified" for naked functions with arbitrary ABI #140566
• Tracking Issue for abi_custom #140829
• add extern "custom" functions #140770
• check extern "custom" function pointers #159780

unresolved questions

None

[folkertdev]

r? tgross35

[rustbot]

tgross35 is currently at their maximum review capacity.
They may take a while to respond.

[rustbot]

compiler-builtins is developed in its own repository. If possible, consider making this change to rust-lang/compiler-builtins instead.

cc @tgross35

[bjorn3]

Do we deny this on wasm? Wasm requires the function signature to be known when defining or importing it.

[folkertdev]

In that case, does the whole concept of a naked function even make sense there? That is, can it do something that a normal function whose body is an asm! block cannot do?

Practically inline assembly is unstable (and extremely incomplete) for wasm, so I'm not sure if/how it'll eventually fit in.

But we can deny extern "custom" on wasm if that seems best right now.

[bjorn3]

Naked asm can avoid touching the stack, maybe there are cases that is useful? Naked asm doesn't help for defining functions that use ref types or GC types as there isn't a way to express those using rust syntax, so those still need global_asm!().

[823984418]

Allowing only assembly calls seems quite restrictive in terms of cross platform capabilities, as a special case, an extern "custom" fn function that internally calls only another extern "custom" fn function should be allowed and may be treated as syntactic sugar for a direct jump.

[asquared31415]

This is a restriction on naked functions already, they must not contain Rust code.

[folkertdev]

Just making this explicit: #158621 removes support for wasm and spirv targets. Neither have stable assembly, so practically this doesn't change anything, but the whole concept of extern "custom" doesn't really make sense there.

[traviscross]

Makes sense to me. Thanks @folkertdev for your work on this.

@rfcbot fcp merge lang

cc @Amanieu

[rust-rfcbot]

@traviscross has proposed to merge this. The next step is review by the rest of the tagged team members:

• @joshtriplett
• @nikomatsakis
• @scottmcm
• @tmandry
• @traviscross

Concerns:

• needs-rfc resolved by Stabilize extern "custom" #158504 (comment)

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns.
See this document for info about what commands tagged team members can give me.

[tgross35]

It looks like we currently accept generics, this currently builds:

#![feature(abi_custom)]

use std::arch::naked_asm;

#[unsafe(naked)]
unsafe extern "custom" fn increment<T>() {
    naked_asm!(
        "add rax, 1",
        "ret",
    )
}

https://rust.godbolt.org/z/cv77sYnqE:

I think we should reject this for the same reason we reject arguments. @rust-lang/lang to confirm

[tgross35]

Changing my mind here, I can see valid usecases for something like:

#![feature(abi_custom)]

use std::arch::asm;
use std::arch::naked_asm;

/// # Safety
/// Something about ABI and overflow...
#[unsafe(naked)]
unsafe extern "custom" fn increment<T, const N: usize>() {
    naked_asm!(
        "add rax, {x}",
        "add rax, {N}",
        "ret",
        x = const size_of::<T>(),
        N = const N,
    )
}

fn main() {
    let mut x: u64 = 0;
    unsafe {
        asm!(
            "call {}",
            sym increment::<u32, 10>,
            inout("rax") x,
        );
    }
    assert_eq!(x, 4);
}

https://rust.godbolt.org/z/odj6Tcsbd

But lang still needs to weigh in since this didn't come up in the RFC or reference PR as far as I can tell.

@rustbot label +I-lang-nominated

Edit: I see we actually test for this already, at least with const generics

rust/tests/ui/abi/custom.rs

Lines 48 to 54 in 4667d75

	unsafe extern "C" fn const_generic<const N: u64>() {
	naked_asm!(
	"mov rax, {}",
	"ret",
	const N,
	);
	}

[tgross35]

Second question: should we reject #[cold]? I don't think there's anything we can do with it, assuming it can't bubble up through inline asm somehow.

Currently accepted https://rust.godbolt.org/z/rh5aMbcGK

[tgross35]

Some small test suggestions, otherwise stabilization looks fine to me against rust-lang/reference#2300 at 66b3bc5b8688 with the possible exception of generics and #[cold]. I guess generics are probably fine, would just like somebody from lang to confirm since I haven't seen it discussed anywhere.

View changes since this review

[traviscross]

Thanks @tgross35 for flagging those. For my own part, I agree that generics should be accepted and that cold likely should not be.

[joshtriplett]

As I understand it, in some cases #[cold] might cause something to get placed differently in the binary (e.g. adjacent to other cold items)? If that were the behavior, that could still work, right?

[joshtriplett]

If #[cold] is currently having zero effect on a custom function, then we should reject it at compile time.

If at some point this becomes capable of affecting function placement in the binary, that makes sense to apply to custom, and at that point we should accept it.

[traviscross]

We discussed this in the lang call. We agree that generics should be supported and that, for now, cold on these items should be made an error.

[traviscross]

In the lang call, we also discussed the rule for the return type. (Thanks to @folkertdev for raising this.) The return type on these items is constrained to unit. Is that constraint syntactic (i.e., the return type must be elided or () written explicitly) or semantic (i.e., true after normalization of type aliases)?

We agreed this rule should be enforced syntactically.

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[tgross35]

#[cold] is rejected since #161002 and lang decided generics should be accepted, so everything seems in order to me!

@bors r+

View changes since this review

[rust-bors]

📌 Commit 652ab85 has been approved by tgross35

It is now in the queue for this repository.