Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[EH/SjLj] Add tests for mixing exceptions and sjlj #14732

Merged
merged 6 commits into from Jul 27, 2021

Conversation

aheejin
Copy link
Member

@aheejin aheejin commented Jul 22, 2021

This adds three tests for mixing exceptions and sjlj.
We currently pass test_exceptions_longjmp1, and other two tests will
pass after https://reviews.llvm.org/D106525 lands.

  • test_exceptions_longjmp1
    In a function with a setjmp call, invoke exists in LLVM IR. We have to check
    both for EH and sjlj when a function throws.
  • test_exceptions_longjmp2
    In a function without a setjmp call, invoke exists in LLVM IR. We
    only have to handle exceptions, but when longjmps occur, we shouldn't
    swallow them.
  • test_exceptions_longjmp3
    In a function with a setjmp call, there's no invoke in LLVM IR. We
    only have to handle longjmps, but w hen an exceptions occcur, we
    shouldn't swallow them.

Each of these tests corresponds with tests in
test/CodeGen/WebAssembly/lower-em-sjlj.ll in
https://reviews.llvm.org/D106525.

This adds three tests for mixing exceptions and sjlj.
We currently pass `test_exceptions_longjmp1`, and other two tests will
pass after https://reviews.llvm.org/D106525 lands.

- `test_exceptions_longjmp1`
  In a function with a `setjmp` call, `invoke` exists in LLVM IR. We have to check
  both for EH and sjlj when a function throws.
- `test_exceptions_longjmp2`
  In a function without a `setjmp` call, `invoke` exists in LLVM IR. We
  only have to handle exceptions, but when longjmps occur, we shouldn't
  swallow them.
- `test_exceptions_longjmp3`
  In a function with a `setjmp` call, there's no `invoke` in LLVM IR. We
  only have to handle longjmps, but w hen an exceptions occcur, we
  shouldn't swallow them.

Each of these tests corresponds with tests in
test/CodeGen/WebAssembly/lower-em-sjlj.ll in
https://reviews.llvm.org/D106525.
@aheejin aheejin requested review from kripken and sbc100 July 22, 2021 18:44
aheejin added a commit to llvm/llvm-project that referenced this pull request Jul 26, 2021
When Emscripten EH mixes with Emscripten SjLj, we are not currently
handling some of them correctly. There are three cases:
1. The current function calls `setjmp` and there is an `invoke` to a
   function that can either throw or longjmp. In this case, we have to
   check both for exception and longjmp. We are currently handling this
   case correctly:
   https://github.com/llvm/llvm-project/blob/0c0eb76782d5224b8d81a5afbb9a152bcf7c94c7/llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp#L1058-L1090
   When inserting routines for functions that can longjmp, which we do
   only for setjmp-calling functions, we check if the function was
   previously an `invoke` and handle it correctly.

2. The current function does NOT call `setjmp` and there is an `invoke`
   to a function that can either throw or longjmp. Because there is no
   `setjmp` call, we haven't been doing any check for functions that can
   longjmp. But in that case, for `invoke`, we only check for an
   exception and if it is not an exception we reset `__THREW__` to 0,
   which can silently swallow the longjmp:
   https://github.com/llvm/llvm-project/blob/0c0eb76782d5224b8d81a5afbb9a152bcf7c94c7/llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp#L70-L80
   This CL fixes this.

3. The current function calls `setjmp` and there is no `invoke`. Because
   it is not an `invoke`, we haven't been doing any check for functions
   that can throw, and only insert longjmp-checking routines for
   functions that can longjmp. But in that case, if a longjmpable
   function throws, we only check for a longjmp so if it is not a
   longjmp we reset `__THREW__` to 0, which can silently swallow the
   exception:
   https://github.com/llvm/llvm-project/blob/0c0eb76782d5224b8d81a5afbb9a152bcf7c94c7/llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp#L156-L169
   This CL fixes this.

To do that, this moves around some code, so we register necessary
functions for both EH and SjLj and precompute some data (the set of
functions that contains `setjmp`) before doing actual EH or SjLj
transformation.

This CL makes 2nd and 3rd tests in
emscripten-core/emscripten#14732 work.

Reviewed By: dschuff

Differential Revision: https://reviews.llvm.org/D106525
@aheejin aheejin merged commit 6f9a294 into emscripten-core:main Jul 27, 2021
@aheejin aheejin deleted the test_eh_sjlj branch July 27, 2021 18:30
arichardson pushed a commit to CTSRD-CHERI/llvm-project that referenced this pull request Sep 29, 2021
When Emscripten EH mixes with Emscripten SjLj, we are not currently
handling some of them correctly. There are three cases:
1. The current function calls `setjmp` and there is an `invoke` to a
   function that can either throw or longjmp. In this case, we have to
   check both for exception and longjmp. We are currently handling this
   case correctly:
   https://github.com/llvm/llvm-project/blob/0c0eb76782d5224b8d81a5afbb9a152bcf7c94c7/llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp#L1058-L1090
   When inserting routines for functions that can longjmp, which we do
   only for setjmp-calling functions, we check if the function was
   previously an `invoke` and handle it correctly.

2. The current function does NOT call `setjmp` and there is an `invoke`
   to a function that can either throw or longjmp. Because there is no
   `setjmp` call, we haven't been doing any check for functions that can
   longjmp. But in that case, for `invoke`, we only check for an
   exception and if it is not an exception we reset `__THREW__` to 0,
   which can silently swallow the longjmp:
   https://github.com/llvm/llvm-project/blob/0c0eb76782d5224b8d81a5afbb9a152bcf7c94c7/llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp#L70-L80
   This CL fixes this.

3. The current function calls `setjmp` and there is no `invoke`. Because
   it is not an `invoke`, we haven't been doing any check for functions
   that can throw, and only insert longjmp-checking routines for
   functions that can longjmp. But in that case, if a longjmpable
   function throws, we only check for a longjmp so if it is not a
   longjmp we reset `__THREW__` to 0, which can silently swallow the
   exception:
   https://github.com/llvm/llvm-project/blob/0c0eb76782d5224b8d81a5afbb9a152bcf7c94c7/llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp#L156-L169
   This CL fixes this.

To do that, this moves around some code, so we register necessary
functions for both EH and SjLj and precompute some data (the set of
functions that contains `setjmp`) before doing actual EH or SjLj
transformation.

This CL makes 2nd and 3rd tests in
emscripten-core/emscripten#14732 work.

Reviewed By: dschuff

Differential Revision: https://reviews.llvm.org/D106525
mem-frob pushed a commit to draperlaboratory/hope-llvm-project that referenced this pull request Oct 7, 2022
When Emscripten EH mixes with Emscripten SjLj, we are not currently
handling some of them correctly. There are three cases:
1. The current function calls `setjmp` and there is an `invoke` to a
   function that can either throw or longjmp. In this case, we have to
   check both for exception and longjmp. We are currently handling this
   case correctly:
   https://github.com/llvm/llvm-project/blob/0c0eb76782d5224b8d81a5afbb9a152bcf7c94c7/llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp#L1058-L1090
   When inserting routines for functions that can longjmp, which we do
   only for setjmp-calling functions, we check if the function was
   previously an `invoke` and handle it correctly.

2. The current function does NOT call `setjmp` and there is an `invoke`
   to a function that can either throw or longjmp. Because there is no
   `setjmp` call, we haven't been doing any check for functions that can
   longjmp. But in that case, for `invoke`, we only check for an
   exception and if it is not an exception we reset `__THREW__` to 0,
   which can silently swallow the longjmp:
   https://github.com/llvm/llvm-project/blob/0c0eb76782d5224b8d81a5afbb9a152bcf7c94c7/llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp#L70-L80
   This CL fixes this.

3. The current function calls `setjmp` and there is no `invoke`. Because
   it is not an `invoke`, we haven't been doing any check for functions
   that can throw, and only insert longjmp-checking routines for
   functions that can longjmp. But in that case, if a longjmpable
   function throws, we only check for a longjmp so if it is not a
   longjmp we reset `__THREW__` to 0, which can silently swallow the
   exception:
   https://github.com/llvm/llvm-project/blob/0c0eb76782d5224b8d81a5afbb9a152bcf7c94c7/llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp#L156-L169
   This CL fixes this.

To do that, this moves around some code, so we register necessary
functions for both EH and SjLj and precompute some data (the set of
functions that contains `setjmp`) before doing actual EH or SjLj
transformation.

This CL makes 2nd and 3rd tests in
emscripten-core/emscripten#14732 work.

Reviewed By: dschuff

Differential Revision: https://reviews.llvm.org/D106525
vgvassilev pushed a commit to vgvassilev/llvm that referenced this pull request Dec 28, 2022
When Emscripten EH mixes with Emscripten SjLj, we are not currently
handling some of them correctly. There are three cases:
1. The current function calls `setjmp` and there is an `invoke` to a
   function that can either throw or longjmp. In this case, we have to
   check both for exception and longjmp. We are currently handling this
   case correctly:
   https://github.com/llvm/llvm-project/blob/0c0eb76782d5224b8d81a5afbb9a152bcf7c94c7/llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp#L1058-L1090
   When inserting routines for functions that can longjmp, which we do
   only for setjmp-calling functions, we check if the function was
   previously an `invoke` and handle it correctly.

2. The current function does NOT call `setjmp` and there is an `invoke`
   to a function that can either throw or longjmp. Because there is no
   `setjmp` call, we haven't been doing any check for functions that can
   longjmp. But in that case, for `invoke`, we only check for an
   exception and if it is not an exception we reset `__THREW__` to 0,
   which can silently swallow the longjmp:
   https://github.com/llvm/llvm-project/blob/0c0eb76782d5224b8d81a5afbb9a152bcf7c94c7/llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp#L70-L80
   This CL fixes this.

3. The current function calls `setjmp` and there is no `invoke`. Because
   it is not an `invoke`, we haven't been doing any check for functions
   that can throw, and only insert longjmp-checking routines for
   functions that can longjmp. But in that case, if a longjmpable
   function throws, we only check for a longjmp so if it is not a
   longjmp we reset `__THREW__` to 0, which can silently swallow the
   exception:
   https://github.com/llvm/llvm-project/blob/0c0eb76782d5224b8d81a5afbb9a152bcf7c94c7/llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp#L156-L169
   This CL fixes this.

To do that, this moves around some code, so we register necessary
functions for both EH and SjLj and precompute some data (the set of
functions that contains `setjmp`) before doing actual EH or SjLj
transformation.

This CL makes 2nd and 3rd tests in
emscripten-core/emscripten#14732 work.

Reviewed By: dschuff

Differential Revision: https://reviews.llvm.org/D106525

llvm-monorepo: c285a11efdb0821e6be9fcef695097890f44aa03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants