Skip to content

Commit f5c6dda

Browse files
committed
Allow JS -> native dependencies to be specified in __deps. NFC
Now that LLD_REPORT_UNDEFINED is always on, we can depend on JS library processing before link time. Extend the existing symbol list to be a list of symbols + native dependencies. This way we can begin to move reverse dependencies out of deps_info.py and potentially one day completely remove that file. For now it is still needed because indirect dependencies can't be specified in the JS library code yet. Replaces (and inspired by): #15982
1 parent ca19311 commit f5c6dda

File tree

10 files changed

+74
-79
lines changed

10 files changed

+74
-79
lines changed

emcc.py

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import json
2929
import logging
3030
import os
31+
import pprint
3132
import re
3233
import shlex
3334
import shutil
@@ -50,6 +51,7 @@
5051
from tools.minimal_runtime_shell import generate_minimal_runtime_html
5152
import tools.line_endings
5253
from tools import feature_matrix
54+
from tools import deps_info
5355
from tools import js_manipulation
5456
from tools import wasm2c
5557
from tools import webassembly
@@ -542,14 +544,28 @@ def build_symbol_list(filename):
542544
"""Only called when there is no existing symbol list for a given content hash.
543545
"""
544546
library_syms = generate_js_symbols()
545-
write_file(filename, '\n'.join(library_syms) + '\n')
547+
lines = []
548+
549+
for name, deps in library_syms.items():
550+
if deps:
551+
lines.append('%s: %s' % (name, ','.join(deps)))
552+
else:
553+
lines.append(name)
554+
write_file(filename, '\n'.join(lines) + '\n')
546555

547556
# We need to use a separate lock here for symbol lists because, unlike with system libraries,
548557
# it's normally for these file to get pruned as part of normal operation. This means that it
549558
# can be deleted between the `cache.get()` then the `read_file`.
550559
with filelock.FileLock(cache.get_path(cache.get_path('symbol_lists.lock'))):
551560
filename = cache.get(f'symbol_lists/{content_hash}.txt', build_symbol_list)
552-
library_syms = read_file(filename).splitlines()
561+
lines = read_file(filename).splitlines()
562+
library_syms = {}
563+
for line in lines:
564+
if ':' in line:
565+
name, deps = line.split(':')
566+
library_syms[name] = deps.strip().split(',')
567+
else:
568+
library_syms[line] = []
553569

554570
# Limit of the overall size of the cache to 100 files.
555571
# This code will get test coverage since a full test run of `other` or `core`
@@ -1315,9 +1331,15 @@ def run(args):
13151331
logger.debug('stopping after linking to object file')
13161332
return 0
13171333

1334+
js_syms = {}
1335+
if not settings.SIDE_MODULE:
1336+
js_syms = get_all_js_syms()
1337+
deps_info.append_deps_info(js_syms)
1338+
logger.debug("JS symbols: " + pprint.pformat(js_syms))
1339+
13181340
phase_calculate_system_libraries(state, linker_arguments, linker_inputs, newargs)
13191341

1320-
phase_link(linker_arguments, wasm_target)
1342+
phase_link(linker_arguments, wasm_target, js_syms)
13211343

13221344
# Special handling for when the user passed '-Wl,--version'. In this case the linker
13231345
# does not create the output file, but just prints its version and exits with 0.
@@ -3060,7 +3082,7 @@ def phase_calculate_system_libraries(state, linker_arguments, linker_inputs, new
30603082

30613083

30623084
@ToolchainProfiler.profile_block('link')
3063-
def phase_link(linker_arguments, wasm_target):
3085+
def phase_link(linker_arguments, wasm_target, js_syms):
30643086
logger.debug(f'linking: {linker_arguments}')
30653087

30663088
# Make a final pass over settings.EXPORTED_FUNCTIONS to remove any
@@ -3070,13 +3092,6 @@ def phase_link(linker_arguments, wasm_target):
30703092
settings.REQUIRED_EXPORTS = dedup_list(settings.REQUIRED_EXPORTS)
30713093
settings.EXPORT_IF_DEFINED = dedup_list(settings.EXPORT_IF_DEFINED)
30723094

3073-
# if EMCC_DEBUG=2 then we must link now, so the temp files are complete.
3074-
# if using the wasm backend, we might be using vanilla LLVM, which does not allow our
3075-
# fastcomp deferred linking opts.
3076-
# TODO: we could check if this is a fastcomp build, and still speed things up here
3077-
js_syms = None
3078-
if settings.ERROR_ON_UNDEFINED_SYMBOLS and not settings.SIDE_MODULE:
3079-
js_syms = get_all_js_syms()
30803095
building.link_lld(linker_arguments, wasm_target, external_symbols=js_syms)
30813096

30823097

src/jsifier.js

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ function isDefined(symName) {
6868

6969
function runJSify(symbolsOnly = false) {
7070
const libraryItems = [];
71+
const symbolDeps = {};
7172
let postSets = [];
7273

7374
LibraryManager.load();
@@ -245,9 +246,16 @@ function ${name}(${args}) {
245246
return;
246247
}
247248

249+
const deps = LibraryManager.library[symbol + '__deps'] || [];
250+
if (!Array.isArray(deps)) {
251+
error(`JS library directive ${symbol}__deps=${deps.toString()} is of type ${typeof deps}, but it should be an array!`);
252+
return;
253+
}
254+
248255
if (symbolsOnly) {
249256
if (!isJsOnlySymbol(symbol) && LibraryManager.library.hasOwnProperty(symbol)) {
250-
librarySymbols.push(symbol);
257+
externalDeps = deps.filter((d) => !(d in LibraryManager.library) && typeof d === 'string');
258+
symbolDeps[symbol] = externalDeps;
251259
}
252260
return;
253261
}
@@ -319,11 +327,6 @@ function ${name}(${args}) {
319327

320328
const original = LibraryManager.library[symbol];
321329
let snippet = original;
322-
const deps = LibraryManager.library[symbol + '__deps'] || [];
323-
if (!Array.isArray(deps)) {
324-
error(`JS library directive ${symbol}__deps=${deps.toString()} is of type ${typeof deps}, but it should be an array!`);
325-
return;
326-
}
327330

328331
const isUserSymbol = LibraryManager.library[symbol + '__user'];
329332
deps.forEach((dep) => {
@@ -542,7 +545,7 @@ function ${name}(${args}) {
542545
}
543546

544547
if (symbolsOnly) {
545-
print(JSON.stringify(librarySymbols));
548+
print(JSON.stringify(symbolDeps));
546549
return;
547550
}
548551

src/library.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2111,7 +2111,7 @@ mergeInto(LibraryManager.library, {
21112111
list: [],
21122112
map: {}
21132113
},
2114-
setprotoent__deps: ['$Protocols', '$writeAsciiToMemory'],
2114+
setprotoent__deps: ['$Protocols', '$writeAsciiToMemory', 'malloc'],
21152115
setprotoent: function(stayopen) {
21162116
// void setprotoent(int stayopen);
21172117

src/library_browser.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -804,6 +804,7 @@ var LibraryBrowser = {
804804
},
805805

806806
emscripten_run_preload_plugins_data__proxy: 'sync',
807+
emscripten_run_preload_plugins_data__deps: ['malloc'],
807808
emscripten_run_preload_plugins_data__sig: 'viiiiii',
808809
emscripten_run_preload_plugins_data: function(data, size, suffix, arg, onload, onerror) {
809810
{{{ runtimeKeepalivePush() }}}
@@ -1355,7 +1356,7 @@ var LibraryBrowser = {
13551356
return info.awaited;
13561357
},
13571358

1358-
emscripten_get_preloaded_image_data__deps: ['$PATH_FS'],
1359+
emscripten_get_preloaded_image_data__deps: ['$PATH_FS', 'malloc'],
13591360
emscripten_get_preloaded_image_data__proxy: 'sync',
13601361
emscripten_get_preloaded_image_data__sig: 'iiii',
13611362
emscripten_get_preloaded_image_data: function(path, w, h) {

src/library_sdl.js

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1348,7 +1348,7 @@ var LibrarySDL = {
13481348
return SDL.version;
13491349
},
13501350

1351-
SDL_Init__deps: ['$zeroMemory'],
1351+
SDL_Init__deps: ['$zeroMemory', 'malloc', 'free', 'memcpy'],
13521352
SDL_Init__proxy: 'sync',
13531353
SDL_Init__sig: 'ii',
13541354
SDL_Init__docs: '/** @param{number=} initFlags */',
@@ -1846,15 +1846,18 @@ var LibrarySDL = {
18461846
SDL_SetError: function() {},
18471847

18481848
SDL_malloc__sig: 'ii',
1849+
SDL_malloc__deps: ['malloc'],
18491850
SDL_malloc: function(size) {
18501851
return _malloc(size);
18511852
},
18521853

18531854
SDL_free__sig: 'vi',
1855+
SDL_free__deps: ['free'],
18541856
SDL_free: function(ptr) {
18551857
_free(ptr);
18561858
},
18571859

1860+
SDL_CreateRGBSurface__deps: ['malloc', 'free'],
18581861
SDL_CreateRGBSurface__proxy: 'sync',
18591862
SDL_CreateRGBSurface__sig: 'iiiiiiiii',
18601863
SDL_CreateRGBSurface: function(flags, width, height, depth, rmask, gmask, bmask, amask) {
@@ -2067,6 +2070,7 @@ var LibrarySDL = {
20672070

20682071
SDL_PushEvent__proxy: 'sync',
20692072
SDL_PushEvent__sig: 'ii',
2073+
SDL_PushEvent__deps: ['malloc'],
20702074
SDL_PushEvent: function(ptr) {
20712075
var copy = _malloc({{{ C_STRUCTS.SDL_KeyboardEvent.__size__ }}});
20722076
_memcpy(copy, ptr, {{{ C_STRUCTS.SDL_KeyboardEvent.__size__ }}});
@@ -2117,6 +2121,7 @@ var LibrarySDL = {
21172121
// An Emscripten-specific extension to SDL: Some browser APIs require that they are called from within an event handler function.
21182122
// Allow recording a callback that will be called for each received event.
21192123
emscripten_SDL_SetEventHandler__proxy: 'sync',
2124+
emscripten_SDL_SetEventHandler__deps: ['malloc'],
21202125
emscripten_SDL_SetEventHandler__sig: 'vii',
21212126
emscripten_SDL_SetEventHandler: function(handler, userdata) {
21222127
SDL.eventHandler = handler;
@@ -2245,7 +2250,7 @@ var LibrarySDL = {
22452250
return flags; // We support JPG, PNG, TIF because browsers do
22462251
},
22472252

2248-
IMG_Load_RW__deps: ['SDL_LockSurface', 'SDL_FreeRW', '$PATH_FS'],
2253+
IMG_Load_RW__deps: ['SDL_LockSurface', 'SDL_FreeRW', '$PATH_FS', 'malloc'],
22492254
IMG_Load_RW__proxy: 'sync',
22502255
IMG_Load_RW__sig: 'iii',
22512256
IMG_Load_RW: function(rwopsID, freeSrc) {
@@ -2413,7 +2418,7 @@ var LibrarySDL = {
24132418

24142419
// SDL_Audio
24152420

2416-
SDL_OpenAudio__deps: ['$autoResumeAudioContext', '$safeSetTimeout'],
2421+
SDL_OpenAudio__deps: ['$autoResumeAudioContext', '$safeSetTimeout', 'malloc'],
24172422
SDL_OpenAudio__proxy: 'sync',
24182423
SDL_OpenAudio__sig: 'iii',
24192424
SDL_OpenAudio: function(desired, obtained) {

src/library_uuid.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ mergeInto(LibraryManager.library, {
2424
},
2525

2626
// Copies the 'compact' UUID variable from src to dst.
27+
uuid_copy__deps: ['memcpy'],
2728
uuid_copy: function(dst, src) {
2829
// void uuid_copy(uuid_t dst, const uuid_t src);
2930
_memcpy(dst, src, 16);

src/library_webgpu.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1851,7 +1851,7 @@ var LibraryWebGPU = {
18511851

18521852
// In webgpu.h offset and size are passed in as size_t.
18531853
// And library_webgpu assumes that size_t is always 32bit in emscripten.
1854-
wgpuBufferGetConstMappedRange__deps: ['$warnOnce'],
1854+
wgpuBufferGetConstMappedRange__deps: ['$warnOnce', 'malloc', 'free'],
18551855
wgpuBufferGetConstMappedRange: function(bufferId, offset, size) {
18561856
var bufferWrapper = WebGPU.mgrBuffer.objects[bufferId];
18571857
{{{ gpu.makeCheckDefined('bufferWrapper') }}}
@@ -1882,7 +1882,7 @@ var LibraryWebGPU = {
18821882

18831883
// In webgpu.h offset and size are passed in as size_t.
18841884
// And library_webgpu assumes that size_t is always 32bit in emscripten.
1885-
wgpuBufferGetMappedRange__deps: ['$warnOnce'],
1885+
wgpuBufferGetMappedRange__deps: ['$warnOnce', 'malloc', 'free'],
18861886
wgpuBufferGetMappedRange: function(bufferId, offset, size) {
18871887
var bufferWrapper = WebGPU.mgrBuffer.objects[bufferId];
18881888
{{{ gpu.makeCheckDefined('bufferWrapper') }}}

test/test_other.py

Lines changed: 14 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -6735,26 +6735,22 @@ def test_no_missing_symbols(self): # simple hello world should not show any miss
67356735
mergeInto(LibraryManager.library, {
67366736
my_js__deps: ['main'],
67376737
my_js: (function() {
6738-
return function() {
6739-
console.log("hello " + _nonexistingvariable);
6740-
};
6738+
return () => console.log("hello " + _nonexistingvariable);
67416739
}()),
67426740
});
67436741
''')
6744-
create_file('test.cpp', '''\
6742+
create_file('test.c', '''\
67456743
#include <stdio.h>
67466744
#include <stdlib.h>
67476745

6748-
extern "C" {
6749-
extern void my_js();
6750-
}
6746+
void my_js();
67516747

67526748
int main() {
67536749
my_js();
67546750
return EXIT_SUCCESS;
67556751
}
67566752
''')
6757-
self.run_process([EMXX, 'test.cpp', '--js-library', 'library_foo.js'])
6753+
self.run_process([EMCC, 'test.c', '--js-library', 'library_foo.js'])
67586754

67596755
# but we do error on a missing js var
67606756
create_file('library_foo_missing.js', '''
@@ -6767,16 +6763,16 @@ def test_no_missing_symbols(self): # simple hello world should not show any miss
67676763
}()),
67686764
});
67696765
''')
6770-
err = self.expect_fail([EMXX, 'test.cpp', '--js-library', 'library_foo_missing.js'])
6771-
self.assertContained('undefined symbol: nonexistingvariable', err)
6766+
err = self.expect_fail([EMCC, 'test.c', '--js-library', 'library_foo_missing.js'])
6767+
self.assertContained('wasm-ld: error: symbol exported via --export not found: nonexistingvariable', err)
67726768

67736769
# and also for missing C code, of course (without the --js-library, it's just a missing C method)
6774-
err = self.expect_fail([EMXX, 'test.cpp'])
6770+
err = self.expect_fail([EMCC, 'test.c'])
67756771
self.assertContained('undefined symbol: my_js', err)
67766772

6777-
def test_js_lib_to_system_lib(self):
6778-
# memset is in compiled code, so a js library __deps can't access it. it
6779-
# would need to be in deps_info.json or EXPORTED_FUNCTIONS
6773+
def test_js_lib_native_deps(self):
6774+
# Verify that memset (which lives in compiled code), can be specified as a JS library
6775+
# dependency.
67806776
create_file('lib.js', r'''
67816777
mergeInto(LibraryManager.library, {
67826778
depper__deps: ['memset'],
@@ -6785,38 +6781,19 @@ def test_js_lib_to_system_lib(self):
67856781
},
67866782
});
67876783
''')
6788-
create_file('test.cpp', r'''
6789-
#include <string.h>
6784+
create_file('test.c', r'''
67906785
#include <stdio.h>
67916786

6792-
extern "C" {
6793-
extern void depper(char*);
6794-
}
6787+
void depper(char*);
67956788

67966789
int main(int argc, char** argv) {
6797-
char buffer[11];
6798-
buffer[10] = '\0';
6799-
// call by a pointer, to force linking of memset, no llvm intrinsic here
6800-
volatile auto ptr = memset;
6801-
(*ptr)(buffer, 'a', 10);
6790+
char buffer[11] = { 0 };
68026791
depper(buffer);
68036792
puts(buffer);
68046793
}
68056794
''')
68066795

6807-
err = self.expect_fail([EMXX, 'test.cpp', '--js-library', 'lib.js'])
6808-
self.assertContained('_memset may need to be added to EXPORTED_FUNCTIONS if it arrives from a system library', err)
6809-
6810-
# without the dep, and with EXPORTED_FUNCTIONS, it works ok
6811-
create_file('lib.js', r'''
6812-
mergeInto(LibraryManager.library, {
6813-
depper: function(ptr) {
6814-
_memset(ptr, 'd'.charCodeAt(0), 10);
6815-
},
6816-
});
6817-
''')
6818-
self.run_process([EMXX, 'test.cpp', '--js-library', 'lib.js', '-sEXPORTED_FUNCTIONS=_main,_memset'])
6819-
self.assertContained('dddddddddd', self.run_js('a.out.js'))
6796+
self.do_runf('test.c', 'dddddddddd\n', emcc_args=['--js-library', 'lib.js'])
68206797

68216798
def test_realpath(self):
68226799
create_file('src.c', r'''

tools/building.py

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ def link_to_object(args, target):
148148

149149
def lld_flags_for_executable(external_symbols):
150150
cmd = []
151-
if external_symbols:
151+
if settings.ERROR_ON_UNDEFINED_SYMBOLS:
152152
undefs = shared.get_temp_files().get('.undefined').name
153153
utils.write_file(undefs, '\n'.join(external_symbols))
154154
cmd.append('--allow-undefined-file=%s' % undefs)
@@ -176,9 +176,8 @@ def lld_flags_for_executable(external_symbols):
176176
# Strip the leading underscores
177177
c_exports = [demangle_c_symbol_name(e) for e in c_exports]
178178
c_exports += settings.EXPORT_IF_DEFINED
179-
if external_symbols:
180-
# Filter out symbols external/JS symbols
181-
c_exports = [e for e in c_exports if e not in external_symbols]
179+
# Filter out symbols external/JS symbols
180+
c_exports = [e for e in c_exports if e not in external_symbols]
182181
for export in c_exports:
183182
cmd.append('--export-if-defined=' + export)
184183

0 commit comments

Comments
 (0)