Fix mismatch in prologue and epilogue for funclets on AArch64 Windows
ClosedPublic
Actions

Authored by danielframpton on Mar 29 2020, 9:37 AM.

Details

Reviewers

efriedma
rnk

Commits

rG69fb858731e8: [AArch64] Fix mismatch in prologue and epilogue for funclets on Windows
rG522b4c4b88a5: [AArch64] Fix mismatch in prologue and epilogue for funclets on Windows

Summary

The generated code for a funclet can have an add to sp in the epilogue for which there is no corresponding sub in the prologue.

This patch removes the early return from emitPrologue that was preventing the sub to sp, and instead conditionalizes the appropriate parts of the rest of the function.

Test case included.

Fixes: https://bugs.llvm.org/show_bug.cgi?id=45345

Diff Detail

Repository: rG LLVM Github Monorepo

Event Timeline

danielframpton created this revision.Mar 29 2020, 9:37 AM

Herald added a project: Restricted Project. · View Herald TranscriptMar 29 2020, 9:37 AM

Herald added subscribers: llvm-commits, hiraditya. · View Herald Transcript

Harbormaster failed remote builds in B50867: Diff 253428!Mar 29 2020, 10:40 AM

If I'm understanding correctly, the space that wasn't getting allocated in the prologue is the space for arguments to calls inside the funclet?

llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
1038	I think this COPY still needs to happen after the SEH_PrologEnd, or else we need to emit an SEH opcode for it. Otherwise, the SEH opcodes won't match the instructions. (The mismatch probably doesn't have many visible side-effects here, but still.)

In D77015#1948788, @efriedma wrote:

If I'm understanding correctly, the space that wasn't getting allocated in the prologue is the space for arguments to calls inside the funclet?

Correct, although the relevant value (MaxCallFrameSize) is computed for the function, so the call does not actually need to be in the funclet.

Move the COPY to after the end of the prologue (SEH_PrologEnd) as suggested in the review.

danielframpton marked 2 inline comments as done.Mar 29 2020, 12:46 PM

danielframpton added inline comments.

llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
1038	Done

Harbormaster failed remote builds in B50878: Diff 253447!Mar 29 2020, 1:23 PM

Updated to include context.

LGTM

I assume you don't have commit access, so I'll commit it for you.

efriedma accepted this revision.Mar 30 2020, 2:08 PM

This revision is now accepted and ready to land.Mar 30 2020, 2:08 PM

danielframpton edited the summary of this revision. (Show Details)Mar 30 2020, 5:18 PM

In D77015#1950924, @efriedma wrote:

LGTM

I assume you don't have commit access, so I'll commit it for you.

That would be great. I am doing some final validation of the version you accepted (using it to self-host rustc). I will let you know that is complete.

In D77015#1951716, @danielframpton wrote:

In D77015#1950924, @efriedma wrote:

LGTM

I assume you don't have commit access, so I'll commit it for you.

That would be great. I am doing some final validation of the version you accepted (using it to self-host rustc). I will let you know that is complete.