Skip to content

Commit 6a86669

Browse files
aheejintstellar
authored andcommitted
[WebAssembly] Ignore filters in Emscripten EH landingpads
We have been handling filters and landingpads incorrectly all along. We pass clauses' (catches') types to `__cxa_find_matching_catch` in JS glue code, which returns the thrown pointer and sets the selector using `setTempRet0()`. We apparently have been doing the same for filters' (exception specs') types; we pass them to `__cxa_find_matching_catch` just the same way as clauses. And `__cxa_find_matching_catch` treats all given types as clauses. So it is a little surprising; maybe we intended to do something from the JS side and didn't end up doing? So anyway, I don't think supporting exception specs in Emscripten EH is a priority, but this can actually cause incorrect results for normal catches when functions are inlined and the inlined spec type has a parent-child relationship with the catch's type. --- The below is an example of a bug that can happen when inlining and class hierarchy is mixed. If you are busy you can skip this part: ``` struct A {}; struct B : A {}; void bar() throw (B) { throw B(); } void foo() { try { bar(); } catch (A &) { fputs ("Expected result\n", stdout); } } ``` In the unoptimized code, `bar`'s landingpad will have a filter for `B` and `foo`'s landingpad will have a clause for `A`. But when `bar` is inlined into `foo`, `foo`'s landingpad has both a filter for `B` and a clause for `A`, and it passes the both types to `__cxa_find_matching_catch`: ``` __cxa_find_matching_catch(typeinfo for B, typeinfo for A) ``` `__cxa_find_matching_catch` thinks both are clauses, and looks at the first type `B`, which belongs to a filter. And the thrown type is `B`, so it thinks the first type `B` is caught. But this makes it return an incorrect selector, because it is supposed to catch the exception using the second type `A`, which is a parent of `B`. As a result, the `foo` in the example program above does not print "Expected result" but just throws the exception to the caller. (This wouldn't have happened if `A` and `B` are completely disjoint types, such as `float` and `int`) Fixes https://bugs.llvm.org/show_bug.cgi?id=50357. Reviewed By: dschuff, kripken Differential Revision: https://reviews.llvm.org/D102795 (cherry picked from commit 412a338)
1 parent f1b1151 commit 6a86669

File tree

2 files changed

+9
-16
lines changed

2 files changed

+9
-16
lines changed

llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -885,16 +885,9 @@ bool WebAssemblyLowerEmscriptenEHSjLj::runEHOnFunction(Function &F) {
885885
SmallVector<Value *, 16> FMCArgs;
886886
for (unsigned I = 0, E = LPI->getNumClauses(); I < E; ++I) {
887887
Constant *Clause = LPI->getClause(I);
888-
// As a temporary workaround for the lack of aggregate varargs support
889-
// in the interface between JS and wasm, break out filter operands into
890-
// their component elements.
891-
if (LPI->isFilter(I)) {
892-
auto *ATy = cast<ArrayType>(Clause->getType());
893-
for (unsigned J = 0, E = ATy->getNumElements(); J < E; ++J) {
894-
Value *EV = IRB.CreateExtractValue(Clause, makeArrayRef(J), "filter");
895-
FMCArgs.push_back(EV);
896-
}
897-
} else
888+
// TODO Handle filters (= exception specifications).
889+
// https://bugs.llvm.org/show_bug.cgi?id=50396
890+
if (LPI->isCatch(I))
898891
FMCArgs.push_back(Clause);
899892
}
900893

llvm/test/CodeGen/WebAssembly/lower-em-exceptions.ll

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,9 @@ catch: ; preds = %catch.dispatch
6868
}
6969

7070
; Test invoke instruction with filters (functions with throw(...) declaration)
71+
; Currently we don't support exception specifications correctly in JS glue code,
72+
; so we ignore all filters here.
73+
; See https://bugs.llvm.org/show_bug.cgi?id=50396.
7174
define void @filter() personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*) {
7275
; CHECK-LABEL: @filter(
7376
entry:
@@ -91,12 +94,9 @@ lpad: ; preds = %entry
9194
%2 = extractvalue { i8*, i32 } %0, 1
9295
br label %filter.dispatch
9396
; CHECK: lpad:
94-
; CHECK-NEXT: %[[FMC:.*]] = call i8* @__cxa_find_matching_catch_4(i8* bitcast (i8** @_ZTIi to i8*), i8* bitcast (i8** @_ZTIc to i8*))
95-
; CHECK-NEXT: %[[IVI1:.*]] = insertvalue { i8*, i32 } undef, i8* %[[FMC]], 0
96-
; CHECK-NEXT: %[[TEMPRET0_VAL:.*]] = call i32 @getTempRet0()
97-
; CHECK-NEXT: %[[IVI2:.*]] = insertvalue { i8*, i32 } %[[IVI1]], i32 %[[TEMPRET0_VAL]], 1
98-
; CHECK-NEXT: extractvalue { i8*, i32 } %[[IVI2]], 0
99-
; CHECK-NEXT: extractvalue { i8*, i32 } %[[IVI2]], 1
97+
; We now temporarily ignore filters because of the bug, so we pass nothing to
98+
; __cxa_find_matching_catch
99+
; CHECK-NEXT: %[[FMC:.*]] = call i8* @__cxa_find_matching_catch_2()
100100

101101
filter.dispatch: ; preds = %lpad
102102
%ehspec.fails = icmp slt i32 %2, 0

0 commit comments

Comments
 (0)