Skip to content

Commit

Permalink
[SEH] Fix assertin when return scalar value from __try block. (#71488)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jyu2-git committed Nov 8, 2023
1 parent 64ed4ed commit c79b544
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 1 deletion.
4 changes: 3 additions & 1 deletion clang/lib/CodeGen/CGCall.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3507,7 +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.
assert(!SI->isAtomic() && !SI->isVolatile());
// All memory instructions inside __try block are volatile.
assert(!SI->isAtomic() &&
(!SI->isVolatile() || CGF.currentFunctionUsesSEHTry()));
return SI;
};
// If there are multiple uses of the return-value slot, just check
Expand Down
23 changes: 23 additions & 0 deletions clang/test/CodeGen/windows-seh-EHa-TryInFinally.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
}

0 comments on commit c79b544

Please sign in to comment.