Skip to content

Commit d21ef58

Browse files
authored
[js-compiler] Remove special handling for JS library aliases. NFC (#25441)
This change remove some special casing in `addFromLibrary` which simplifies the code, and also lays the groundwork for #25441 which explicitly allows JS symbol to alias native ones. This special handling was added in #19046 in order to deal with case where `glXXX` symbols were simultaneously aliased by `emscripten_glXX` and exported (in the case of MAIN_MODULE=1). However, this work is no longer needed since in #21785 we stopped stopped included symbol exported due to `MAIN_MODULE=1` as part of `WASM_EXPORTS` (the symbols are only exported to side modules, not to JS). In original problem was that `emscripten_glXX` should always points to the original JS implementation, even if a native version is exported. Just in case somebody is explicitly exporting these symbols this change also flips the aliases such that the `glXX` functions are now aliases of `emscripten_glXX` and not the other way around. This means that exporting `glXXX` will replace/override the alias, but will not effect `emscripten_glXX`.
1 parent bd4676a commit d21ef58

File tree

5 files changed

+48
-31
lines changed

5 files changed

+48
-31
lines changed

src/jsifier.mjs

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -505,7 +505,7 @@ function(${args}) {
505505
// what we just added to the library.
506506
}
507507

508-
function addFromLibrary(symbol, dependent, force = false) {
508+
function addFromLibrary(symbol, dependent) {
509509
// don't process any special identifiers. These are looked up when
510510
// processing the base name of the identifier.
511511
if (isDecorator(symbol)) {
@@ -514,7 +514,7 @@ function(${args}) {
514514

515515
// if the function was implemented in compiled code, there is no need to
516516
// include the js version
517-
if (WASM_EXPORTS.has(symbol) && !force) {
517+
if (WASM_EXPORTS.has(symbol)) {
518518
return;
519519
}
520520

@@ -639,7 +639,6 @@ function(${args}) {
639639
});
640640

641641
let isFunction = false;
642-
let aliasTarget;
643642

644643
const postsetId = symbol + '__postset';
645644
const postset = LibraryManager.library[postsetId];
@@ -659,7 +658,7 @@ function(${args}) {
659658
// Redirection for aliases. We include the parent, and at runtime
660659
// make ourselves equal to it. This avoid having duplicate
661660
// functions with identical content.
662-
aliasTarget = snippet;
661+
const aliasTarget = snippet;
663662
snippet = mangleCSymbolName(aliasTarget);
664663
deps.push(aliasTarget);
665664
}
@@ -690,7 +689,7 @@ function(${args}) {
690689
'noExitRuntime cannot be referenced via __deps mechanism. Use DEFAULT_LIBRARY_FUNCS_TO_INCLUDE or EXPORTED_RUNTIME_METHODS',
691690
);
692691
}
693-
return addFromLibrary(dep, `${symbol}, referenced by ${dependent}`, dep === aliasTarget);
692+
return addFromLibrary(dep, `${symbol}, referenced by ${dependent}`);
694693
}
695694
let contentText;
696695
if (isFunction) {
@@ -777,10 +776,6 @@ function(${args}) {
777776
}
778777

779778
let commentText = '';
780-
if (force) {
781-
commentText += '/** @suppress {duplicate } */\n';
782-
}
783-
784779
let docs = LibraryManager.library[symbol + '__docs'];
785780
// Add the docs if they exist and if we are actually emitting a declaration.
786781
// See the TODO about wasmTable above.

src/lib/libwebgl.js

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4346,17 +4346,37 @@ createGLPassthroughFunctions(LibraryGL, glPassthroughFuncs);
43464346

43474347
autoAddDeps(LibraryGL, '$GL');
43484348

4349+
function renameSymbol(lib, oldName, newName) {
4350+
lib[newName] = lib[oldName];
4351+
delete lib[oldName];
4352+
for (const suffix of decoratorSuffixes) {
4353+
const oldDecorator = oldName + suffix;
4354+
if (lib.hasOwnProperty(oldDecorator)) {
4355+
const newDecorator = newName + suffix;
4356+
lib[newDecorator] = lib[oldDecorator];
4357+
delete lib[oldDecorator];
4358+
}
4359+
}
4360+
}
4361+
43494362
function recordGLProcAddressGet(lib) {
43504363
// GL proc address retrieval - allow access through glX and emscripten_glX, to
43514364
// allow name collisions with user-implemented things having the same name
43524365
// (see gl.c)
4366+
//
4367+
// We do this by renaming `glX` symbols to `emscripten_glX` and then setting
4368+
// `glX` as an alias of `emscripten_glX`. The reason for this renaming is to
4369+
// ensure that `emscripten_glX` is always available, even in cases where native
4370+
// code defines `glX`.
4371+
const glSyms = [];
43534372
for (const sym of Object.keys(lib)) {
43544373
if (sym.startsWith('gl') && !isDecorator(sym)) {
4355-
const alias = 'emscripten_' + sym;
4356-
lib[alias] = sym;
4374+
const newSym = 'emscripten_' + sym;
4375+
renameSymbol(lib, sym, newSym);
4376+
lib[sym] = newSym;
43574377
var sig = LibraryManager.library[sym + '__sig'];
43584378
if (sig) {
4359-
lib[alias + '__sig'] = sig;
4379+
lib[newSym + '__sig'] = sig;
43604380
}
43614381
}
43624382
}

src/utility.mjs

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ export function mergeInto(obj, other, options = null) {
157157
const deps = other[key];
158158
if (!Array.isArray(deps)) {
159159
error(
160-
`JS library directive ${key}=${deps.toString()} is of type '${type}', but it should be an array`,
160+
`JS library directive ${key}=${deps} is of type '${type}', but it should be an array`,
161161
);
162162
}
163163
for (let dep of deps) {
@@ -199,22 +199,23 @@ export function isJsOnlySymbol(symbol) {
199199
return symbol[0] == '$';
200200
}
201201

202+
export const decoratorSuffixes = [
203+
'__sig',
204+
'__proxy',
205+
'__asm',
206+
'__deps',
207+
'__postset',
208+
'__docs',
209+
'__nothrow',
210+
'__noleakcheck',
211+
'__internal',
212+
'__user',
213+
'__async',
214+
'__i53abi',
215+
];
216+
202217
export function isDecorator(ident) {
203-
const suffixes = [
204-
'__sig',
205-
'__proxy',
206-
'__asm',
207-
'__deps',
208-
'__postset',
209-
'__docs',
210-
'__nothrow',
211-
'__noleakcheck',
212-
'__internal',
213-
'__user',
214-
'__async',
215-
'__i53abi',
216-
];
217-
return suffixes.some((suffix) => ident.endsWith(suffix));
218+
return decoratorSuffixes.some((suffix) => ident.endsWith(suffix));
218219
}
219220

220221
export function readFile(filename) {
@@ -330,6 +331,7 @@ export function runInMacroContext(code, options) {
330331

331332
addToCompileTimeContext({
332333
assert,
334+
decoratorSuffixes,
333335
error,
334336
isDecorator,
335337
isJsOnlySymbol,

test/codesize/test_codesize_hello_dylink_all.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
2-
"a.out.js": 245842,
2+
"a.out.js": 245841,
33
"a.out.nodebug.wasm": 597755,
4-
"total": 843597,
4+
"total": 843596,
55
"sent": [
66
"IMG_Init",
77
"IMG_Load",

test/test_override_system_js_lib_symbol.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
if (!LibraryManager.library.glTexImage3D) throw 'This file should be getting processed after library_webgl2.js!';
22

33
addToLibrary({
4-
orig_glTexImage3D__deps: LibraryManager.library.glTexImage3D__deps,
4+
orig_glTexImage3D__deps: LibraryManager.library.glTexImage3D__deps || [],
55
orig_glTexImage3D: LibraryManager.library.glTexImage3D,
66

77
glTexImage3D__deps: ['orig_glTexImage3D'],

0 commit comments

Comments
 (0)