[win][aarch64] Place catch objects in the fixed object area #147421
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-backend-aarch64 Author: Daniel Paoliello (dpaoliello) ChangesFixes #146973 When an object with alignment requirements is placed on the stack, this causes a stack realignment which causes AArch64 to use x19 to refer to objects on the stack as there may be a gap between local variables and the Stack Pointer. This causes issues with the MSVC C++ exception personality as the offset to the catch object recorded in the handler table no longer matches the object being used in the catch block itself. The fix for this is to place catch objects into the fixed object area. Full diff: https://github.com/llvm/llvm-project/pull/147421.diff 4 Files Affected:
diff --git a/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp b/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
index 6f1ce5bdbe286..1f19506bcff0c 100644
--- a/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
@@ -435,6 +435,8 @@ AArch64FrameLowering::getStackIDForScalableVectors() const {
static unsigned getFixedObjectSize(const MachineFunction &MF,
const AArch64FunctionInfo *AFI, bool IsWin64,
bool IsFunclet) {
+ assert(AFI->getTailCallReservedStack() % 16 == 0 &&
+ "Tail call reserved stack must be aligned to 16 bytes");
if (!IsWin64 || IsFunclet) {
return AFI->getTailCallReservedStack();
} else {
@@ -442,12 +444,29 @@ static unsigned getFixedObjectSize(const MachineFunction &MF,
!MF.getFunction().getAttributes().hasAttrSomewhere(
Attribute::SwiftAsync))
report_fatal_error("cannot generate ABI-changing tail call for Win64");
+ unsigned FixedObjectSize = AFI->getTailCallReservedStack();
+
// Var args are stored here in the primary function.
- const unsigned VarArgsArea = AFI->getVarArgsGPRSize();
- // To support EH funclets we allocate an UnwindHelp object
- const unsigned UnwindHelpObject = (MF.hasEHFunclets() ? 8 : 0);
- return AFI->getTailCallReservedStack() +
- alignTo(VarArgsArea + UnwindHelpObject, 16);
+ FixedObjectSize += AFI->getVarArgsGPRSize();
+
+ if (MF.hasEHFunclets()) {
+ // Catch objects are stored here in the primary function.
+ const MachineFrameInfo &MFI = MF.getFrameInfo();
+ const WinEHFuncInfo &EHInfo = *MF.getWinEHFuncInfo();
+ for (const WinEHTryBlockMapEntry &TBME : EHInfo.TryBlockMap) {
+ for (const WinEHHandlerType &H : TBME.HandlerArray) {
+ int FrameIndex = H.CatchObj.FrameIndex;
+ if (FrameIndex != INT_MAX) {
+ FixedObjectSize = alignTo(FixedObjectSize,
+ MFI.getObjectAlign(FrameIndex).value()) +
+ MFI.getObjectSize(FrameIndex);
+ }
+ }
+ }
+ // To support EH funclets we allocate an UnwindHelp object
+ FixedObjectSize += 8;
+ }
+ return alignTo(FixedObjectSize, 16);
}
}
@@ -4641,23 +4660,39 @@ void AArch64FrameLowering::processFunctionBeforeFrameFinalized(
// anything.
if (!MF.hasEHFunclets())
return;
- const TargetInstrInfo &TII = *MF.getSubtarget().getInstrInfo();
- WinEHFuncInfo &EHInfo = *MF.getWinEHFuncInfo();
- MachineBasicBlock &MBB = MF.front();
- auto MBBI = MBB.begin();
- while (MBBI != MBB.end() && MBBI->getFlag(MachineInstr::FrameSetup))
- ++MBBI;
+ // Win64 C++ EH needs to allocate space for the catch objects in the fixed
+ // object area right next to the UnwindHelp object.
+ WinEHFuncInfo &EHInfo = *MF.getWinEHFuncInfo();
+ int64_t CurrentOffset =
+ AFI->getVarArgsGPRSize() + AFI->getTailCallReservedStack();
+ for (WinEHTryBlockMapEntry &TBME : EHInfo.TryBlockMap) {
+ for (WinEHHandlerType &H : TBME.HandlerArray) {
+ int FrameIndex = H.CatchObj.FrameIndex;
+ if (FrameIndex != INT_MAX) {
+ CurrentOffset =
+ alignTo(CurrentOffset, MFI.getObjectAlign(FrameIndex).value());
+ CurrentOffset += MFI.getObjectSize(FrameIndex);
+ MFI.setObjectOffset(FrameIndex, -CurrentOffset);
+ }
+ }
+ }
// Create an UnwindHelp object.
// The UnwindHelp object is allocated at the start of the fixed object area
- int64_t FixedObject =
- getFixedObjectSize(MF, AFI, /*IsWin64*/ true, /*IsFunclet*/ false);
- int UnwindHelpFI = MFI.CreateFixedObject(/*Size*/ 8,
- /*SPOffset*/ -FixedObject,
+ int64_t UnwindHelpOffset = alignTo(CurrentOffset + 8, Align(16));
+ assert(UnwindHelpOffset == getFixedObjectSize(MF, AFI, /*IsWin64*/ true,
+ /*IsFunclet*/ false) &&
+ "UnwindHelpOffset must be at the start of the fixed object area");
+ int UnwindHelpFI = MFI.CreateFixedObject(/*Size*/ 8, -UnwindHelpOffset,
/*IsImmutable=*/false);
EHInfo.UnwindHelpFrameIdx = UnwindHelpFI;
+ MachineBasicBlock &MBB = MF.front();
+ auto MBBI = MBB.begin();
+ while (MBBI != MBB.end() && MBBI->getFlag(MachineInstr::FrameSetup))
+ ++MBBI;
+
// We need to store -2 into the UnwindHelp object at the start of the
// function.
DebugLoc DL;
@@ -4665,6 +4700,7 @@ void AArch64FrameLowering::processFunctionBeforeFrameFinalized(
RS->backward(MBBI);
Register DstReg = RS->FindUnusedReg(&AArch64::GPR64commonRegClass);
assert(DstReg && "There must be a free register after frame setup");
+ const TargetInstrInfo &TII = *MF.getSubtarget().getInstrInfo();
BuildMI(MBB, MBBI, DL, TII.get(AArch64::MOVi64imm), DstReg).addImm(-2);
BuildMI(MBB, MBBI, DL, TII.get(AArch64::STURXi))
.addReg(DstReg, getKillRegState(true))
diff --git a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
index fb8bd81c033af..1b9a97273349c 100644
--- a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
@@ -28513,9 +28513,7 @@ void AArch64TargetLowering::finalizeLowering(MachineFunction &MF) const {
}
// Unlike X86, we let frame lowering assign offsets to all catch objects.
-bool AArch64TargetLowering::needsFixedCatchObjects() const {
- return false;
-}
+bool AArch64TargetLowering::needsFixedCatchObjects() const { return true; }
bool AArch64TargetLowering::shouldLocalize(
const MachineInstr &MI, const TargetTransformInfo *TTI) const {
diff --git a/llvm/test/CodeGen/AArch64/wineh-aligned-stack-obj.ll b/llvm/test/CodeGen/AArch64/wineh-aligned-stack-obj.ll
new file mode 100644
index 0000000000000..ed633ce5e8f17
--- /dev/null
+++ b/llvm/test/CodeGen/AArch64/wineh-aligned-stack-obj.ll
@@ -0,0 +1,145 @@
+; RUN: llc %s --mtriple=aarch64-pc-windows-msvc -o - | FileCheck %s
+
+; Regression test for handling MSVC C++ exceptions when there's an aligned
+; object on the stack.
+
+; Generated from this C++ code:
+; https://godbolt.org/z/cGzGfqq34
+; > clang --target=aarch64-pc-windows-msvc test.cpp
+; ```
+; // Large object: alignment seems to be important?
+; struct alignas(128) BigObj {
+; int value;
+; // Destructor so it's kept alive.
+; ~BigObj() { }
+; };
+;
+; // Exception type need to be large enough to not fit in a register.
+; struct Error {
+; int value;
+; int padding[3];
+; };
+;
+; int main() {
+; BigObj bo{};
+;
+; try {
+; throw Error { 42, {0, 0, 0} };
+; } catch (const Error& e) {
+; return e.value;
+; }
+; return 0;
+; }
+; ```
+
+; CHECK-LABEL: main:
+; CHECK: sub x[[SPTMP:[0-9]+]], sp, #336
+; CHECK: and sp, x[[SPTMP]], #0xffffffffffffff80
+; CHECK: mov x[[FP:[0-9]+]], sp
+; CHECK: str wzr, [x[[FP]], #332]
+
+; CHECK-LABEL: "?catch$3@?0?main@4HA":
+; CHECK: str w8, [x[[FP]], #332]
+; CHECK-NEXT: .seh_startepilogue
+; CHECK: ret
+
+; CHECK-LABEL: $cppxdata$main:
+; CHECK: .word -16 // UnwindHelp
+; CHECK-LABEL: $handlerMap$0$main:
+; CHECK-NEXT: .word 8 // Adjectives
+; CHECK-NEXT: .word "??_R0?AUError@@@8"@IMGREL // Type
+; CHECK-NEXT: .word -8 // CatchObjOffset
+; CHECK-NEXT: .word "?catch$3@?0?main@4HA"@IMGREL // Handler
+
+%rtti.TypeDescriptor11 = type { ptr, ptr, [12 x i8] }
+%eh.CatchableType = type { i32, i32, i32, i32, i32, i32, i32 }
+%eh.CatchableTypeArray.1 = type { i32, [1 x i32] }
+%eh.ThrowInfo = type { i32, i32, i32, i32 }
+%struct.BigObj = type { i32, [124 x i8] }
+%struct.Error = type { i32, [3 x i32] }
+
+$"??1BigObj@@QEAA@XZ" = comdat any
+
+$"??_R0?AUError@@@8" = comdat any
+
+$"_CT??_R0?AUError@@@816" = comdat any
+
+$"_CTA1?AUError@@" = comdat any
+
+$"_TI1?AUError@@" = comdat any
+
+@"??_7type_info@@6B@" = external constant ptr
+@"??_R0?AUError@@@8" = linkonce_odr global %rtti.TypeDescriptor11 { ptr @"??_7type_info@@6B@", ptr null, [12 x i8] c".?AUError@@\00" }, comdat
+@__ImageBase = external dso_local constant i8
+@"_CT??_R0?AUError@@@816" = linkonce_odr unnamed_addr constant %eh.CatchableType { i32 0, i32 trunc (i64 sub nuw nsw (i64 ptrtoint (ptr @"??_R0?AUError@@@8" to i64), i64 ptrtoint (ptr @__ImageBase to i64)) to i32), i32 0, i32 -1, i32 0, i32 16, i32 0 }, section ".xdata", comdat
+@"_CTA1?AUError@@" = linkonce_odr unnamed_addr constant %eh.CatchableTypeArray.1 { i32 1, [1 x i32] [i32 trunc (i64 sub nuw nsw (i64 ptrtoint (ptr @"_CT??_R0?AUError@@@816" to i64), i64 ptrtoint (ptr @__ImageBase to i64)) to i32)] }, section ".xdata", comdat
+@"_TI1?AUError@@" = linkonce_odr unnamed_addr constant %eh.ThrowInfo { i32 0, i32 0, i32 0, i32 trunc (i64 sub nuw nsw (i64 ptrtoint (ptr @"_CTA1?AUError@@" to i64), i64 ptrtoint (ptr @__ImageBase to i64)) to i32) }, section ".xdata", comdat
+
+define dso_local noundef i32 @main() personality ptr @__CxxFrameHandler3 {
+entry:
+ %retval = alloca i32, align 4
+ %bo = alloca %struct.BigObj, align 128
+ %tmp = alloca %struct.Error, align 4
+ %e = alloca ptr, align 8
+ %cleanup.dest.slot = alloca i32, align 4
+ store i32 0, ptr %retval, align 4
+ call void @llvm.memset.p0.i64(ptr align 128 %bo, i8 0, i64 128, i1 false)
+ %value = getelementptr inbounds nuw %struct.BigObj, ptr %bo, i32 0, i32 0
+ %value1 = getelementptr inbounds nuw %struct.Error, ptr %tmp, i32 0, i32 0
+ store i32 42, ptr %value1, align 4
+ %padding = getelementptr inbounds nuw %struct.Error, ptr %tmp, i32 0, i32 1
+ store i32 0, ptr %padding, align 4
+ %arrayinit.element = getelementptr inbounds i32, ptr %padding, i64 1
+ store i32 0, ptr %arrayinit.element, align 4
+ %arrayinit.element2 = getelementptr inbounds i32, ptr %padding, i64 2
+ store i32 0, ptr %arrayinit.element2, align 4
+ invoke void @_CxxThrowException(ptr %tmp, ptr @"_TI1?AUError@@") #3
+ to label %unreachable unwind label %catch.dispatch
+
+catch.dispatch:
+ %0 = catchswitch within none [label %catch] unwind label %ehcleanup
+
+catch:
+ %1 = catchpad within %0 [ptr @"??_R0?AUError@@@8", i32 8, ptr %e]
+ %2 = load ptr, ptr %e, align 8
+ %value3 = getelementptr inbounds nuw %struct.Error, ptr %2, i32 0, i32 0
+ %3 = load i32, ptr %value3, align 4
+ store i32 %3, ptr %retval, align 4
+ store i32 1, ptr %cleanup.dest.slot, align 4
+ catchret from %1 to label %catchret.dest
+
+catchret.dest:
+ br label %cleanup
+
+try.cont:
+ store i32 0, ptr %retval, align 4
+ store i32 1, ptr %cleanup.dest.slot, align 4
+ br label %cleanup
+
+cleanup:
+ call void @"??1BigObj@@QEAA@XZ"(ptr noundef nonnull align 128 dereferenceable(4) %bo) #4
+ %4 = load i32, ptr %retval, align 4
+ ret i32 %4
+
+ehcleanup:
+ %5 = cleanuppad within none []
+ call void @"??1BigObj@@QEAA@XZ"(ptr noundef nonnull align 128 dereferenceable(4) %bo) [ "funclet"(token %5) ]
+ cleanupret from %5 unwind to caller
+
+unreachable:
+ unreachable
+}
+
+declare void @llvm.memset.p0.i64(ptr writeonly captures(none), i8, i64, i1 immarg) #1
+
+declare dso_local void @_CxxThrowException(ptr, ptr)
+
+declare dso_local i32 @__CxxFrameHandler3(...)
+
+define linkonce_odr dso_local void @"??1BigObj@@QEAA@XZ"(ptr noundef nonnull align 128 dereferenceable(4) %this) unnamed_addr comdat {
+entry:
+ %this.addr = alloca ptr, align 8
+ store ptr %this, ptr %this.addr, align 8
+ %this1 = load ptr, ptr %this.addr, align 8
+ ret void
+}
diff --git a/llvm/test/CodeGen/AArch64/wineh-try-catch.ll b/llvm/test/CodeGen/AArch64/wineh-try-catch.ll
index e10b05e2488fd..6bb22fca24591 100644
--- a/llvm/test/CodeGen/AArch64/wineh-try-catch.ll
+++ b/llvm/test/CodeGen/AArch64/wineh-try-catch.ll
@@ -20,7 +20,7 @@
; CHECK: str x28, [sp, #24]
; CHECK: stp x29, x30, [sp, #32]
; CHECK: add x29, sp, #32
-; CHECK: sub sp, sp, #624
+; CHECK: sub sp, sp, #608
; CHECK: mov x19, sp
; CHECK: mov x0, #-2
; CHECK: stur x0, [x29, #16]
@@ -51,7 +51,7 @@
; CHECK: str x21, [sp, #16]
; CHECK: str x28, [sp, #24]
; CHECK: stp x29, x30, [sp, #32]
-; CHECK: add x20, x19, #12
+; CHECK: add x20, x19, #0
; Check that there are no further stack updates.
; CHECK-NOT: sub sp, sp
|
| const WinEHFuncInfo &EHInfo = *MF.getWinEHFuncInfo(); | ||
| for (const WinEHTryBlockMapEntry &TBME : EHInfo.TryBlockMap) { | ||
| for (const WinEHHandlerType &H : TBME.HandlerArray) { | ||
| int FrameIndex = H.CatchObj.FrameIndex; |
There was a problem hiding this comment.
This looks like it can allocate space for the same FrameIndex multiple times? I think a given FrameIndex can show up multple times in the TryBlockMap. (See FunctionLoweringInfo::set)
There was a problem hiding this comment.
Good catch: yes, if the catchpads reuse the same alloca then we'd allocate too many fixed objects. I've fixed it for AArch64 and added a test, I'll create a followup PR to fix x64.
I couldn't figure out a way to make Clang generate this IR, so that could be a potential optimization (i.e., only emit one alloca for all by-reference and by-pointer catch blocks).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #146973
When an object with alignment requirements is placed on the stack, this causes a stack realignment which causes AArch64 to use x19 to refer to objects on the stack as there may be a gap between local variables and the Stack Pointer. This causes issues with the MSVC C++ exception personality as the offset to the catch object recorded in the handler table no longer matches the object being used in the catch block itself.
The fix for this is to place catch objects into the fixed object area.