Assert if we try to use link-time setting before link time. NFC. (#14109)

We should not be reading or writing to linker-only settings until after
the compile phase.  This change keeps us honest and allows us to be sure
the list of compile time settings that we have is correct (or at least
sufficient, it could contain false positives).

Author: sbc100

emcc.py

@@ -50,7 +50,7 @@
 from tools import wasm2c
 from tools import webassembly
 from tools import config
-from tools.settings import settings, MEM_SIZE_SETTINGS
+from tools.settings import settings, MEM_SIZE_SETTINGS, COMPILE_TIME_SETTINGS
 
 logger = logging.getLogger('emcc')
 
@@ -213,6 +213,15 @@ def __init__(self, args):
     self.forced_stdlibs = []
 
 
+def add_link_flag(state, i, f):
+  if f.startswith('-l'):
+    state.libs.append((i, f[2:]))
+  if f.startswith('-L'):
+    state.lib_dirs.append(f[2:])
+
+  state.link_flags.append((i, f))
+
+
 class EmccOptions:
   def __init__(self):
     self.output_file = None
@@ -1032,9 +1041,15 @@ def run(args):
   ## Process argument and setup the compiler
   state = EmccState(args)
   options, newargs, settings_map = phase_parse_arguments(state)
+
+  # For internal consistency, ensure we don't attempt or read or write any link time
+  # settings until we reach the linking phase.
+  settings.limit_settings(COMPILE_TIME_SETTINGS)
+
   newargs, input_files = phase_setup(options, state, newargs, settings_map)
 
   if options.post_link:
+    settings.limit_settings(None)
     target, wasm_target = phase_linker_setup(options, state, newargs, settings_map)
     process_libraries(state.libs, state.lib_dirs, [])
     if len(input_files) != 1:
@@ -1043,8 +1058,7 @@ def run(args):
     return 0
 
   ## Compile source code to object files
-  linker_inputs = []
-  phase_compile_inputs(options, state, newargs, input_files, linker_inputs)
+  linker_inputs = phase_compile_inputs(options, state, newargs, input_files)
 
   if state.compile_only:
     logger.debug('stopping after compile phase')
@@ -1055,6 +1069,9 @@ def run(args):
 
     return 0
 
+  # We have now passed the compile phase, allow reading/writing of all settings.
+  settings.limit_settings(None)
+
   if options.output_file and options.output_file.startswith('-'):
     exit_with_error('invalid output filename: `%s`' % options.output_file)
 
@@ -1141,29 +1158,6 @@ def phase_parse_arguments(state):
   if options.post_link or options.oformat == OFormat.BARE:
     diagnostics.warning('experimental', '--oformat=base/--post-link are experimental and subject to change.')
 
-  if options.emrun:
-    options.pre_js += open(shared.path_from_root('src', 'emrun_prejs.js')).read() + '\n'
-    options.post_js += open(shared.path_from_root('src', 'emrun_postjs.js')).read() + '\n'
-    # emrun mode waits on program exit
-    settings.EXIT_RUNTIME = 1
-
-  if options.cpu_profiler:
-    options.post_js += open(shared.path_from_root('src', 'cpuprofiler.js')).read() + '\n'
-
-  if options.memory_profiler:
-    settings.MEMORYPROFILER = 1
-
-  if options.thread_profiler:
-    options.post_js += open(shared.path_from_root('src', 'threadprofiler.js')).read() + '\n'
-
-  if options.memory_init_file is None:
-    options.memory_init_file = settings.OPT_LEVEL >= 2
-
-  # TODO: support source maps with js_transform
-  if options.js_transform and settings.GENERATE_SOURCE_MAP:
-    logger.warning('disabling source maps because a js transform is being done')
-    settings.GENERATE_SOURCE_MAP = 0
-
   explicit_settings_changes, newargs = parse_s_args(newargs)
   settings_changes += explicit_settings_changes
 
@@ -1206,14 +1200,6 @@ def phase_setup(options, state, newargs, settings_map):
   # arguments that expand into multiple processed arguments, as in -Wl,-f1,-f2.
   input_files = []
 
-  def add_link_flag(i, f):
-    if f.startswith('-l'):
-      state.libs.append((i, f[2:]))
-    if f.startswith('-L'):
-      state.lib_dirs.append(f[2:])
-
-    state.link_flags.append((i, f))
-
   # find input files with a simple heuristic. we should really analyze
   # based on a full understanding of gcc params, right now we just assume that
   # what is left contains no more |-x OPT| things
@@ -1259,21 +1245,21 @@ def add_link_flag(i, f):
       else:
         input_files.append((i, arg))
     elif arg.startswith('-L'):
-      add_link_flag(i, arg)
+      add_link_flag(state, i, arg)
       newargs[i] = ''
     elif arg.startswith('-l'):
-      add_link_flag(i, arg)
+      add_link_flag(state, i, arg)
       newargs[i] = ''
     elif arg.startswith('-Wl,'):
       # Multiple comma separated link flags can be specified. Create fake
       # fractional indices for these: -Wl,a,b,c,d at index 4 becomes:
       # (4, a), (4.25, b), (4.5, c), (4.75, d)
       link_flags_to_add = arg.split(',')[1:]
       for flag_index, flag in enumerate(link_flags_to_add):
-        add_link_flag(i + float(flag_index) / len(link_flags_to_add), flag)
+        add_link_flag(state, i + float(flag_index) / len(link_flags_to_add), flag)
       newargs[i] = ''
     elif arg == '-Xlinker':
-      add_link_flag(i + 1, newargs[i + 1])
+      add_link_flag(state, i + 1, newargs[i + 1])
       newargs[i] = ''
       newargs[i + 1] = ''
     elif arg == '-s':
@@ -1307,10 +1293,6 @@ def add_link_flag(i, f):
     # for key in settings_map:
     #   if key not in COMPILE_TIME_SETTINGS:
     #     diagnostics.warning('unused-command-line-argument', "linker setting ignored during compilation: '%s'" % key)
-  else:
-    ldflags = emsdk_ldflags(newargs)
-    for f in ldflags:
-      add_link_flag(sys.maxsize, f)
 
   if state.has_dash_c or state.has_dash_S or state.has_dash_E or '-M' in newargs or '-MM' in newargs:
     if state.has_dash_c:
@@ -1362,6 +1344,33 @@ def phase_linker_setup(options, state, newargs, settings_map):
     # Add `#!` line to output JS and make it executable.
     options.executable = True
 
+  ldflags = emsdk_ldflags(newargs)
+  for f in ldflags:
+    add_link_flag(state, sys.maxsize, f)
+
+  if options.emrun:
+    options.pre_js += open(shared.path_from_root('src', 'emrun_prejs.js')).read() + '\n'
+    options.post_js += open(shared.path_from_root('src', 'emrun_postjs.js')).read() + '\n'
+    # emrun mode waits on program exit
+    settings.EXIT_RUNTIME = 1
+
+  if options.cpu_profiler:
+    options.post_js += open(shared.path_from_root('src', 'cpuprofiler.js')).read() + '\n'
+
+  if options.memory_profiler:
+    settings.MEMORYPROFILER = 1
+
+  if options.thread_profiler:
+    options.post_js += open(shared.path_from_root('src', 'threadprofiler.js')).read() + '\n'
+
+  if options.memory_init_file is None:
+    options.memory_init_file = settings.OPT_LEVEL >= 2
+
+  # TODO: support source maps with js_transform
+  if options.js_transform and settings.GENERATE_SOURCE_MAP:
+    logger.warning('disabling source maps because a js transform is being done')
+    settings.GENERATE_SOURCE_MAP = 0
+
   # options.output_file is the user-specified one, target is what we will generate
   if options.output_file:
     target = options.output_file
@@ -2244,7 +2253,7 @@ def get_full_import_name(name):
 
 
 @ToolchainProfiler.profile_block('compile inputs')
-def phase_compile_inputs(options, state, newargs, input_files, linker_inputs):
+def phase_compile_inputs(options, state, newargs, input_files):
   def is_link_flag(flag):
     if flag.startswith('-nostdlib'):
       return True
@@ -2321,7 +2330,7 @@ def get_clang_command_asm(src_file):
       # with -MF! (clang seems to not recognize it)
       logger.debug(('just preprocessor ' if state.has_dash_E else 'just dependencies: ') + ' '.join(cmd))
       shared.check_call(cmd)
-    return
+    return []
 
   # Precompiled headers support
   if state.has_header_inputs:
@@ -2334,8 +2343,9 @@ def get_clang_command_asm(src_file):
         cmd += ['-o', options.output_file]
       logger.debug("running (for precompiled headers): " + cmd[0] + ' ' + ' '.join(cmd[1:]))
       shared.check_call(cmd)
-      return
+      return []
 
+  linker_inputs = []
   seen_names = {}
 
   def uniquename(name):
@@ -2392,6 +2402,8 @@ def compile_source_file(i, input_file):
       logger.debug('using object file: ' + input_file)
       linker_inputs.append((i, input_file))
 
+  return linker_inputs
+
 
 @ToolchainProfiler.profile_block('calculate system libraries')
 def phase_calculate_system_libraries(state, linker_arguments, linker_inputs, newargs):

tools/ports/__init__.py

@@ -27,6 +27,8 @@ def read_ports():
       assert hasattr(port, a), 'port %s is missing %s' % (port, a)
     if not hasattr(port, 'process_dependencies'):
       port.process_dependencies = lambda x: 0
+    if not hasattr(port, 'linker_setup'):
+      port.linker_setup = lambda x, y: 0
     if not hasattr(port, 'deps'):
       port.deps = []
 

tools/ports/regal.py

@@ -143,7 +143,7 @@ def clear(ports, settings, shared):
   shared.Cache.erase_lib(get_lib_name(settings))
 
 
-def process_dependencies(settings):
+def linker_setup(ports, settings):
   settings.FULL_ES2 = 1
 
 

tools/ports/sdl2.py

@@ -90,7 +90,7 @@ def clear(ports, settings, shared):
   shared.Cache.erase_lib(get_lib_name(settings))
 
 
-def process_dependencies(settings):
+def linker_setup(ports, settings):
   settings.DEFAULT_LIBRARY_FUNCS_TO_INCLUDE += ['$autoResumeAudioContext', '$dynCall']
 
 

tools/settings.py

@@ -21,24 +21,7 @@
     'DEFAULT_PTHREAD_STACK_SIZE'
 )
 
-# Subset of settings that apply at compile time.
-# (Keep in sync with [compile] comments in settings.js)
-COMPILE_TIME_SETTINGS = (
-    'MEMORY64',
-    'INLINING_LIMIT',
-    'EXCEPTION_CATCHING_ALLOWED',
-    'DISABLE_EXCEPTION_CATCHING',
-    'DISABLE_EXCEPTION_THROWING',
-    'MAIN_MODULE',
-    'SIDE_MODULE',
-    'RELOCATABLE',
-    'STRICT',
-    'EMSCRIPTEN_TRACING',
-    'USE_PTHREADS',
-    'SUPPORT_LONGJMP',
-    'DEFAULT_TO_CXX',
-    'WASM_OBJECT_FILES',
-
+PORTS_SETTINGS = (
     # All port-related settings are valid at compile time
     'USE_SDL',
     'USE_LIBPNG',
@@ -62,11 +45,45 @@
     'USE_MPG123',
     'USE_GIFLIB',
     'USE_FREETYPE',
+    'SDL2_MIXER_FORMATS',
+    'SDL2_IMAGE_FORMATS',
 )
 
+# Subset of settings that apply at compile time.
+# (Keep in sync with [compile] comments in settings.js)
+COMPILE_TIME_SETTINGS = (
+    'MEMORY64',
+    'INLINING_LIMIT',
+    'DISABLE_EXCEPTION_CATCHING',
+    'DISABLE_EXCEPTION_THROWING',
+    'MAIN_MODULE',
+    'SIDE_MODULE',
+    'RELOCATABLE',
+    'STRICT',
+    'EMSCRIPTEN_TRACING',
+    'USE_PTHREADS',
+    'SUPPORT_LONGJMP',
+    'DEFAULT_TO_CXX',
+    'WASM_OBJECT_FILES',
+
+    # Internal settings used during compilation
+    'EXCEPTION_CATCHING_ALLOWED',
+    'EXCEPTION_HANDLING',
+    'LTO',
+    'OPT_LEVEL',
+    'DEBUG_LEVEL',
+
+    # This is legacy setting that we happen to handle very early on
+    'RUNTIME_LINKED_LIBS',
+    # TODO: should not be here
+    'AUTO_ARCHIVE_INDEXES',
+    'DEFAULT_LIBRARY_FUNCS_TO_INCLUDE',
+) + PORTS_SETTINGS
+
 
 class SettingsManager:
   attrs = {}
+  allowed_settings = []
   legacy_settings = {}
   alt_names = {}
   internal_settings = set()
@@ -76,6 +93,7 @@ def __init__(self):
     self.legacy_settings.clear()
     self.alt_names.clear()
     self.internal_settings.clear()
+    self.allowed_settings.clear()
 
     # Load the JS defaults into python.
     settings = open(path_from_root('src', 'settings.js')).read().replace('//', '#')
@@ -118,13 +136,24 @@ def dict(self):
   def keys(self):
     return self.attrs.keys()
 
+  def limit_settings(self, allowed):
+    self.allowed_settings.clear()
+    if allowed:
+      self.allowed_settings.extend(allowed)
+
   def __getattr__(self, attr):
+    if self.allowed_settings:
+      assert attr in self.allowed_settings, f"internal error: attempt to read setting '{attr}' while in limited settings mode"
+
     if attr in self.attrs:
       return self.attrs[attr]
     else:
-      raise AttributeError("no such setting: '%s'" % attr)
+      raise AttributeError(f"no such setting: '{attr}'")
 
   def __setattr__(self, name, value):
+    if self.allowed_settings:
+      assert name in self.allowed_settings, f"internal error: attempt to write setting '{name}' while in limited settings mode"
+
     if name == 'STRICT' and value:
       for a in self.legacy_settings:
         self.attrs.pop(a, None)

tools/system_libs.py

@@ -1880,6 +1880,7 @@ def get_ports_libs(settings):
 
   for port in dependency_order(needed):
     if port.needed(settings):
+      port.linker_setup(Ports, settings)
       # ports return their output files, which will be linked, or a txt file
       ret += [f for f in port.get(Ports, settings, shared) if not f.endswith('.txt')]
 
@@ -1900,7 +1901,7 @@ def add_ports_cflags(args, settings): # noqa: U100
 
   needed = get_needed_ports(settings)
 
-  # Now get (i.e. build) the ports independency order.  This is important because the
+  # Now get (i.e. build) the ports in dependency order.  This is important because the
   # headers from one ports might be needed before we can build the next.
   for port in dependency_order(needed):
     port.get(Ports, settings, shared)