Remove fastcomp-only ALIASING_FUNCTION_POINTERS setting (#11882)

Also removes the code block around it in emscripten.py. That's an obvious
easy win as it's a fastcomp-only function, and it's faster to remove such a
thing (when it's easily identifiable like here) rather than let future PRs remove
it line by line. Also removes the call chain to that function, also of fastcomp-
only functions (also easy wins, and necessary due to flake8).

See #11860

Author: kripken

emcc.py

@@ -1444,9 +1444,6 @@ def check(input_file):
          not shared.Settings.USE_PTHREADS:
         shared.Settings.EXPORT_READY_PROMISE = 0
 
-    if shared.Settings.EMULATE_FUNCTION_POINTER_CASTS:
-      shared.Settings.ALIASING_FUNCTION_POINTERS = 0
-
     if shared.Settings.LEGACY_VM_SUPPORT:
       if not shared.Settings.WASM or shared.Settings.WASM2JS:
         shared.Settings.POLYFILL_OLD_MATH_FUNCTIONS = 1

emscripten.py

@@ -53,70 +53,6 @@ def access_quote(prop):
     return '.' + prop
 
 
-def emscript_fastcomp(infile, outfile, memfile, temp_files, DEBUG):
-  """Runs the emscripten LLVM-to-JS compiler.
-
-  Args:
-    infile: The path to the input LLVM assembly file.
-    outfile: An open file object where the output is written.
-  """
-
-  assert shared.Settings.ASM_JS, 'fastcomp is asm.js-only (mode 1 or 2)'
-
-  success = False
-
-  try:
-
-    # Overview:
-    #   * Run LLVM backend to emit JS. JS includes function bodies, memory initializer,
-    #     and various metadata
-    #   * Run compiler.js on the metadata to emit the shell js code, pre/post-ambles,
-    #     JS library dependencies, etc.
-
-    # metadata is modified by reference in some of the below
-    # these functions are split up to force variables to go out of scope and allow
-    # memory to be reclaimed
-
-    with ToolchainProfiler.profile_block('get_and_parse_backend'):
-      backend_output = compile_js(infile, temp_files, DEBUG)
-      funcs, metadata, mem_init = parse_fastcomp_output(backend_output, DEBUG)
-      fixup_metadata_tables(metadata)
-      funcs = fixup_functions(funcs, metadata)
-    with ToolchainProfiler.profile_block('compiler_glue'):
-      glue, forwarded_data = compiler_glue(metadata, temp_files, DEBUG)
-
-    with ToolchainProfiler.profile_block('function_tables_and_exports'):
-      (post, function_table_data, bundled_args) = (
-          function_tables_and_exports(funcs, metadata, mem_init, glue, forwarded_data, outfile, DEBUG))
-    with ToolchainProfiler.profile_block('write_output_file'):
-      finalize_output(outfile, post, function_table_data, bundled_args, metadata, DEBUG)
-    success = True
-
-  finally:
-    outfile.close()
-    if not success:
-      shared.try_delete(outfile.name) # remove partial output
-
-
-def compile_js(infile, temp_files, DEBUG):
-  """Compile infile with asm.js backend, return the contents of the compiled js"""
-  with temp_files.get_file('.4.js') as temp_js:
-    backend_cmd = create_backend_cmd(infile, temp_js)
-
-    if DEBUG:
-      logger.debug('emscript: llvm backend: ' + ' '.join(backend_cmd))
-      t = time.time()
-    shared.print_compiler_stage(backend_cmd)
-    with ToolchainProfiler.profile_block('emscript_llvm_backend'):
-      shared.check_call(backend_cmd)
-    if DEBUG:
-      logger.debug('  emscript: llvm backend took %s seconds' % (time.time() - t))
-
-    # Split up output
-    backend_output = open(temp_js).read()
-  return backend_output
-
-
 def parse_fastcomp_output(backend_output, DEBUG):
   start_funcs_marker = '// EMSCRIPTEN_START_FUNCTIONS'
   end_funcs_marker = '// EMSCRIPTEN_END_FUNCTIONS'
@@ -543,49 +479,6 @@ def write_output_file(outfile, post, module):
   outfile.write(post)
 
 
-def create_backend_cmd(infile, temp_js):
-  """Create asm.js backend command from settings dict"""
-  args = [
-    shared.LLVM_COMPILER, infile, '-march=js', '-filetype=asm', '-o', temp_js,
-    '-emscripten-stack-size=%d' % shared.Settings.TOTAL_STACK,
-    '-O%s' % shared.Settings.OPT_LEVEL,
-  ]
-  if shared.Settings.USE_PTHREADS:
-    args += ['-emscripten-enable-pthreads']
-  if shared.Settings.WARN_UNALIGNED:
-    args += ['-emscripten-warn-unaligned']
-  if shared.Settings.RESERVED_FUNCTION_POINTERS > 0:
-    args += ['-emscripten-reserved-function-pointers=%d' % shared.Settings.RESERVED_FUNCTION_POINTERS]
-  if shared.Settings.ASSERTIONS > 0:
-    args += ['-emscripten-assertions=%d' % shared.Settings.ASSERTIONS]
-  if shared.Settings.ALIASING_FUNCTION_POINTERS == 0:
-    args += ['-emscripten-no-aliasing-function-pointers']
-  if shared.Settings.EMULATE_FUNCTION_POINTER_CASTS:
-    args += ['-emscripten-emulate-function-pointer-casts']
-  if shared.Settings.RELOCATABLE:
-    args += ['-emscripten-relocatable']
-    args += ['-emscripten-global-base=0']
-  elif shared.Settings.GLOBAL_BASE >= 0:
-    args += ['-emscripten-global-base=%d' % shared.Settings.GLOBAL_BASE]
-  if shared.Settings.SIDE_MODULE:
-    args += ['-emscripten-side-module']
-  if shared.Settings.LEGALIZE_JS_FFI != 1:
-    args += ['-emscripten-legalize-javascript-ffi=0']
-  if shared.Settings.DISABLE_EXCEPTION_CATCHING != 1:
-    args += ['-enable-emscripten-cpp-exceptions']
-    if shared.Settings.DISABLE_EXCEPTION_CATCHING == 2:
-      args += ['-emscripten-cpp-exceptions-whitelist=' + ','.join(shared.Settings.EXCEPTION_CATCHING_ALLOWED or ['fake'])]
-  if not shared.Settings.EXIT_RUNTIME:
-    args += ['-emscripten-no-exit-runtime']
-  if shared.Settings.WORKAROUND_IOS_9_RIGHT_SHIFT_BUG:
-    args += ['-emscripten-asmjs-work-around-ios-9-right-shift-bug']
-  if shared.Settings.WASM:
-    args += ['-emscripten-wasm']
-    if building.is_wasm_only():
-      args += ['-emscripten-only-wasm']
-  return args
-
-
 def optimize_syscalls(declares, DEBUG):
   """Disables filesystem if only a limited subset of syscalls is used.
 
@@ -2444,7 +2337,7 @@ def run(infile, outfile, memfile):
 
   outfile_obj = open(outfile, 'w')
 
-  emscripter = emscript_wasm_backend if shared.Settings.WASM_BACKEND else emscript_fastcomp
+  emscripter = emscript_wasm_backend
   return temp_files.run_and_clean(lambda: emscripter(
       infile, outfile_obj, memfile, temp_files, shared.DEBUG)
   )

site/source/docs/porting/Debugging.rst

@@ -251,14 +251,11 @@ In order to debug these sorts of issues:
 - Compile with ``-Werror``. This turns warnings into errors, which can be useful as some cases of undefined behavior would otherwise show warnings.
 - Use ``-s ASSERTIONS=2`` to get some useful information about the function pointer being called, and its type.
 - Look at the browser stack trace to see where the error occurs and which function should have been called.
-- Build with :ref:`SAFE_HEAP=1 <debugging-SAFE-HEAP>` and function pointer aliasing disabled (``ALIASING_FUNCTION_POINTERS=0``). This should make it impossible for a function pointer to be called with the wrong type without raising an error: ``-s SAFE_HEAP=1 -s ALIASING_FUNCTION_POINTERS=0``
-
+- Build with :ref:`SAFE_HEAP=1 <debugging-SAFE-HEAP>`.
+- :ref:`Sanitizers` can help here, in particular UBSan.
 
 Another function pointer issue is when the wrong function is called. :ref:`SAFE_HEAP=1 <debugging-SAFE-HEAP>` can help with this as it detects some possible errors with function table accesses.
 
-``ALIASING_FUNCTION_POINTERS=0`` is also useful because it ensures that calls to function pointer addresses in the wrong table result in clear errors. Without this setting such calls just execute whatever function is at the address, which can be much harder to debug.
-
-
 
 Infinite loops
 --------------

src/settings.js

@@ -290,12 +290,6 @@ var SAFE_HEAP_LOG = 0;
 // [fastcomp-only]
 var RESERVED_FUNCTION_POINTERS = 0;
 
-// Whether to allow function pointers to alias if they have a different type.
-// This can greatly decrease table sizes in asm.js, but can break code that
-// compares function pointers across different types.
-// [fastcomp-only]
-var ALIASING_FUNCTION_POINTERS = 0;
-
 // Allows function pointers to be cast, wraps each call of an incorrect type
 // with a runtime correction.  This adds overhead and should not be used
 // normally.  It also forces ALIASING_FUNCTION_POINTERS to 0.  Aside from making
@@ -1745,4 +1739,5 @@ var LEGACY_SETTINGS = [
   ['FAST_UNROLLED_MEMCPY_AND_MEMSET', [0, 1], 'The wasm backend implements memcpy/memset in C'],
   ['DOUBLE_MODE', [0, 1], 'The wasm backend always implements doubles normally'],
   ['PRECISE_F32', [0, 1, 2], 'The wasm backend always implements floats normally'],
+  ['ALIASING_FUNCTION_POINTERS', [0, 1], 'The wasm backend always uses a single index space for function pointers, in a single Table'],
 ];

tests/test_core.py

@@ -6085,10 +6085,6 @@ def test_lua(self):
   @needs_make('configure script')
   @is_slow_test
   def test_freetype(self):
-    if self.run_name == 'asm2g':
-      # flip for some more coverage here
-      self.set_setting('ALIASING_FUNCTION_POINTERS', 1 - self.get_setting('ALIASING_FUNCTION_POINTERS'))
-
     self.add_pre_run("FS.createDataFile('/', 'font.ttf', %s, true, false, false);" % str(
       list(bytearray(open(path_from_root('tests', 'freetype', 'LiberationSansBold.ttf'), 'rb').read()))
     ))
@@ -6898,11 +6894,6 @@ def test_add_function(self):
       self.emcc_args = old
       print(old)
 
-    print('with ALIASING_FUNCTION_POINTERS')
-    self.set_setting('ALIASING_FUNCTION_POINTERS', 1)
-    self.do_run_in_out_file_test('tests', 'interop', 'test_add_function')
-    self.clear_setting('ALIASING_FUNCTION_POINTERS')
-
     print('with RESERVED_FUNCTION_POINTERS=0')
     self.set_setting('RESERVED_FUNCTION_POINTERS', 0)
 

tests/test_other.py

@@ -3929,56 +3929,6 @@ def test_bad_function_pointer_cast(self):
                   # non-informative error
                   self.assertContained(('abort(', 'exception'), output)
 
-  @no_wasm_backend('asm.js function table feature')
-  def test_aliased_func_pointers(self):
-    create_test_file('src.cpp', r'''
-#include <stdio.h>
-
-int impl1(int foo) { return foo; }
-float impla(float foo) { return foo; }
-int impl2(int foo) { return foo+1; }
-float implb(float foo) { return foo+1; }
-int impl3(int foo) { return foo+2; }
-float implc(float foo) { return foo+2; }
-
-int main(int argc, char **argv) {
-  volatile void *f = (void*)impl1;
-  if (argc == 50) f = (void*)impla;
-  if (argc == 51) f = (void*)impl2;
-  if (argc == 52) f = (void*)implb;
-  if (argc == 53) f = (void*)impl3;
-  if (argc == 54) f = (void*)implc;
-  return (int)f;
-}
-''')
-
-    print('aliasing')
-
-    sizes_ii = {}
-    sizes_dd = {}
-
-    for alias in [None, 0, 1]:
-      cmd = [EMCC, 'src.cpp', '-O1', '-s', 'WASM=0']
-      if alias is not None:
-        cmd += ['-s', 'ALIASING_FUNCTION_POINTERS=' + str(alias)]
-      else:
-        alias = -1
-      print(cmd)
-      self.run_process(cmd)
-      src = open('a.out.js').read().split('\n')
-      for line in src:
-        if line.strip().startswith('var FUNCTION_TABLE_ii = '):
-          sizes_ii[alias] = line.count(',')
-        if line.strip().startswith('var FUNCTION_TABLE_dd = '):
-          sizes_dd[alias] = line.count(',')
-
-    print('ii', sizes_ii)
-    print('dd', sizes_dd)
-
-    for sizes in [sizes_ii, sizes_dd]:
-      self.assertEqual(sizes[-1], sizes[1]) # default is to alias
-      self.assertLess(sizes[1], sizes[0]) # without aliasing, we have more unique values and fat tables
-
   def test_bad_export(self):
     for m in ['', ' ']:
       self.clear()

tools/shared.py

@@ -1001,7 +1001,6 @@ def apply_opt_level(cls, opt_level, shrink_level=0, noisy=False):
       if opt_level >= 1:
         cls.attrs['ASM_JS'] = 1
         cls.attrs['ASSERTIONS'] = 0
-        cls.attrs['ALIASING_FUNCTION_POINTERS'] = 1
       if shrink_level >= 2:
         cls.attrs['EVAL_CTORS'] = 1