Skip to content

Build-time error on use of emscripten_promise_any if Promise.any is not available #19172

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

Merged
merged 1 commit into from
Apr 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
28 changes: 16 additions & 12 deletions emcc.py
Original file line number Diff line number Diff line change
Expand Up @@ -2420,6 +2420,14 @@ def phase_linker_setup(options, state, newargs):
if settings.INCLUDE_FULL_LIBRARY and not settings.DISABLE_EXCEPTION_CATCHING:
settings.EXPORTED_FUNCTIONS += ['___get_exception_message', '_free']

if settings.MEMORY64:
if settings.ASYNCIFY and settings.MEMORY64 == 1:
exit_with_error('MEMORY64=1 is not compatible with ASYNCIFY')
if not settings.DISABLE_EXCEPTION_CATCHING:
exit_with_error('MEMORY64 is not compatible with DISABLE_EXCEPTION_CATCHING=0')
# Any "pointers" passed to JS will now be i64's, in both modes.
settings.WASM_BIGINT = 1

if settings.WASM_WORKERS:
# TODO: After #15982 is resolved, these dependencies can be declared in library_wasm_worker.js
# instead of having to record them here.
Expand All @@ -2432,13 +2440,19 @@ def phase_linker_setup(options, state, newargs):
settings.WASM_WORKER_FILE = unsuffixed(os.path.basename(target)) + '.ww.js'
settings.JS_LIBRARIES.append((0, shared.path_from_root('src', 'library_wasm_worker.js')))

# Set min browser versions based on certain settings such as WASM_BIGINT,
# PTHREADS, AUDIO_WORKLET
# Such setting must be set before this point
feature_matrix.apply_min_browser_versions()

# TODO(sbc): Find make a generic way to expose the feature matrix to JS
# compiler rather then adding them all ad-hoc as internal settings
settings.SUPPORTS_GLOBALTHIS = feature_matrix.caniuse(feature_matrix.Feature.GLOBALTHIS)
settings.SUPPORTS_PROMISE_ANY = feature_matrix.caniuse(feature_matrix.Feature.PROMISE_ANY)
if not settings.BULK_MEMORY:
settings.BULK_MEMORY = feature_matrix.caniuse(feature_matrix.Feature.BULK_MEMORY)

if settings.AUDIO_WORKLET:
if not settings.SUPPORTS_GLOBALTHIS:
exit_with_error('Must target recent enough browser versions that will support globalThis in order to target Wasm Audio Worklets!')
if settings.AUDIO_WORKLET == 1:
settings.AUDIO_WORKLET_FILE = unsuffixed(os.path.basename(target)) + '.aw.js'
settings.JS_LIBRARIES.append((0, shared.path_from_root('src', 'library_webaudio.js')))
Expand Down Expand Up @@ -2552,14 +2566,6 @@ def check_memory_setting(setting):
if settings.SHARED_MEMORY or settings.RELOCATABLE or settings.ASYNCIFY_LAZY_LOAD_CODE or settings.WASM2JS:
settings.IMPORTED_MEMORY = 1

if settings.MEMORY64:
if settings.ASYNCIFY and settings.MEMORY64 == 1:
exit_with_error('MEMORY64=1 is not compatible with ASYNCIFY')
if not settings.DISABLE_EXCEPTION_CATCHING:
exit_with_error('MEMORY64 is not compatible with DISABLE_EXCEPTION_CATCHING=0')
# Any "pointers" passed to JS will now be i64's, in both modes.
settings.WASM_BIGINT = 1

if settings.WASM_BIGINT:
settings.LEGALIZE_JS_FFI = 0

Expand Down Expand Up @@ -2900,8 +2906,6 @@ def get_full_import_name(name):
if settings.WASM_EXCEPTIONS:
settings.EXPORTED_FUNCTIONS += ['___cpp_exception', '___cxa_increment_exception_refcount', '___cxa_decrement_exception_refcount', '___thrown_object_from_unwind_exception']

feature_matrix.apply_min_browser_versions()

if settings.SIDE_MODULE:
# For side modules, we ignore all REQUIRED_EXPORTS that might have been added above.
# They all come from either libc or compiler-rt. The exception is __wasm_call_ctors
Expand Down
8 changes: 7 additions & 1 deletion src/library_promise.js
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,13 @@ mergeInto(LibraryManager.library, {
return id;
},

emscripten_promise_any__deps: ['$promiseMap', '$idsToPromises'],

emscripten_promise_any__deps: [
'$promiseMap', '$idsToPromises',
#if !SUPPORTS_PROMISE_ANY && !INCLUDE_FULL_LIBRARY
() => error("emscripten_promise_any used, but Promise.any is not supported by the current runtime configuration (run with EMCC_DEBUG=1 in the env for more details)"),
#endif
],
emscripten_promise_any: function(idBuf, errorBuf, size) {
var promises = idsToPromises(idBuf, size);
#if RUNTIME_DEBUG
Expand Down
3 changes: 3 additions & 0 deletions src/settings_internal.js
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,9 @@ var TARGET_NOT_SUPPORTED = 0x7FFFFFFF;
// Used to track whether target environment supports the 'globalThis' attribute.
var SUPPORTS_GLOBALTHIS = false;

// Used to track whether target environment supports the 'Promise.any'.
var SUPPORTS_PROMISE_ANY = false;

// Wasm backend symbols that are considered system symbols and don't
// have the normal C symbol name mangled applied (== prefix with an underscore)
// (Also implicily on this list is any function that starts with string "dynCall_")
Expand Down
8 changes: 8 additions & 0 deletions test/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -9761,6 +9761,14 @@ def test_main_reads_args(self):

@requires_node
def test_promise(self):
# This test depends on Promise.any, which in turn requires a modern target. Check that it
# fails to even build without bumping the min versions:
err = self.expect_fail([EMCC, test_file('core/test_promise.c')])
Copy link
Member

Choose a reason for hiding this comment

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

Don't all the versions below support Promise.any, so this build should succeed rather than fail? I must be missing something.

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 default versions fail, so we run a compile here to check that it fails, then we set the correct settings and then we expect the test to pass, right?

Copy link
Member

Choose a reason for hiding this comment

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

Aha, yes, makes sense. I didn't grok that this was doing the build twice, once before and once after setting the settings.

self.assertContained('error: emscripten_promise_any used, but Promise.any is not supported by the current runtime configuration', err)
self.set_setting('MIN_NODE_VERSION', '150000')
self.set_setting('MIN_SAFARI_VERSION', '150000')
self.set_setting('MIN_FIREFOX_VERSION', '79')
self.set_setting('MIN_CHROME_VERSION', '85')
self.do_core_test('test_promise.c')


Expand Down
24 changes: 23 additions & 1 deletion tools/feature_matrix.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ class Feature(IntEnum):
JS_BIGINT_INTEGRATION = auto()
THREADS = auto()
GLOBALTHIS = auto()
PROMISE_ANY = auto()


default_features = {Feature.SIGN_EXT, Feature.MUTABLE_GLOBALS}
Expand Down Expand Up @@ -62,25 +63,44 @@ class Feature(IntEnum):
'edge': 79,
'firefox': 65,
'safari': 120100,
# 'node': 120000
'node': 120000,
},
Feature.PROMISE_ANY: {
'chrome': 85,
'firefox': 79,
'safari': 140000,
'node': 150000,
},
}


def caniuse(feature):
min_versions = min_browser_versions[feature]

def report_missing(setting_name):
setting_value = getattr(settings, setting_name)
logger.debug(f'cannot use {feature.name} because {setting_name} is too old: {setting_value}')

if settings.MIN_CHROME_VERSION < min_versions['chrome']:
report_missing('MIN_CHROME_VERSION')
return False
# For edge we just use the same version requirements as chrome since,
# at least for modern versions of edge, they share version numbers.
if settings.MIN_EDGE_VERSION < min_versions['chrome']:
report_missing('MIN_EDGE_VERSION')
return False
if settings.MIN_FIREFOX_VERSION < min_versions['firefox']:
report_missing('MIN_FIREFOX_VERSION')
return False
if settings.MIN_SAFARI_VERSION < min_versions['safari']:
report_missing('MIN_SAFARI_VERSION')
return False
# IE don't support any non-MVP features
if settings.MIN_IE_VERSION != 0x7FFFFFFF:
report_missing('MIN_IE_VERSION')
return False
if 'node' in min_versions and settings.MIN_NODE_VERSION < min_versions['node']:
report_missing('MIN_NODE_VERSION')
return False
return True

Expand Down Expand Up @@ -112,3 +132,5 @@ def apply_min_browser_versions():
if settings.PTHREADS:
enable_feature(Feature.THREADS, 'pthreads')
enable_feature(Feature.BULK_MEMORY, 'pthreads')
if settings.AUDIO_WORKLET:
enable_feature(Feature.GLOBALTHIS, 'AUDIO_WORKLET')