[SEH] Fix assertin when return scalar value from __try block. #71488
Conversation
Current compler assert with `!SI->isAtomic() && !SI->isVolatile()' failed This due to following rule: First, no exception can move in or out of _try region., i.e., no "potential faulty instruction can be moved across _try boundary. Second, the order of exceptions for instructions 'directly' under a _try must be preserved (not applied to those in callees). Finally, global states (local/global/heap variables) that can be read outside of _try region must be updated in memory (not just in register) before the subsequent exception occurs. All memory instructions inside a _try are considered as 'volatile' to assure 2nd and 3rd rules for C-code above. This is a little sub-optimized. But it's acceptable as the amount of code directly under _try is very small. However during findDominatingStoreToReturnValue: those are not allowed. To fix just skip the assertion when current function has seh try.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: None (jyu2-git) ChangesCurrent compler assert with `!SI->isAtomic() && !SI->isVolatile()' failed This due to following rule: All memory instructions inside a _try are considered as 'volatile' to assure 2nd and 3rd rules for C-code above. This is a little sub-optimized. But it's acceptable as the amount of code directly under _try is very small. However during findDominatingStoreToReturnValue: those are not allowed. To fix just skip the assertion when current function has seh try. Full diff: https://github.com/llvm/llvm-project/pull/71488.diff 2 Files Affected:
diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index e7951b3a3f4d855..bc367b89992bbd9 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -3507,6 +3507,9 @@ static llvm::StoreInst *findDominatingStoreToReturnValue(CodeGenFunction &CGF) {
return nullptr;
// These aren't actually possible for non-coerced returns, and we
// only care about non-coerced returns on this code path.
+ // All memory instructions inside __try block are volatile.
+ if (CGF.currentFunctionUsesSEHTry() && SI->isVolatile())
+ return SI;
assert(!SI->isAtomic() && !SI->isVolatile());
return SI;
};
diff --git a/clang/test/CodeGen/windows-seh-EHa-TryInFinally.cpp b/clang/test/CodeGen/windows-seh-EHa-TryInFinally.cpp
index fab567e763df443..ce2a9528e1908a9 100644
--- a/clang/test/CodeGen/windows-seh-EHa-TryInFinally.cpp
+++ b/clang/test/CodeGen/windows-seh-EHa-TryInFinally.cpp
@@ -67,3 +67,26 @@ void foo()
}
}
}
+
+// CHECK-LABEL:@"?bar@@YAHXZ"()
+// CHECK: invoke.cont:
+// CHECK: invoke void @llvm.seh.try.begin()
+// CHECK: invoke.cont1:
+// CHECK: store volatile i32 1, ptr %cleanup.dest.slot
+// CHECK: invoke void @llvm.seh.try.end()
+// CHECK: invoke.cont2:
+// CHECK: call void @"?fin$0@0@bar@@"
+// CHECK: %cleanup.dest3 = load i32, ptr %cleanup.dest.slot
+// CHECK: return:
+// CHECK: ret i32 11
+int bar()
+{
+ int x;
+ __try {
+ return 11;
+ } __finally {
+ if (_abnormal_termination()) {
+ x = 9;
+ }
+ }
+}
|
|
@llvm/pr-subscribers-clang-codegen Author: None (jyu2-git) ChangesCurrent compler assert with `!SI->isAtomic() && !SI->isVolatile()' failed This due to following rule: All memory instructions inside a _try are considered as 'volatile' to assure 2nd and 3rd rules for C-code above. This is a little sub-optimized. But it's acceptable as the amount of code directly under _try is very small. However during findDominatingStoreToReturnValue: those are not allowed. To fix just skip the assertion when current function has seh try. Full diff: https://github.com/llvm/llvm-project/pull/71488.diff 2 Files Affected:
diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index e7951b3a3f4d855..bc367b89992bbd9 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -3507,6 +3507,9 @@ static llvm::StoreInst *findDominatingStoreToReturnValue(CodeGenFunction &CGF) {
return nullptr;
// These aren't actually possible for non-coerced returns, and we
// only care about non-coerced returns on this code path.
+ // All memory instructions inside __try block are volatile.
+ if (CGF.currentFunctionUsesSEHTry() && SI->isVolatile())
+ return SI;
assert(!SI->isAtomic() && !SI->isVolatile());
return SI;
};
diff --git a/clang/test/CodeGen/windows-seh-EHa-TryInFinally.cpp b/clang/test/CodeGen/windows-seh-EHa-TryInFinally.cpp
index fab567e763df443..ce2a9528e1908a9 100644
--- a/clang/test/CodeGen/windows-seh-EHa-TryInFinally.cpp
+++ b/clang/test/CodeGen/windows-seh-EHa-TryInFinally.cpp
@@ -67,3 +67,26 @@ void foo()
}
}
}
+
+// CHECK-LABEL:@"?bar@@YAHXZ"()
+// CHECK: invoke.cont:
+// CHECK: invoke void @llvm.seh.try.begin()
+// CHECK: invoke.cont1:
+// CHECK: store volatile i32 1, ptr %cleanup.dest.slot
+// CHECK: invoke void @llvm.seh.try.end()
+// CHECK: invoke.cont2:
+// CHECK: call void @"?fin$0@0@bar@@"
+// CHECK: %cleanup.dest3 = load i32, ptr %cleanup.dest.slot
+// CHECK: return:
+// CHECK: ret i32 11
+int bar()
+{
+ int x;
+ __try {
+ return 11;
+ } __finally {
+ if (_abnormal_termination()) {
+ x = 9;
+ }
+ }
+}
|
Current compler assert with `!SI->isAtomic() && !SI->isVolatile()' failed
This due to following rule:
First, no exception can move in or out of _try region., i.e., no "potential faulty instruction can be moved across _try boundary. Second, the order of exceptions for instructions 'directly' under a _try must be preserved (not applied to those in callees). Finally, global states (local/global/heap variables) that can be read outside of _try region must be updated in memory (not just in register) before the subsequent exception occurs.
All memory instructions inside a _try are considered as 'volatile' to assure 2nd and 3rd rules for C-code above. This is a little sub-optimized. But it's acceptable as the amount of code directly under _try is very small. However during findDominatingStoreToReturnValue: those are not allowed.
To fix just skip the assertion when current function has seh try.