Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[WinEH] Missing register reload in catch funclet #60766

Closed
kalle-llvm opened this issue Feb 15, 2023 · 3 comments
Closed

[WinEH] Missing register reload in catch funclet #60766

kalle-llvm opened this issue Feb 15, 2023 · 3 comments
Labels
clang:codegen IR generation bugs: mangling, exceptions, etc.

Comments

@kalle-llvm
Copy link
Contributor

This code:

https://godbolt.org/z/xxoY4zhTG

is miscompiled by clang, a register is not reloaded in the catch-funclet so 'this' becomes garbage inside the catch. I'm using 15.0.7 and I have tried different versions on godbolt. Starting with 8.0.0 they give essentially the same code. (Versions before this give correct code, but that may just be a fluke, this bug was hard to reproduce...)

Karl-Johan Johnsson

repro.cpp.txt
@EugeneZelenko EugeneZelenko added clang:codegen IR generation bugs: mangling, exceptions, etc. and removed new issue labels Feb 15, 2023
@llvmbot
Copy link
Member

llvmbot commented Feb 15, 2023

@llvm/issue-subscribers-clang-codegen

@kalle-llvm
Copy link
Contributor Author

BTW, I think this is probably a backend bug, the IR that comes from clang looks correct as far as I can see.

repro.ll.txt

@kalle-llvm
Copy link
Contributor Author

It seems that the bug is caused by MachineLICM hoisting the register reload out of the loop (and so out of the catch).

I guess a conservative fix would be to have MachineLICM skip loops that contain catchswitch blocks.

llvmbot pushed a commit to llvm/llvm-project-release-prs that referenced this issue Aug 24, 2023
@kalle-llvm @llvmbot
[MachineLICM][WinEH] Don't hoist register reloads out of funclets
8751d3a 
This fixes llvm/llvm-project#60766

With MSVC style exception-handling (funclets), no registers are
alive when entering the funclet so they must be reloaded from the
stack.  MachineLICM can sometimes hoist such reloads out of the
funclet which is not correct, the register will have been clobbered
when entering the funclet.  This can happen in any loop that
contains a try-catch.

This has been tested on x86_64-pc-window-msvc.  I'm not sure if
funclets work the same on the other windows archs.

Reviewed By: rnk, arsenm

Differential Revision: https://reviews.llvm.org/D153337

(cherry picked from commit 917574d)
tru pushed a commit to llvm/llvm-project-release-prs that referenced this issue Aug 25, 2023
@kalle-llvm @tru
[MachineLICM][WinEH] Don't hoist register reloads out of funclets
1856972 
This fixes llvm/llvm-project#60766

With MSVC style exception-handling (funclets), no registers are
alive when entering the funclet so they must be reloaded from the
stack.  MachineLICM can sometimes hoist such reloads out of the
funclet which is not correct, the register will have been clobbered
when entering the funclet.  This can happen in any loop that
contains a try-catch.

This has been tested on x86_64-pc-window-msvc.  I'm not sure if
funclets work the same on the other windows archs.

Reviewed By: rnk, arsenm

Differential Revision: https://reviews.llvm.org/D153337

(cherry picked from commit 917574d)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
clang:codegen IR generation bugs: mangling, exceptions, etc.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@EugeneZelenko @llvmbot @kalle-llvm