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.

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

crash in wasm-opt at -O2 #3697

Closed
sbc100 opened this issue Mar 17, 2021 · 9 comments · Fixed by #3700
Closed

crash in wasm-opt at -O2 #3697

sbc100 opened this issue Mar 17, 2021 · 9 comments · Fixed by #3700

Comments

@sbc100
Copy link
Member

sbc100 commented Mar 17, 2021

The wasm2.test_poppler test is failing consistenly with recent builds of wasm-opt:

https://app.circleci.com/pipelines/github/emscripten-core/emscripten/11775/workflows/4dd05dca-592a-47f3-9e74-671debdfd795/jobs/404629

terminate called after throwing an instance of 'std::out_of_range'
  what():  _Map_base::at
em++: error: '/root/emsdk/upstream/bin/wasm-opt --post-emscripten --no-exit-runtime -O2 --low-memory-unused --zero-filled-memory --strip-producers pdfimages.wasm -o pdfimages.wasm -g --mvp-features' failed (-6)
make[2]: *** [pdfimages] Error 1
make[2]: *** Waiting for unfinished jobs....

I have not yet bisected to a particular binaryen commit but I suspect its something in the last 24 hours.
@kripken
Copy link
Member

kripken commented Mar 17, 2021

I don't see this locally. Perhaps it is running out of memory. I don't think we've changed anything on the Binaryen side that could explain it. Perhaps an LLVM change is causing more code or locals to be emitted?

@sbc100
Copy link
Member Author

sbc100 commented Mar 17, 2021

I can't repro it locally either. Strange. I wonder if its circleci change?

@sbc100
Copy link
Member Author

sbc100 commented Mar 17, 2021

Actually I can repro locally if I do ./emsdk install tot and use that.

@sbc100
Copy link
Member Author

sbc100 commented Mar 17, 2021

Seems to be related to the binaryen binaryen built on the waterfall.

sbc100 added a commit to emscripten-core/emscripten that referenced this issue Mar 17, 2021
@sbc100
Disable test_poppler
8fe6b2f 
See WebAssembly/binaryen#3697.
@sbc100 sbc100 mentioned this issue Mar 17, 2021
Merged
@kripken
Copy link
Member

kripken commented Mar 17, 2021

Interesting, if you can reproduce this with the waterfall builds then we can bisect it at least.

Looks like the sheriff is @aheejin - do you have time? Or did you intend to @sbc100 ? If neither then I can find time.

sbc100 added a commit to emscripten-core/emscripten that referenced this issue Mar 17, 2021
@sbc100
Disable test_poppler (#13692)
5aabf10 
See WebAssembly/binaryen#3697.
@aheejin aheejin self-assigned this Mar 17, 2021
@kripken
Copy link
Member

kripken commented Mar 17, 2021

@aheejin Actually I just tried this again before I saw you self-assigned this and I can get this to reproduce locally. From the stack trace it is in debug info code, so likely my fault or wouter - I can verify shortly.

@aheejin aheejin removed their assignment Mar 17, 2021
@kripken kripken mentioned this issue Mar 18, 2021
Merged
@kripken
Copy link
Member

kripken commented Mar 18, 2021

This was actually LLVM's "fault" for emitting different DWARF now. So this was a crash in all opt levels actually, whenever we rewrote DWARF. Fix in #3700

kripken added a commit that referenced this issue Mar 18, 2021
@kripken
Ignore missing CUs in DWARF rewriting (#3700)
647ef50 
A recent change in LLVM causes it to sometimes end up with a thing
with no parent. That is, a debug_line or a debug_loc that has no CU that
refers to it. This is perhaps LLVM DCEing CUs, or something else that
changed - not sure. But it seems like valid DWARF we should handle.

This PR handles that in our code. Two things broke here. First, locs must
be simply ignored when there is no CU. Second, lines are trickier as we
used to compute their position by scanning them, and that list contained
only ones with a CU. So we missed some and ended up with wrong offsets.
To make things simpler and more robust, just track the position of each
line table on itself.

Fixes #3697
@sbc100
Copy link
Member Author

sbc100 commented Mar 18, 2021

Do you want to make a PR to re-enable the poppler test? (or I can if you like)

@kripken
Copy link
Member

kripken commented Mar 18, 2021

I will. Still waiting on the binaryen roll.

@kripken kripken mentioned this issue Mar 18, 2021
Merged
kripken added a commit to emscripten-core/emscripten that referenced this issue Mar 20, 2021
@kripken
Restore test_poppler (#13704)
79092f7 
WebAssembly/binaryen#3697 has been fixed.
@truboxl truboxl mentioned this issue Apr 9, 2021
Merged
@kg kg mentioned this issue May 4, 2021
Closed
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

Successfully merging a pull request may close this issue.

Ignore missing CUs in DWARF rewriting WebAssembly/binaryen
3 participants
@kripken @sbc100 @aheejin