Add type checking for all JS library decorators

See #20107
emscripten-core · Aug 24, 2023 · 5a88fcf · 5a88fcf
1 parent 0c35c4a
commit 5a88fcf
Show file tree

Hide file tree

Showing 5 changed files with 72 additions and 23 deletions.
diff --git a/ChangeLog.md b/ChangeLog.md
@@ -21,6 +21,8 @@ See docs/process.md for more on how version tagging works.
 3.1.46 (in development)
 -----------------------
 - libunwind updated to LLVM 16.0.6. (#20088)
+- 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
 -----------------

diff --git a/src/jsifier.js b/src/jsifier.js
@@ -227,7 +227,7 @@ function(${args}) {
 
     if (SHARED_MEMORY) {
       const proxyingMode = LibraryManager.library[symbol + '__proxy'];
-      if (proxyingMode) {
+      if (proxyingMode && proxyingMode !== 'none') {
         if (proxyingMode !== 'sync' && proxyingMode !== 'async') {
           throw new Error(`Invalid proxyingMode ${symbol}__proxy: '${proxyingMode}' specified!`);
         }

diff --git a/src/library_syscall.js b/src/library_syscall.js
@@ -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)
   },

diff --git a/src/utility.js b/src/utility.js
@@ -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)) {
+          error(`JS library directive ${key}=${deps.toString()} is of type '${type}', but it should be an array`);
+        }
+        for (let dep of deps) {
+          if (typeof dep !== 'string') {
+            error(`__deps entries must be of type 'string' 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}'`);
+        }
       }
     }
   }

diff --git a/test/test_other.py b/test/test_other.py
@@ -3958,15 +3958,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' 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'''