Skip to content

Commit 745e1bb

Browse files
committed
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 are two appoaches we can choose to avoid them showing up as lsan failures. 1. Use `emscripten_builtin_malloc` so that allocations is not tracked by lsan. 2. Ensure memory is free'd on exit (in a couple of cases I've used `atexit` to do this. Its not clear to be exactly when one method is preferable over another. The atexit approach I think is reasonable when its used to implement a feature that already has a high overhead (such as `RETAIN_COMPILER_SETTINGS`).
1 parent 9d63033 commit 745e1bb

19 files changed

+101
-40
lines changed

emcc.py

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1926,11 +1926,6 @@ def default_setting(name, new_default):
19261926
'removeRunDependency',
19271927
]
19281928

1929-
if not settings.MINIMAL_RUNTIME or settings.EXIT_RUNTIME:
1930-
# MINIMAL_RUNTIME only needs callRuntimeCallbacks in certain cases, but the normal runtime
1931-
# always does.
1932-
settings.DEFAULT_LIBRARY_FUNCS_TO_INCLUDE += ['$callRuntimeCallbacks']
1933-
19341929
if settings.USE_PTHREADS:
19351930
# memalign is used to ensure allocated thread stacks are aligned.
19361931
settings.EXPORTED_FUNCTIONS += ['_memalign']
@@ -2041,9 +2036,6 @@ def check_memory_setting(setting):
20412036
if options.shell_path == utils.path_from_root('src/shell.html'):
20422037
options.shell_path = utils.path_from_root('src/shell_minimal_runtime.html')
20432038

2044-
if settings.EXIT_RUNTIME:
2045-
settings.DEFAULT_LIBRARY_FUNCS_TO_INCLUDE += ['proc_exit']
2046-
20472039
if settings.ASSERTIONS:
20482040
# In ASSERTIONS-builds, functions UTF8ArrayToString() and stringToUTF8Array() (which are not JS library functions), both
20492041
# 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):
21312123

21322124
if 'leak' in sanitize:
21332125
settings.USE_LSAN = 1
2134-
settings.EXIT_RUNTIME = 1
2126+
default_setting('EXIT_RUNTIME', 1)
21352127

21362128
if settings.LINKABLE:
21372129
exit_with_error('LSan does not support dynamic linking')
21382130

21392131
if 'address' in sanitize:
21402132
settings.USE_ASAN = 1
2133+
default_setting('EXIT_RUNTIME', 1)
21412134
if not settings.UBSAN_RUNTIME:
21422135
settings.UBSAN_RUNTIME = 2
21432136

@@ -2217,6 +2210,14 @@ def check_memory_setting(setting):
22172210
# enables the --post-emscripten pass
22182211
settings.GLOBAL_BASE = 1024
22192212

2213+
if settings.MINIMAL_RUNTIME:
2214+
if settings.EXIT_RUNTIME:
2215+
settings.DEFAULT_LIBRARY_FUNCS_TO_INCLUDE += ['proc_exit', '$callRuntimeCallbacks']
2216+
else:
2217+
# MINIMAL_RUNTIME only needs callRuntimeCallbacks in certain cases, but the normal runtime
2218+
# always does.
2219+
settings.DEFAULT_LIBRARY_FUNCS_TO_INCLUDE += ['$callRuntimeCallbacks']
2220+
22202221
# various settings require malloc/free support from JS
22212222
if settings.RELOCATABLE or \
22222223
settings.BUILD_AS_WORKER or \

emscripten.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ def apply_static_code_hooks(forwarded_json, code):
156156
code = shared.do_replace(code, '<<< ATINITS >>>', str(forwarded_json['ATINITS']))
157157
if settings.HAS_MAIN:
158158
code = shared.do_replace(code, '<<< ATMAINS >>>', str(forwarded_json['ATMAINS']))
159-
if settings.EXIT_RUNTIME:
159+
if settings.EXIT_RUNTIME and (not settings.MINIMAL_RUNTIME or settings.HAS_MAIN):
160160
code = shared.do_replace(code, '<<< ATEXITS >>>', str(forwarded_json['ATEXITS']))
161161
return code
162162

src/library.js

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -441,7 +441,6 @@ LibraryManager.library = {
441441
#endif
442442
},
443443
__cxa_atexit: 'atexit',
444-
445444
#endif
446445

447446
// TODO: There are currently two abort() functions that get imported to asm module scope: the built-in runtime function abort(),
@@ -2646,6 +2645,7 @@ LibraryManager.library = {
26462645
},
26472646

26482647
emscripten_run_script_string__sig: 'ii',
2648+
emscripten_run_script_string__deps: ['malloc', 'free'],
26492649
emscripten_run_script_string: function(ptr) {
26502650
{{{ makeEval("var s = eval(UTF8ToString(ptr));") }}}
26512651
if (s == null) {
@@ -2655,6 +2655,9 @@ LibraryManager.library = {
26552655
var me = _emscripten_run_script_string;
26562656
var len = lengthBytesUTF8(s);
26572657
if (!me.bufferSize || me.bufferSize < len+1) {
2658+
if (!me.bufferSize) {
2659+
_atexit(function() { _free(me.buffer); });
2660+
}
26582661
if (me.bufferSize) _free(me.buffer);
26592662
me.bufferSize = len+1;
26602663
me.buffer = _malloc(me.bufferSize);
@@ -2930,19 +2933,26 @@ LibraryManager.library = {
29302933
_emscripten_log_js(flags, str);
29312934
},
29322935

2936+
emscripten_get_compiler_setting__deps: ['atexit', 'free'],
29332937
emscripten_get_compiler_setting: function(name) {
29342938
#if RETAIN_COMPILER_SETTINGS
29352939
name = UTF8ToString(name);
29362940

29372941
var ret = getCompilerSetting(name);
29382942
if (typeof ret === 'number') return ret;
29392943

2940-
if (!_emscripten_get_compiler_setting.cache) _emscripten_get_compiler_setting.cache = {};
2944+
if (!_emscripten_get_compiler_setting.cache) {
2945+
_emscripten_get_compiler_setting.cache = {};
2946+
_atexit(function() {
2947+
for (var key in _emscripten_get_compiler_setting.cache) {
2948+
_free(_emscripten_get_compiler_setting.cache[key]);
2949+
}
2950+
});
2951+
}
29412952
var cache = _emscripten_get_compiler_setting.cache;
2942-
var fullname = name + '__str';
2943-
var fullret = cache[fullname];
2953+
var fullret = cache[name];
29442954
if (fullret) return fullret;
2945-
return cache[fullname] = allocate(intArrayFromString(ret + ''), ALLOC_NORMAL);
2955+
return cache[name] = allocate(intArrayFromString(ret + ''), ALLOC_NORMAL);
29462956
#else
29472957
throw 'You must build with -s RETAIN_COMPILER_SETTINGS=1 for getCompilerSetting or emscripten_get_compiler_setting to work';
29482958
#endif

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
#ifdef __EMSCRIPTEN__
44
#include <stdlib.h>
55
#include <wasi/api.h>
6+
#include <emscripten/emmalloc.h>
67
#endif
78

89
char **__environ = 0;
@@ -21,11 +22,11 @@ void __emscripten_environ_constructor(void) {
2122
return;
2223
}
2324

24-
__environ = malloc(sizeof(char *) * (environ_count + 1));
25+
__environ = emscripten_builtin_malloc(sizeof(char *) * (environ_count + 1));
2526
if (__environ == 0) {
2627
return;
2728
}
28-
char *environ_buf = malloc(sizeof(char) * environ_buf_size);
29+
char *environ_buf = emscripten_builtin_malloc(sizeof(char) * environ_buf_size);
2930
if (environ_buf == 0) {
3031
__environ = 0;
3132
return;

tests/core/emscripten_get_compiler_setting.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
#include <emscripten.h>
1111

1212
int main() {
13-
printf("EXIT_RUNTIME: %d\n", emscripten_get_compiler_setting("EXIT_RUNTIME"));
13+
printf("INVOKE_RUN: %d\n", emscripten_get_compiler_setting("INVOKE_RUN"));
1414
assert((unsigned)emscripten_get_compiler_setting("OPT_LEVEL") <= 3);
1515
assert((unsigned)emscripten_get_compiler_setting("DEBUG_LEVEL") <= 4);
1616
printf("EMSCRIPTEN_VERSION: %s\n", (char*)emscripten_get_compiler_setting("EMSCRIPTEN_VERSION"));
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
1-
EXIT_RUNTIME: 0
1+
INVOKE_RUN: 1
22
EMSCRIPTEN_VERSION: waka

tests/core/test_dynamic_cast_b.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,5 +34,9 @@ int main() {
3434
printf("c2: %d\n", dynamic_cast<CDerived *>(pc) != NULL);
3535
printf("c3: %d\n", dynamic_cast<CBase *>(pc) != NULL);
3636

37+
delete pa;
38+
delete pb;
39+
delete pc;
40+
3741
return 0;
3842
}

tests/core/test_em_js.cpp

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
#include <emscripten.h>
77
#include <stdio.h>
8+
#include <stdlib.h>
89

910
EM_JS(void, noarg, (void), { out("no args works"); });
1011
EM_JS(int, noarg_int, (void), {
@@ -54,15 +55,15 @@ EM_JS(int, user_comma, (void), {
5455
return x[y][1];
5556
});
5657

57-
EM_JS(const char*, return_utf8_str, (void), {
58+
EM_JS(char*, return_utf8_str, (void), {
5859
var jsString = 'こんにちは';
5960
var lengthBytes = lengthBytesUTF8(jsString)+1;
6061
var stringOnWasmHeap = _malloc(lengthBytes);
6162
stringToUTF8(jsString, stringOnWasmHeap, lengthBytes);
6263
return stringOnWasmHeap;
6364
});
6465

65-
EM_JS(const char*, return_str, (void), {
66+
EM_JS(char*, return_str, (void), {
6667
var jsString = 'hello from js';
6768
var lengthBytes = jsString.length+1;
6869
var stringOnWasmHeap = _malloc(lengthBytes);
@@ -91,8 +92,12 @@ int main() {
9192
printf(" user_separator returned: %d\n", user_separator());
9293
printf(" user_comma returned: %d\n", user_comma());
9394

94-
printf(" return_str returned: %s\n", return_str());
95-
printf(" return_utf8_str returned: %s\n", return_utf8_str());
95+
char* s1 = return_str();
96+
printf(" return_str returned: %s\n", s1);
97+
free(s1);
98+
char* s2 = return_utf8_str();
99+
printf(" return_utf8_str returned: %s\n", s2);
100+
free(s2);
96101

97102
printf(" _prefixed: %d\n", _prefixed());
98103

tests/core/test_embind_polymorphic_class_no_rtti.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
EM_JS(void, calltest, (), {
1212
var foo = new Module.Foo();
1313
console.log("foo.test() returned: " + foo.test());
14+
foo.delete();
1415
});
1516

1617
class Foo {

tests/core/test_newlocale.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ void test(const char* name) {
77
printf("newlocale '%s' succeeded\n", name);
88
else
99
printf("newlocale '%s' failed\n", name);
10+
freelocale(loc);
1011
}
1112

1213
int main(int argc, char* argv[]) {

tests/core/test_polymorph.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,5 +44,8 @@ int main() {
4444
Ls = &Other::four;
4545
printf("*%d*\n", (o->*(Ls))());
4646

47+
delete x;
48+
delete y;
49+
delete o;
4750
return 0;
4851
}

tests/core/test_printf_more.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
*/
77

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

tests/core/test_reinterpreted_ptrs.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,9 @@ static void runTest() {
4040

4141
printf("%s\n",
4242
(b == magic1 ? "magic1" : (b == magic2 ? "magic2" : "neither")));
43+
delete a;
4344
};
4445

45-
extern "C" {
46-
int main(int argc, char **argv) { runTest(); }
46+
int main(int argc, char **argv) {
47+
runTest();
4748
}

tests/core/test_tinyfuncstr.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,6 @@ struct Class {
1313
};
1414

1515
int main() {
16-
printf("*%s,%s*\n", Class::name1(), (new Class())->name2());
16+
printf("*%s,%s*\n", Class::name1(), Class().name2());
1717
return 0;
1818
}

tests/core/test_utf.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
#include <stdio.h>
99
#include <emscripten.h>
10+
#include <stdlib.h>
1011

1112
int main() {
1213
char *c = "μ†ℱ ╋ℯ╳╋ 😇";
@@ -18,5 +19,7 @@ int main() {
1819
"out([UTF8ToString(cheez), Module.getValue(cheez, "
1920
"'i8')&0xff, Module.getValue(cheez+1, 'i8')&0xff, "
2021
"Module.getValue(cheez+2, 'i8')&0xff, Module.getValue(cheez+3, "
21-
"'i8')&0xff].join(','));");
22+
"'i8')&0xff].join(','));"
23+
"_free(cheez);"
24+
);
2225
}

tests/embind/test_i64_binding.cpp

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,8 @@ int main()
8989
printf("start\n");
9090

9191
test("vector<int64_t>");
92-
val::global().set("v64", val(vector<int64_t>{1, 2, 3, -4}));
92+
val myval(std::vector<int64_t>{1, 2, 3, -4});
93+
val::global().set("v64", myval);
9394
ensure_js("v64.get(0) === 1n");
9495
ensure_js("v64.get(1) === 2n");
9596
ensure_js("v64.get(2) === 3n");
@@ -106,7 +107,8 @@ int main()
106107
ensure_js_throws("v64.push_back(12345678901234567890123456n)", "TypeError");
107108

108109
test("vector<uint64_t>");
109-
val::global().set("vU64", val(vector<uint64_t>{1, 2, 3, 4}));
110+
val myval2(vector<uint64_t>{1, 2, 3, 4});
111+
val::global().set("vU64", myval2);
110112
ensure_js("vU64.get(0) === 1n");
111113
ensure_js("vU64.get(1) === 2n");
112114
ensure_js("vU64.get(2) === 3n");
@@ -125,6 +127,9 @@ int main()
125127
test("vector<uint64_t> Cannot convert bigint that is negative");
126128
ensure_js_throws("vU64.push_back(-1n)", "TypeError");
127129

130+
myval.call<void>("delete");
131+
myval2.call<void>("delete");
128132
printf("end\n");
133+
129134
return 0;
130135
}

tests/files.cpp

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ int main()
2020
rewind (file);
2121
printf("size: %d\n", size);
2222

23-
char *buffer = (char*) malloc (sizeof(char)*size);
23+
char *buffer = (char*)malloc(sizeof(char)*size);
2424
assert(buffer);
2525

2626
size_t read = fread(buffer, 1, size, file);
@@ -31,8 +31,8 @@ int main()
3131
printf(",%d", buffer[i]);
3232
printf("\n");
3333

34-
fclose (file);
35-
free (buffer);
34+
fclose(file);
35+
free(buffer);
3636

3737
// Do it again, with a loop on feof
3838

@@ -48,7 +48,8 @@ int main()
4848

4949
// Standard streams
5050

51-
printf("input:%s\n", gets((char*)malloc(1024)));
51+
char gets_buffer[1024];
52+
printf("input:%s\n", gets(gets_buffer));
5253
fwrite("texto\n", 1, 6, stdout);
5354
fwrite("texte\n", 1, 6, stderr);
5455
putchar('$');

tests/fs/test_mmap.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -285,6 +285,7 @@ int main() {
285285
#if !__has_feature(address_sanitizer) // the following is invalid, and asan complains rightfully
286286
assert(map[textsize] != 'b');
287287
#endif
288+
munmap(map, textsize);
288289

289290
close(fd);
290291
}

0 commit comments

Comments
 (0)