Skip to content

Commit a741786

Browse files
authored
Detect direct/internal usage of all missing library symbols (#17418)
Previously this mechanism only worked for missing function symbols and would only fire once the function was called. The new mechanism will catch any access to missing functions, even if they never called, and will also catch access to missing number symbols. I've also optimized the size of the `missingLibrarySymbol` block in the generate code: Old: missingLibrarySymbol('foo'); missingLibrarySymbol('bar'); ... New: var missingLibrarySymbols = ['foo', 'bar' ...]; missingLibrarySymbols.forEach(missingLibrarySymbol);
1 parent 36899b5 commit a741786

12 files changed

+108
-67
lines changed

src/library.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3680,6 +3680,8 @@ DEFAULT_LIBRARY_FUNCS_TO_INCLUDE.push(
36803680
'$addFunction',
36813681
'$removeFunction',
36823682
'$allocate',
3683+
'$ALLOC_NORMAL',
3684+
'$ALLOC_STACK',
36833685
'$AsciiToString',
36843686
'$stringToAscii',
36853687
'$UTF16ToString',

src/modules.js

Lines changed: 30 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -266,23 +266,6 @@ function isFSPrefixed(name) {
266266
return name.length > 3 && name[0] === 'F' && name[1] === 'S' && name[2] === '_';
267267
}
268268

269-
// forcing the filesystem exports a few things by default
270-
function isExportedByForceFilesystem(name) {
271-
if (!WASMFS) {
272-
// The old FS has some functionality that WasmFS lacks.
273-
if (name === 'FS_createLazyFile' ||
274-
name === 'FS_createDevice') {
275-
return true;
276-
}
277-
}
278-
return name === 'FS_createPath' ||
279-
name === 'FS_createDataFile' ||
280-
name === 'FS_createPreloadedFile' ||
281-
name === 'FS_unlink' ||
282-
name === 'addRunDependency' ||
283-
name === 'removeRunDependency';
284-
}
285-
286269
function isInternalSymbol(ident) {
287270
return ident + '__internal' in LibraryManager.library;
288271
}
@@ -297,16 +280,25 @@ function addMissingLibraryStubs() {
297280
if (!ASSERTIONS) return '';
298281
let rtn = '';
299282
const librarySymbolSet = new Set(librarySymbols);
283+
const missingSyms = [];
300284
for (const ident in LibraryManager.library) {
301-
if (typeof LibraryManager.library[ident] === 'function') {
285+
if (typeof LibraryManager.library[ident] === 'function' || typeof LibraryManager.library[ident] === 'number') {
302286
if (ident[0] === '$' && !isJsLibraryConfigIdentifier(ident) && !isInternalSymbol(ident)) {
303287
const name = ident.substr(1);
304288
if (!librarySymbolSet.has(name)) {
305-
rtn += `var ${name} = missingLibraryFunc('${name}');\n`;
289+
missingSyms.push(name);
306290
}
307291
}
308292
}
309293
}
294+
if (missingSyms.length) {
295+
rtn += 'var missingLibrarySymbols = [\n';
296+
for (const sym of missingSyms) {
297+
rtn += ` '${sym}',\n`;
298+
}
299+
rtn += '];\n';
300+
rtn += 'missingLibrarySymbols.forEach(missingLibrarySymbol)\n';
301+
}
310302
return rtn;
311303
}
312304

@@ -333,14 +325,6 @@ function exportRuntime() {
333325
}
334326
return `Module["${name}"] = ${exported};`;
335327
}
336-
// do not export it. but if ASSERTIONS, emit a
337-
// stub with an error, so the user gets a message
338-
// if it is used, that they should export it
339-
if (ASSERTIONS) {
340-
// check if it already exists, to support EXPORT_ALL and other cases
341-
const fssymbol = isExportedByForceFilesystem(name);
342-
return `unexportedRuntimeSymbol('${name}', ${fssymbol});`;
343-
}
344328
}
345329

346330
// All possible runtime elements that can be exported
@@ -439,6 +423,7 @@ function exportRuntime() {
439423
}
440424
}
441425

426+
let unexportedStubs = '';
442427
if (ASSERTIONS) {
443428
// check all exported things exist, warn about typos
444429
const runtimeElementsSet = new Set(runtimeElements);
@@ -447,8 +432,25 @@ function exportRuntime() {
447432
warn(`invalid item in EXPORTED_RUNTIME_METHODS: ${name}`);
448433
}
449434
}
435+
436+
const unexported = [];
437+
for (const name of runtimeElements) {
438+
if (!EXPORTED_RUNTIME_METHODS_SET.has(name)) {
439+
unexported.push(name);
440+
}
441+
}
442+
443+
if (unexported.length) {
444+
unexportedStubs += 'var unexportedRuntimeSymbols = [\n';
445+
for (const sym of unexported) {
446+
unexportedStubs += ` '${sym}',\n`;
447+
}
448+
unexportedStubs += '];\n';
449+
unexportedStubs += 'unexportedRuntimeSymbols.forEach(unexportedRuntimeSymbol);\n';
450+
}
450451
}
452+
451453
let exports = runtimeElements.map((name) => maybeExport(name));
452454
exports = exports.filter((name) => name);
453-
return exports.join('\n') + '\n' + addMissingLibraryStubs();
455+
return exports.join('\n') + '\n' + unexportedStubs + addMissingLibraryStubs();
454456
}

src/runtime_debug.js

Lines changed: 35 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -22,24 +22,49 @@ function ignoredModuleProp(prop) {
2222
}
2323
}
2424

25-
function unexportedMessage(sym, isFSSybol) {
26-
var msg = "'" + sym + "' was not exported. add it to EXPORTED_RUNTIME_METHODS (see the FAQ)";
27-
if (isFSSybol) {
28-
msg += '. Alternatively, forcing filesystem support (-sFORCE_FILESYSTEM) can export this for you';
29-
}
30-
return msg;
25+
// forcing the filesystem exports a few things by default
26+
function isExportedByForceFilesystem(name) {
27+
return name === 'FS_createPath' ||
28+
name === 'FS_createDataFile' ||
29+
name === 'FS_createPreloadedFile' ||
30+
name === 'FS_unlink' ||
31+
name === 'addRunDependency' ||
32+
#if !WASMFS
33+
// The old FS has some functionality that WasmFS lacks.
34+
name === 'FS_createLazyFile' ||
35+
name === 'FS_createDevice' ||
36+
#endif
37+
name === 'removeRunDependency';
3138
}
3239

33-
function missingLibraryFunc(sym) {
34-
return () => abort('Call to `' + sym + '` which is a library function and not included by default; add it to your library.js __deps or to DEFAULT_LIBRARY_FUNCS_TO_INCLUDE on the command line');
40+
function missingLibrarySymbol(sym) {
41+
if (typeof globalThis !== 'undefined' && !Object.getOwnPropertyDescriptor(globalThis, sym)) {
42+
Object.defineProperty(globalThis, sym, {
43+
configurable: true,
44+
get: function() {
45+
// Can't `abort()` here because it would break code that does runtime
46+
// checks. e.g. `if (typeof SDL === 'undefined')`.
47+
var msg = '`' + sym + '` is a library symbol and not included by default; add it to your library.js __deps or to DEFAULT_LIBRARY_FUNCS_TO_INCLUDE on the command line';
48+
if (isExportedByForceFilesystem(sym)) {
49+
msg += '. Alternatively, forcing filesystem support (-sFORCE_FILESYSTEM) can export this for you';
50+
}
51+
warnOnce(msg);
52+
return undefined;
53+
}
54+
});
55+
}
3556
}
3657

37-
function unexportedRuntimeSymbol(sym, isFSSybol) {
58+
function unexportedRuntimeSymbol(sym) {
3859
if (!Object.getOwnPropertyDescriptor(Module, sym)) {
3960
Object.defineProperty(Module, sym, {
4061
configurable: true,
4162
get: function() {
42-
abort(unexportedMessage(sym, isFSSybol));
63+
var msg = "'" + sym + "' was not exported. add it to EXPORTED_RUNTIME_METHODS (see the FAQ)";
64+
if (isExportedByForceFilesystem(sym)) {
65+
msg += '. Alternatively, forcing filesystem support (-sFORCE_FILESYSTEM) can export this for you';
66+
}
67+
abort(msg);
4368
}
4469
});
4570
}

tests/core/legacy_exported_runtime_numbers.cpp

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,19 @@
33
// University of Illinois/NCSA Open Source License. Both these licenses can be
44
// found in the LICENSE file.
55

6-
#include<emscripten.h>
6+
#include <emscripten.h>
7+
#include <stdio.h>
78

89
int main() {
10+
int rtn = EM_ASM_INT({
911
#ifdef DIRECT
10-
EM_ASM({
11-
out('|' + ALLOC_STACK + '|');
12-
});
12+
out('|' + ALLOC_STACK + '|');
13+
return ALLOC_STACK;
1314
#else
14-
EM_ASM({
1515
out('|' + Module['ALLOC_STACK'] + '|');
16-
});
16+
return Module['ALLOC_STACK'];
1717
#endif
18+
});
19+
printf("|%d|\n", rtn);
20+
return 0;
1821
}
19-

tests/core/legacy_exported_runtime_numbers.out

Lines changed: 0 additions & 1 deletion
This file was deleted.
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
26679
1+
27500
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
21458
1+
22356
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
95154
1+
84065
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
56605
1+
56607
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
76442
1+
66130

0 commit comments

Comments
 (0)