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

Avoid calling addCxaCatch when not linking as C++ #21386

Merged
merged 1 commit into from
Feb 22, 2024
+17 −1
Merged

Avoid calling addCxaCatch when not linking as C++ #21386

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/jsifier.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ function(${args}) {
// the number specifies the number of arguments. In Emscripten, route all
// these to a single function 'findMatchingCatch' that takes an array
// of argument.
if (!WASM_EXCEPTIONS && symbol.startsWith('__cxa_find_matching_catch_')) {
if (LINK_AS_CXX && !WASM_EXCEPTIONS && symbol.startsWith('__cxa_find_matching_catch_')) {
Copy link
Member

Choose a reason for hiding this comment

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

How does this interact with linking as C but adding the C++ standard libraries manually (-lc++ etc.)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The function being called withing this block (addCxaCatch is defined in library_exceptions.js) which is only included in LINK_AS_CXX is set (see src/modules.js). So using LINK_AS_CXX seems correct here at least for now.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i.e. using -lc++ when linking with the C compiler for not currently enable library_exceptions.js and so it should also not enable this block.

Copy link
Member

Choose a reason for hiding this comment

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

Makes sense, though I worry we might want to change modules.js because of this situation, eventually... but maybe that is very unlikely.

if (DISABLE_EXCEPTION_THROWING) {
error('DISABLE_EXCEPTION_THROWING was set (likely due to -fno-exceptions), which means no C++ exception throwing support code is linked in, but exception catching code appears. Either do not set DISABLE_EXCEPTION_THROWING (if you do want exception throwing) or compile all source files with -fno-except (so that no exceptions support code is required); also make sure DISABLE_EXCEPTION_CATCHING is set to the right value - if you want exceptions, it should be off, and vice versa.');
return;
Expand Down
10 changes: 10 additions & 0 deletions test/other/test_exceptions_c_linker.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
#include <stdio.h>

__attribute__((import_name("__cxa_find_matching_catch_1")))
void __cxa_find_matching_catch_1();

int main() {
__cxa_find_matching_catch_1();
printf("done");
return 0;
}
6 changes: 6 additions & 0 deletions test/test_other.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8796,6 +8796,12 @@ def test_wasm_nope(self):
out = self.run_js('a.out.js', assert_returncode=NON_ZERO)
self.assertContained('no native wasm support detected', out)

def test_exceptions_c_linker(self):
# Test that we don't try to create __cxa_find_matching_catch_xx function automatically
# when not linking as C++.
stderr = self.expect_fail([EMCC, '-sSTRICT', test_file('other/test_exceptions_c_linker.c')])
self.assertContained('error: undefined symbol: __cxa_find_matching_catch_1', stderr)

@parameterized({
'': (False,),
'wasm': (True,),
Expand Down
Loading