Skip to content

Commit 9bff7a2

Browse files
authored
Don't allow legacy settings in strict mode (#8261)
In strict mode (`EMCC_STRICT=1` or `-s STRICT`) treat any usage of legacy settings as an error. We don't even attach the legacy settings in this case so it becomes impossible to depend on legacy settings. Also, move SAFE_HEAP and ASM_JS out of legacy settings. These settings are still used, they just need to have their values verified. See #8257
1 parent b4fdb5f commit 9bff7a2

File tree

5 files changed

+67
-28
lines changed

5 files changed

+67
-28
lines changed

emcc.py

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -829,8 +829,11 @@ def detect_fixed_language_mode(args):
829829
has_header_inputs = False
830830
lib_dirs = [shared.path_from_root('system', 'local', 'lib'),
831831
shared.path_from_root('system', 'lib')]
832-
for i in range(len(newargs)): # find input files XXX this a simple heuristic. we should really analyze based on a full understanding of gcc params,
833-
# right now we just assume that what is left contains no more |-x OPT| things
832+
833+
# find input files this a simple heuristic. we should really analyze
834+
# based on a full understanding of gcc params, right now we just assume that
835+
# what is left contains no more |-x OPT| things
836+
for i in range(len(newargs)):
834837
arg = newargs[i]
835838
if i > 0:
836839
prev = newargs[i - 1]
@@ -945,18 +948,16 @@ def detect_fixed_language_mode(args):
945948
if options.separate_asm:
946949
shared.Settings.SEPARATE_ASM = shared.JS.get_subresource_location(asm_target)
947950

948-
if 'EMCC_STRICT' in os.environ:
949-
shared.Settings.STRICT = os.environ.get('EMCC_STRICT') != '0'
950-
951-
# Libraries are searched before settings_changes are applied, so apply the value for STRICT and ERROR_ON_MISSING_LIBRARIES from
952-
# command line already now.
951+
# Libraries are searched before settings_changes are applied, so apply the
952+
# value for STRICT and ERROR_ON_MISSING_LIBRARIES from command line already
953+
# now.
953954

954955
def get_last_setting_change(setting):
955956
return ([None] + [x for x in settings_changes if x.startswith(setting + '=')])[-1]
956957

957958
strict_cmdline = get_last_setting_change('STRICT')
958959
if strict_cmdline:
959-
shared.Settings.STRICT = int(strict_cmdline[len('STRICT='):])
960+
shared.Settings.STRICT = int(strict_cmdline.split('=', 1)[1])
960961

961962
if shared.Settings.STRICT:
962963
shared.Settings.ERROR_ON_MISSING_LIBRARIES = 1

src/settings.js

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1392,14 +1392,14 @@ var DISABLE_DEPRECATED_FIND_EVENT_TARGET_BEHAVIOR = 0;
13921392
// to explicitly choose to disable HTML minification altogether.
13931393
var MINIFY_HTML = 1;
13941394

1395-
// Legacy settings that have been removed, and the values they are now fixed to. These can no
1396-
// longer be changed:
1397-
// [OPTION_NAME, POSSIBLE_VALUES, ERROR_EXPLANATION], where POSSIBLE_VALUES is an array of values that will
1398-
// still be silently accepted by the compiler. First element in the list is the canonical/fixed value going forward.
1399-
// This allows existing build systems to keep specifying one of the supported settings, for backwards compatibility.
1395+
// Legacy settings that have been removed, and the values they are now fixed to.
1396+
// These can no longer be changed:
1397+
// [OPTION_NAME, POSSIBLE_VALUES, ERROR_EXPLANATION], where POSSIBLE_VALUES is
1398+
// an array of values that will still be silently accepted by the compiler.
1399+
// First element in the list is the canonical/fixed value going forward.
1400+
// This allows existing build systems to keep specifying one of the supported
1401+
// settings, for backwards compatibility.
14001402
var LEGACY_SETTINGS = [
1401-
['ASM_JS', [1, 2], 'ASM_JS must be enabled in fastcomp'],
1402-
['SAFE_HEAP', [0, 1], 'safe heap must be 0 or 1 in fastcomp'],
14031403
['UNALIGNED_MEMORY', [0], 'forced unaligned memory not supported in fastcomp'],
14041404
['FORCE_ALIGNED_MEMORY', [0], 'forced aligned memory is not supported in fastcomp'],
14051405
['PGO', [0], 'pgo no longer supported'],

tests/test_other.py

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9042,9 +9042,16 @@ def test_minimal_runtime_code_size(self):
90429042
asmjs = ['-s', 'WASM=0', '--separate-asm', '-s', 'ELIMINATE_DUPLICATE_FUNCTIONS=1', '--memory-init-file', '1']
90439043
opts = ['-O3', '--closure', '1', '-DNDEBUG', '-ffast-math']
90449044

9045-
hello_world_sources = [path_from_root('tests', 'small_hello_world.c'), '-s', 'RUNTIME_FUNCS_TO_IMPORT=[]', '-s', 'USES_DYNAMIC_ALLOC=0', '-s', 'ASM_PRIMITIVE_VARS=[STACKTOP]']
9046-
hello_webgl_sources = [path_from_root('tests', 'minimal_webgl', 'main.cpp'), path_from_root('tests', 'minimal_webgl', 'webgl.c'), '--js-library', path_from_root('tests', 'minimal_webgl', 'library_js.js'),
9047-
'-s', 'RUNTIME_FUNCS_TO_IMPORT=[]', '-s', 'USES_DYNAMIC_ALLOC=2', '-lGL', '-s', 'MODULARIZE=1']
9045+
hello_world_sources = [path_from_root('tests', 'small_hello_world.c'),
9046+
'-s', 'RUNTIME_FUNCS_TO_IMPORT=[]',
9047+
'-s', 'USES_DYNAMIC_ALLOC=0',
9048+
'-s', 'ASM_PRIMITIVE_VARS=[STACKTOP]']
9049+
hello_webgl_sources = [path_from_root('tests', 'minimal_webgl', 'main.cpp'),
9050+
path_from_root('tests', 'minimal_webgl', 'webgl.c'),
9051+
'--js-library', path_from_root('tests', 'minimal_webgl', 'library_js.js'),
9052+
'-s', 'RUNTIME_FUNCS_TO_IMPORT=[]',
9053+
'-s', 'USES_DYNAMIC_ALLOC=2', '-lGL',
9054+
'-s', 'MODULARIZE=1']
90489055
hello_webgl2_sources = hello_webgl_sources + ['-s', 'USE_WEBGL2=1']
90499056

90509057
test_cases = [
@@ -9102,3 +9109,16 @@ def test_legacy_settings_forbidden_to_change(self):
91029109

91039110
run_process([PYTHON, EMCC, '-s', 'MEMFS_APPEND_TO_TYPED_ARRAYS=1', path_from_root('tests', 'hello_world.c')])
91049111
run_process([PYTHON, EMCC, '-s', 'PRECISE_I64_MATH=2', path_from_root('tests', 'hello_world.c')])
9112+
9113+
def test_legacy_settings_strict_mode(self):
9114+
cmd = [PYTHON, EMCC, path_from_root('tests', 'hello_world.c'), '-s', 'SPLIT_MEMORY=0']
9115+
run_process(cmd)
9116+
9117+
with env_modify({'EMCC_STRICT': '1'}):
9118+
proc = run_process(cmd, stderr=PIPE, check=False)
9119+
self.assertNotEqual(proc.returncode, 0)
9120+
self.assertContained('legacy setting used in strict mode: SPLIT_MEMORY', proc.stderr)
9121+
9122+
proc = run_process(cmd + ['-s', 'STRICT=1'], stderr=PIPE, check=False)
9123+
self.assertNotEqual(proc.returncode, 0)
9124+
self.assertContained('legacy setting used in strict mode: SPLIT_MEMORY', proc.stderr)

tests/test_sanity.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -331,7 +331,7 @@ def test_llvm_fastcomp(self):
331331

332332
restore_and_set_up()
333333

334-
self.check_working([EMCC] + MINIMAL_HELLO_WORLD + ['-s', 'ASM_JS=0'], '''ASM_JS must be enabled in fastcomp''')
334+
self.check_working([EMCC] + MINIMAL_HELLO_WORLD + ['-s', 'ASM_JS=0'], 'ASM_JS can only be set to either 1 or 2')
335335

336336
def test_node(self):
337337
NODE_WARNING = 'node version appears too old'

tools/shared.py

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1138,8 +1138,15 @@ def reset(self):
11381138
settings = re.sub(r'var ([\w\d]+)', r'attrs["\1"]', settings)
11391139
exec(settings, {'attrs': self.attrs})
11401140

1141-
for opt, fixed_values, _ in self.attrs['LEGACY_SETTINGS']:
1142-
self.attrs[opt] = fixed_values[0]
1141+
if 'EMCC_STRICT' in os.environ:
1142+
self.attrs['STRICT'] = int(os.environ.get('EMCC_STRICT'))
1143+
1144+
self.legacy_settings = {}
1145+
for name, fixed_values, err in self.attrs['LEGACY_SETTINGS']:
1146+
self.legacy_settings[name] = (fixed_values, err)
1147+
assert name not in self.attrs, 'legacy setting (%s) cannot also be a regular setting' % name
1148+
if not self.attrs['STRICT']:
1149+
self.attrs[name] = fixed_values[0]
11431150

11441151
if get_llvm_target() == WASM_TARGET:
11451152
self.attrs['WASM_BACKEND'] = 1
@@ -1172,19 +1179,24 @@ def apply_opt_level(self, opt_level, shrink_level=0, noisy=False):
11721179
if shrink_level >= 2:
11731180
self.attrs['EVAL_CTORS'] = 1
11741181

1182+
def keys(self):
1183+
return self.attrs.keys()
1184+
11751185
def __getattr__(self, attr):
11761186
if attr in self.attrs:
11771187
return self.attrs[attr]
11781188
else:
1179-
raise AttributeError
1189+
raise AttributeError("Settings object has no attribute '%s'" % attr)
11801190

11811191
def __setattr__(self, attr, value):
1182-
for legacy_attr, fixed_values, error_message in self.attrs['LEGACY_SETTINGS']:
1183-
if attr == legacy_attr:
1184-
if value not in fixed_values:
1185-
exit_with_error('Invalid command line option -s ' + attr + '=' + str(value) + ': ' + error_message)
1186-
else:
1187-
logger.debug('Option -s ' + attr + '=' + str(value) + ' has been removed from the codebase. (' + error_message + ')')
1192+
if attr in self.legacy_settings:
1193+
if self.attrs['STRICT']:
1194+
exit_with_error('legacy setting used in strict mode: %s', attr)
1195+
fixed_values, error_message = self.legacy_settings[attr]
1196+
if value not in fixed_values:
1197+
exit_with_error('Invalid command line option -s ' + attr + '=' + str(value) + ': ' + error_message)
1198+
else:
1199+
logger.debug('Option -s ' + attr + '=' + str(value) + ' has been removed from the codebase. (' + error_message + ')')
11881200

11891201
if attr not in self.attrs:
11901202
logger.error('Assigning a non-existent settings attribute "%s"' % attr)
@@ -1230,6 +1242,12 @@ def __getitem__(self, key):
12301242

12311243

12321244
def verify_settings():
1245+
if Settings.ASM_JS not in [1, 2]:
1246+
exit_with_error('ASM_JS can only be set to either 1 or 2')
1247+
1248+
if Settings.SAFE_HEAP not in [0, 1]:
1249+
exit_with_error('SAVE_HEAP must be 0 or 1 in fastcomp')
1250+
12331251
if Settings.WASM and Settings.EXPORT_FUNCTION_TABLES:
12341252
exit_with_error('emcc: EXPORT_FUNCTION_TABLES incompatible with WASM')
12351253

0 commit comments

Comments
 (0)