release/21.x: [clang] Fix catching pointers by reference on mingw targets (#162546) #163714
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
|
@efriedma-quic What do you think about merging this PR to the release branch? |
llvmbot
added
clang
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-codegen Author: None (llvmbot) ChangesBackport 10be254 Requested by: @mstorsjo Full diff: https://github.com/llvm/llvm-project/pull/163714.diff 2 Files Affected:
diff --git a/clang/lib/CodeGen/TargetInfo.cpp b/clang/lib/CodeGen/TargetInfo.cpp
index 277d69daf493c..584b0d2e37f3f 100644
--- a/clang/lib/CodeGen/TargetInfo.cpp
+++ b/clang/lib/CodeGen/TargetInfo.cpp
@@ -75,6 +75,8 @@ TargetCodeGenInfo::~TargetCodeGenInfo() = default;
// If someone can figure out a general rule for this, that would be great.
// It's probably just doomed to be platform-dependent, though.
unsigned TargetCodeGenInfo::getSizeOfUnwindException() const {
+ if (getABIInfo().getCodeGenOpts().hasSEHExceptions())
+ return getABIInfo().getDataLayout().getPointerSizeInBits() > 32 ? 64 : 48;
// Verified for:
// x86-64 FreeBSD, Linux, Darwin
// x86-32 FreeBSD, Linux, Darwin
diff --git a/clang/test/CodeGenCXX/sizeof-unwind-exception.cpp b/clang/test/CodeGenCXX/sizeof-unwind-exception.cpp
index 4fb977a5367e7..e40b2d7ae43ea 100644
--- a/clang/test/CodeGenCXX/sizeof-unwind-exception.cpp
+++ b/clang/test/CodeGenCXX/sizeof-unwind-exception.cpp
@@ -3,6 +3,8 @@
// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -emit-llvm -fcxx-exceptions -fexceptions %s -O2 -o - | FileCheck %s --check-prefix=ARM-DARWIN
// RUN: %clang_cc1 -triple arm-unknown-gnueabi -emit-llvm -fcxx-exceptions -fexceptions %s -O2 -o - | FileCheck %s --check-prefix=ARM-EABI
// RUN: %clang_cc1 -triple mipsel-unknown-unknown -emit-llvm -fcxx-exceptions -fexceptions %s -O2 -o - | FileCheck %s --check-prefix=MIPS
+// RUN: %clang_cc1 -triple x86_64-windows-gnu -emit-llvm -fcxx-exceptions -fexceptions -exception-model=seh %s -O2 -o - | FileCheck %s --check-prefix=MINGW-X86-64
+// RUN: %clang_cc1 -triple thumbv7-windows-gnu -emit-llvm -fcxx-exceptions -fexceptions -exception-model=seh %s -O2 -o - | FileCheck %s --check-prefix=MINGW-ARMV7
void foo();
void test() {
@@ -25,9 +27,15 @@ void test() {
// ARM-EABI-NEXT: [[T1:%.*]] = getelementptr i8, ptr [[EXN]], i32 88
// MIPS: [[T0:%.*]] = tail call ptr @__cxa_begin_catch(ptr [[EXN:%.*]]) [[NUW:#[0-9]+]]
// MIPS-NEXT: [[T1:%.*]] = getelementptr i8, ptr [[EXN]], i32 24
+// MINGW-X86-64: [[T0:%.*]] = tail call ptr @__cxa_begin_catch(ptr [[EXN:%.*]]) [[NUW:#[0-9]+]]
+// MINGW-X86-64-NEXT:[[T1:%.*]] = getelementptr i8, ptr [[EXN]], i64 64
+// MINGW-ARMV7: [[T0:%.*]] = tail call arm_aapcs_vfpcc ptr @__cxa_begin_catch(ptr [[EXN:%.*]]) [[NUW:#[0-9]+]]
+// MINGW-ARMV7-NEXT: [[T1:%.*]] = getelementptr i8, ptr [[EXN]], i32 48
// X86-64: attributes [[NUW]] = { nounwind }
// X86-32: attributes [[NUW]] = { nounwind }
// ARM-DARWIN: attributes [[NUW]] = { nounwind }
// ARM-EABI: attributes [[NUW]] = { nounwind }
// MIPS: attributes [[NUW]] = { nounwind }
+// MINGW-X86-64: attributes [[NUW]] = { nounwind }
+// MINGW-ARMV7: attributes [[NUW]] = { nounwind }
|
efriedma-quic
approved these changes
Oct 16, 2025
github-project-automation
bot
moved this from Needs Review
to Needs Merge
in LLVM Release Status
|
this doesn't build: please take a look |
Oh, that's surprising. It turns out that 76058c0 moved things around, and before that, we need to do things slightly differently. With a diff like this on top, I can make it work: diff --git a/clang/lib/CodeGen/TargetInfo.cpp b/clang/lib/CodeGen/TargetInfo.cpp
index 584b0d2e37f3..af711c14d4ed 100644
--- a/clang/lib/CodeGen/TargetInfo.cpp
+++ b/clang/lib/CodeGen/TargetInfo.cpp
@@ -75,7 +75,7 @@ TargetCodeGenInfo::~TargetCodeGenInfo() = default;
// If someone can figure out a general rule for this, that would be great.
// It's probably just doomed to be platform-dependent, though.
unsigned TargetCodeGenInfo::getSizeOfUnwindException() const {
- if (getABIInfo().getCodeGenOpts().hasSEHExceptions())
+ if (getABIInfo().getContext().getLangOpts().hasSEHExceptions())
return getABIInfo().getDataLayout().getPointerSizeInBits() > 32 ? 64 : 48;
// Verified for:
// x86-64 FreeBSD, Linux, DarwinI updated the PR with that, which should fix things. |
) For this specific case, when catching a pointer data type, by reference, Clang generates a special code pattern, which directly accesses the exception data by skipping past the `_Unwind_Exception` manually (rather than using the return value of `__cxa_begin_catch`). On most platforms, `_Unwind_Exception` is 32 bytes, but in some configurations it's different. (ARM EHABI is one preexisting case.) In the case of SEH, it's also different - it is 48 bytes in 32 bit mode and 64 bytes in 64 bit mode. (See the SEH ifdef in `_Unwind_Exception` in `clang/lib/Headers/unwind.h`.) Handle this case in `TargetCodeGenInfo::getSizeOfUnwindException`, fixing the code generation for catching pointers by reference. This fixes mstorsjo/llvm-mingw#522. (cherry picked from commit 10be254)
github-project-automation
bot
moved this from Needs Merge
to Done
in LLVM Release Status
|
@mstorsjo (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR. |
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.
Backport 10be254
Requested by: @mstorsjo