Skip to content

Commit

Permalink
Fix for wasmOffsetConverter with MODULARIZE
Browse files Browse the repository at this point in the history
I tried to fix this in #13236 but I don't think it was ever working
correctly.  In MODULARIZE mode `wasmOffsetConverter` was not being
defined within the module itself (where it is used).  his change moves
the contruction of `wasmOffsetConverter` and `wasmSourceMap` so they
always occur inside the module.

I've also improved the testing of `wasmOffsetConverter` so we actually
verify that it works in `tests/core/test_return_address.c`.
  • Loading branch information
sbc100 committed Jul 26, 2021
1 parent 94b70c5 commit 5b98a7a
Show file tree
Hide file tree
Showing 6 changed files with 51 additions and 66 deletions.
3 changes: 3 additions & 0 deletions src/library.js
Original file line number Diff line number Diff line change
Expand Up @@ -2952,6 +2952,9 @@ LibraryManager.library = {
#if !USE_OFFSET_CONVERTER
abort('Cannot use emscripten_generate_pc (needed by __builtin_return_address) without -s USE_OFFSET_CONVERTER');
#else
#if ASSERTIONS
assert(wasmOffsetConverter);
#endif
var match;

if (match = /\bwasm-function\[\d+\]:(0x[0-9a-f]+)/.exec(frame)) {
Expand Down
1 change: 0 additions & 1 deletion src/postamble_minimal.js
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,6 @@ WebAssembly.instantiate(Module['wasm'], imports).then(function(output) {
#if USE_PTHREADS
// This Worker is now ready to host pthreads, tell the main thread we can proceed.
if (ENVIRONMENT_IS_PTHREAD) {
moduleLoaded();
postMessage({ 'cmd': 'loaded' });
}
#endif
Expand Down
45 changes: 27 additions & 18 deletions src/preamble.js
Original file line number Diff line number Diff line change
Expand Up @@ -919,6 +919,21 @@ function instantiateSync(file, info) {
}
#endif
#if LOAD_SOURCE_MAP || USE_OFFSET_CONVERTER
// When using postMessage to send an object, it is processed by the structured clone algorithm.
// The prototype, and hence methods, on that object is then lost. This function adds back the lost prototype.
// This does not work with nested objects that has prototypes, but it suffices for WasmSourceMap and WasmOffsetConverter.
function resetPrototype(constructor, attrs) {
var object = Object.create(constructor.prototype);
for (var key in attrs) {
if (attrs.hasOwnProperty(key)) {
object[key] = attrs[key];
}
}
return object;
}
#endif
// Create the wasm instance.
// Receives the wasm imports, returns the exports.
function createWasm() {
Expand Down Expand Up @@ -1174,28 +1189,22 @@ function createWasm() {
// to manually instantiate the Wasm module themselves. This allows pages to run the instantiation parallel
// to any other async startup actions they are performing.
if (Module['instantiateWasm']) {
#if USE_OFFSET_CONVERTER
#if ASSERTIONS
assert(Module['wasmOffsetData'], 'wasmOffsetData required when overriding instantiateWasm');
#endif
wasmOffsetConverter = resetPrototype(WasmOffsetConverter, Module['wasmOffsetData']);
#endif
#if LOAD_SOURCE_MAP
#if ASSERTIONS
assert(Module['wasmSourceMapData'], 'wasmSourceMapData required when overriding instantiateWasm');
#endif
wasmSourceMap = resetPrototype(Module['WasmSourceMap'], Module['wasmSourceMapData']);
#endif
try {
var exports = Module['instantiateWasm'](info, receiveInstance);
#if ASYNCIFY
exports = Asyncify.instrumentWasmExports(exports);
#endif
#if USE_OFFSET_CONVERTER
{{{
runOnMainThread(`
// We have no way to create an OffsetConverter in this code path since
// we have no access to the wasm binary (only the user does). Instead,
// create a fake one that reports we cannot identify functions from
// their binary offsets.
// Note that we only do this on the main thread, as the workers
// receive the OffsetConverter data from there.
wasmOffsetConverter = {
getName: function() {
return 'unknown-due-to-instantiateWasm';
}
};
removeRunDependency('offset-converter');
`)
}}}
#endif
return exports;
} catch(e) {
Expand Down
48 changes: 3 additions & 45 deletions src/worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,37 +99,6 @@ Module['instantiateWasm'] = function(info, receiveInstance) {
};
#endif

#if LOAD_SOURCE_MAP || USE_OFFSET_CONVERTER
// When using postMessage to send an object, it is processed by the structured clone algorithm.
// The prototype, and hence methods, on that object is then lost. This function adds back the lost prototype.
// This does not work with nested objects that has prototypes, but it suffices for WasmSourceMap and WasmOffsetConverter.
function resetPrototype(constructor, attrs) {
var object = Object.create(constructor.prototype);
for (var key in attrs) {
if (attrs.hasOwnProperty(key)) {
object[key] = attrs[key];
}
}
return object;
}
#endif

#if LOAD_SOURCE_MAP
var wasmSourceMapData;
#endif
#if USE_OFFSET_CONVERTER
var wasmOffsetData, wasmOffsetConverter;
#endif

function moduleLoaded() {
#if LOAD_SOURCE_MAP
wasmSourceMap = resetPrototype(Module['WasmSourceMap'], wasmSourceMapData);
#endif
#if USE_OFFSET_CONVERTER
wasmOffsetConverter = resetPrototype(Module['WasmOffsetConverter'], wasmOffsetData);
#endif
}

self.onmessage = function(e) {
try {
if (e.data.cmd === 'load') { // Preload command that is called once per worker to parse and load the Emscripten code.
Expand All @@ -155,10 +124,10 @@ self.onmessage = function(e) {
{{{ makeAsmImportsAccessInPthread('wasmMemory') }}} = e.data.wasmMemory;

#if LOAD_SOURCE_MAP
wasmSourceMapData = e.data.wasmSourceMap;
Module['wasmSourceMapData'] = e.data.wasmSourceMap;
#endif
#if USE_OFFSET_CONVERTER
wasmOffsetData = e.data.wasmOffsetConverter;
Module['wasmOffsetData'] = e.data.wasmOffsetConverter;
#endif

{{{ makeAsmImportsAccessInPthread('buffer') }}} = {{{ makeAsmImportsAccessInPthread('wasmMemory') }}}.buffer;
Expand All @@ -172,7 +141,6 @@ self.onmessage = function(e) {
return exports.default(Module);
}).then(function(instance) {
Module = instance;
moduleLoaded();
});
#else
if (typeof e.data.urlOrBlob === 'string') {
Expand All @@ -186,24 +154,14 @@ self.onmessage = function(e) {
#if MINIMAL_RUNTIME
{{{ EXPORT_NAME }}}(imports).then(function (instance) {
Module = instance;
moduleLoaded();
});
#else
{{{ EXPORT_NAME }}}(Module).then(function (instance) {
Module = instance;
moduleLoaded();
});
#endif
#endif

#if !MODULARIZE && !MINIMAL_RUNTIME
// MINIMAL_RUNTIME always compiled Wasm (&Wasm2JS) asynchronously, even in pthreads. But
// regular runtime and asm.js are loaded synchronously, so in those cases
// we are now loaded, and can post back to main thread.
moduleLoaded();
#endif

#endif
#endif // MODULARIZE && EXPORT_ES6
} else if (e.data.cmd === 'objectTransfer') {
Module['PThread'].receiveObjectTransfer(e.data);
} else if (e.data.cmd === 'run') {
Expand Down
16 changes: 14 additions & 2 deletions tests/core/test_return_address.c
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,24 @@

#include <assert.h>
#include <stdio.h>
#include <string.h>

void *emscripten_return_address(int level);
const char *emscripten_pc_get_function(void* pc);

void func() {
assert(emscripten_return_address(0) != 0);
__attribute__((noinline)) void func() {
void* rtn_addr = emscripten_return_address(0);
void* rtn_addr2 = __builtin_return_address(0);
const char* caller_name = emscripten_pc_get_function(rtn_addr);
printf("emscripten_return_address: %p\n", rtn_addr);
printf("emscripten_pc_get_function: %s\n", caller_name);
assert(rtn_addr != 0);
assert(rtn_addr2 != 0);
assert(rtn_addr == rtn_addr2);
#ifdef DEBUG
assert(strcmp(caller_name, "main") == 0);
assert(emscripten_return_address(50) == 0);
#endif
}

// We need to take these two arguments or clang can potentially generate
Expand Down
4 changes: 4 additions & 0 deletions tests/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -8317,6 +8317,8 @@ def test_pthread_offset_converter(self):
self.set_setting('PROXY_TO_PTHREAD')
self.set_setting('EXIT_RUNTIME')
self.set_setting('USE_OFFSET_CONVERTER')
if '-g' in self.emcc_args:
self.emcc_args += ['-DDEBUG']
self.do_runf(test_file('core/test_return_address.c'), 'passed')

@node_pthreads
Expand All @@ -8328,6 +8330,8 @@ def test_pthread_offset_converter_modularize(self):
self.set_setting('MODULARIZE')
create_file('post.js', 'var m = require("./test_return_address.js"); m();')
self.emcc_args += ['--extern-post-js', 'post.js', '-s', 'EXPORT_NAME=foo']
if '-g' in self.emcc_args:
self.emcc_args += ['-DDEBUG']
self.do_runf(test_file('core/test_return_address.c'), 'passed')

def test_emscripten_atomics_stub(self):
Expand Down

0 comments on commit 5b98a7a

Please sign in to comment.