Skip to content

[Wasm Exceptions] Assertion on "Active sort region list not finished" #13554

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.

Sign up for GitHub

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

Jump to bottom
Closed
kripken opened this issue Feb 24, 2021 · 1 comment
Closed

[Wasm Exceptions] Assertion on "Active sort region list not finished" #13554

kripken opened this issue Feb 24, 2021 · 1 comment

Comments

@kripken
Copy link
Member

kripken commented Feb 24, 2021 •
edited
Loading

Full error:

clang++: /b/s/w/ir/cache/builder/emscripten-releases/llvm-project/llvm/lib/Target/WebAssembly/WebAssemblyCFGSort.cpp:330: void sortBlocks(llvm::MachineFunction &, const llvm::MachineLoopInfo &, const llvm::WebAssemblyExceptionInfo &, const llvm::MachineDominatorTree &): Assertion `Entries.empty() && "Active sort region list not finished"' failed.

Testcase:

int main() {
  try {
    try {
      throw 0;
    } catch (int) {
    }
  } catch (int) {
  }
}

STR: ./em++ a.cpp -c -fwasm-exceptions on #13485 (where this was found, using tot from right now, sdk-releases-upstream-0fcee855ada6358c4fe6e8c2a7f291c54cde4876-64bit).
@kripken
Copy link
Member Author

kripken commented Feb 24, 2021

Btw @aheejin I think I saw while reducing this that the testcases could alternate between this error and either #13514 or #13515. Makes me wonder if maybe there's an underlying single cause perhaps? But I'm not 100% sure I saw that. I can check tomorrow if the information would be useful.

morehouse pushed a commit to morehouse/llvm-project that referenced this issue Mar 4, 2021
@aheejin @morehouse
[WebAssembly] Fix more ExceptionInfo grouping bugs
5e17c81 
This fixes two bugs in `WebAssemblyExceptionInfo` grouping, created by
D97247. These two bugs are not easy to split into two different CLs,
because tests that fail for one also tend to fail for the other.

- In D97247, when fixing `ExceptionInfo` grouping by taking out
  the unwind destination' exception from the unwind src's exception, we
  just iterated the BBs in the function order, but this was incorrect;
  this changes it to dominator tree preorder. Please refer to the
  comments in the code for the reason and an example.

- After this subexception-taking-out fix, there still can be remaining
  BBs we have to take out. When Exception B is taken out of Exception A
  (because EHPad B is the unwind destination of EHPad A), there can
  still be BBs within Exception A that are reachable from Exception B,
  which also should be taken out. Please refer to the comments in the
  code for more detailed explanation on why this can happen. To make
  this possible, this splits `WebAssemblyException::addBlock` into two
  parts: adding to a set and adding to a vector. We need to iterate on
  BBs within a `WebAssemblyException` to fix this, so we add BBs to sets
  first. But we add BBs to vectors later after we fix all incorrectness
  because deleting BBs from vectors is expensive. I considered removing
  the vector from `WebAssemblyException`, but it was not easy because
  this class has to maintain a similar interface with `MachineLoop` to
  be wrapped into a single interface `SortRegion`, which is used in
  CFGSort.

Other misc. drive-by fixes:
- Make `WebAssemblyExceptionInfo` do not even run when wasm EH is not
  used or the function doesn't have any EH pads, not to waste time
- Add `LLVM_DEBUG` lines for easy debugging
- Fix `preds` comments in cfg-stackify-eh.ll
- Fix `__cxa_throw`'s signature in cfg-stackify-eh.ll

Fixes emscripten-core/emscripten#13554.

Reviewed By: dschuff, tlively

Differential Revision: https://reviews.llvm.org/D97677
mem-frob pushed a commit to draperlaboratory/hope-llvm-project that referenced this issue Oct 7, 2022
@aheejin
[WebAssembly] Fix more ExceptionInfo grouping bugs
6c2b2fb 
This fixes two bugs in `WebAssemblyExceptionInfo` grouping, created by
D97247. These two bugs are not easy to split into two different CLs,
because tests that fail for one also tend to fail for the other.

- In D97247, when fixing `ExceptionInfo` grouping by taking out
  the unwind destination' exception from the unwind src's exception, we
  just iterated the BBs in the function order, but this was incorrect;
  this changes it to dominator tree preorder. Please refer to the
  comments in the code for the reason and an example.

- After this subexception-taking-out fix, there still can be remaining
  BBs we have to take out. When Exception B is taken out of Exception A
  (because EHPad B is the unwind destination of EHPad A), there can
  still be BBs within Exception A that are reachable from Exception B,
  which also should be taken out. Please refer to the comments in the
  code for more detailed explanation on why this can happen. To make
  this possible, this splits `WebAssemblyException::addBlock` into two
  parts: adding to a set and adding to a vector. We need to iterate on
  BBs within a `WebAssemblyException` to fix this, so we add BBs to sets
  first. But we add BBs to vectors later after we fix all incorrectness
  because deleting BBs from vectors is expensive. I considered removing
  the vector from `WebAssemblyException`, but it was not easy because
  this class has to maintain a similar interface with `MachineLoop` to
  be wrapped into a single interface `SortRegion`, which is used in
  CFGSort.

Other misc. drive-by fixes:
- Make `WebAssemblyExceptionInfo` do not even run when wasm EH is not
  used or the function doesn't have any EH pads, not to waste time
- Add `LLVM_DEBUG` lines for easy debugging
- Fix `preds` comments in cfg-stackify-eh.ll
- Fix `__cxa_throw`'s signature in cfg-stackify-eh.ll

Fixes emscripten-core/emscripten#13554.

Reviewed By: dschuff, tlively

Differential Revision: https://reviews.llvm.org/D97677
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@kripken