Add extern "custom"

[folkertdev]

View all comments

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__();
}

History

• Lang proposal: extern "unspecified" for naked functions with arbitrary ABI rust#140566
• Tracking Issue for abi_custom rust#140829
• add extern "custom" functions rust#140770
• Stabilize extern "custom" rust#158504

Important

Since RFCs involve many conversations at once that can be difficult to follow, please use review comment threads on the text changes instead of direct comments on the RFC.

If you don't have a particular section of the RFC to comment on, you can click on the "Comment on this file" button on the top-right corner of the diff, to the right of the "Viewed" checkbox. This will create a separate thread even if others have commented on the file too.

Rendered

[traviscross]

Thanks @folkertdev for putting this together.

@rfcbot fcp merge lang

[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

No concerns currently listed.

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.

[tmandry]

@rfcbot reviewed

[rust-rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[nikomatsakis]

@rfcbot reviewed

[ChayimFriedman2]

Seeing a RFC entering a final comment period 1 hour after it was opened is extremely rare, and personally makes me worried. Even if it was discussed extensively in the past and all relevant team members are on-board, I believe we should let the community more time to discuss it.

[programmerjake]

imo it's fine since that's what the final comment period is for, announcing and giving people time to look at something before it's accepted, when the team already thinks it's good enough.

[folkertdev]

Right, this is the (final) ping for concerns to be raised. This was coordinated during the T-lang triage meeting.

This feature is based on the following lang proposal, it is basically unchanged from that thread.

• Lang proposal: extern "unspecified" for naked functions with arbitrary ABI rust#140566

[clarfonthey]

I also felt a bit concerned by the sudden FCP, but the methodology here seems pretty canonical since it uses naked functions.

The only potential concern is bikeshedding the name "custom" which could potentially be worded as "none" (since this effectively is no ABI) but that's a pretty weak point and not worth blocking IMHO. Plus, it could be changed even post-acceptance but pre-stabilisation.

I see no reason to slow down on the FCP process.

[steffahn]

Plus, it could be changed even post-acceptance but pre-stabilisation.

I would expect this time frame to be fairly short in this case.

[Lokathor]

"none" is incorrect, as explained in the RFC, there is an ABI happening, it's just not an ABI the compiler knows how to use without assistance.

[clarfonthey]

"none" is incorrect, as explained in the RFC, there is an ABI happening, it's just not an ABI the compiler knows how to use without assistance.

Yes, this is another reason justifying the choice, although I do think that "none" that the compiler knows about is still appropriate: the compiler is not using any ABI for the function, even though yes, technically, ABI will always exist no matter what. Even just jumping to a label is still an ABI, after all.

My point is that this is technically a weak point of contention, but not one I think is worth discussing here, since the name is more than adequately justified.

[rust-rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

[clarfonthey]

A little uncomfortable that not only an FCP here, but an FCP in the main repo to stabilise both passed while there still seem to be a few active discussions. I don't think we need to block stabilisation on another FCP but I think we should at least address all the feedback on the RFC before merging the feature into stable.

[folkertdev]

We discussed this again in the T-lang meeting on July 22nd (see notes), deciding to reject argument and return types, but to note that they can be a future extension.

Waffle (in DMs) requested that we don't special-case ! here. Like any other return type, it is purely documentation and can go out of sync with the implementation. Looking at the implementation and comments from the initial design again, I don't think that including -> ! was all that deliberate, but rather a result of the shared logic with the extern "interrupt-*" calling conventions. The PR linked below removes support for -> !, and I've added allowing specifically ! as a return type as a future extension.

Finally, I've added function pointer checks in rust-lang/rust#159780 and updated the text to reflect that we do enforce the restrictions (must be unsafe, no argument and return types) everywhere, including function pointer types.

With that, I believe the RFC text matches the current consensus of T-lang and friends, and can be merged. We can then let the dust settle a bit, and move forward with the stabilization PR in a couple of weeks.

I understand that there are some people in the thread here that would like to e.g. see arguments: that door is not closed. It just doesn't seem needed right now. If someone feels they have a compelling use case after stabilization of this minimal version, they can pick up the future extensions.

[traviscross]

The lang team has accepted this RFC, and we've now merged it.

Thanks to @folkertdev for pushing forward this important work, and thanks to all those who reviewed this RFC and provided useful feedback.

For further updates, please follow the tracking issue:

• Tracking Issue for abi_custom rust#140829