From ff3e19a526bfdabb3dd4bf2d7080def719714cd0 Mon Sep 17 00:00:00 2001 From: Derek Schuff Date: Tue, 19 Nov 2024 16:51:10 -0800 Subject: [PATCH 1/4] Allow user override of default-enabled compile-time wasm features The signext feature is enabled by default by clang and can be overridden with the -msign-ext/-mno-sign-ext flags at compile time. However it cannot be individually controlled at link time (it can only be lowered by setting the min browser version). This PR allows -mno-sign-ext to be used at link time as well, and override the browser version control. --- emcc.py | 6 ++++++ src/settings_internal.js | 2 ++ test/test_other.py | 5 +++++ tools/feature_matrix.py | 10 ++++++++++ 4 files changed, 23 insertions(+) diff --git a/emcc.py b/emcc.py index 520ad404108b3..cc12bbe80f090 100644 --- a/emcc.py +++ b/emcc.py @@ -46,6 +46,7 @@ from tools import cache from tools.settings import default_setting, user_settings, settings, MEM_SIZE_SETTINGS, COMPILE_TIME_SETTINGS from tools.utils import read_file, removeprefix +from tools import feature_matrix logger = logging.getLogger('emcc') @@ -1405,6 +1406,11 @@ def consume_arg_file(): settings.BULK_MEMORY = 1 elif arg == '-mno-bulk-memory': settings.BULK_MEMORY = 0 + elif arg == '-msign-ext': + settings.SIGN_EXT = 1 + feature_matrix.enable_feature(feature_matrix.Feature.SIGN_EXT, 'SIGN_EXT') + elif arg == '-mno-sign-ext': + feature_matrix.disable_feature(feature_matrix.Feature.SIGN_EXT) elif arg == '-fexceptions': # TODO Currently -fexceptions only means Emscripten EH. Switch to wasm # exception handling by default when -fexceptions is given when wasm diff --git a/src/settings_internal.js b/src/settings_internal.js index b0ef0f162b17a..b0777ede0e61f 100644 --- a/src/settings_internal.js +++ b/src/settings_internal.js @@ -254,6 +254,8 @@ var PTHREADS = false; var BULK_MEMORY = false; +var SIGN_EXT = true; + var MINIFY_WHITESPACE = true; var ASYNCIFY_IMPORTS_EXCEPT_JS_LIBS = []; diff --git a/test/test_other.py b/test/test_other.py index db997372b2d0a..487d7e06d256a 100644 --- a/test/test_other.py +++ b/test/test_other.py @@ -10425,6 +10425,11 @@ def compile(flags): verify_features_sec('multivalue', True) verify_features_sec('reference-types', True) + compile(['-mno-sign-ext', '-c']) + verify_features_sec('sign-ext', False) + compile(['-mno-sign-ext']) + verify_features_sec_linked('sign-ext', False) + compile(['-mnontrapping-fptoint', '-c']) verify_features_sec('nontrapping-fptoint', True) diff --git a/tools/feature_matrix.py b/tools/feature_matrix.py index 0b6a26368e99e..08d41093a3880 100644 --- a/tools/feature_matrix.py +++ b/tools/feature_matrix.py @@ -39,6 +39,7 @@ class Feature(IntEnum): default_features = {Feature.SIGN_EXT, Feature.MUTABLE_GLOBALS} +disable_override_features = set() min_browser_versions = { Feature.NON_TRAPPING_FPTOINT: { @@ -93,6 +94,9 @@ class Feature(IntEnum): def caniuse(feature): + if feature in disable_override_features: + return False + min_versions = min_browser_versions[feature] def report_missing(setting_name): @@ -132,6 +136,12 @@ def enable_feature(feature, reason): setattr(settings, name, min_version) +def disable_feature(feature): + """Allow the user to disable a feature that would otherwise be on by default. + """ + disable_override_features.add(feature) + + # apply minimum browser version defaults based on user settings. if # a user requests a feature that we know is only supported in browsers # from a specific version and above, we can assume that browser version. From 86d3d4f3dcecd4b5b3f2879d9eac9793e3f08281 Mon Sep 17 00:00:00 2001 From: Derek Schuff Date: Tue, 19 Nov 2024 17:23:06 -0800 Subject: [PATCH 2/4] improve reason field --- emcc.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/emcc.py b/emcc.py index cc12bbe80f090..266108c58a7f7 100644 --- a/emcc.py +++ b/emcc.py @@ -1408,7 +1408,7 @@ def consume_arg_file(): settings.BULK_MEMORY = 0 elif arg == '-msign-ext': settings.SIGN_EXT = 1 - feature_matrix.enable_feature(feature_matrix.Feature.SIGN_EXT, 'SIGN_EXT') + feature_matrix.enable_feature(feature_matrix.Feature.SIGN_EXT, '-msign-ext') elif arg == '-mno-sign-ext': feature_matrix.disable_feature(feature_matrix.Feature.SIGN_EXT) elif arg == '-fexceptions': From 27935e518c50f28666f75d2aa23646244ad08d60 Mon Sep 17 00:00:00 2001 From: Derek Schuff Date: Wed, 20 Nov 2024 10:52:05 -0800 Subject: [PATCH 3/4] also allow explicit enable override --- emcc.py | 9 +++++++-- src/settings_internal.js | 2 -- test/test_other.py | 19 ++++++++++++++----- tools/feature_matrix.py | 7 ++++++- 4 files changed, 27 insertions(+), 10 deletions(-) diff --git a/emcc.py b/emcc.py index 266108c58a7f7..37b8210da7d81 100644 --- a/emcc.py +++ b/emcc.py @@ -1404,11 +1404,16 @@ def consume_arg_file(): settings.WASM_EXCEPTIONS = 0 elif arg == '-mbulk-memory': settings.BULK_MEMORY = 1 + feature_matrix.enable_feature(feature_matrix.Feature.BULK_MEMORY, + '-mbulk-memory', + is_explicit=True) elif arg == '-mno-bulk-memory': settings.BULK_MEMORY = 0 + feature_matrix.disable_feature(feature_matrix.Feature.BULK_MEMORY) elif arg == '-msign-ext': - settings.SIGN_EXT = 1 - feature_matrix.enable_feature(feature_matrix.Feature.SIGN_EXT, '-msign-ext') + feature_matrix.enable_feature(feature_matrix.Feature.SIGN_EXT, + '-msign-ext', + is_explicit=True) elif arg == '-mno-sign-ext': feature_matrix.disable_feature(feature_matrix.Feature.SIGN_EXT) elif arg == '-fexceptions': diff --git a/src/settings_internal.js b/src/settings_internal.js index b0777ede0e61f..b0ef0f162b17a 100644 --- a/src/settings_internal.js +++ b/src/settings_internal.js @@ -254,8 +254,6 @@ var PTHREADS = false; var BULK_MEMORY = false; -var SIGN_EXT = true; - var MINIFY_WHITESPACE = true; var ASYNCIFY_IMPORTS_EXCEPT_JS_LIBS = []; diff --git a/test/test_other.py b/test/test_other.py index 487d7e06d256a..b1fd3e57ea785 100644 --- a/test/test_other.py +++ b/test/test_other.py @@ -10425,28 +10425,37 @@ def compile(flags): verify_features_sec('multivalue', True) verify_features_sec('reference-types', True) + # Disable a feature compile(['-mno-sign-ext', '-c']) verify_features_sec('sign-ext', False) + # Disabling overrides default browser versions compile(['-mno-sign-ext']) verify_features_sec_linked('sign-ext', False) + # Disabling overrides manual browser versions + compile(['-sMIN_SAFARI_VERSION=150000', '-mno-sign-ext']) + # Disable via browser selection + compile(['-sMIN_FIREFOX_VERSION=61']) + verify_features_sec_linked('sign-ext', False) + # Manual enable overrides browser version + compile(['-sMIN_FIREFOX_VERSION=61', '-msign-ext']) + verify_features_sec_linked('sign-ext', True) compile(['-mnontrapping-fptoint', '-c']) verify_features_sec('nontrapping-fptoint', True) - # BIGINT causes binaryen to not run, and keeps the target_features section after link # Setting this SAFARI_VERSION should enable bulk memory because it links in emscripten_memcpy_bulkmem # However it does not enable nontrapping-fptoint yet because it has no effect at compile time and # no libraries include nontrapping yet. - compile(['-sMIN_SAFARI_VERSION=150000', '-sWASM_BIGINT']) + compile(['-sMIN_SAFARI_VERSION=150000']) verify_features_sec_linked('sign-ext', True) verify_features_sec_linked('mutable-globals', True) verify_features_sec_linked('multivalue', True) verify_features_sec_linked('bulk-memory', True) verify_features_sec_linked('nontrapping-fptoint', False) - compile(['-sMIN_SAFARI_VERSION=150000', '-mno-bulk-memory', '-sWASM_BIGINT']) - # FIXME? -mno-bulk-memory at link time does not override MIN_SAFARI_VERSION. it probably should? - verify_features_sec_linked('bulk-memory', True) + compile(['-sMIN_SAFARI_VERSION=150000', '-mno-bulk-memory']) + # -mno-bulk-memory at link time overrides MIN_SAFARI_VERSION + verify_features_sec_linked('bulk-memory', False) def test_js_preprocess(self): # Use stderr rather than stdout here because stdout is redirected to the output JS file itself. diff --git a/tools/feature_matrix.py b/tools/feature_matrix.py index 08d41093a3880..3337c1f69addd 100644 --- a/tools/feature_matrix.py +++ b/tools/feature_matrix.py @@ -40,6 +40,7 @@ class Feature(IntEnum): default_features = {Feature.SIGN_EXT, Feature.MUTABLE_GLOBALS} disable_override_features = set() +enable_override_features = set() min_browser_versions = { Feature.NON_TRAPPING_FPTOINT: { @@ -96,6 +97,8 @@ class Feature(IntEnum): def caniuse(feature): if feature in disable_override_features: return False + if feature in enable_override_features: + return True min_versions = min_browser_versions[feature] @@ -118,10 +121,12 @@ def report_missing(setting_name): return True -def enable_feature(feature, reason): +def enable_feature(feature, reason, is_explicit=False): """Updates default settings for browser versions such that the given feature is available everywhere. """ + if is_explicit: + enable_override_features.add(feature) for name, min_version in min_browser_versions[feature].items(): name = f'MIN_{name.upper()}_VERSION' if settings[name] < min_version: From a62c634dd458cd8e581caef82ae57bb2564c353a Mon Sep 17 00:00:00 2001 From: Derek Schuff Date: Wed, 20 Nov 2024 14:24:43 -0800 Subject: [PATCH 4/4] review feedback --- emcc.py | 4 ++-- tools/feature_matrix.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/emcc.py b/emcc.py index 37b8210da7d81..dfe088e7b75c0 100644 --- a/emcc.py +++ b/emcc.py @@ -1406,14 +1406,14 @@ def consume_arg_file(): settings.BULK_MEMORY = 1 feature_matrix.enable_feature(feature_matrix.Feature.BULK_MEMORY, '-mbulk-memory', - is_explicit=True) + override=True) elif arg == '-mno-bulk-memory': settings.BULK_MEMORY = 0 feature_matrix.disable_feature(feature_matrix.Feature.BULK_MEMORY) elif arg == '-msign-ext': feature_matrix.enable_feature(feature_matrix.Feature.SIGN_EXT, '-msign-ext', - is_explicit=True) + override=True) elif arg == '-mno-sign-ext': feature_matrix.disable_feature(feature_matrix.Feature.SIGN_EXT) elif arg == '-fexceptions': diff --git a/tools/feature_matrix.py b/tools/feature_matrix.py index 3337c1f69addd..53ee6f66ce716 100644 --- a/tools/feature_matrix.py +++ b/tools/feature_matrix.py @@ -121,11 +121,11 @@ def report_missing(setting_name): return True -def enable_feature(feature, reason, is_explicit=False): +def enable_feature(feature, reason, override=False): """Updates default settings for browser versions such that the given feature is available everywhere. """ - if is_explicit: + if override: enable_override_features.add(feature) for name, min_version in min_browser_versions[feature].items(): name = f'MIN_{name.upper()}_VERSION'