[win][x64] Various fixes for unwind v2 #154834
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
Member
|
@llvm/pr-subscribers-backend-x86 Author: Daniel Paoliello (dpaoliello) Changes
Full diff: https://github.com/llvm/llvm-project/pull/154834.diff 4 Files Affected:
diff --git a/llvm/lib/MC/MCWin64EH.cpp b/llvm/lib/MC/MCWin64EH.cpp
index a87648afde7d6..8111ccb8bc69c 100644
--- a/llvm/lib/MC/MCWin64EH.cpp
+++ b/llvm/lib/MC/MCWin64EH.cpp
@@ -22,6 +22,7 @@ class MCSection;
/// MCExpr that represents the epilog unwind code in an unwind table.
class MCUnwindV2EpilogTargetExpr final : public MCTargetExpr {
+ const MCSymbol *Function;
const MCSymbol *FunctionEnd;
const MCSymbol *UnwindV2Start;
const MCSymbol *EpilogEnd;
@@ -31,7 +32,7 @@ class MCUnwindV2EpilogTargetExpr final : public MCTargetExpr {
MCUnwindV2EpilogTargetExpr(const WinEH::FrameInfo &FrameInfo,
const WinEH::FrameInfo::Epilog &Epilog,
uint8_t EpilogSize_)
- : FunctionEnd(FrameInfo.FuncletOrFuncEnd),
+ : Function(FrameInfo.Function), FunctionEnd(FrameInfo.FuncletOrFuncEnd),
UnwindV2Start(Epilog.UnwindV2Start), EpilogEnd(Epilog.End),
EpilogSize(EpilogSize_), Loc(Epilog.Loc) {}
@@ -253,13 +254,15 @@ static void EmitUnwindInfo(MCStreamer &streamer, WinEH::FrameInfo *info) {
OS->getAssembler(), LastEpilog.End, LastEpilog.UnwindV2Start);
if (!MaybeSize) {
context.reportError(LastEpilog.Loc,
- "Failed to evaluate epilog size for Unwind v2");
+ "Failed to evaluate epilog size for Unwind v2 in " +
+ info->Function->getName());
return;
}
assert(*MaybeSize >= 0);
if (*MaybeSize >= (int64_t)UINT8_MAX) {
context.reportError(LastEpilog.Loc,
- "Epilog size is too large for Unwind v2");
+ "Epilog size is too large for Unwind v2 in " +
+ info->Function->getName());
return;
}
EpilogSize = *MaybeSize + 1;
@@ -282,7 +285,8 @@ static void EmitUnwindInfo(MCStreamer &streamer, WinEH::FrameInfo *info) {
// Too many epilogs to handle.
if ((size_t)numCodes + numEpilogCodes > UINT8_MAX) {
context.reportError(info->FunctionLoc,
- "Too many unwind codes with Unwind v2 enabled");
+ "Too many unwind codes with Unwind v2 enabled in " +
+ info->Function->getName());
return;
}
@@ -383,14 +387,16 @@ bool MCUnwindV2EpilogTargetExpr::evaluateAsRelocatableImpl(
auto Offset = GetOptionalAbsDifference(*Asm, FunctionEnd, UnwindV2Start);
if (!Offset) {
Asm->getContext().reportError(
- Loc, "Failed to evaluate epilog offset for Unwind v2");
+ Loc, "Failed to evaluate epilog offset for Unwind v2 in " +
+ Function->getName());
return false;
}
assert(*Offset > 0);
constexpr uint16_t MaxEpilogOffset = 0x0fff;
if (*Offset > MaxEpilogOffset) {
- Asm->getContext().reportError(Loc,
- "Epilog offset is too large for Unwind v2");
+ Asm->getContext().reportError(
+ Loc,
+ "Epilog offset is too large for Unwind v2 in " + Function->getName());
return false;
}
@@ -398,8 +404,8 @@ bool MCUnwindV2EpilogTargetExpr::evaluateAsRelocatableImpl(
auto Size = GetOptionalAbsDifference(*Asm, EpilogEnd, UnwindV2Start);
if (Size != (EpilogSize - 1)) {
Asm->getContext().reportError(
- Loc,
- "Size of this epilog does not match size of last epilog in function");
+ Loc, "Size of this epilog does not match size of last epilog in " +
+ Function->getName());
return false;
}
diff --git a/llvm/lib/Target/X86/X86WinEHUnwindV2.cpp b/llvm/lib/Target/X86/X86WinEHUnwindV2.cpp
index ea8b88f41bb87..009ff7554314a 100644
--- a/llvm/lib/Target/X86/X86WinEHUnwindV2.cpp
+++ b/llvm/lib/Target/X86/X86WinEHUnwindV2.cpp
@@ -90,10 +90,15 @@ DebugLoc findDebugLoc(const MachineBasicBlock &MBB) {
}
bool X86WinEHUnwindV2::runOnMachineFunction(MachineFunction &MF) {
+ // noreturn functions don't have epilogs.
+ Function &F = MF.getFunction();
+ if (F.doesNotReturn())
+ return false;
+
WinX64EHUnwindV2Mode Mode =
ForceMode.getNumOccurrences()
? static_cast<WinX64EHUnwindV2Mode>(ForceMode.getValue())
- : MF.getFunction().getParent()->getWinX64EHUnwindV2Mode();
+ : F.getParent()->getWinX64EHUnwindV2Mode();
if (Mode == WinX64EHUnwindV2Mode::Disabled)
return false;
@@ -105,6 +110,7 @@ bool X86WinEHUnwindV2::runOnMachineFunction(MachineFunction &MF) {
// Prolog information.
SmallVector<int64_t> PushedRegs;
bool HasStackAlloc = false;
+ bool HasSetFrame = false;
unsigned ApproximatePrologCodeCount = 0;
// Requested changes.
@@ -130,15 +136,20 @@ bool X86WinEHUnwindV2::runOnMachineFunction(MachineFunction &MF) {
break;
case X86::SEH_StackAlloc:
- case X86::SEH_SetFrame:
if (State != FunctionState::InProlog)
- llvm_unreachable("SEH_StackAlloc or SEH_SetFrame outside of prolog");
+ llvm_unreachable("SEH_StackAlloc outside of prolog");
// Assume a large alloc...
- ApproximatePrologCodeCount +=
- (MI.getOpcode() == X86::SEH_StackAlloc) ? 3 : 1;
+ ApproximatePrologCodeCount += 3;
HasStackAlloc = true;
break;
+ case X86::SEH_SetFrame:
+ if (State != FunctionState::InProlog)
+ llvm_unreachable("SEH_SetFrame outside of prolog");
+ ApproximatePrologCodeCount++;
+ HasSetFrame = true;
+ break;
+
case X86::SEH_SaveReg:
case X86::SEH_SaveXMM:
if (State != FunctionState::InProlog)
@@ -190,8 +201,30 @@ bool X86WinEHUnwindV2::runOnMachineFunction(MachineFunction &MF) {
State = FunctionState::FinishedEpilog;
break;
- case X86::LEA64r:
case X86::MOV64rr:
+ if (State == FunctionState::InEpilog) {
+ // If the prolog contains a stack allocation, then the first
+ // instruction in the epilog must be to adjust the stack pointer.
+ if (!HasSetFrame)
+ return rejectCurrentFunctionInternalError(
+ MF, Mode,
+ "The epilog is setting frame back, but prolog did not set it");
+ if (PoppedRegCount > 0)
+ return rejectCurrentFunctionInternalError(
+ MF, Mode,
+ "The epilog is setting the frame back after popping "
+ "registers");
+ if (HasStackDealloc)
+ return rejectCurrentFunctionInternalError(
+ MF, Mode,
+ "Cannot set the frame back after the stack "
+ "allocation has been deallocated");
+ } else if (State == FunctionState::FinishedEpilog)
+ return rejectCurrentFunctionInternalError(
+ MF, Mode, "Unexpected mov instruction after the epilog");
+ break;
+
+ case X86::LEA64r:
case X86::ADD64ri32:
if (State == FunctionState::InEpilog) {
// If the prolog contains a stack allocation, then the first
@@ -211,8 +244,7 @@ bool X86WinEHUnwindV2::runOnMachineFunction(MachineFunction &MF) {
HasStackDealloc = true;
} else if (State == FunctionState::FinishedEpilog)
return rejectCurrentFunctionInternalError(
- MF, Mode,
- "Unexpected lea, mov or add instruction after the epilog");
+ MF, Mode, "Unexpected lea or add instruction after the epilog");
break;
case X86::POP64r:
diff --git a/llvm/test/CodeGen/X86/win64-eh-unwindv2-errors.mir b/llvm/test/CodeGen/X86/win64-eh-unwindv2-errors.mir
index de76d90bf6b6c..474b776658671 100644
--- a/llvm/test/CodeGen/X86/win64-eh-unwindv2-errors.mir
+++ b/llvm/test/CodeGen/X86/win64-eh-unwindv2-errors.mir
@@ -106,7 +106,7 @@ body: |
# RUN: -x86-wineh-unwindv2-force-mode=1 | FileCheck %s \
# RUN: --check-prefix=BESTEFFORT
# DEALLOC-AFTER-EPILOG: LLVM ERROR: Windows x64 Unwind v2 is required, but LLVM has generated incompatible code in function 'dealloc_after_epilog':
-# DEALLOC-AFTER-EPILOG-SAME: Unexpected lea, mov or add instruction after the epilog
+# DEALLOC-AFTER-EPILOG-SAME: Unexpected lea or add instruction after the epilog
--- |
define dso_local void @dealloc_after_epilog() local_unnamed_addr {
@@ -161,6 +161,135 @@ body: |
RET64
...
+;--- mov_no_setframe.mir
+# RUN: not --crash llc -mtriple=x86_64-pc-windows-msvc -o - \
+# RUN: %t/mov_no_setframe.mir -run-pass=x86-wineh-unwindv2 2>&1 | \
+# RUN: FileCheck %s --check-prefix=MOV-NO-SETFRAME
+# RUN: llc -mtriple=x86_64-pc-windows-msvc -o - %t/mov_no_setframe.mir \
+# RUN: -run-pass=x86-wineh-unwindv2 -x86-wineh-unwindv2-force-mode=1 | \
+# RUN: FileCheck %s --check-prefix=BESTEFFORT
+# MOV-NO-SETFRAME: LLVM ERROR: Windows x64 Unwind v2 is required, but LLVM has generated incompatible code in function 'mov_no_setframe':
+# MOV-NO-SETFRAME-SAME: The epilog is setting frame back, but prolog did not set it
+
+--- |
+ define dso_local void @mov_no_setframe() local_unnamed_addr {
+ entry:
+ ret void
+ }
+ !llvm.module.flags = !{!0}
+ !0 = !{i32 1, !"winx64-eh-unwindv2", i32 2}
+...
+---
+name: mov_no_setframe
+body: |
+ bb.0.entry:
+ frame-setup SEH_EndPrologue
+ SEH_BeginEpilogue
+ $rsp = MOV64rr $rbp
+ SEH_EndEpilogue
+ RET64
+...
+
+;--- mov_after_epilog.mir
+# RUN: not --crash llc -mtriple=x86_64-pc-windows-msvc -o - \
+# RUN: %t/mov_after_epilog.mir -run-pass=x86-wineh-unwindv2 2>&1 | \
+# RUN: FileCheck %s --check-prefix=MOV-AFTER-EPILOG
+# RUN: llc -mtriple=x86_64-pc-windows-msvc -o - \
+# RUN: %t/mov_after_epilog.mir -run-pass=x86-wineh-unwindv2 \
+# RUN: -x86-wineh-unwindv2-force-mode=1 | FileCheck %s \
+# RUN: --check-prefix=BESTEFFORT
+# MOV-AFTER-EPILOG: LLVM ERROR: Windows x64 Unwind v2 is required, but LLVM has generated incompatible code in function 'mov_after_epilog':
+# MOV-AFTER-EPILOG-SAME: Unexpected mov instruction after the epilog
+
+--- |
+ define dso_local void @mov_after_epilog() local_unnamed_addr {
+ entry:
+ ret void
+ }
+ !llvm.module.flags = !{!0}
+ !0 = !{i32 1, !"winx64-eh-unwindv2", i32 2}
+...
+---
+name: mov_after_epilog
+body: |
+ bb.0.entry:
+ $rbp = MOV64rr $rsp
+ frame-setup SEH_SetFrame 52, 0
+ frame-setup SEH_EndPrologue
+ SEH_BeginEpilogue
+ SEH_EndEpilogue
+ $rsp = MOV64rr $rbp
+ RET64
+...
+
+;--- pop_before_mov.mir
+# RUN: not --crash llc -mtriple=x86_64-pc-windows-msvc -o - \
+# RUN: %t/pop_before_mov.mir -run-pass=x86-wineh-unwindv2 2>&1 | \
+# RUN: FileCheck %s --check-prefix=POP-BEFORE-MOV
+# RUN: llc -mtriple=x86_64-pc-windows-msvc -o - %t/pop_before_mov.mir \
+# RUN: -run-pass=x86-wineh-unwindv2 -x86-wineh-unwindv2-force-mode=1 | \
+# RUN: FileCheck %s --check-prefix=BESTEFFORT
+# POP-BEFORE-MOV: LLVM ERROR: Windows x64 Unwind v2 is required, but LLVM has generated incompatible code in function 'pop_before_mov':
+# POP-BEFORE-MOV-SAME: The epilog is setting the frame back after popping registers
+
+--- |
+ define dso_local void @pop_before_mov() local_unnamed_addr {
+ entry:
+ ret void
+ }
+ !llvm.module.flags = !{!0}
+ !0 = !{i32 1, !"winx64-eh-unwindv2", i32 2}
+...
+---
+name: pop_before_mov
+body: |
+ bb.0.entry:
+ frame-setup PUSH64r killed $rdi, implicit-def $rsp, implicit $rsp
+ frame-setup SEH_PushReg 55
+ $rbp = MOV64rr $rsp
+ frame-setup SEH_SetFrame 52, 0
+ frame-setup SEH_EndPrologue
+ SEH_BeginEpilogue
+ $rdi = frame-destroy POP64r implicit-def $rsp, implicit $rsp
+ $rsp = MOV64rr $rbp
+ SEH_EndEpilogue
+ RET64
+...
+
+;--- mov_after_dealloc.mir
+# RUN: not --crash llc -mtriple=x86_64-pc-windows-msvc -o - \
+# RUN: %t/mov_after_dealloc.mir -run-pass=x86-wineh-unwindv2 2>&1 | \
+# RUN: FileCheck %s --check-prefix=MOV-AFTER-DEALLOC
+# RUN: llc -mtriple=x86_64-pc-windows-msvc -o - %t/mov_after_dealloc.mir \
+# RUN: -run-pass=x86-wineh-unwindv2 -x86-wineh-unwindv2-force-mode=1 | \
+# RUN: FileCheck %s --check-prefix=BESTEFFORT
+# MOV-AFTER-DEALLOC: LLVM ERROR: Windows x64 Unwind v2 is required, but LLVM has generated incompatible code in function 'mov_after_dealloc':
+# MOV-AFTER-DEALLOC-SAME: Cannot set the frame back after the stack allocation has been deallocated
+
+--- |
+ define dso_local void @mov_after_dealloc() local_unnamed_addr {
+ entry:
+ ret void
+ }
+ !llvm.module.flags = !{!0}
+ !0 = !{i32 1, !"winx64-eh-unwindv2", i32 2}
+...
+---
+name: mov_after_dealloc
+body: |
+ bb.0.entry:
+ $rbp = MOV64rr $rsp
+ frame-setup SEH_SetFrame 52, 0
+ $rsp = frame-setup SUB64ri32 $rsp, 40, implicit-def dead $eflags
+ frame-setup SEH_StackAlloc 40
+ frame-setup SEH_EndPrologue
+ SEH_BeginEpilogue
+ $rsp = frame-destroy ADD64ri32 $rsp, 40, implicit-def dead $eflags
+ $rsp = MOV64rr $rbp
+ SEH_EndEpilogue
+ RET64
+...
+
;--- too_many_pops.mir
# RUN: not --crash llc -mtriple=x86_64-pc-windows-msvc -o - %t/too_many_pops.mir \
# RUN: -run-pass=x86-wineh-unwindv2 2>&1 | FileCheck %s \
diff --git a/llvm/test/CodeGen/X86/win64-eh-unwindv2.ll b/llvm/test/CodeGen/X86/win64-eh-unwindv2.ll
index 326127a919f3a..0d92d044e1b94 100644
--- a/llvm/test/CodeGen/X86/win64-eh-unwindv2.ll
+++ b/llvm/test/CodeGen/X86/win64-eh-unwindv2.ll
@@ -171,9 +171,44 @@ define dso_local void @large_aligned_alloc() align 16 {
; CHECK-NEXT: retq
; CHECK-NEXT: .seh_endproc
+define dso_local void @set_frame_only() local_unnamed_addr {
+ tail call i64 @llvm.x86.flags.read.u64()
+ ret void
+}
+
+; CHECK-LABEL: set_frame_only:
+; CHECK: .seh_unwindversion 2
+; CHECK: .seh_pushreg %rbp
+; CHECK: .seh_setframe %rbp, 0
+; CHECK: .seh_endprologue
+; CHECK-NOT: .seh_endproc
+; CHECK: .seh_startepilogue
+; CHECK-NEXT: .seh_unwindv2start
+; CHECK-NEXT: popq %rbp
+; CHECK-NEXT: .seh_endepilogue
+; CHECK-NEXT: retq
+; CHECK-NEXT: .seh_endproc
+
+attributes #1 = { noreturn }
+define dso_local void @no_return_func() local_unnamed_addr #1 {
+entry:
+ call void @d()
+ unreachable
+}
+; CHECK-LABEL: no_return_func:
+; CHECK-NOT: .seh_unwindversion 2
+; CHECK: .seh_stackalloc
+; CHECK-NEXT: .seh_endprologue
+; CHECK-NOT: .seh_startepilogue
+; CHECK-NOT: .seh_unwindv2start
+; CHECK: int3
+; CHECK-NEXT: .seh_endproc
+
+declare i64 @llvm.x86.flags.read.u64()
declare void @a() local_unnamed_addr
declare i32 @b() local_unnamed_addr
declare i32 @c(i32) local_unnamed_addr
+declare void @d() local_unnamed_addr #1
!llvm.module.flags = !{!0}
-!0 = !{i32 1, !"winx64-eh-unwindv2", i32 1}
+!0 = !{i32 1, !"winx64-eh-unwindv2", i32 2}
|
Collaborator
There was a problem hiding this comment.
doesNotReturn doesn't mean what you want it to mean here. It just checks for the IR noreturn attribute. Which is only very loosely related to whether the function has any epilogues. (Particularly when optimizations are disabled.)
You should just handle however many epilogues you see. If you don't see any, then that number is zero.
* `SetFrame` does not count as a stack allocation. * `mov` in the epilog undoes `SetFrame` (but is not required), it does not deallocate a stack allocation. * `mov` in the epilog MUST be before any stack deallocation or register popping. * Do not try to enable unwind v2 for `noreturn` functions as they don't have epilogs. * Improve the errors in `MC` to include the problematic function name.
| MF, Mode, "Unexpected mov instruction after the epilog"); | ||
| break; | ||
|
|
||
| case X86::LEA64r: |
Collaborator
There was a problem hiding this comment.
Can an LEA64r correspond to a SetFrame? (I think we might generate lea 16(%rbp), %rsp or something like that.) Not sure how much it matters.
Contributor
Author
There was a problem hiding this comment.
Only if there was a SetFrame AND a stack alloc, otherwise MOV is used:
llvm-project/llvm/lib/Target/X86/X86FrameLowering.cpp
Lines 2574 to 2597 in 1277a11
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/163/builds/25626 Here is the relevant piece of the build log for the reference |
Contributor
Author
Unrelated to my change: |
sivadeilra
pushed a commit
to sivadeilra/llvm-project
that referenced
this pull request
Nov 13, 2025
* `SetFrame` does not count as a stack allocation. * `mov` in the epilog undoes `SetFrame` (but is not required), it does not deallocate a stack allocation. * `mov` in the epilog MUST be before any stack deallocation or register popping. * Remove assert for having a prolog without any epilogs (this is possible for `noreturn` functions, for instance). * Improve the errors in `MC` to include the problematic function name.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
SetFramedoes not count as a stack allocation.movin the epilog undoesSetFrame(but is not required), it does not deallocate a stack allocation.movin the epilog MUST be before any stack deallocation or register popping.noreturnfunctions, for instance).MCto include the problematic function name.