Skip to content

Commit 8f354ae

Browse files
authored
Fix bulk memory feature removal in optimizing builds (#25719)
The bug here was the BINARYEN_FEATURES was been stashed right away after linking and then used unmodified for future binaryen calls. Fixes: #25479
1 parent fb11334 commit 8f354ae

File tree

2 files changed

+31
-8
lines changed

2 files changed

+31
-8
lines changed

test/test_other.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9974,6 +9974,23 @@ def compile(flags):
99749974
compile(['-sMIN_SAFARI_VERSION=140100', '-mbulk-memory'])
99759975
verify_features_sec_linked('nontrapping-fptoint', False)
99769976

9977+
def test_no_bulk_memory(self):
9978+
# The test_wasm_features test (above) uses the feature section to confirm
9979+
# if a feature is present, but that doesn't work in optimizing builds
9980+
# since we strip the feature section in release builds.
9981+
# This test confirms that no DATACOUNT section is present in the final
9982+
# binary.
9983+
9984+
def has_data_count(filename):
9985+
with webassembly.Module(filename) as wasm:
9986+
return wasm.get_section(webassembly.SecType.DATACOUNT)
9987+
9988+
self.emcc('hello_world.c', ['-O3', '-o', 'bulk.js'])
9989+
self.assertTrue(has_data_count('bulk.wasm'))
9990+
9991+
self.emcc('hello_world.c', ['-O3', '-o', 'nobulk.js', '-mno-bulk-memory', '-mno-bulk-memory-opt'])
9992+
self.assertFalse(has_data_count('nobulk.wasm'))
9993+
99779994
@crossplatform
99789995
def test_html_preprocess(self):
99799996
src_file = test_file('module/test_stdin.c')

tools/link.py

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
webassembly,
3535
)
3636
from .cmdline import OFormat
37+
from .feature_matrix import Feature
3738
from .minimal_runtime_shell import generate_minimal_runtime_html
3839
from .settings import (
3940
DEPRECATED_SETTINGS,
@@ -367,16 +368,21 @@ def get_binaryen_lowering_passes():
367368
# This can happen if the feature is explicitly disabled on the command line,
368369
# or when targeting an VM/engine that does not support the feature.
369370

370-
# List of [<feature_name>, <lowering_flag>] pairs.
371+
# List of [<feature_name>, <lowering_flag>, <feature_flags>] triples.
371372
features = [
372-
[feature_matrix.Feature.SIGN_EXT, '--signext-lowering'],
373-
[feature_matrix.Feature.NON_TRAPPING_FPTOINT, '--llvm-nontrapping-fptoint-lowering'],
374-
[feature_matrix.Feature.BULK_MEMORY, '--llvm-memory-copy-fill-lowering'],
373+
[Feature.SIGN_EXT, '--signext-lowering', ['--enable-sign-ext']],
374+
[Feature.NON_TRAPPING_FPTOINT, '--llvm-nontrapping-fptoint-lowering', ['--enable-nontrapping-float-to-int']],
375+
[Feature.BULK_MEMORY, '--llvm-memory-copy-fill-lowering', ['--enable-bulk-memory', '--enable-bulk-memory-opt']],
375376
]
376377

377-
for feature, lowering_flag in features:
378+
for feature, lowering_flag, feature_flags in features:
378379
if not feature_matrix.caniuse(feature):
379380
logger.debug(f'lowering {feature.name} feature due to incompatible target browser engines')
381+
for f in feature_flags:
382+
# Remove features from binaryen_features, otherwise future runs of binaryen
383+
# could re-introduce the feature.
384+
if f in building.binaryen_features:
385+
building.binaryen_features.remove(f)
380386
passes.append(lowering_flag)
381387

382388
return passes
@@ -1044,7 +1050,7 @@ def limit_incoming_module_api():
10441050
if user_settings.get('WASM_BIGINT') and settings.WASM_BIGINT:
10451051
exit_with_error('WASM_BIGINT=1 is not compatible with wasm2js')
10461052
settings.WASM_BIGINT = 0
1047-
feature_matrix.disable_feature(feature_matrix.Feature.JS_BIGINT_INTEGRATION)
1053+
feature_matrix.disable_feature(Feature.JS_BIGINT_INTEGRATION)
10481054

10491055
if options.oformat == OFormat.WASM and not settings.SIDE_MODULE:
10501056
# if the output is just a wasm file, it will normally be a standalone one,
@@ -1583,8 +1589,8 @@ def limit_incoming_module_api():
15831589

15841590
# TODO(sbc): Find make a generic way to expose the feature matrix to JS
15851591
# compiler rather then adding them all ad-hoc as internal settings
1586-
settings.SUPPORTS_PROMISE_ANY = feature_matrix.caniuse(feature_matrix.Feature.PROMISE_ANY)
1587-
default_setting('WASM_BIGINT', feature_matrix.caniuse(feature_matrix.Feature.JS_BIGINT_INTEGRATION))
1592+
settings.SUPPORTS_PROMISE_ANY = feature_matrix.caniuse(Feature.PROMISE_ANY)
1593+
default_setting('WASM_BIGINT', feature_matrix.caniuse(Feature.JS_BIGINT_INTEGRATION))
15881594

15891595
if settings.AUDIO_WORKLET:
15901596
add_system_js_lib('libwebaudio.js')

0 commit comments

Comments
 (0)