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

Add helper function for C++ exception formatting #16343

Merged
merged 30 commits into from
Feb 24, 2022

Conversation

hoodmane
Copy link
Contributor

@hoodmane hoodmane commented Feb 19, 2022

Resolves #16326
This adds a function that converts a C++ exception pointer into a message like:
CppException myexception: My exception happened
or:
CppException int: An object of type 'int' at address 15738976 was thrown as a C++ exception

You would use as follows:

try {
	Module._some_cpp_function(arg);
} catch(e){
	if(typeof e === "number"){
		// The number was a raw C++ exception
		const msg = Module.formatException(e);
		// ...
	}
}

TODO:

  • Figure out how to make -sFORMAT_EXCEPTION_SUPPORT actually export this function
  • Add a test
  • Update docs?

@hoodmane
Copy link
Contributor Author

@sbc100 I can't figure out how to force this function to be included from emcc.py. I want to add code like:

  if settings.FORMAT_EXCEPTION_SUPPORT:
    settings.REQUIRED_EXPORTS += ['formatException']

to force the library manager to put this in but I don't understand how.

@sbc100
Copy link
Collaborator

sbc100 commented Feb 19, 2022

@sbc100 I can't figure out how to force this function to be included from emcc.py. I want to add code like:

  if settings.FORMAT_EXCEPTION_SUPPORT:
    settings.REQUIRED_EXPORTS += ['formatException']

to force the library manager to put this in but I don't understand how.

REQUIRED_EXPORTS only applies to native wasm function. I think want DEFAULT_LIBRARY_FUNCS_TO_INCLUDE and possible also EXPORTED_FUNCTIONS. See the include_and_export export helper in emcc.py

tools/system_libs.py Outdated Show resolved Hide resolved
system/lib/format_exception.cpp Outdated Show resolved Hide resolved
system/lib/format_exception.cpp Outdated Show resolved Hide resolved
tools/system_libs.py Outdated Show resolved Hide resolved
tools/system_libs.py Outdated Show resolved Hide resolved
src/library_exceptions.js Outdated Show resolved Hide resolved
src/library_exceptions.js Outdated Show resolved Hide resolved
src/library_exceptions.js Outdated Show resolved Hide resolved
src/library_exceptions.js Outdated Show resolved Hide resolved
emcc.py Outdated Show resolved Hide resolved
tools/system_libs.py Outdated Show resolved Hide resolved
src/library_exceptions.js Outdated Show resolved Hide resolved
src/library_exceptions.js Outdated Show resolved Hide resolved
src/library_exceptions.js Outdated Show resolved Hide resolved
src/settings.js Outdated Show resolved Hide resolved
tests/test_core.py Outdated Show resolved Hide resolved
tests/test_core.py Outdated Show resolved Hide resolved
tests/test_core.py Outdated Show resolved Hide resolved
tests/test_core.py Outdated Show resolved Hide resolved
tests/test_core.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@sbc100 sbc100 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm % some last comments

src/library_exceptions.js Outdated Show resolved Hide resolved
tests/test_core.py Outdated Show resolved Hide resolved
emcc.py Outdated Show resolved Hide resolved
emcc.py Outdated Show resolved Hide resolved
emcc.py Outdated Show resolved Hide resolved
tests/test_core.py Outdated Show resolved Hide resolved
tests/test_core.py Outdated Show resolved Hide resolved
@@ -1543,6 +1543,61 @@ def test_exceptions_rethrow_missing(self):
create_file('main.cpp', 'int main() { throw; }')
self.do_runf('main.cpp', None, assert_returncode=NON_ZERO)

def test_format_exception(self):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add @with_both_eh_sjlj to this test?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then you can also remove DISABLE_EXCEPTION_CATCHING below.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't work with wasm-exceptions. I am not sure how to make it work with wasm-exceptions, if you like I can look into it. It would probably be entertaining. But I think it should be a separate PR.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, can you open a bug about that so that we remember to get it fixed. I imagine @aheejin will want it to work for sure.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I opened #16380 which is where I got stuck in the wasm-exceptions case.

Copy link
Collaborator

@sbc100 sbc100 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm with one final change

@sbc100 sbc100 changed the title Implement C++ exception formatting Add helper function for C++ exception formatting Feb 24, 2022
@sbc100 sbc100 merged commit 7df4038 into emscripten-core:main Feb 24, 2022
@kripken
Copy link
Member

kripken commented Feb 24, 2022

It would be good to add docs for this, perhaps around here:

https://emscripten.org/docs/porting/Debugging.html#handling-c-exceptions-from-javascript

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.

Feature suggestion: C++ exception formatting
3 participants