This is an archive of the discontinued LLVM Phabricator instance.

[WebAssembly] Ignore filters in Emscripten EH landingpads
ClosedPublic

Authored by aheejin on May 19 2021, 11:56 AM.

Details

Summary

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.

Diff Detail

Event Timeline

aheejin created this revision.May 19 2021, 11:56 AM
aheejin requested review of this revision.May 19 2021, 11:56 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 19 2021, 11:56 AM
aheejin edited the summary of this revision. (Show Details)May 19 2021, 11:58 AM
kripken accepted this revision.May 19 2021, 12:40 PM

Nice find!

And interesting how subtle a bug that is... I guess that's how it went unnoticed for so long.

This revision is now accepted and ready to land.May 19 2021, 12:40 PM
dschuff accepted this revision.May 19 2021, 3:42 PM
This revision was automatically updated to reflect the committed changes.