Skip to content

Allow JS -> native dependencies to be specified in __deps. NFC #18849

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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