Skip to content

Commit ca5f2bc

Browse files
authored
Allow JS -> native dependencies to be specified in __deps. NFC (#18849)
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 810f7c5 commit ca5f2bc

File tree

11 files changed

+75
-84
lines changed

11 files changed

+75
-84
lines changed

emcc.py

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@
5050
from tools.minimal_runtime_shell import generate_minimal_runtime_html
5151
import tools.line_endings
5252
from tools import feature_matrix
53+
from tools import deps_info
5354
from tools import js_manipulation
5455
from tools import wasm2c
5556
from tools import webassembly
@@ -542,14 +543,28 @@ def build_symbol_list(filename):
542543
"""Only called when there is no existing symbol list for a given content hash.
543544
"""
544545
library_syms = generate_js_symbols()
545-
write_file(filename, '\n'.join(library_syms) + '\n')
546+
lines = []
547+
548+
for name, deps in library_syms.items():
549+
if deps:
550+
lines.append('%s: %s' % (name, ','.join(deps)))
551+
else:
552+
lines.append(name)
553+
write_file(filename, '\n'.join(lines) + '\n')
546554

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

554569
# Limit of the overall size of the cache to 100 files.
555570
# This code will get test coverage since a full test run of `other` or `core`
@@ -1315,9 +1330,14 @@ def run(args):
13151330
logger.debug('stopping after linking to object file')
13161331
return 0
13171332

1333+
js_syms = {}
1334+
if not settings.SIDE_MODULE:
1335+
js_syms = get_all_js_syms()
1336+
deps_info.append_deps_info(js_syms)
1337+
13181338
phase_calculate_system_libraries(state, linker_arguments, linker_inputs, newargs)
13191339

1320-
phase_link(linker_arguments, wasm_target)
1340+
phase_link(linker_arguments, wasm_target, js_syms)
13211341

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

30633083

30643084
@ToolchainProfiler.profile_block('link')
3065-
def phase_link(linker_arguments, wasm_target):
3085+
def phase_link(linker_arguments, wasm_target, js_syms):
30663086
logger.debug(f'linking: {linker_arguments}')
30673087

30683088
# Make a final pass over settings.EXPORTED_FUNCTIONS to remove any
@@ -3072,13 +3092,6 @@ def phase_link(linker_arguments, wasm_target):
30723092
settings.REQUIRED_EXPORTS = dedup_list(settings.REQUIRED_EXPORTS)
30733093
settings.EXPORT_IF_DEFINED = dedup_list(settings.EXPORT_IF_DEFINED)
30743094

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

30843097

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) => !isJsOnlySymbol(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_html5.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2325,7 +2325,7 @@ var LibraryHTML5 = {
23252325

23262326
$battery: function() { return navigator.battery || navigator.mozBattery || navigator.webkitBattery; },
23272327

2328-
$registerBatteryEventCallback__deps: ['$JSEvents', '$fillBatteryEventData', '$battery', '$findEventTarget'],
2328+
$registerBatteryEventCallback__deps: ['$JSEvents', '$fillBatteryEventData', '$battery', '$findEventTarget', 'malloc'],
23292329
$registerBatteryEventCallback: function(target, userData, useCapture, callbackfunc, eventTypeId, eventTypeString, targetThread) {
23302330
#if USE_PTHREADS
23312331
targetThread = JSEvents.getTargetThreadForEventCallback(targetThread);
@@ -2359,7 +2359,7 @@ var LibraryHTML5 = {
23592359

23602360
emscripten_set_batterychargingchange_callback_on_thread__proxy: 'sync',
23612361
emscripten_set_batterychargingchange_callback_on_thread__sig: 'iii',
2362-
emscripten_set_batterychargingchange_callback_on_thread__deps: ['$registerBatteryEventCallback', '$battery'],
2362+
emscripten_set_batterychargingchange_callback_on_thread__deps: ['$registerBatteryEventCallback', '$battery', 'malloc'],
23632363
emscripten_set_batterychargingchange_callback_on_thread: function(userData, callbackfunc, targetThread) {
23642364
if (!battery()) return {{{ cDefine('EMSCRIPTEN_RESULT_NOT_SUPPORTED') }}};
23652365
registerBatteryEventCallback(battery(), userData, true, callbackfunc, {{{ cDefine('EMSCRIPTEN_EVENT_BATTERYCHARGINGCHANGE') }}}, "chargingchange", targetThread);
@@ -2368,7 +2368,7 @@ var LibraryHTML5 = {
23682368

23692369
emscripten_set_batterylevelchange_callback_on_thread__proxy: 'sync',
23702370
emscripten_set_batterylevelchange_callback_on_thread__sig: 'iii',
2371-
emscripten_set_batterylevelchange_callback_on_thread__deps: ['$registerBatteryEventCallback', '$battery'],
2371+
emscripten_set_batterylevelchange_callback_on_thread__deps: ['$registerBatteryEventCallback', '$battery', 'malloc'],
23722372
emscripten_set_batterylevelchange_callback_on_thread: function(userData, callbackfunc, targetThread) {
23732373
if (!battery()) return {{{ cDefine('EMSCRIPTEN_RESULT_NOT_SUPPORTED') }}};
23742374
registerBatteryEventCallback(battery(), userData, true, callbackfunc, {{{ cDefine('EMSCRIPTEN_EVENT_BATTERYLEVELCHANGE') }}}, "levelchange", targetThread);

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
@@ -6782,26 +6782,22 @@ def test_no_missing_symbols(self): # simple hello world should not show any miss
67826782
mergeInto(LibraryManager.library, {
67836783
my_js__deps: ['main'],
67846784
my_js: (function() {
6785-
return function() {
6786-
console.log("hello " + _nonexistingvariable);
6787-
};
6785+
return () => console.log("hello " + _nonexistingvariable);
67886786
}()),
67896787
});
67906788
''')
6791-
create_file('test.cpp', '''\
6789+
create_file('test.c', '''\
67926790
#include <stdio.h>
67936791
#include <stdlib.h>
67946792

6795-
extern "C" {
6796-
extern void my_js();
6797-
}
6793+
void my_js();
67986794

67996795
int main() {
68006796
my_js();
68016797
return EXIT_SUCCESS;
68026798
}
68036799
''')
6804-
self.run_process([EMXX, 'test.cpp', '--js-library', 'library_foo.js'])
6800+
self.run_process([EMCC, 'test.c', '--js-library', 'library_foo.js'])
68056801

68066802
# but we do error on a missing js var
68076803
create_file('library_foo_missing.js', '''
@@ -6814,16 +6810,16 @@ def test_no_missing_symbols(self): # simple hello world should not show any miss
68146810
}()),
68156811
});
68166812
''')
6817-
err = self.expect_fail([EMXX, 'test.cpp', '--js-library', 'library_foo_missing.js'])
6818-
self.assertContained('undefined symbol: nonexistingvariable', err)
6813+
err = self.expect_fail([EMCC, 'test.c', '--js-library', 'library_foo_missing.js'])
6814+
self.assertContained('wasm-ld: error: symbol exported via --export not found: nonexistingvariable', err)
68196815

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

6824-
def test_js_lib_to_system_lib(self):
6825-
# memset is in compiled code, so a js library __deps can't access it. it
6826-
# would need to be in deps_info.json or EXPORTED_FUNCTIONS
6820+
def test_js_lib_native_deps(self):
6821+
# Verify that memset (which lives in compiled code), can be specified as a JS library
6822+
# dependency.
68276823
create_file('lib.js', r'''
68286824
mergeInto(LibraryManager.library, {
68296825
depper__deps: ['memset'],
@@ -6832,38 +6828,19 @@ def test_js_lib_to_system_lib(self):
68326828
},
68336829
});
68346830
''')
6835-
create_file('test.cpp', r'''
6836-
#include <string.h>
6831+
create_file('test.c', r'''
68376832
#include <stdio.h>
68386833

6839-
extern "C" {
6840-
extern void depper(char*);
6841-
}
6834+
void depper(char*);
68426835

68436836
int main(int argc, char** argv) {
6844-
char buffer[11];
6845-
buffer[10] = '\0';
6846-
// call by a pointer, to force linking of memset, no llvm intrinsic here
6847-
volatile auto ptr = memset;
6848-
(*ptr)(buffer, 'a', 10);
6837+
char buffer[11] = { 0 };
68496838
depper(buffer);
68506839
puts(buffer);
68516840
}
68526841
''')
68536842

6854-
err = self.expect_fail([EMXX, 'test.cpp', '--js-library', 'lib.js'])
6855-
self.assertContained('_memset may need to be added to EXPORTED_FUNCTIONS if it arrives from a system library', err)
6856-
6857-
# without the dep, and with EXPORTED_FUNCTIONS, it works ok
6858-
create_file('lib.js', r'''
6859-
mergeInto(LibraryManager.library, {
6860-
depper: function(ptr) {
6861-
_memset(ptr, 'd'.charCodeAt(0), 10);
6862-
},
6863-
});
6864-
''')
6865-
self.run_process([EMXX, 'test.cpp', '--js-library', 'lib.js', '-sEXPORTED_FUNCTIONS=_main,_memset'])
6866-
self.assertContained('dddddddddd', self.run_js('a.out.js'))
6843+
self.do_runf('test.c', 'dddddddddd\n', emcc_args=['--js-library', 'lib.js'])
68676844

68686845
def test_realpath(self):
68696846
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)