Remove remaining uses of setErrNo and deprecate SUPPORT_ERRNO setting (#21074)

Aside from the `setErrNo` library function `SUPPORT_ERRNO` also
controlled the building the malloc and sbrk.  However, just setting
errno to ENOMEM didn't seem to have any effect on the code size for any
of our micro benchmarks.  This may be because LTO can completely
eliminate the write to `errno` when there are no readers, or it could be
that errno usage exists in in even the smallest program already so
removing from malloc alone makes no difference.

Author: sbc100

ChangeLog.md

@@ -57,6 +57,9 @@ See docs/process.md for more on how version tagging works.
 - C++ objects passed into embind's val via constructors, methods, and call
   function will not be automatically destroyed after the function call. This
   makes the behavior consistent for invocations. 
+- The `SUPPORT_ERRNO` setting is now deprecated as it only controlled setting
+  errno from JS library functions and emscripten no longer requires this.
+  (#21074)
 
 3.1.51 - 12/13/23
 -----------------

embuilder.py

@@ -46,7 +46,6 @@
     'libc++-noexcept',
     'libal',
     'libdlmalloc',
-    'libdlmalloc-noerrno',
     'libdlmalloc-tracing',
     'libdlmalloc-debug',
     'libembind',

site/source/docs/tools_reference/settings_reference.rst

@@ -315,6 +315,8 @@ options: 'quiet', 'warn', 'error'. If set to 'warn', Closure warnings are
 printed out to console. If set to 'error', Closure warnings are treated like
 errors, similar to -Werror compiler flag.
 
+.. note:: This setting is deprecated
+
 .. _ignore_closure_compiler_errors:
 
 IGNORE_CLOSURE_COMPILER_ERRORS
@@ -1177,6 +1179,8 @@ EXTRA_EXPORTED_RUNTIME_METHODS
 
 Deprecated, use EXPORTED_RUNTIME_METHODS instead.
 
+.. note:: This setting is deprecated
+
 .. _incoming_module_js_api:
 
 INCOMING_MODULE_JS_API
@@ -2447,13 +2451,11 @@ Minimum supported value is 101900, which was released 2020-02-05.
 SUPPORT_ERRNO
 =============
 
-Tracks whether we are building with errno support enabled. Set to 0
-to disable compiling errno support in altogether. This saves a little
-bit of generated code size in applications that do not care about
-POSIX errno variable. Setting this to 0 also requires using --closure
-for effective code size optimizations to take place.
+Whether we support setting errno from JS library code.
 In MINIMAL_RUNTIME builds, this option defaults to 0.
 
+.. note:: This setting is deprecated
+
 .. _minimal_runtime:
 
 MINIMAL_RUNTIME

src/library.js

@@ -328,8 +328,7 @@ addToLibrary({
     updateMemoryViews();
   },
 
-  system__deps: ['$setErrNo'],
-  system: (command) => {
+  _emscripten_system: (command) => {
 #if ENVIRONMENT_MAY_BE_NODE
     if (ENVIRONMENT_IS_NODE) {
       if (!command) return 1; // shell is available
@@ -368,8 +367,7 @@ addToLibrary({
     // http://pubs.opengroup.org/onlinepubs/000095399/functions/system.html
     // Can't call external programs.
     if (!command) return 0; // no shell available
-    setErrNo({{{ cDefs.ENOSYS }}});
-    return -1;
+    return -{{{ cDefs.ENOSYS }}};
   },
 
   // ==========================================================================
@@ -1522,20 +1520,6 @@ addToLibrary({
     {{{ cDefs.EOWNERDEAD }}}: 'Previous owner died',
     {{{ cDefs.ESTRPIPE }}}: 'Streams pipe error',
   },
-#if SUPPORT_ERRNO
-  $setErrNo__deps: ['__errno_location'],
-  $setErrNo: (value) => {
-    {{{makeSetValue("___errno_location()", 0, 'value', 'i32') }}};
-    return value;
-  },
-#else
-  $setErrNo: (value) => {
-#if ASSERTIONS
-    err('failed to set errno from JS');
-#endif
-    return 0;
-  },
-#endif
 
 #if PROXY_POSIX_SOCKETS == 0
   // ==========================================================================
@@ -2138,11 +2122,10 @@ addToLibrary({
   // nonblocking
   // ==========================================================================
 #if SOCKET_WEBRTC
-  $Sockets__deps: ['$setErrNo',
+  $Sockets__deps: [
     () => 'var SocketIO = ' + read('../third_party/socket.io.js') + ';\n',
-    () => 'var Peer = ' + read('../third_party/wrtcp.js') + ';\n'],
-#else
-  $Sockets__deps: ['$setErrNo'],
+    () => 'var Peer = ' + read('../third_party/wrtcp.js') + ';\n'
+  ],
 #endif
   $Sockets: {
     BUFFER_SIZE: 10*1024, // initial size

src/library_legacy.js

@@ -75,4 +75,19 @@ addToLibrary({
 
   $allocateUTF8: '$stringToNewUTF8',
   $allocateUTF8OnStack: '$stringToUTF8OnStack',
+
+#if SUPPORT_ERRNO
+  $setErrNo__deps: ['__errno_location'],
+  $setErrNo: (value) => {
+    {{{makeSetValue("___errno_location()", 0, 'value', 'i32') }}};
+    return value;
+  },
+#else
+  $setErrNo: (value) => {
+#if ASSERTIONS
+    err('failed to set errno from JS');
+#endif
+    return 0;
+  },
+#endif
 });

src/library_sigs.js

@@ -331,6 +331,7 @@ sigs = {
   _emscripten_receive_on_main_thread_js__sig: 'dipip',
   _emscripten_runtime_keepalive_clear__sig: 'v',
   _emscripten_set_offscreencanvas_size__sig: 'ipii',
+  _emscripten_system__sig: 'ip',
   _emscripten_thread_exit_joinable__sig: 'vp',
   _emscripten_thread_mailbox_await__sig: 'vp',
   _emscripten_thread_set_strongref__sig: 'vp',
@@ -1541,7 +1542,6 @@ sigs = {
   strftime_l__sig: 'pppppp',
   strptime__sig: 'pppp',
   strptime_l__sig: 'ppppp',
-  system__sig: 'ip',
   uuid_clear__sig: 'vp',
   uuid_compare__sig: 'ipp',
   uuid_copy__sig: 'vpp',

src/settings.js

@@ -274,6 +274,7 @@ var USE_CLOSURE_COMPILER = false;
 // printed out to console. If set to 'error', Closure warnings are treated like
 // errors, similar to -Werror compiler flag.
 // [link]
+// [deprecated]
 var CLOSURE_WARNINGS = 'quiet';
 
 // Ignore closure warnings and errors (like on duplicate definitions)
@@ -922,6 +923,7 @@ var ASYNCIFY_EXPORTS = [];
 var EXPORTED_RUNTIME_METHODS = [];
 
 // Deprecated, use EXPORTED_RUNTIME_METHODS instead.
+// [deprecated]
 var EXTRA_EXPORTED_RUNTIME_METHODS = [];
 
 // A list of incoming values on the Module object in JS that we care about. If
@@ -1839,13 +1841,10 @@ var MIN_CHROME_VERSION = 85;
 // Minimum supported value is 101900, which was released 2020-02-05.
 var MIN_NODE_VERSION = 160000;
 
-// Tracks whether we are building with errno support enabled. Set to 0
-// to disable compiling errno support in altogether. This saves a little
-// bit of generated code size in applications that do not care about
-// POSIX errno variable. Setting this to 0 also requires using --closure
-// for effective code size optimizations to take place.
+// Whether we support setting errno from JS library code.
 // In MINIMAL_RUNTIME builds, this option defaults to 0.
 // [link]
+// [deprecated]
 var SUPPORT_ERRNO = true;
 
 // If true, uses minimal sized runtime without POSIX features, Module,

system/lib/libc/emscripten_internal.h

@@ -148,6 +148,8 @@ void llvm_eh_typeid_for(void* exn);
 
 uint32_t _emscripten_lookup_name(const char *name);
 
+int _emscripten_system(const char *command);
+
 #ifdef __cplusplus
 }
 #endif

system/lib/libc/sbrk.c

@@ -12,9 +12,7 @@
 #define __NEED_max_align_t
 #endif
 
-#ifndef EMSCRIPTEN_NO_ERRNO
 #include <errno.h>
-#endif
 #include <limits.h>
 #include <stddef.h>
 #include <stdint.h>
@@ -31,12 +29,6 @@ void emscripten_memprof_sbrk_grow(intptr_t old, intptr_t new);
 
 #include <emscripten/heap.h>
 
-#ifndef EMSCRIPTEN_NO_ERRNO
-#define SET_ERRNO() { errno = ENOMEM; }
-#else
-#define SET_ERRNO()
-#endif
-
 extern size_t __heap_base;
 
 static uintptr_t sbrk_val = (uintptr_t)&__heap_base;
@@ -79,7 +71,7 @@ void *sbrk(intptr_t increment_) {
     // increase the WebAssembly Memory size, and abort if that fails.
     if ((increment > 0 && new_brk <= old_brk)
      || (new_brk > emscripten_get_heap_size() && !emscripten_resize_heap(new_brk))) {
-      SET_ERRNO();
+      errno = ENOMEM;
       return (void*)-1;
     }
 #ifdef __EMSCRIPTEN_SHARED_MEMORY__

system/lib/libc/system.c

@@ -0,0 +1,10 @@
+#include <unistd.h>
+#include <stdlib.h>
+#include <errno.h>
+#include "pthread_impl.h"
+#include "emscripten_internal.h"
+
+int system(const char *cmd)
+{
+	return __syscall_ret(_emscripten_system(cmd));
+}

test/minimal_webgl/CMakeLists.txt

@@ -71,9 +71,6 @@ if (EMSCRIPTEN)
 	# Optimization flag to optimize aggressively for size. (other options -Os, -O3, -O2, -O1, -O0)
 	append_linker_flags("-Oz")
 
-	# Reduce code size: We do not need libc errno field support in our build output.
-	append_linker_flags("-sSUPPORT_ERRNO=0")
-
 	# Reduce code size: We do not need native POSIX filesystem emulation support (Emscripten FS/MEMFS)
 	append_linker_flags("-sFILESYSTEM=0")
 endif()

test/other/metadce/test_metadce_hello_O0.gzsize

@@ -1 +1 @@
-8422
+8420

test/other/metadce/test_metadce_minimal_O0.gzsize

@@ -1 +1 @@
-7251
+7250

test/other/test_unoptimized_code_size_strict.js.size

@@ -1 +1 @@
-57121
+57107

test/test_other.py

@@ -10596,7 +10596,6 @@ def test_minimal_runtime_code_size(self, test_name, js, compare_js_output=False)
                                '-sTEXTDECODER=2',
                                '-sABORTING_MALLOC=0',
                                '-sALLOW_MEMORY_GROWTH=0',
-                               '-sSUPPORT_ERRNO=0',
                                '-sDECLARE_ASM_MODULE_EXPORTS',
                                '-sMALLOC=emmalloc',
                                '-sGL_EMULATE_GLES_VERSION_STRING_FORMAT=0',
@@ -11921,7 +11920,7 @@ def test_default_to_cxx(self):
     'minimal': (['-sMINIMAL_RUNTIME', '-sSUPPORT_ERRNO'],),
   })
   def test_support_errno(self, args):
-    self.emcc_args += args + ['-sEXPORTED_FUNCTIONS=_main,___errno_location']
+    self.emcc_args += args + ['-sEXPORTED_FUNCTIONS=_main,___errno_location', '-Wno-deprecated']
 
     self.do_other_test('test_support_errno.c')
     size_default = os.path.getsize('test_support_errno.js')

tools/link.py

@@ -729,6 +729,9 @@ def phase_linker_setup(options, state, newargs):
 
   final_suffix = get_file_suffix(target)
 
+  if 'SUPPORT_ERRNO' in user_settings:
+    diagnostics.warning('deprecated', 'SUPPORT_ERRNO is deprecated since emscripten no longer uses the setErrNo library function')
+
   if settings.EXTRA_EXPORTED_RUNTIME_METHODS:
     diagnostics.warning('deprecated', 'EXTRA_EXPORTED_RUNTIME_METHODS is deprecated, please use EXPORTED_RUNTIME_METHODS instead')
     settings.EXPORTED_RUNTIME_METHODS += settings.EXTRA_EXPORTED_RUNTIME_METHODS
@@ -879,7 +882,6 @@ def phase_linker_setup(options, state, newargs):
   # It is unlikely that developers targeting "native web" APIs with MINIMAL_RUNTIME need
   # errno support by default.
   if settings.MINIMAL_RUNTIME:
-    default_setting('SUPPORT_ERRNO', 0)
     # Require explicit -lfoo.js flags to link with JS libraries.
     default_setting('AUTO_JS_LIBRARIES', 0)
     # When using MINIMAL_RUNTIME, symbols should only be exported if requested.

tools/maint/update_settings_docs.py

@@ -50,6 +50,7 @@
   'compile+link': 'Applicable during both linking and compilation',
   'compile': 'Only applicable during compilation',
   'experimental': 'This is an experimental setting',
+  'deprecated': 'This setting is deprecated',
 }
 
 output_file = path_from_root('site/source/docs/tools_reference/settings_reference.rst')

tools/system_libs.py

@@ -1267,6 +1267,7 @@ def get_files(self):
           'sigtimedwait.c',
           'wasi-helpers.c',
           'sbrk.c',
+          'system.c',
         ])
 
     if settings.RELOCATABLE:
@@ -1666,7 +1667,6 @@ def __init__(self, **kwargs):
     if self.malloc not in ('dlmalloc', 'emmalloc', 'emmalloc-debug', 'emmalloc-memvalidate', 'emmalloc-verbose', 'emmalloc-memvalidate-verbose', 'mimalloc', 'none'):
       raise Exception('malloc must be one of "emmalloc[-debug|-memvalidate][-verbose]", "dlmalloc" or "none", see settings.js')
 
-    self.use_errno = kwargs.pop('use_errno')
     self.is_tracing = kwargs.pop('is_tracing')
     self.memvalidate = kwargs.pop('memvalidate')
     self.verbose = kwargs.pop('verbose')
@@ -1691,8 +1691,6 @@ def get_cflags(self):
       cflags += ['-UNDEBUG', '-DDLMALLOC_DEBUG']
     else:
       cflags += ['-DNDEBUG']
-    if not self.use_errno:
-      cflags += ['-DMALLOC_FAILURE_ACTION=', '-DEMSCRIPTEN_NO_ERRNO']
     if self.is_tracing:
       cflags += ['--tracing']
     return cflags
@@ -1704,9 +1702,6 @@ def get_base_name(self):
     name = super().get_base_name()
     if self.is_debug and not self.memvalidate and not self.verbose:
       name += '-debug'
-    if not self.use_errno:
-      # emmalloc doesn't actually use errno, but it's easier to build it again
-      name += '-noerrno'
     if self.is_tracing:
       name += '-tracing'
     return name
@@ -1716,14 +1711,13 @@ def can_use(self):
 
   @classmethod
   def vary_on(cls):
-    return super().vary_on() + ['is_debug', 'use_errno', 'is_tracing', 'memvalidate', 'verbose']
+    return super().vary_on() + ['is_debug', 'is_tracing', 'memvalidate', 'verbose']
 
   @classmethod
   def get_default_variation(cls, **kwargs):
     return super().get_default_variation(
       malloc=settings.MALLOC,
       is_debug=settings.ASSERTIONS >= 2,
-      use_errno=settings.SUPPORT_ERRNO,
       is_tracing=settings.EMSCRIPTEN_TRACING,
       memvalidate='memvalidate' in settings.MALLOC,
       verbose='verbose' in settings.MALLOC,