Skip to content

Commit

Permalink
Allow JS -> native dependencies to be specified in __deps. NFC (emscr…
Browse files Browse the repository at this point in the history
…ipten-core#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): emscripten-core#15982
  • Loading branch information
sbc100 authored and impact-maker committed Mar 17, 2023
1 parent 539e766 commit 3a52945
Show file tree
Hide file tree
Showing 11 changed files with 75 additions and 84 deletions.
35 changes: 24 additions & 11 deletions emcc.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
from tools.minimal_runtime_shell import generate_minimal_runtime_html
import tools.line_endings
from tools import feature_matrix
from tools import deps_info
from tools import js_manipulation
from tools import wasm2c
from tools import webassembly
Expand Down Expand Up @@ -542,14 +543,28 @@ def build_symbol_list(filename):
"""Only called when there is no existing symbol list for a given content hash.
"""
library_syms = generate_js_symbols()
write_file(filename, '\n'.join(library_syms) + '\n')
lines = []

for name, deps in library_syms.items():
if deps:
lines.append('%s: %s' % (name, ','.join(deps)))
else:
lines.append(name)
write_file(filename, '\n'.join(lines) + '\n')

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

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

js_syms = {}
if not settings.SIDE_MODULE:
js_syms = get_all_js_syms()
deps_info.append_deps_info(js_syms)

phase_calculate_system_libraries(state, linker_arguments, linker_inputs, newargs)

phase_link(linker_arguments, wasm_target)
phase_link(linker_arguments, wasm_target, js_syms)

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


@ToolchainProfiler.profile_block('link')
def phase_link(linker_arguments, wasm_target):
def phase_link(linker_arguments, wasm_target, js_syms):
logger.debug(f'linking: {linker_arguments}')

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

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


Expand Down
17 changes: 10 additions & 7 deletions src/jsifier.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ function isDefined(symName) {

function runJSify(symbolsOnly = false) {
const libraryItems = [];
const symbolDeps = {};
let postSets = [];

LibraryManager.load();
Expand Down Expand Up @@ -245,9 +246,16 @@ function ${name}(${args}) {
return;
}

const deps = LibraryManager.library[symbol + '__deps'] || [];
if (!Array.isArray(deps)) {
error(`JS library directive ${symbol}__deps=${deps.toString()} is of type ${typeof deps}, but it should be an array!`);
return;
}

if (symbolsOnly) {
if (!isJsOnlySymbol(symbol) && LibraryManager.library.hasOwnProperty(symbol)) {
librarySymbols.push(symbol);
externalDeps = deps.filter((d) => !isJsOnlySymbol(d) && !(d in LibraryManager.library) && typeof d === 'string');
symbolDeps[symbol] = externalDeps;
}
return;
}
Expand Down Expand Up @@ -319,11 +327,6 @@ function ${name}(${args}) {

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

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

if (symbolsOnly) {
print(JSON.stringify(librarySymbols));
print(JSON.stringify(symbolDeps));
return;
}

Expand Down
2 changes: 1 addition & 1 deletion src/library.js
Original file line number Diff line number Diff line change
Expand Up @@ -2111,7 +2111,7 @@ mergeInto(LibraryManager.library, {
list: [],
map: {}
},
setprotoent__deps: ['$Protocols', '$writeAsciiToMemory'],
setprotoent__deps: ['$Protocols', '$writeAsciiToMemory', 'malloc'],
setprotoent: function(stayopen) {
// void setprotoent(int stayopen);

Expand Down
3 changes: 2 additions & 1 deletion src/library_browser.js
Original file line number Diff line number Diff line change
Expand Up @@ -804,6 +804,7 @@ var LibraryBrowser = {
},

emscripten_run_preload_plugins_data__proxy: 'sync',
emscripten_run_preload_plugins_data__deps: ['malloc'],
emscripten_run_preload_plugins_data__sig: 'viiiiii',
emscripten_run_preload_plugins_data: function(data, size, suffix, arg, onload, onerror) {
{{{ runtimeKeepalivePush() }}}
Expand Down Expand Up @@ -1355,7 +1356,7 @@ var LibraryBrowser = {
return info.awaited;
},

emscripten_get_preloaded_image_data__deps: ['$PATH_FS'],
emscripten_get_preloaded_image_data__deps: ['$PATH_FS', 'malloc'],
emscripten_get_preloaded_image_data__proxy: 'sync',
emscripten_get_preloaded_image_data__sig: 'iiii',
emscripten_get_preloaded_image_data: function(path, w, h) {
Expand Down
6 changes: 3 additions & 3 deletions src/library_html5.js
Original file line number Diff line number Diff line change
Expand Up @@ -2325,7 +2325,7 @@ var LibraryHTML5 = {

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

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

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

emscripten_set_batterylevelchange_callback_on_thread__proxy: 'sync',
emscripten_set_batterylevelchange_callback_on_thread__sig: 'iii',
emscripten_set_batterylevelchange_callback_on_thread__deps: ['$registerBatteryEventCallback', '$battery'],
emscripten_set_batterylevelchange_callback_on_thread__deps: ['$registerBatteryEventCallback', '$battery', 'malloc'],
emscripten_set_batterylevelchange_callback_on_thread: function(userData, callbackfunc, targetThread) {
if (!battery()) return {{{ cDefine('EMSCRIPTEN_RESULT_NOT_SUPPORTED') }}};
registerBatteryEventCallback(battery(), userData, true, callbackfunc, {{{ cDefine('EMSCRIPTEN_EVENT_BATTERYLEVELCHANGE') }}}, "levelchange", targetThread);
Expand Down
11 changes: 8 additions & 3 deletions src/library_sdl.js
Original file line number Diff line number Diff line change
Expand Up @@ -1348,7 +1348,7 @@ var LibrarySDL = {
return SDL.version;
},

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

SDL_malloc__sig: 'ii',
SDL_malloc__deps: ['malloc'],
SDL_malloc: function(size) {
return _malloc(size);
},

SDL_free__sig: 'vi',
SDL_free__deps: ['free'],
SDL_free: function(ptr) {
_free(ptr);
},

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

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

IMG_Load_RW__deps: ['SDL_LockSurface', 'SDL_FreeRW', '$PATH_FS'],
IMG_Load_RW__deps: ['SDL_LockSurface', 'SDL_FreeRW', '$PATH_FS', 'malloc'],
IMG_Load_RW__proxy: 'sync',
IMG_Load_RW__sig: 'iii',
IMG_Load_RW: function(rwopsID, freeSrc) {
Expand Down Expand Up @@ -2413,7 +2418,7 @@ var LibrarySDL = {

// SDL_Audio

SDL_OpenAudio__deps: ['$autoResumeAudioContext', '$safeSetTimeout'],
SDL_OpenAudio__deps: ['$autoResumeAudioContext', '$safeSetTimeout', 'malloc'],
SDL_OpenAudio__proxy: 'sync',
SDL_OpenAudio__sig: 'iii',
SDL_OpenAudio: function(desired, obtained) {
Expand Down
1 change: 1 addition & 0 deletions src/library_uuid.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ mergeInto(LibraryManager.library, {
},

// Copies the 'compact' UUID variable from src to dst.
uuid_copy__deps: ['memcpy'],
uuid_copy: function(dst, src) {
// void uuid_copy(uuid_t dst, const uuid_t src);
_memcpy(dst, src, 16);
Expand Down
4 changes: 2 additions & 2 deletions src/library_webgpu.js
Original file line number Diff line number Diff line change
Expand Up @@ -1851,7 +1851,7 @@ var LibraryWebGPU = {
// In webgpu.h offset and size are passed in as size_t.
// And library_webgpu assumes that size_t is always 32bit in emscripten.
wgpuBufferGetConstMappedRange__deps: ['$warnOnce'],
wgpuBufferGetConstMappedRange__deps: ['$warnOnce', 'malloc', 'free'],
wgpuBufferGetConstMappedRange: function(bufferId, offset, size) {
var bufferWrapper = WebGPU.mgrBuffer.objects[bufferId];
{{{ gpu.makeCheckDefined('bufferWrapper') }}}
Expand Down Expand Up @@ -1882,7 +1882,7 @@ var LibraryWebGPU = {
// In webgpu.h offset and size are passed in as size_t.
// And library_webgpu assumes that size_t is always 32bit in emscripten.
wgpuBufferGetMappedRange__deps: ['$warnOnce'],
wgpuBufferGetMappedRange__deps: ['$warnOnce', 'malloc', 'free'],
wgpuBufferGetMappedRange: function(bufferId, offset, size) {
var bufferWrapper = WebGPU.mgrBuffer.objects[bufferId];
{{{ gpu.makeCheckDefined('bufferWrapper') }}}
Expand Down
51 changes: 14 additions & 37 deletions test/test_other.py
Original file line number Diff line number Diff line change
Expand Up @@ -6782,26 +6782,22 @@ def test_no_missing_symbols(self): # simple hello world should not show any miss
mergeInto(LibraryManager.library, {
my_js__deps: ['main'],
my_js: (function() {
return function() {
console.log("hello " + _nonexistingvariable);
};
return () => console.log("hello " + _nonexistingvariable);
}()),
});
''')
create_file('test.cpp', '''\
create_file('test.c', '''\
#include <stdio.h>
#include <stdlib.h>

extern "C" {
extern void my_js();
}
void my_js();

int main() {
my_js();
return EXIT_SUCCESS;
}
''')
self.run_process([EMXX, 'test.cpp', '--js-library', 'library_foo.js'])
self.run_process([EMCC, 'test.c', '--js-library', 'library_foo.js'])

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

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

def test_js_lib_to_system_lib(self):
# memset is in compiled code, so a js library __deps can't access it. it
# would need to be in deps_info.json or EXPORTED_FUNCTIONS
def test_js_lib_native_deps(self):
# Verify that memset (which lives in compiled code), can be specified as a JS library
# dependency.
create_file('lib.js', r'''
mergeInto(LibraryManager.library, {
depper__deps: ['memset'],
Expand All @@ -6832,38 +6828,19 @@ def test_js_lib_to_system_lib(self):
},
});
''')
create_file('test.cpp', r'''
#include <string.h>
create_file('test.c', r'''
#include <stdio.h>

extern "C" {
extern void depper(char*);
}
void depper(char*);

int main(int argc, char** argv) {
char buffer[11];
buffer[10] = '\0';
// call by a pointer, to force linking of memset, no llvm intrinsic here
volatile auto ptr = memset;
(*ptr)(buffer, 'a', 10);
char buffer[11] = { 0 };
depper(buffer);
puts(buffer);
}
''')

err = self.expect_fail([EMXX, 'test.cpp', '--js-library', 'lib.js'])
self.assertContained('_memset may need to be added to EXPORTED_FUNCTIONS if it arrives from a system library', err)

# without the dep, and with EXPORTED_FUNCTIONS, it works ok
create_file('lib.js', r'''
mergeInto(LibraryManager.library, {
depper: function(ptr) {
_memset(ptr, 'd'.charCodeAt(0), 10);
},
});
''')
self.run_process([EMXX, 'test.cpp', '--js-library', 'lib.js', '-sEXPORTED_FUNCTIONS=_main,_memset'])
self.assertContained('dddddddddd', self.run_js('a.out.js'))
self.do_runf('test.c', 'dddddddddd\n', emcc_args=['--js-library', 'lib.js'])

def test_realpath(self):
create_file('src.c', r'''
Expand Down
7 changes: 3 additions & 4 deletions tools/building.py
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ def link_to_object(args, target):

def lld_flags_for_executable(external_symbols):
cmd = []
if external_symbols:
if settings.ERROR_ON_UNDEFINED_SYMBOLS:
undefs = shared.get_temp_files().get('.undefined').name
utils.write_file(undefs, '\n'.join(external_symbols))
cmd.append('--allow-undefined-file=%s' % undefs)
Expand Down Expand Up @@ -176,9 +176,8 @@ def lld_flags_for_executable(external_symbols):
# Strip the leading underscores
c_exports = [demangle_c_symbol_name(e) for e in c_exports]
c_exports += settings.EXPORT_IF_DEFINED
if external_symbols:
# Filter out symbols external/JS symbols
c_exports = [e for e in c_exports if e not in external_symbols]
# Filter out symbols external/JS symbols
c_exports = [e for e in c_exports if e not in external_symbols]
for export in c_exports:
cmd.append('--export-if-defined=' + export)

Expand Down
Loading

0 comments on commit 3a52945

Please sign in to comment.