asyncify: support *-matching in whitelist and blacklist (#2344)

See emscripten-core/emscripten#9381 for rationale.

Author: Beuc

src/passes/Asyncify.cpp

@@ -229,15 +229,15 @@
 //      input might reach code paths you missed during testing, so it's hard
 //      to know you got this right), so this is not recommended unless you
 //      really know what are doing, and need to optimize every bit of speed
-//      and size.
+//      and size. '*' wildcard matching supported.
 //
 //      As with --asyncify-imports, you can use a response file here.
 //
 //   --pass-arg=asyncify-whitelist@name1,name2,name3
 //
 //      If the whitelist is provided, then only the functions in the list
 //      will be instrumented. Like the blacklist, getting this wrong will
-//      break your application.
+//      break your application. '*' wildcard matching supported.
 //
 //      As with --asyncify-imports, you can use a response file here.
 //
@@ -334,6 +334,65 @@ class GlobalHelper {
   std::map<Name, Type> rev;
 };
 
+class PatternMatcher {
+public:
+  std::string designation;
+  std::set<Name> names;
+  std::set<std::string> patterns;
+  std::set<std::string> patternsMatched;
+
+  PatternMatcher(std::string designation,
+                 Module& module,
+                 const String::Split& list)
+    : designation(designation) {
+    // The lists contain human-readable strings. Turn them into the
+    // internal escaped names for later comparisons
+    for (auto& name : list) {
+      auto escaped = WasmBinaryBuilder::escape(name);
+      if (name.find('*') != std::string::npos) {
+        patterns.insert(std::string(escaped.str));
+      } else {
+        auto* func = module.getFunctionOrNull(escaped);
+        if (!func) {
+          std::cerr << "warning: Asyncify " << designation
+                    << "list contained a non-existing function name: " << name
+                    << " (" << escaped << ")\n";
+        } else if (func->imported()) {
+          Fatal() << "Asyncify " << designation
+                  << "list contained an imported function name (use the import "
+                     "list for imports): "
+                  << name << '\n';
+        }
+        names.insert(escaped.str);
+      }
+    }
+  }
+
+  bool match(Name funcName) {
+    if (names.count(funcName) > 0) {
+      return true;
+    } else {
+      for (auto& pattern : patterns) {
+        if (String::wildcardMatch(pattern, funcName.str)) {
+          patternsMatched.insert(pattern);
+          return true;
+        }
+      }
+    }
+    return false;
+  }
+
+  void checkPatternsMatches() {
+    for (auto& pattern : patterns) {
+      if (patternsMatched.count(pattern) == 0) {
+        std::cerr << "warning: Asyncify " << designation
+                  << "list contained a non-matching pattern: " << pattern
+                  << "\n";
+      }
+    }
+  }
+};
+
 // Analyze the entire module to see which calls may change the state, that
 // is, start an unwind or rewind), either in itself or in something called
 // by it.
@@ -374,8 +433,8 @@ class ModuleAnalyzer {
     : module(module), canIndirectChangeState(canIndirectChangeState),
       globals(module), asserts(asserts) {
 
-    blacklist.insert(blacklistInput.begin(), blacklistInput.end());
-    whitelist.insert(whitelistInput.begin(), whitelistInput.end());
+    PatternMatcher blacklist("black", module, blacklistInput);
+    PatternMatcher whitelist("white", module, whitelistInput);
 
     // Scan to see which functions can directly change the state.
     // Also handle the asyncify imports, removing them (as we will implement
@@ -456,7 +515,7 @@ class ModuleAnalyzer {
     map.swap(scanner.map);
 
     // Functions in the blacklist are assumed to not change the state.
-    for (auto& name : blacklist) {
+    for (auto& name : blacklist.names) {
       if (auto* func = module.getFunctionOrNull(name)) {
         map[func].canChangeState = false;
       }
@@ -492,21 +551,24 @@ class ModuleAnalyzer {
       auto* func = work.pop();
       for (auto* caller : map[func].calledBy) {
         if (!map[caller].canChangeState && !map[caller].isBottomMostRuntime &&
-            !blacklist.count(caller->name)) {
+            !blacklist.match(caller->name)) {
           map[caller].canChangeState = true;
           work.push(caller);
         }
       }
     }
 
-    if (!whitelist.empty()) {
+    if (!whitelistInput.empty()) {
       // Only the functions in the whitelist can change the state.
       for (auto& func : module.functions) {
         if (!func->imported()) {
-          map[func.get()].canChangeState = whitelist.count(func->name) > 0;
+          map[func.get()].canChangeState = whitelist.match(func->name);
         }
       }
     }
+
+    blacklist.checkPatternsMatches();
+    whitelist.checkPatternsMatches();
   }
 
   bool needsInstrumentation(Function* func) {
@@ -566,8 +628,6 @@ class ModuleAnalyzer {
   }
 
   GlobalHelper globals;
-  std::set<Name> blacklist;
-  std::set<Name> whitelist;
   bool asserts;
 };
 
@@ -1138,28 +1198,6 @@ struct Asyncify : public Pass {
     blacklist = handleBracketingOperators(blacklist);
     whitelist = handleBracketingOperators(whitelist);
 
-    // The lists contain human-readable strings. Turn them into the internal
-    // escaped names for later comparisons
-    auto processList = [module](String::Split& list, const std::string& which) {
-      for (auto& name : list) {
-        auto escaped = WasmBinaryBuilder::escape(name);
-        auto* func = module->getFunctionOrNull(escaped);
-        if (!func) {
-          std::cerr << "warning: Asyncify " << which
-                    << "list contained a non-existing function name: " << name
-                    << " (" << escaped << ")\n";
-        } else if (func->imported()) {
-          Fatal() << "Asyncify " << which
-                  << "list contained an imported function name (use the import "
-                     "list for imports): "
-                  << name << '\n';
-        }
-        name = escaped.str;
-      }
-    };
-    processList(blacklist, "black");
-    processList(whitelist, "white");
-
     if (!blacklist.empty() && !whitelist.empty()) {
       Fatal() << "It makes no sense to use both a blacklist and a whitelist "
                  "with asyncify.";

src/support/string.h

@@ -87,17 +87,18 @@ inline String::Split handleBracketingOperators(String::Split split) {
   return ret;
 }
 
-// Does a simple wildcard match between a pattern and a value. Currently
-// supports a '*' at the end of the pattern.
+// Does a simple '*' wildcard match between a pattern and a value.
 inline bool wildcardMatch(const std::string& pattern,
                           const std::string& value) {
   for (size_t i = 0; i < pattern.size(); i++) {
+    if (pattern[i] == '*') {
+      return wildcardMatch(pattern.substr(i + 1), value.substr(i)) ||
+             (value.size() > 0 &&
+              wildcardMatch(pattern.substr(i), value.substr(i + 1)));
+    }
     if (i >= value.size()) {
       return false;
     }
-    if (pattern[i] == '*') {
-      return true;
-    }
     if (pattern[i] != value[i]) {
       return false;
     }

test/unit/test_asyncify.py

@@ -36,7 +36,12 @@ def test_asyncify_list_bad(self):
             ('--pass-arg=asyncify-whitelist@nonexistent', 'nonexistent'),
             ('--pass-arg=asyncify-blacklist@main', None),
             ('--pass-arg=asyncify-whitelist@main', None),
-            ('--pass-arg=asyncify-whitelist@main', None),
+            ('--pass-arg=asyncify-blacklist@m*n', None),
+            ('--pass-arg=asyncify-whitelist@m*n', None),
+            ('--pass-arg=asyncify-whitelist@main*', None),
+            ('--pass-arg=asyncify-whitelist@*main', None),
+            ('--pass-arg=asyncify-blacklist@non*existent', 'non*existent'),
+            ('--pass-arg=asyncify-whitelist@non*existent', 'non*existent'),
             ('--pass-arg=asyncify-whitelist@DOS_ReadFile(unsigned short, unsigned char*, unsigned short*, bool)', None),
         ]:
             print(arg, warning)