[llvm] 412a338 - [WebAssembly] Ignore filters in Emscripten EH landingpads

[Heejin Ahn via llvm-commits]

Author: Heejin Ahn
Date: 2021-05-20T01:28:16-07:00
New Revision: 412a3381f721452fb6cd33bc30e7700102639e3f

URL: https://github.com/llvm/llvm-project/commit/412a3381f721452fb6cd33bc30e7700102639e3f
DIFF: https://github.com/llvm/llvm-project/commit/412a3381f721452fb6cd33bc30e7700102639e3f.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

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 73c064523ab58..35ec8946c8c41 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp
@@ -896,16 +896,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 8df0b5dcba7b9..af04b2f1466e4 100644
--- a/llvm/test/CodeGen/WebAssembly/lower-em-exceptions.ll
+++ b/llvm/test/CodeGen/WebAssembly/lower-em-exceptions.ll
@@ -69,6 +69,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:
@@ -92,12 +95,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