Skip to content
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

Add type checking for all JS library decorators #20110

Merged
merged 1 commit into from
Aug 29, 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
2 changes: 2 additions & 0 deletions ChangeLog.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ See docs/process.md for more on how version tagging works.
incoming module but forget to include them in `-sINCOMING_MODULE_API`
will see an error in debug builds so this change will not generate any
silent failures.
- JS library decorators such as `__deps` and `__async` are now type checked so
that errors are not silently ignored.

3.1.45 - 08/23/23
-----------------
Expand Down
2 changes: 1 addition & 1 deletion src/jsifier.js
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ function(${args}) {

const proxyingMode = LibraryManager.library[symbol + '__proxy'];
if (proxyingMode) {
if (proxyingMode !== 'sync' && proxyingMode !== 'async') {
if (proxyingMode !== 'sync' && proxyingMode !== 'async' && proxyingMode !== 'none') {
throw new Error(`Invalid proxyingMode ${symbol}__proxy: '${proxyingMode}' specified!`);
}
if (SHARED_MEMORY) {
Expand Down
2 changes: 1 addition & 1 deletion src/library_syscall.js
Original file line number Diff line number Diff line change
Expand Up @@ -838,7 +838,7 @@ var SyscallsLibrary = {
return ___syscall_statfs64(0, size, buf);
},
__syscall_fadvise64__nothrow: true,
__syscall_fadvise64__proxy: false,
__syscall_fadvise64__proxy: 'none',
__syscall_fadvise64: (fd, offset, len, advice) => {
return 0; // your advice is important to us (but we can't use it)
},
Expand Down
59 changes: 46 additions & 13 deletions src/utility.js
Original file line number Diff line number Diff line change
Expand Up @@ -145,22 +145,55 @@ function mergeInto(obj, other, options = null) {
}

for (const key of Object.keys(other)) {
if (key.endsWith('__sig')) {
if (obj.hasOwnProperty(key)) {
const oldsig = obj[key];
const newsig = other[key];
if (oldsig == newsig) {
warn(`signature redefinition for: ${key}`);
} else {
error(`signature redefinition for: ${key}. (old=${oldsig} vs new=${newsig})`);
if (isDecorator(key)) {
if (key.endsWith('__sig')) {
if (obj.hasOwnProperty(key)) {
const oldsig = obj[key];
const newsig = other[key];
if (oldsig == newsig) {
warn(`signature redefinition for: ${key}`);
} else {
error(`signature redefinition for: ${key}. (old=${oldsig} vs new=${newsig})`);
}
}
}
}

if (key.endsWith('__deps')) {
const deps = other[key];
if (!Array.isArray(deps)) {
error(`JS library directive ${key}=${deps.toString()} is of type ${typeof deps}, but it should be an array`);
const index = key.lastIndexOf('__');
const decoratorName = key.slice(index);
const type = typeof other[key];

// Specific type checking for `__deps` which is expected to be an array
// (not just any old `object`)
if (decoratorName === '__deps') {
const deps = other[key];
if (!Array.isArray(deps)) {
sbc100 marked this conversation as resolved.
Show resolved Hide resolved
error(`JS library directive ${key}=${deps.toString()} is of type '${type}', but it should be an array`);
}
for (let dep of deps) {
if (dep && typeof dep !== 'string' && typeof dep !== 'function') {
error(`__deps entries must be of type 'string' or 'function' not '${typeof dep}': ${key}`)
}
}
} else {
// General type checking for all other decorators
const decoratorTypes = {
'__sig': 'string',
'__proxy': 'string',
'__asm': 'boolean',
'__inline': 'boolean',
'__postset': ['string', 'function'],
'__docs': 'string',
'__nothrow': 'boolean',
'__noleakcheck': 'boolean',
'__internal': 'boolean',
'__user': 'boolean',
'__async': 'boolean',
'__i53abi': 'boolean',
};
const expected = decoratorTypes[decoratorName];
if (type !== expected && !expected.includes(type)) {
error(`Decorator (${key}} has wrong type. Expected '${expected}' not '${type}'`);
}
}
}
}
Expand Down
30 changes: 22 additions & 8 deletions test/test_other.py
Original file line number Diff line number Diff line change
Expand Up @@ -3963,15 +3963,29 @@ def test_js_lib_invalid_deps(self):
jslibfunc: (x) => {},
});
''')
create_file('src.c', r'''
#include <stdio.h>
int jslibfunc();
int main() {
printf("c calling: %d\n", jslibfunc());
}

err = self.expect_fail([EMCC, test_file('hello_world.c'), '--js-library', 'lib.js'])
self.assertContained('lib.js: JS library directive jslibfunc__deps=hello is of type \'string\', but it should be an array', err)

create_file('lib2.js', r'''
addToLibrary({
jslibfunc__deps: [1,2,3],
jslibfunc: (x) => {},
});
''')

err = self.expect_fail([EMCC, test_file('hello_world.c'), '--js-library', 'lib2.js'])
self.assertContained("lib2.js: __deps entries must be of type 'string' or 'function' not 'number': jslibfunc__deps", err)

def test_js_lib_invalid_decorator(self):
create_file('lib.js', r'''
addToLibrary({
jslibfunc__async: 'hello',
jslibfunc: (x) => {},
});
''')
err = self.expect_fail([EMCC, 'src.c', '--js-library', 'lib.js'])
self.assertContained('lib.js: JS library directive jslibfunc__deps=hello is of type string, but it should be an array', err)
err = self.expect_fail([EMCC, test_file('hello_world.c'), '--js-library', 'lib.js'])
self.assertContained("lib.js: Decorator (jslibfunc__async} has wrong type. Expected 'boolean' not 'string'", err)

def test_js_lib_legacy(self):
create_file('lib.js', r'''
Expand Down