This is an archive of the discontinued LLVM Phabricator instance.

[X86] Align stack to 16-bytes on 32-bit with X86_INTR call convention
ClosedPublic

Authored by antangelo on May 24 2023, 7:57 PM.

Details

Summary

Adds a dynamic stack alignment to functions under the interrupt call
convention on x86-32. This fixes the issue where the stack can be
misaligned on entry, since x86-32 makes no guarantees about the stack
pointer position when the interrupt service routine is called.

The alignment is done by overriding X86RegisterInfo::shouldRealignStack,
and by setting the correct alignment in X86FrameLowering::calculateMaxStackAlign.
This forces the interrupt handler to be dynamically aligned, generating
the appropriate and instruction in the prologue and lea in the
epilogue. The no-realign-stack attribute can be used as an opt-out.

Fixes #26851

Diff Detail

Event Timeline

antangelo created this revision.May 24 2023, 7:57 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 24 2023, 7:57 PM
Herald added a subscriber: hiraditya. · View Herald Transcript
antangelo requested review of this revision.May 24 2023, 7:57 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 24 2023, 7:57 PM
llvm/test/CodeGen/X86/x86-32-intrcc.ll
52–54

Update all these descriptions

pengfei added inline comments.May 25 2023, 6:32 AM
llvm/lib/Target/X86/X86FrameLowering.cpp
1248

I think max is enough. We don't have no-power-of-2 alginment.

1248

Where's the 16 request from, ABI?

llvm/test/CodeGen/X86/x86-32-intrcc.ll
11–13

This seems conflict with the intention here.

51–54

ditto.

146–173

This should not be affected.

antangelo updated this revision to Diff 526650.May 30 2023, 8:43 AM

Update test descriptions, use max instead of lcm for alignment

antangelo added inline comments.May 30 2023, 8:57 AM
llvm/lib/Target/X86/X86FrameLowering.cpp
1248

The 16 byte alignment is from SysV ABI. I believe the latest revision is here https://gitlab.com/x86-psABIs/i386-ABI/-/tree/hjl/x86/master

The end of the input argument area shall be aligned on a 16 (32 or 64, if __m256 or __m512 is passed on stack) byte boundary

pengfei added inline comments.May 30 2023, 11:27 PM
llvm/lib/Target/X86/X86FrameLowering.cpp
1248

My understanding is psABI just defines the default calling conversion. I didn't see description about interrupt in the ABI doc.
What's the GCC behavior? Is this to match with it?

antangelo added inline comments.May 30 2023, 11:57 PM
llvm/lib/Target/X86/X86FrameLowering.cpp
1248

GCC aligns the stack to 16 bytes if the interrupt handler makes calls to functions outside of the current compilation unit (I believe this is where the ABI alignment comes into play unless I'm misunderstanding). The goal is to align with GCC's behavior.

pengfei accepted this revision.May 31 2023, 3:50 AM

LGTM except for one nit.

llvm/lib/Target/X86/X86FrameLowering.cpp
1248

Got it, thanks!

llvm/test/CodeGen/X86/x86-32-intrcc.ll
408–409

Add nounwind to remove .cfi*?

This revision is now accepted and ready to land.May 31 2023, 3:50 AM
antangelo updated this revision to Diff 527270.May 31 2023, 8:48 PM

Added nounwind to new x86-32 interrupt tests to remove .cfi*

I don't have commit access to commit the changes myself, if someone can commit on my behalf. My name and email are:

Name: Antonio Abbatangelo
Email: contact@antangelo.com