Add better error message when mixing exceptions modes. NFC #23738
Conversation
Depends on llvm/llvm-project#128564 to allow wasm-ld to report the object file in question. Fixes: emscripten-core#23732
| #if WASM_EXCEPTIONS | ||
| #if ASSERTIONS | ||
| // Try to give a more useful error message than simply `Undefined reference to `emscripten_longjmp` | ||
| emscripten_longjmp__deps: [() => error('undefined reference to `emscripten_longjmp`. One or more object files was not compiled with `-fwasm-exceptions`. Build with `-sASSERTIONS=0` to have wasm-ld report which one.')], |
There was a problem hiding this comment.
Just to confirm, -sASSERTIONS=0 is the default right?
There was a problem hiding this comment.
-sASSERTIONS=1 is the default for -O0 but -sASSERTIONS=0 is the default of -O1 and above.
| self.assertContained('error: undefined reference to `emscripten_longjmp`. One or more object files was not compiled with `-fwasm-exceptions`. Build with `-sASSERTIONS=0` to have wasm-ld report which one.', err) | ||
|
|
||
| err = self.expect_fail([EMCC, 'main.o', '-fwasm-exceptions', '-sASSERTIONS=0']) | ||
| self.assertContained('wasm-ld: error: main.o: undefined symbol: emscripten_longjmp', err) |
There was a problem hiding this comment.
Looks like a big improvement to me.
When Emscripten invokes wasm-ld it would be nice if it could detect this error and add the extra context about this since one doesn't see the instruction to pass -fwasm-exceptions as a cflag here. I assume we are already redirecting stderr when we invoke wasm-ld?
If it's too inconvenient this by itself is quite helpful.
There was a problem hiding this comment.
Emscripten doesn't currently capture and parse the output of wasm-ld. Its just fails if/when wasm-ld fails.
I'm not sure I want to go down the route of trying to get fancy with parsing the error output of wasm-ld. I decided that showing the more specific error when -O0 / -sASSERTIONS=1 are enabled was a good compromise.
|
What's the procedure for an emscripten change that requires an llvm change? You test emscripten locally against the llvm change and then wait for the llvm commit to land and then wait for the roller to ingest it and then rebase the emscripten pr? |
Yes, exactly. |
Depends on llvm/llvm-project#128564 to allow wasm-ld to report the object file in question.
Fixes: #23732