Skip to content

embind: Add workaround for unstable type-ids between shared libraries and executable #18418

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
embind: Add workaround for unstable type-ids when requiring type
Due to TypeIDs not being stable between modules when the type info
is not visible outside the side-module, embind fails to look up
the registered type for a TypeID that has been registered from
another module.

This is for example the case for emscripten::val, which is registered
by embind.cpp. If another module uses the templated val(T&& value)
constructor, it will result in a call to _emval_take_value where
the passed type id is resolved in the context of the calling
module.

If these two differ, it will result in a binding error thrown:

 BindingError: emval::as has unknown type N10emscripten3valE

As long as we can't guarantee stable type-ids across modules,
we work around the issue by looking for a matching existing
registered type based on the type name, and if one is found
we register the missing type-id with the same information.

Fixes: #16711
  • Loading branch information
torarnv committed Dec 22, 2022
commit e9f86cebc3eb18f52e03ec67730f3edaa91d4564
22 changes: 21 additions & 1 deletion src/embind/embind.js
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ var LibraryEmbind = {
$registeredPointers: {},

$registerType__deps: [
'$awaitingDependencies', '$registeredTypes',
'$awaitingDependencies', '$registeredTypes', '$getTypeName',
'$typeDependencies', '$throwBindingError',
'$whenDependentTypesAreResolved'],
$registerType__docs: '/** @param {Object=} options */',
Expand All @@ -319,6 +319,12 @@ var LibraryEmbind = {
}
}

#if MAIN_MODULE
// Store type name as well, which may or may not match the registered
// name, depending on mangling and the actual calls to register_foo.
registeredInstance.typeName = getTypeName(rawType);
#endif

registeredTypes[rawType] = registeredInstance;
delete typeDependencies[rawType];

Expand Down Expand Up @@ -416,6 +422,20 @@ var LibraryEmbind = {
$requireRegisteredType: function(rawType, humanName) {
var impl = registeredTypes[rawType];
if (undefined === impl) {
#if MAIN_MODULE
// Fallback, in case of shared libraries, where the TypeID (rawType)
// for each type is not stable across library boundaries.
var typeName = getTypeName(rawType);
for (const typeId in registeredTypes) {
var type = registeredTypes[typeId];
if (type.typeName == typeName) {
registerType(rawType, type, {
ignoreDuplicateRegistrations: true,
});
return type;
}
}
#endif
throwBindingError(humanName + " has unknown type " + getTypeName(rawType));
}
return impl;
Expand Down
25 changes: 25 additions & 0 deletions test/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -7684,6 +7684,31 @@ def test_embind_no_rtti_followed_by_rtti(self):
self.emcc_args += ['-lembind', '-fno-rtti', '-frtti']
self.do_run(src, '418\ndotest returned: 42\n')

@needs_dylink
def test_embind_type_registration_when_typeid_is_not_stable(self):
# See test_dylink_typeid
self.emcc_args += ['-lembind', '-fvisibility=hidden']
self.dylink_test(header=r'''
#include <emscripten.h>
#include <emscripten/bind.h>
#include <emscripten/val.h>
#include <stdio.h>
''', main=r'''
#include "header.h"
int main() {
emscripten::val intVal(42);
emscripten::val valVal(intVal);
puts("success");
return 0;
}
''', side=r'''
#include "header.h"
__attribute__((constructor)) void kaboom() {
emscripten::val intVal(42);
emscripten::val valVal(intVal);
}
''', expected=['success'], need_reverse=False)

@no_wasm64('webidl not compatible with MEMORY64 yet')
@parameterized({
'': ('DEFAULT', False),
Expand Down