Skip to content

Commit ea4c450

Browse files
authored
Allow marking of library functions as __internal. (#15294)
Functions marked in this way will generate a warning if depended on my external (user) library JS files. See: #15242
1 parent 95ef06b commit ea4c450

File tree

8 files changed

+65
-3
lines changed

8 files changed

+65
-3
lines changed

ChangeLog.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,10 @@ See docs/process.md for more on how version tagging works.
2020

2121
2.0.32
2222
------
23+
- Internal-only library functions can now be marked as `__internal: true` in JS
24+
system libraries. Such symbols should not be used by external libraries and
25+
are subject to change. As of now we generate warning when external libraries
26+
depend on the these symbols.
2327
- Stub functions from `library_syscall.js` and `library.js` were replaced with
2428
native code stubs (See `system/lib/libc/emscripten_syscall_stubs.c`). This
2529
should be better for wasm module portability as well as code size. As part

src/compiler.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
// LLVM => JavaScript compiler, main entry point
99

1010
var nodeFS = require('fs');
11-
var nodePath = require('path');
11+
nodePath = require('path');
1212

1313
print = (x) => {
1414
process['stdout'].write(x + '\n');

src/jsifier.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,10 +205,14 @@ function ${name}(${args}) {
205205
error(`JS library directive ${ident}__deps=${deps.toString()} is of type ${typeof deps}, but it should be an array!`);
206206
return;
207207
}
208+
var isUserSymbol = LibraryManager.library[ident + '__user'];
208209
deps.forEach((dep) => {
209210
if (typeof snippet === 'string' && !(dep in LibraryManager.library)) {
210211
warn(`missing library dependency ${dep}, make sure you are compiling with the right options (see #if in src/library*.js)`);
211212
}
213+
if (isUserSymbol && LibraryManager.library[dep + '__internal']) {
214+
warn(`user library symbol '${ident}' depends on internal symbol '${dep}'`)
215+
}
212216
});
213217
var isFunction = false;
214218

src/library.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3395,6 +3395,7 @@ LibraryManager.library = {
33953395
#endif
33963396
},
33973397

3398+
$callRuntimeCallbacks__internal: true,
33983399
$callRuntimeCallbacks: function(callbacks) {
33993400
while (callbacks.length > 0) {
34003401
var callback = callbacks.shift();

src/library_dylink.js

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

66
var LibraryDylink = {
77
#if RELOCATABLE
8+
$resolveGlobalSymbol__internal: true,
89
$resolveGlobalSymbol__deps: ['$asmjsMangle'],
910
$resolveGlobalSymbol: function(symName, direct) {
1011
var sym;
@@ -39,6 +40,7 @@ var LibraryDylink = {
3940

4041
// Create globals to each imported symbol. These are all initialized to zero
4142
// and get assigned later in `updateGOT`
43+
$GOTHandler__internal: true,
4244
$GOTHandler__deps: ['$GOT'],
4345
$GOTHandler: {
4446
'get': function(obj, symName) {
@@ -52,6 +54,7 @@ var LibraryDylink = {
5254
}
5355
},
5456

57+
$isInternalSym__internal: true,
5558
$isInternalSym: function(symName) {
5659
// TODO: find a way to mark these in the binary or avoid exporting them.
5760
return [
@@ -72,6 +75,7 @@ var LibraryDylink = {
7275
;
7376
},
7477

78+
$updateGOT__internal: true,
7579
$updateGOT__deps: ['$GOT', '$isInternalSym'],
7680
$updateGOT: function(exports, replace) {
7781
#if DYLINK_DEBUG
@@ -122,6 +126,7 @@ var LibraryDylink = {
122126
},
123127

124128
// Applies relocations to exported things.
129+
$relocateExports__internal: true,
125130
$relocateExports__deps: ['$updateGOT'],
126131
$relocateExports: function(exports, memoryBase, replace) {
127132
var relocated = {};
@@ -149,6 +154,7 @@ var LibraryDylink = {
149154
return relocated;
150155
},
151156

157+
$reportUndefinedSymbols__internal: true,
152158
$reportUndefinedSymbols__deps: ['$GOT', '$resolveGlobalSymbol'],
153159
$reportUndefinedSymbols: function() {
154160
#if DYLINK_DEBUG
@@ -200,6 +206,7 @@ var LibraryDylink = {
200206
loadedLibsByHandle: {},
201207
},
202208

209+
$dlSetError__internal: true,
203210
$dlSetError: ['___dl_seterr',
204211
#if MINIMAL_RUNTIME
205212
'$intArrayFromString'
@@ -215,6 +222,7 @@ var LibraryDylink = {
215222
// Dynamic version of shared.py:make_invoke. This is needed for invokes
216223
// that originate from side modules since these are not known at JS
217224
// generation time.
225+
$createInvokeFunction__internal: true,
218226
$createInvokeFunction__deps: ['$dynCall', 'setThrew'],
219227
$createInvokeFunction: function(sig) {
220228
return function() {
@@ -256,6 +264,7 @@ var LibraryDylink = {
256264

257265
// returns the side module metadata as an object
258266
// { memorySize, memoryAlign, tableSize, tableAlign, neededDynlibs}
267+
$getDylinkMetadata__internal: true,
259268
$getDylinkMetadata: function(binary) {
260269
var offset = 0;
261270
var end = 0;
@@ -734,6 +743,7 @@ var LibraryDylink = {
734743
return true;
735744
},
736745

746+
$preloadDylibs__internal: true,
737747
$preloadDylibs__deps: ['$loadDynamicLibrary', '$reportUndefinedSymbols'],
738748
$preloadDylibs: function() {
739749
#if DYLINK_DEBUG

src/modules.js

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -185,11 +185,30 @@ var LibraryManager = {
185185
this.libraries = libraries;
186186

187187
for (var filename of libraries) {
188-
var src = read(filename);
188+
var isUserLibrary = nodePath.isAbsolute(filename);
189189
if (VERBOSE) {
190-
printErr('processing: ' + filename);
190+
if (isUserLibrary) {
191+
printErr('processing user library: ' + filename);
192+
} else {
193+
printErr('processing system library: ' + filename);
194+
}
191195
}
196+
var src = read(filename);
197+
var origLibrary = undefined;
192198
var processed = undefined;
199+
// When we parse user libraries also set `__user` attribute
200+
// on each element so that we can distinguish them later.
201+
if (isUserLibrary) {
202+
origLibrary = this.library
203+
this.library = new Proxy(this.library, {
204+
set: (target, prop, value) => {
205+
target[prop] = value;
206+
if (!isJsLibraryConfigIdentifier(prop)) {
207+
target[prop + '__user'] = true;
208+
}
209+
}
210+
});
211+
}
193212
try {
194213
processed = processMacros(preprocess(src, filename));
195214
eval(processed);
@@ -214,6 +233,10 @@ var LibraryManager = {
214233
}
215234
}
216235
throw e;
236+
} finally {
237+
if (origLibrary) {
238+
this.library = origLibrary;
239+
}
217240
}
218241
}
219242

src/utility.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,7 @@ function isJsLibraryConfigIdentifier(ident) {
122122
'__import',
123123
'__nothrow',
124124
'__noleakcheck',
125+
'__internal',
125126
];
126127
return suffixes.some((suffix) => ident.endsWith(suffix));
127128
}

tests/test_other.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3169,6 +3169,25 @@ def test_js_lib_using_asm_lib(self):
31693169
self.run_process([EMXX, 'src.cpp', '--js-library', 'lib.js'])
31703170
self.assertContained('c calling: 14\n', self.run_js('a.out.js'))
31713171

3172+
def test_js_internal_deps(self):
3173+
create_file('lib.js', r'''
3174+
mergeInto(LibraryManager.library, {
3175+
jslibfunc__deps: ['$callRuntimeCallbacks'],
3176+
jslibfunc: function(x) {
3177+
callRuntimeCallbacks();
3178+
},
3179+
});
3180+
''')
3181+
create_file('src.cpp', r'''
3182+
#include <stdio.h>
3183+
extern "C" int jslibfunc();
3184+
int main() {
3185+
printf("c calling: %d\n", jslibfunc());
3186+
}
3187+
''')
3188+
err = self.run_process([EMXX, 'src.cpp', '--js-library', 'lib.js'], stderr=PIPE).stderr
3189+
self.assertContained("warning: user library symbol 'jslibfunc' depends on internal symbol '$callRuntimeCallbacks'", err)
3190+
31723191
def test_EMCC_BUILD_DIR(self):
31733192
# EMCC_BUILD_DIR env var contains the dir we were building in, when running the js compiler (e.g. when
31743193
# running a js library). We force the cwd to be src/ for technical reasons, so this lets you find out

0 commit comments

Comments
 (0)