Skip to content

Cherry-pick: fix stack update for x86_intrcc with error code #146

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

Conversation

Freax13
Copy link

@Freax13 Freax13 commented May 12, 2023

Cherry-pick of llvm@f615436 to fix rust-lang/rust#109918.

The x86_intrcc calling convention will build two STACKALLOC_W_PROBING machine instructions if the function takes an error code. This is caused by an additional call to emitSPUpdate in llvm/lib/Target/X86/X86FrameLowering.cpp:1650. Previously only the first STACKALLOC_W_PROBING machine instruction was properly handled, the second one was simply ignored. This lead to miscompilations where the stack pointer wasn't properly updated (see rust-lang/rust#109918). This patch fixes this by handling all STACKALLOC_W_PROBING machine instructions.

To be honest I don't quite understand why this didn't lead to more noticeable miscompilations previously.

This is my first time contributing to LLVM.

Reviewed By: pengfei

Differential Revision: https://reviews.llvm.org/D150033
@nikic
Copy link

nikic commented May 13, 2023

Has an upstream backport for this been requested already?

@Freax13
Copy link
Author

Freax13 commented May 13, 2023

I'm not aware of any such requests. I have to admit that I don't know the workflow following patches being accepted. Should I request an upstream backport?

@nikic
Copy link

nikic commented May 13, 2023

I've filed an issue at llvm#62694.

@nikic
Copy link

nikic commented May 17, 2023

LLVM 16.0.4 has been released with this fix. Update PR in rust-lang/rust#111672.

@nikic nikic closed this May 17, 2023
vext01 pushed a commit to vext01/llvm-project that referenced this pull request May 1, 2024
…ts_into_calls

Serialise stack-maps on demand
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants