Default to EXIT_RUNTIME=1 when under ASan

We were already setting `EXIT_RUNTIME=1` under LSan because without this
leaks are not reported.  Since ASan includes LSan by default it makes
sense to do that same for ASan.

Because this change also effectivly enables leak detection in the `asan`
test suite it also includes a bunch of fixes for memory leaks.

The code in `emcc.py` that has been moved depend on `EXIT_RUNTIME` and
so must be run only after this setting has been finalized.

As part of this change I found a couple of issues related to
`EXIT_RUNTIME` when combined with other flags:

#15080
#15081

Regarding memory that leaked from JS and/or system libraries there in
some cases we want to ignore the leaks rather then adding code to free
the memory and in this case I've used `emscripten_builtin_malloc`.

Author: sbc100

emcc.py

@@ -1926,11 +1926,6 @@ def default_setting(name, new_default):
       'removeRunDependency',
     ]
 
-  if not settings.MINIMAL_RUNTIME or settings.EXIT_RUNTIME:
-    # MINIMAL_RUNTIME only needs callRuntimeCallbacks in certain cases, but the normal runtime
-    # always does.
-    settings.DEFAULT_LIBRARY_FUNCS_TO_INCLUDE += ['$callRuntimeCallbacks']
-
   if settings.USE_PTHREADS:
     # memalign is used to ensure allocated thread stacks are aligned.
     settings.EXPORTED_FUNCTIONS += ['_memalign']
@@ -2041,9 +2036,6 @@ def check_memory_setting(setting):
     if options.shell_path == utils.path_from_root('src/shell.html'):
       options.shell_path = utils.path_from_root('src/shell_minimal_runtime.html')
 
-    if settings.EXIT_RUNTIME:
-      settings.DEFAULT_LIBRARY_FUNCS_TO_INCLUDE += ['proc_exit']
-
     if settings.ASSERTIONS:
       # In ASSERTIONS-builds, functions UTF8ArrayToString() and stringToUTF8Array() (which are not JS library functions), both
       # use warnOnce(), which in MINIMAL_RUNTIME is a JS library function, so explicitly have to mark dependency to warnOnce()
@@ -2131,13 +2123,14 @@ def check_memory_setting(setting):
 
   if 'leak' in sanitize:
     settings.USE_LSAN = 1
-    settings.EXIT_RUNTIME = 1
+    default_setting('EXIT_RUNTIME', 1)
 
     if settings.LINKABLE:
       exit_with_error('LSan does not support dynamic linking')
 
   if 'address' in sanitize:
     settings.USE_ASAN = 1
+    default_setting('EXIT_RUNTIME', 1)
     if not settings.UBSAN_RUNTIME:
       settings.UBSAN_RUNTIME = 2
 
@@ -2217,6 +2210,14 @@ def check_memory_setting(setting):
     # enables the --post-emscripten pass
     settings.GLOBAL_BASE = 1024
 
+  if settings.MINIMAL_RUNTIME:
+    if settings.EXIT_RUNTIME:
+      settings.DEFAULT_LIBRARY_FUNCS_TO_INCLUDE += ['proc_exit', '$callRuntimeCallbacks']
+  else:
+    # MINIMAL_RUNTIME only needs callRuntimeCallbacks in certain cases, but the normal runtime
+    # always does.
+    settings.DEFAULT_LIBRARY_FUNCS_TO_INCLUDE += ['$callRuntimeCallbacks']
+
   # various settings require malloc/free support from JS
   if settings.RELOCATABLE or \
      settings.BUILD_AS_WORKER or \

emscripten.py

@@ -156,7 +156,7 @@ def apply_static_code_hooks(forwarded_json, code):
   code = shared.do_replace(code, '<<< ATINITS >>>', str(forwarded_json['ATINITS']))
   if settings.HAS_MAIN:
     code = shared.do_replace(code, '<<< ATMAINS >>>', str(forwarded_json['ATMAINS']))
-  if settings.EXIT_RUNTIME:
+  if settings.EXIT_RUNTIME and (not settings.MINIMAL_RUNTIME or settings.HAS_MAIN):
     code = shared.do_replace(code, '<<< ATEXITS >>>', str(forwarded_json['ATEXITS']))
   return code
 

src/library.js

@@ -441,7 +441,6 @@ LibraryManager.library = {
 #endif
   },
   __cxa_atexit: 'atexit',
-
 #endif
 
   // TODO: There are currently two abort() functions that get imported to asm module scope: the built-in runtime function abort(),
@@ -2645,6 +2644,9 @@ LibraryManager.library = {
     {{{ makeEval('return eval(UTF8ToString(ptr))|0;') }}}
   },
 
+  // We use builtin_malloc and builtin_free here because otherwise lsan will
+  // report the last returned string as a leak.
+  emscripten_run_script_string__deps: ['emscripten_builtin_malloc', 'emscripten_builtin_free'],
   emscripten_run_script_string__sig: 'ii',
   emscripten_run_script_string: function(ptr) {
     {{{ makeEval("var s = eval(UTF8ToString(ptr));") }}}
@@ -2655,9 +2657,9 @@ LibraryManager.library = {
     var me = _emscripten_run_script_string;
     var len = lengthBytesUTF8(s);
     if (!me.bufferSize || me.bufferSize < len+1) {
-      if (me.bufferSize) _free(me.buffer);
+      if (me.bufferSize) _emscripten_builtin_free(me.buffer);
       me.bufferSize = len+1;
-      me.buffer = _malloc(me.bufferSize);
+      me.buffer = _emscripten_builtin_malloc(me.bufferSize);
     }
     stringToUTF8(s, me.buffer, me.bufferSize);
     return me.buffer;
@@ -2930,6 +2932,9 @@ LibraryManager.library = {
     _emscripten_log_js(flags, str);
   },
 
+  // We never free the return values of this function so we need to allocate
+  // using builtin_malloc to avoid LSan reporting these as leaks.
+  emscripten_get_compiler_setting__deps: ['emscripten_builtin_malloc'],
   emscripten_get_compiler_setting: function(name) {
 #if RETAIN_COMPILER_SETTINGS
     name = UTF8ToString(name);
@@ -2939,10 +2944,11 @@ LibraryManager.library = {
 
     if (!_emscripten_get_compiler_setting.cache) _emscripten_get_compiler_setting.cache = {};
     var cache = _emscripten_get_compiler_setting.cache;
-    var fullname = name + '__str';
-    var fullret = cache[fullname];
+    var fullret = cache[name];
     if (fullret) return fullret;
-    return cache[fullname] = allocate(intArrayFromString(ret + ''), ALLOC_NORMAL);
+    cache[name] = _emscripten_builtin_malloc(ret.length + 1);
+    stringToUTF8(ret + '', cache[name], ret.length + 1);
+    return cache[name];
 #else
     throw 'You must build with -s RETAIN_COMPILER_SETTINGS=1 for getCompilerSetting or emscripten_get_compiler_setting to work';
 #endif

system/lib/libc/musl/src/env/__environ.c

@@ -1,16 +1,19 @@
 #include "libc.h"
 
-#ifdef __EMSCRIPTEN__
-#include <stdlib.h>
-#include <wasi/api.h>
-#endif
-
 char **__environ = 0;
 weak_alias(__environ, ___environ);
 weak_alias(__environ, _environ);
 weak_alias(__environ, environ);
 
 #ifdef __EMSCRIPTEN__
+#include <stdlib.h>
+#include <wasi/api.h>
+// Included for emscripten_builtin_free / emscripten_builtin_malloc
+// TODO(sbc): Should these be in their own header to avoid emmalloc here?
+#include <emscripten/emmalloc.h>
+
+// We use emscripten_builtin_malloc here because this memory is never freed and
+// and we don't want LSan to consider this a leak.
 __attribute__((constructor(100))) // construct this before user code
 void __emscripten_environ_constructor(void) {
     size_t environ_count;
@@ -21,11 +24,11 @@ void __emscripten_environ_constructor(void) {
         return;
     }
 
-    __environ = malloc(sizeof(char *) * (environ_count + 1));
+    __environ = emscripten_builtin_malloc(sizeof(char *) * (environ_count + 1));
     if (__environ == 0) {
         return;
     }
-    char *environ_buf = malloc(sizeof(char) * environ_buf_size);
+    char *environ_buf = emscripten_builtin_malloc(sizeof(char) * environ_buf_size);
     if (environ_buf == 0) {
         __environ = 0;
         return;

system/lib/pthread/pthread_create.c

@@ -13,6 +13,8 @@
 #include <stdbool.h>
 #include <string.h>
 #include <threads.h>
+// Included for emscripten_builtin_free / emscripten_builtin_malloc
+// TODO(sbc): Should these be in their own header to avoid emmalloc here?
 #include <emscripten/emmalloc.h>
 
 // See musl's pthread_create.c

tests/core/emscripten_get_compiler_setting.c

@@ -10,7 +10,7 @@
 #include <emscripten.h>
 
 int main() {
-  printf("EXIT_RUNTIME: %d\n", emscripten_get_compiler_setting("EXIT_RUNTIME"));
+  printf("INVOKE_RUN: %d\n", emscripten_get_compiler_setting("INVOKE_RUN"));
   assert((unsigned)emscripten_get_compiler_setting("OPT_LEVEL") <= 3);
   assert((unsigned)emscripten_get_compiler_setting("DEBUG_LEVEL") <= 4);
   printf("EMSCRIPTEN_VERSION: %s\n", (char*)emscripten_get_compiler_setting("EMSCRIPTEN_VERSION"));

tests/core/emscripten_get_compiler_setting.out

@@ -1,2 +1,2 @@
-EXIT_RUNTIME: 0
+INVOKE_RUN: 1
 EMSCRIPTEN_VERSION: waka

tests/core/test_dynamic_cast_b.cpp

@@ -34,5 +34,9 @@ int main() {
   printf("c2: %d\n", dynamic_cast<CDerived *>(pc) != NULL);
   printf("c3: %d\n", dynamic_cast<CBase *>(pc) != NULL);
 
+  delete pa;
+  delete pb;
+  delete pc;
+
   return 0;
 }

tests/core/test_em_js.cpp

@@ -5,6 +5,7 @@
 
 #include <emscripten.h>
 #include <stdio.h>
+#include <stdlib.h>
 
 EM_JS(void, noarg, (void), { out("no args works"); });
 EM_JS(int, noarg_int, (void), {
@@ -54,15 +55,15 @@ EM_JS(int, user_comma, (void), {
   return x[y][1];
 });
 
-EM_JS(const char*, return_utf8_str, (void), {
+EM_JS(char*, return_utf8_str, (void), {
     var jsString = 'こんにちは';
     var lengthBytes = lengthBytesUTF8(jsString)+1;
     var stringOnWasmHeap = _malloc(lengthBytes);
     stringToUTF8(jsString, stringOnWasmHeap, lengthBytes);
     return stringOnWasmHeap;
 });
 
-EM_JS(const char*, return_str, (void), {
+EM_JS(char*, return_str, (void), {
   var jsString = 'hello from js';
   var lengthBytes = jsString.length+1;
   var stringOnWasmHeap = _malloc(lengthBytes);
@@ -91,8 +92,12 @@ int main() {
   printf("    user_separator returned: %d\n", user_separator());
   printf("    user_comma returned: %d\n", user_comma());
 
-  printf("    return_str returned: %s\n", return_str());
-  printf("    return_utf8_str returned: %s\n", return_utf8_str());
+  char* s1 = return_str();
+  printf("    return_str returned: %s\n", s1);
+  free(s1);
+  char* s2 = return_utf8_str();
+  printf("    return_utf8_str returned: %s\n", s2);
+  free(s2);
 
   printf("    _prefixed: %d\n", _prefixed());
 

tests/core/test_embind_polymorphic_class_no_rtti.cpp

@@ -11,6 +11,7 @@
 EM_JS(void, calltest, (), {
   var foo = new Module.Foo();
   console.log("foo.test() returned: " + foo.test());
+  foo.delete();
 });
 
 class Foo {

tests/core/test_newlocale.c

@@ -7,6 +7,7 @@ void test(const char* name) {
     printf("newlocale '%s' succeeded\n", name);
   else
     printf("newlocale '%s' failed\n", name);
+  freelocale(loc);
 }
 
 int main(int argc, char* argv[]) {

tests/core/test_polymorph.cpp

@@ -44,5 +44,8 @@ int main() {
   Ls = &Other::four;
   printf("*%d*\n", (o->*(Ls))());
 
+  delete x;
+  delete y;
+  delete o;
   return 0;
 }

tests/core/test_printf_more.c

@@ -6,6 +6,8 @@
  */
 
 #include <stdio.h>
+#include <stdlib.h>
+
 int main() {
   int size = snprintf(NULL, 0, "%s %d %.2f\n", "me and myself", 25, 1.345);
   char buf[size];
@@ -14,10 +16,11 @@ int main() {
   char *buff = NULL;
   asprintf(&buff, "%d waka %d\n", 21, 95);
   puts(buff);
+  free(buff);
   // test buffering, write more than a musl buffer at once
   #define X 1026
   char c[X];
-  for(int i=0;i<X;i++) c[i] ='A';
+  for (int i=0;i<X;i++) c[i] ='A';
   c[X-1] = '\0';
   printf("%s\n", c);  /// if X > 1025 this line doesn't print if we don't handle buffering properly
   return 0;

tests/core/test_reinterpreted_ptrs.cpp

@@ -40,8 +40,9 @@ static void runTest() {
 
   printf("%s\n",
          (b == magic1 ? "magic1" : (b == magic2 ? "magic2" : "neither")));
+  delete a;
 };
 
-extern "C" {
-int main(int argc, char **argv) { runTest(); }
+int main(int argc, char **argv) {
+  runTest();
 }

tests/core/test_tinyfuncstr.cpp

@@ -13,6 +13,6 @@ struct Class {
 };
 
 int main() {
-  printf("*%s,%s*\n", Class::name1(), (new Class())->name2());
+  printf("*%s,%s*\n", Class::name1(), Class().name2());
   return 0;
 }

tests/core/test_utf.c

@@ -7,6 +7,7 @@
 
 #include <stdio.h>
 #include <emscripten.h>
+#include <stdlib.h>
 
 int main() {
   char *c = "μ†ℱ ╋ℯ╳╋ 😇";
@@ -18,5 +19,7 @@ int main() {
       "out([UTF8ToString(cheez), Module.getValue(cheez, "
       "'i8')&0xff, Module.getValue(cheez+1, 'i8')&0xff, "
       "Module.getValue(cheez+2, 'i8')&0xff, Module.getValue(cheez+3, "
-      "'i8')&0xff].join(','));");
+      "'i8')&0xff].join(','));"
+      "_free(cheez);"
+      );
 }

tests/embind/test_i64_binding.cpp

@@ -89,7 +89,8 @@ int main()
   printf("start\n");
 
   test("vector<int64_t>");
-  val::global().set("v64", val(vector<int64_t>{1, 2, 3, -4}));
+  val myval(std::vector<int64_t>{1, 2, 3, -4});
+  val::global().set("v64", myval);
   ensure_js("v64.get(0) === 1n");
   ensure_js("v64.get(1) === 2n");
   ensure_js("v64.get(2) === 3n");
@@ -106,7 +107,8 @@ int main()
   ensure_js_throws("v64.push_back(12345678901234567890123456n)", "TypeError");
 
   test("vector<uint64_t>");
-  val::global().set("vU64", val(vector<uint64_t>{1, 2, 3, 4}));
+  val myval2(vector<uint64_t>{1, 2, 3, 4});
+  val::global().set("vU64", myval2);
   ensure_js("vU64.get(0) === 1n");
   ensure_js("vU64.get(1) === 2n");
   ensure_js("vU64.get(2) === 3n");
@@ -125,6 +127,9 @@ int main()
   test("vector<uint64_t> Cannot convert bigint that is negative");
   ensure_js_throws("vU64.push_back(-1n)", "TypeError");
 
+  myval.call<void>("delete");
+  myval2.call<void>("delete");
   printf("end\n");
+
   return 0;
 }

tests/files.cpp

@@ -20,7 +20,7 @@ int main()
   rewind (file);
   printf("size: %d\n", size);
 
-  char *buffer = (char*) malloc (sizeof(char)*size);
+  char *buffer = (char*)malloc(sizeof(char)*size);
   assert(buffer);
 
   size_t read = fread(buffer, 1, size, file);
@@ -31,8 +31,8 @@ int main()
     printf(",%d", buffer[i]);
   printf("\n");
 
-  fclose (file);
-  free (buffer);
+  fclose(file);
+  free(buffer);
 
   // Do it again, with a loop on feof
 
@@ -48,7 +48,8 @@ int main()
 
   // Standard streams
 
-  printf("input:%s\n", gets((char*)malloc(1024)));
+  char gets_buffer[1024];
+  printf("input:%s\n", gets(gets_buffer));
   fwrite("texto\n", 1, 6, stdout);
   fwrite("texte\n", 1, 6, stderr);
   putchar('$');

tests/fs/test_mmap.c

@@ -285,6 +285,7 @@ int main() {
 #if !__has_feature(address_sanitizer) // the following is invalid, and asan complains rightfully
     assert(map[textsize] != 'b');
 #endif
+    munmap(map, textsize);
 
     close(fd);
   }

tests/test_browser.py

@@ -4133,7 +4133,7 @@ def test_pthread_asan_use_after_free_2(self):
     # of proxy-to-pthread, and also the allocation happens on the pthread
     # (which tests that it can use the offset converter to get the stack
     # trace there)
-    self.btest(test_file('pthread/test_pthread_asan_use_after_free_2.cpp'), expected='1', args=['-fsanitize=address', '-s', 'INITIAL_MEMORY=256MB', '-s', 'USE_PTHREADS', '-s', 'PTHREAD_POOL_SIZE=1', '--pre-js', test_file('pthread/test_pthread_asan_use_after_free_2.js')])
+    self.btest_exit(test_file('pthread/test_pthread_asan_use_after_free_2.cpp'), assert_returncode=1, args=['-fsanitize=address', '-s', 'INITIAL_MEMORY=256MB', '-s', 'USE_PTHREADS', '-s', 'PTHREAD_POOL_SIZE=1', '--pre-js', test_file('pthread/test_pthread_asan_use_after_free_2.js')])
 
   @requires_threads
   def test_pthread_exit_process(self):