Author: Heejin Ahn Date: 2021-06-04T20:34:43-07:00 New Revision: 6a86669a6d99179997feaa015b92423cba64ad96
URL: https://github.com/llvm/llvm-project/commit/6a86669a6d99179997feaa015b92423cba64ad96 DIFF: https://github.com/llvm/llvm-project/commit/6a86669a6d99179997feaa015b92423cba64ad96.diff LOG: [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 412a3381f721452fb6cd33bc30e7700102639e3f) Added: Modified: llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp llvm/test/CodeGen/WebAssembly/lower-em-exceptions.ll Removed: ################################################################################ diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp index d3bbadf27478..ff6404c30971 100644 --- a/llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp +++ b/llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp @@ -885,16 +885,9 @@ bool WebAssemblyLowerEmscriptenEHSjLj::runEHOnFunction(Function &F) { SmallVector<Value *, 16> FMCArgs; for (unsigned I = 0, E = LPI->getNumClauses(); I < E; ++I) { Constant *Clause = LPI->getClause(I); - // As a temporary workaround for the lack of aggregate varargs support - // in the interface between JS and wasm, break out filter operands into - // their component elements. - if (LPI->isFilter(I)) { - auto *ATy = cast<ArrayType>(Clause->getType()); - for (unsigned J = 0, E = ATy->getNumElements(); J < E; ++J) { - Value *EV = IRB.CreateExtractValue(Clause, makeArrayRef(J), "filter"); - FMCArgs.push_back(EV); - } - } else + // TODO Handle filters (= exception specifications). + // https://bugs.llvm.org/show_bug.cgi?id=50396 + if (LPI->isCatch(I)) FMCArgs.push_back(Clause); } diff --git a/llvm/test/CodeGen/WebAssembly/lower-em-exceptions.ll b/llvm/test/CodeGen/WebAssembly/lower-em-exceptions.ll index 88073f3a926f..b2e344e88b33 100644 --- a/llvm/test/CodeGen/WebAssembly/lower-em-exceptions.ll +++ b/llvm/test/CodeGen/WebAssembly/lower-em-exceptions.ll @@ -68,6 +68,9 @@ catch: ; preds = %catch.dispatch } ; Test invoke instruction with filters (functions with throw(...) declaration) +; Currently we don't support exception specifications correctly in JS glue code, +; so we ignore all filters here. +; See https://bugs.llvm.org/show_bug.cgi?id=50396. define void @filter() personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*) { ; CHECK-LABEL: @filter( entry: @@ -91,12 +94,9 @@ lpad: ; preds = %entry %2 = extractvalue { i8*, i32 } %0, 1 br label %filter.dispatch ; CHECK: lpad: -; CHECK-NEXT: %[[FMC:.*]] = call i8* @__cxa_find_matching_catch_4(i8* bitcast (i8** @_ZTIi to i8*), i8* bitcast (i8** @_ZTIc to i8*)) -; CHECK-NEXT: %[[IVI1:.*]] = insertvalue { i8*, i32 } undef, i8* %[[FMC]], 0 -; CHECK-NEXT: %[[TEMPRET0_VAL:.*]] = call i32 @getTempRet0() -; CHECK-NEXT: %[[IVI2:.*]] = insertvalue { i8*, i32 } %[[IVI1]], i32 %[[TEMPRET0_VAL]], 1 -; CHECK-NEXT: extractvalue { i8*, i32 } %[[IVI2]], 0 -; CHECK-NEXT: extractvalue { i8*, i32 } %[[IVI2]], 1 +; We now temporarily ignore filters because of the bug, so we pass nothing to +; __cxa_find_matching_catch +; CHECK-NEXT: %[[FMC:.*]] = call i8* @__cxa_find_matching_catch_2() filter.dispatch: ; preds = %lpad %ehspec.fails = icmp slt i32 %2, 0 _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
