RFC: Add realign_stack attribute to rustc

[benisxdxd]

More details are in the MD himself.
For more possible info see:
https://internals.rust-lang.org/t/stack-aligment-in-a-callback-function/2050
https://clang.llvm.org/docs/AttributeReference.html#force-align-arg-pointer

Rendered

[Lokathor]

This is useful on ARM as well, where an interrupt handler could be called when the stack isn't 8-byte aligned, which is what the extern "C" ABI expects.

Ideally, if an align_stack fn is called but it doesn't actually use the stack at all then the function could just skip aligning the stack.

[matthieu-m]

There's a #[realign_stack] attribute mentioned early on, but the rest of the RFC only refers to #[stack_realign], so I think the former is a left-over from a previous iteration and should be renamed.

[eggyal]

Cc rust-lang/rust#112480

[RalfJung]

I don't see how the stated motivation for this attribute makes sense. When I have a stack frame that contains SSE variables, it is the compiler's responsibility to ensure these variables are sufficiently aligned. Otherwise, that's a soundness bug in the compiler. In Rust we don't ask the user to change their code to fix a compiler bug.

The RFC should show some concrete code examples for code that needs this attribute.

rust-lang/rust#112480 is a bug in MSVC. My understanding is that Rust code for that target already guarantees that local variables are sufficiently aligned. It is only when you mix that with C code emitted by the broken Microsoft compiler that you get unaligned pointers. No amount of attributes on the Rust side can fix that. Did I misunderstand something?

[benisxdxd]

I don't see how the stated motivation for this attribute makes sense. When I have a stack frame that contains SSE variables, it is the compiler's responsibility to ensure these variables are sufficiently aligned. Otherwise, that's a soundness bug in the compiler. In Rust we don't ask the user to change their code to fix a compiler bug.

The RFC should show some concrete code examples for code that needs this attribute.

I had a non deterministic bug which was caused by a bugged stack alignment. The rust code assumed a stack alignment of 16, while the system's stack alignment was 4. No matter how much I changed the "data-layout", the problem remained.
A team member of mine forked and updated LLVM and added a "hacky" implementation of the realign_stack.

[Lokathor]

To perhaps clarify the goal: the requirements for data types aren't changed by this attribute. What's changed is the assumptions of what's true on function entry.

• If llvm needs the stack at 16 and can assume it's at 16 by default then it obviously doesn't need to do anything. This is our default in rust (on x86_64 at least).
• If llvm needs the stack at 16 but it can only assume the stack is at 4, then llvm needs to insert the correct adjustments before using the stack, and when returning.

Think of it as an ABI modification attribute.

[Lokathor]

@benisxdxd you could add the additional motivation that interrupt handler functions can't always assume the stack is aligned, and currently they need to do manual stack alignment tricks which are annoying to implement every time. Also, they're costly to do if the stack doesn't actually get used at all.

[RalfJung]

I had a non deterministic bug which was caused by a bugged stack alignment. The rust code assumed a stack alignment of 16, while the system's stack alignment was 4. No matter how much I changed the "data-layout", the problem remained.
A team member of mine forked and updated LLVM and added a "hacky" implementation of the realign_stack.

Again, that sounds like a compiler bug. Why does Rust assume more alignment than the system stack provides? One of them is wrong and should be fixed.

If this is some interrupt handler stuff, then it sounds like interrupt handlers can't use the C ABI. Is it possible to add an interrupt ABI that only assumes the alignment guaranteed by interrupts on the given target, and have the compiler automatically do the necessary adjustments when a function with that ABI calls a C ABI function?

[RalfJung]

What's changed is the assumptions of what's true on function entry.

That sounds like a boolean property: do we assume that the regular alignment is guaranteed, or do we not? One could consider having a C-unaligned ABI that makes no assumptions about stack alignment, so the function itself is responsible for doing whatever needs doing for its locals. I guess this could also be an attribute like #[no_stack_alignment_assumption], modifying the C ABI rather than a new ABI. 🤷

But saying "please align the stack to N" for a user-defined N makes no sense with that motivation. The RFC is conflating policy and mechanism. What you want is a function that can be called with any stack alignment; the way this is internally implemented is likely by having the function re-align the stack to some target-specific value, but that's an implementation detail. There's nothing in Rust that lets you argue "I aligned the stack to 128 and hence I can do X", so #[realign_stack(128)] is entirely meaningless.

[benisxdxd]

#[no_stack_alignment_assumption]

I think that would also solve my problem.

[Lokathor]

I don't think it should be an ABI. The ABI of a function affects what callers need to know about the function to make the call. Having a lower aligned stack isn't a caller concern at all. It's something only the internals of the function needs to potentially care about.

Also, a separate ABI combines poorly with other ABI stuff. We have two C ABI variants now for unwind yes/no, then we add this and now there's 4 C ABI variants. Then we get more and more variants down the road.

In fact, there's absolutely no reason to restrict this to the C ABI. It's not fundamentally unreasonable for a Rust ABI function to sometimes only assume a stack alignment of 4 or 1 or whatever, for example. (edit: since this is only needed for ffi we probably still should restrict it to C ABI, but we don't absolutely need to)

Since min-stack-alignment is a function property that calling code doesn't need to care about, we shouldn't make it an actual fn type difference. I'd say it's similar to the instruction_set attribute, which also doesn't affect the ABI because it's similarly a private property of the function you use the attribute on, and also doesn't change the type of a function you use the attribute on.

[RalfJung]

I don't think it should be an ABI. The ABI of a function affects what callers need to know about the function to make the call. Having a lower aligned stack isn't a caller concern at all. It's something only the internals of the function needs to potentially care about.

The caller needs to know which alignment the callee expects. For C functions that's apparently 16 on some targets?

The bugs you describe above are caused by caller violating this requirement. Clearly the caller is implementing a different ABI than the callee (caller aligns stack to 4, callee expects 16).

Also, a separate ABI combines poorly with other ABI stuff. We have two C ABI variants now for unwind yes/no, then we add this and now there's 4 C ABI variants. Then we get more and more variants down the road.

Yeah, the combinatorial explosion is annoying. It's not too surprising though, ABIs have a lot of knobs that one can tune independently.

Since min-stack-alignment is a function property that calling code doesn't need to care about,

I don't follow. The bugs you are describing are caused by the calling code (that calls the interrupt handler -- I guess it's calling silicon, but that doesn't make a difference) not caring about the callee's requirements (min-stack-alignment = 16)!

[Lokathor]

Ah, I think I understand your confusion. The thing is, calling code can't take advantage of a function having a lower minimum stack alignment. If you call a function, you (the caller) must align the stack to the target specified minimum. The only time this attribute is needed is when a function is expected to be called from weird situations that won't perform the correct alignment on their end.

The reason for the bug is because LLVM assumed the current fn had a stack align of 16 on entry, and so didn't adjust the stack before making the call or using align-16 data. If LLVM had known that the fn didn't have align-16 for the stack on entry, it would fix up the stack within the fn prologue whenever the stack is used (including if any other fn is called).

This is similar to the asm! macro nostack option. If it's left off of an asm! block then LLVM will ensure that the stack is aligned to the C ABI specified minimum stack value before entering the assembly block. If nostack is given, then LLVM might leave the stack in a mis-aligned position (since you promised to not use the stack anyway).

[RalfJung]

LLVM was correct assuming that. The caller in that "weird situation" was wrong to call the function without providing sufficient alignment. It actually wants to call a function with a different ABI that requires less alignment. I think we are talking about the same thing, I am just using different terminology. I think this terminology helps to clarify the actual problem here and can guide towards a solution.

[programmerjake]

looks fine to me. requesting a review from me doesn't do a whole lot though since that's not how RFCs get approved, the appropriate team (probably lang) will review and start a proposed FCP once they think it's ready. I'm not a member of those teams.

[RalfJung]

Another alternative would be to add a new ABI that captures "function which can be called with any stack alignment". This section should discuss why that alternative was not proposed.

[benisxdxd]

added a paragraph about why this was chosen

[RalfJung]

It seems you know have a single paragraph that discusses at least 3 different points, without any structure. Please split this into bullet points or separate paragraph; currently it is very hard to read.

[benisxdxd]

added some spacing

[RalfJung]

"easy to implement" and "it's like what GCC does" are not very strong arguments, IMO. The question is what is best for Rust programmers.

@Lokathor raised some points for why they did not think a separate ABI was a good idea; those should be reflected here.

[benisxdxd]

Added more explaining there

[eggyal]

Just some passing thoughts from me.

[eggyal]

In such cases I can see that instructions/time might be wasted, but wouldn't the net result be no change to the stack?

[benisxdxd]

correct, will remove.

[eggyal]

Macros don't extend what can be expressed in Rust: anything that can be done by a macro can also be done without a macro (just replace the macro invocation with its expansion)—they just make it less verbose/more ergonomic. It therefore isn't clear to me what is meant by this alternative? A more ergonomic way to define a naked function in assembly that realigns its stack, perhaps?

[benisxdxd]

Yeah.
I meant that maybe this RFC could already be implemented purely in rust already with some naked function and inline assembly shenanigans

[eggyal]

You've said that you're currently blocked on this issue. Have you explored this macro idea as a possible solution? @Amanieu's naked_function::naked macro might be a good starting point?

[benisxdxd]

I'm not really good enough with macros and such to do that but yeah I think I will maybe try exploring something along that way.

[eggyal]

I'd suggest that you start by using that naked macro just to implement a stack-realigning prelude that then calls or jumps to your actual logic defined separately in a regular function.

[benisxdxd]

Seems very interesting.
Why did you choose a macro_rules and not proc_macro_attribute?
Also seems very nice that paste crate

[programmerjake]

I just thought of a waay easier workaround that's practically always correct: just guarantee you have a local variable with alignment greater than required by the ABI (so > 16 bytes for x86), LLVM will then have to align the stack for that variable, no complex inline asm required: https://rust.godbolt.org/z/WaExsWvGj

[eggyal]

Why did you choose a macro_rules and not proc_macro_attribute?

Mostly for ease of implementation, but also is faster to compile. If you'd prefer the API of an attribute macro, it'd be pretty easy to make one emit an invocation of the macro above.

[eggyal]

I just thought of a waay easier workaround that's practically always correct: just guarantee you have a local variable with alignment greater than required by the ABI

But isn't the problem that the arguments are placed on the misaligned stack before such forcibly aligned local variable? I can't see any way to cope with that on stable Rust today other than via global_asm (here via naked-function crate). The problem then becomes how one gets from such asm to a function body that has been written in Rust... and alas, I think that has to involve copying the (on-stack) arguments and making a call.

[programmerjake]

I just thought of a waay easier workaround that's practically always correct: just guarantee you have a local variable with alignment greater than required by the ABI

But isn't the problem that the arguments are placed on the misaligned stack before such forcibly aligned local variable? I can't see any way to cope with that on stable Rust today other than via global_asm (here via naked-function crate). The problem then becomes how one gets from such asm to a function body that has been written in Rust... and alas, I think that has to involve copying the (on-stack) arguments and making a call.

make the function called by the trampoline #[inline(never)]:
either:

#[repr(align(32))]
struct Aligned;

pub unsafe extern "C" fn the_fn(a: u32, b: u64, c: &u8) -> u64 {
    let align = Aligned;
    asm!("/* {} */", in(reg) &align);
    the_fn_impl(a, b, c)
}

#[inline(never)]
fn the_fn_impl(a: u32, b: u64, c: &u8) -> u64 {
    todo!()
}

or:

/// workaround unstable attributes on expressions
#[inline(always)]
fn identity<T>(v: T) -> T {
    v
}

#[repr(align(32))]
struct Aligned;

pub unsafe extern "C" fn the_fn(a: u32, b: u64, c: &u8) -> u64 {
    let align = Aligned;
    asm!("/* {} */", in(reg) &align);
    identity(#[inline(never)] || the_fn_impl(a, b, c))()
}

fn the_fn_impl(a: u32, b: u64, c: &u8) -> u64 {
    todo!()
}

[eggyal]

...but at the cost of added overhead where callers abide by the target's stack alignment, as in the majority of cases.

[benisxdxd]

noted

[eggyal]

The key point here is that having the ability to be called with an unaligned stack is orthogonal to Rust's existing ABI definitions; perhaps another approach could be for the language to support ABIs as a composable set of features along different axes, e.g. extern "C" + "realign_stack" fn ... or somesuch? That way, it could still form part of the function's type signature/ABI while avoiding exponential growth in the number of explicitly-defined ABIs.

[benisxdxd]

this looks good enough to me but im not sure it is much different from something like the no_mangle attribute which some people also think should be unsafe.

[programmerjake]

we already have duplicate ABIs for supporting unwinding extern "C-unwind", so I quite like the ABIs as a composable set of features idea! extern "C" + "unwind" + "align_stack" fn()

[benisxdxd]

I did not know that actually and this does look nice.
However I think a different RFC might be better for that.
Also, should be noted, there is still #[unwind(allowed)] and #[unwind(abort)] which might suggest that maybe we should be doing both the ABI and attribute solutions

[RalfJung]

Firstly, leveraging function attributes aligns with Rust's philosophy of providing clear and concise language features, ensuring that developers can easily understand and apply stack realignment where necessary.

I don't know what you mean by this. Rust's general philosophy is to have a strong type system that captures everything needed to ensure soundness. The "Rust approach" to this would IMO clearly be to reflect this in the function type.

I don't think we have a single existing attribute that works like yours. This is not a "concise language feature", it is an attribute that leaks low-level implementation concerns without properly reflecting them in the type system.

If we follow your arguments we should remove the borrow checker since it is a high level of complexity and requires high maintenance over time. But obviously that would be a bad idea, since we put a high priority on having the compiler catch your mistakes. IMO you should at the very least acknowledge that there is a trade-off here and that there are good arguments for both versions. Currently the RFC doesn't read like you are seriously considering the alternative of doing proper ABI treatment of this issue, instead you are dismissing the alternative on vague grounds without giving it a proper chance. In an RFC you are expected to give good arguments for things you do not agree with -- that's how good discussions look like.

But anyway this is getting into the editorial part of how to write a text that weighs the arguments for two sides of a trade-off in a fair manner and justifies the conclusion taken -- I'm afraid I don't currently have the capacity to help with that.

[benisxdxd]

I added an extra paragraph about the whole new ABI option.
I would be glad to more pros for that option if I can think of more (or if you tell me them :) ).
I think we are overthinking this whole thing though. If we see in the future that this causes problems could remove it and do the ABI option instead.

[programmerjake]

If we see in the future that this causes problems could remove it and do the ABI option instead.

removing it may not be backward compatible, so we'd have to do something like deprecate it in all editions < some cutoff and just not have it in editions after that, and have the alternate ABI stuff instead.

[benisxdxd]

ah true you are correct I was not thinking about that.
I hope we won't need to remove it, just like it is still present in GCC and clang c still but yeah.

[benisxdxd]

So, I was looking into the ABI option and came across this, a file @RalfJung actually edited not too away from now.
Seems to me that if every time we want to add something like this we need to change the ABI, it would become very difficult to scale and maintain. @eggyal comment on that seems nice though.

[RalfJung]

As I said, I don't think I have more to add here -- the rest is mostly editorial and unfortunately I don't have the time to shepherd that part of this RFC, sorry.

[benisxdxd]

As I said, I don't think I have more to add here -- the rest is mostly editorial and unfortunately I don't have the time to shepherd that part of this RFC, sorry.

:(
Maybe you know someone else who could help with the editorial part?
This issue really blocks me and I want to get it rolling.
Also, i'm certain it could help other folk as well.

[Jules-Bertholet]

On the ergonomics of adding a separate API string: could we just add a coercion from extern "C-unaligned" fn(...) to extern "C" fn(...) pointers? How much complexity would that add to the compiler?

[benisxdxd]

extern "C-unaligned" fn(...)

That's practically a new ABI, similar to how there is an ABI for extern C with and without unwinding

[benisxdxd]

I don't know the bump rate.
I hope it's ok