Support more valid JS method names

With this change we support defining methods on classes and objects
with non-identifier names, like number and string literals.

Internally, all method names remain strings, reusing any type
information. At lifting, we approximate simple identifiers and
use them unquoted for method definition and for usage in dot
notation. For definitions, we also support quoted strings and
unquoted index values. At call sites, we ensure bracket notation
where needed, supporting index access without quotes.

This covers method names for plain objects and classes.
This does not cover properties, getters and setters yet.

We also add 2 custom method names to the environment that don't
follow the previous identifier naming.

Instructions that define such methods currently are:
ObjectLiteralMethod
ClassInstanceMethod
ClassStaticMethod

Instructions that use such methods are:
CallMethod
CallMethodWithSpread
CallSuperMethod
BindMethod

We ignore definitions and calls of private methods. They also reuse
the same typer logic, but naming rules are more strict here,
non-identifiers are not supported and should never be produced. We
need to separate now identifiers for private and other method names
in the JS environment.

This also extends the compiler to enable importing the new method
types.

Bug: 446634535
Change-Id: I2b8fbb8306e4b6bd901b61952c6da91d4210ae3f
Reviewed-on: https://chrome-internal-review.googlesource.com/c/v8/fuzzilli/+/9047716
Reviewed-by: Dominik Klemba <tacet@google.com>
Reviewed-by: Matthias Liedtke <mliedtke@google.com>
Commit-Queue: Michael Achenbach <machenbach@google.com>

Author: mi-ac

Sources/Fuzzilli/Base/ProgramBuilder.swift

@@ -487,6 +487,13 @@ public class ProgramBuilder {
         return chooseUniform(from: fuzzer.environment.customMethods)
     }
 
+    /// Returns a random custom private method name.
+    ///
+    /// As above but for private methods, where a # symbol will be prepended.
+    public func randomCustomPrivateMethodName() -> String {
+        return chooseUniform(from: fuzzer.environment.customPrivateMethods)
+    }
+
     /// Returns either a builtin or a custom method name, with equal probability.
     public func randomMethodName() -> String {
         return probability(0.5) ? randomBuiltinMethodName() : randomCustomMethodName()

Sources/Fuzzilli/CodeGen/CodeGenerators.swift

@@ -1236,8 +1236,8 @@ public let CodeGenerators: [CodeGenerator] = [
                 inContext: .single(.classDefinition),
                 provides: [.javascript, .subroutine, .method, .classMethod]
             ) { b in
-                // Try to find a private field that hasn't already been added to this class.
-                let methodName = b.generateString(b.randomCustomMethodName,
+                // Try to find a private method that hasn't already been added to this class.
+                let methodName = b.generateString(b.randomCustomPrivateMethodName,
                     notIn: b.currentClassDefinition.privateFields)
                 let parameters = b.randomParameters()
                 b.emit(
@@ -1273,8 +1273,8 @@ public let CodeGenerators: [CodeGenerator] = [
                 inContext: .single(.classDefinition),
                 provides: [.javascript, .subroutine, .method, .classMethod]
             ) { b in
-                // Try to find a private field that hasn't already been added to this class.
-                let methodName = b.generateString(b.randomCustomMethodName,
+                // Try to find a private method that hasn't already been added to this class.
+                let methodName = b.generateString(b.randomCustomPrivateMethodName,
                     notIn: b.currentClassDefinition.privateFields)
                 let parameters = b.randomParameters()
                 b.emit(

Sources/Fuzzilli/Compiler/Compiler.swift

@@ -170,8 +170,12 @@ public class JavaScriptCompiler {
                     } else {
                         head = emit(BeginClassInstanceMethod(methodName: name, parameters: parameters))
                     }
-                case .index:
-                    throw CompilerError.invalidNodeError("Not supported")
+                case .index(let index):
+                    if method.isStatic {
+                        head = emit(BeginClassStaticMethod(methodName: String(index), parameters: parameters))
+                    } else {
+                        head = emit(BeginClassInstanceMethod(methodName: String(index), parameters: parameters))
+                    }
                 case .expression:
                     if method.isStatic {
                         head = emit(BeginClassStaticComputedMethod(parameters: parameters), withInputs: [computedKeys.removeLast()])
@@ -197,7 +201,11 @@ public class JavaScriptCompiler {
                         emit(EndClassInstanceMethod())
                     }
                 case .index:
-                    throw CompilerError.invalidNodeError("Not supported")
+                    if method.isStatic {
+                        emit(EndClassStaticMethod())
+                    } else {
+                        emit(EndClassInstanceMethod())
+                    }
                 case .expression:
                     if method.isStatic {
                         emit(EndClassStaticComputedMethod())

Sources/Fuzzilli/Compiler/Parser/parser.js

@@ -492,8 +492,15 @@ function parse(script, proto) {
                         if (method.computed) {
                             out.expression = visitExpression(method.key);
                         } else {
-                            assert(method.key.type === 'Identifier', "Expected method.key.type to be exactly 'Identifier'")
-                            out.name = method.key.name;
+                            if (method.key.type === 'Identifier') {
+                                out.name = method.key.name;
+                            } else if (method.key.type === 'NumericLiteral') {
+                                out.name = String(method.key.value);
+                            } else if (method.key.type === 'StringLiteral') {
+                                out.name = method.key.value;
+                            } else {
+                                throw "Unknown method key type: " + method.key.type;
+                            }
                         }
 
                         field = {};

Sources/Fuzzilli/Environment/JavaScriptEnvironment.swift

@@ -290,11 +290,13 @@ public class JavaScriptEnvironment: ComponentBase {
 
     /// Identifiers that should be used for custom properties and methods.
     public static let CustomPropertyNames = ["a", "b", "c", "d", "e", "f", "g", "h"]
-    public static let CustomMethodNames = ["m", "n", "o", "p", "valueOf", "toString"]
+    public static let CustomMethodNames = ["m", "n", "o", "?", "67", "valueOf", "toString"]
+    public static let CustomPrivateMethodNames = ["m", "n", "o", "p"]
 
     public private(set) var builtins = Set<String>()
     public let customProperties = Set<String>(CustomPropertyNames)
     public let customMethods = Set<String>(CustomMethodNames)
+    public let customPrivateMethods = Set<String>(CustomPrivateMethodNames)
     public private(set) var builtinProperties = Set<String>()
     public private(set) var builtinMethods = Set<String>()
 
@@ -314,7 +316,35 @@ public class JavaScriptEnvironment: ComponentBase {
     private var producingProperties: [ILType: [(group: String, property: String)]] = [:]
     private var subtypes: [ILType: [ILType]] = [:]
 
+    private let validMethodDefinitionName: Regex<AnyRegexOutput>?
+    private let validDotNotationName: Regex<AnyRegexOutput>?
+    private let validPropertyIndex: Regex<AnyRegexOutput>?
+
     public init(additionalBuiltins: [String: ILType] = [:], additionalObjectGroups: [ObjectGroup] = [], additionalEnumerations: [ILType] = []) {
+        // A simple approximation of a valid JS identifier (used in dot
+        // notation and for method definitions).
+        let simpleId = "[_$a-zA-Z][_$a-zA-Z0-9]*"
+
+        // A non-negative integer (with no leading zero) for index access
+        // without quotes.
+        let index = "[1-9]\\d*|0"
+
+        // Initialize regular expressions to determine valid property
+        // identifiers:
+
+        // At method definition site, we support simple identifiers and
+        // non-negative numbers. Other names will be quoted.
+        // We don't support unquoted doubles.
+        self.validMethodDefinitionName = try! Regex("^(\(index)|\(simpleId))$")
+
+        // For dot notation we only support simple identifiers. We don't
+        // support all possible names according to JS spec.
+        // Unsupported names will be accessed with bracket notation.
+        self.validDotNotationName = try! Regex("^(\(simpleId))$")
+
+        // Valid indexes to use in bracket notation without quotes.
+        self.validPropertyIndex = try! Regex("^(\(index))$")
+
         super.init(name: "JavaScriptEnvironment")
 
         // Build model of the JavaScript environment
@@ -655,6 +685,7 @@ public class JavaScriptEnvironment: ComponentBase {
         logger.info("Have \(builtinMethods.count) builtin method names: \(builtinMethods.sorted())")
         logger.info("Have \(customProperties.count) custom property names: \(customProperties.sorted())")
         logger.info("Have \(customMethods.count) custom method names: \(customMethods.sorted())")
+        logger.info("Have \(customPrivateMethods.count) custom private method names: \(customPrivateMethods.sorted())")
     }
 
     func checkConstructorAvailability() {
@@ -936,6 +967,18 @@ public class JavaScriptEnvironment: ComponentBase {
             $0 || type.Is($1)
         }
     }
+
+    func isValidNameForMethodDefinition(_ name: String) -> Bool {
+        return (try? validMethodDefinitionName?.wholeMatch(in: name)) != nil
+    }
+
+    func isValidDotNotationName(_ name: String) -> Bool {
+        return (try? validDotNotationName?.wholeMatch(in: name)) != nil
+    }
+
+    func isValidPropertyIndex(_ name: String) -> Bool {
+        return (try? validPropertyIndex?.wholeMatch(in: name)) != nil
+    }
 }
 
 /// A struct to encapsulate property and method type information for a group of related objects.

Sources/Fuzzilli/Lifting/JavaScriptLifter.swift

@@ -34,7 +34,7 @@ public class JavaScriptLifter: Lifter {
     let version: ECMAScriptVersion
 
     /// This environment is used if we need to re-type a program before we compile Wasm code.
-    private var environment: JavaScriptEnvironment?
+    private var environment: JavaScriptEnvironment
 
     /// Counter to assist the lifter in detecting nested CodeStrings
     private var codeStringNestingLevel = 0
@@ -78,7 +78,7 @@ public class JavaScriptLifter: Lifter {
     public init(prefix: String = "",
                 suffix: String = "",
                 ecmaVersion: ECMAScriptVersion,
-                environment: JavaScriptEnvironment? = nil,
+                environment: JavaScriptEnvironment,
                 alwaysEmitVariables: Bool = false) {
         self.prefix = prefix
         self.suffix = suffix
@@ -131,7 +131,7 @@ public class JavaScriptLifter: Lifter {
 
         if needToSupportWasm {
             // If we need to support Wasm we need to type all instructions outside of Wasm such that the WasmLifter can access extra type information during lifting.
-            typer = JSTyper(for: environment!)
+            typer = JSTyper(for: environment)
         }
 
         var w = JavaScriptWriter(analyzer: analyzer, version: version, stripComments: !options.contains(.includeComments), includeLineNumbers: options.contains(.includeLineNumbers), alwaysEmitVariables: alwaysEmitVariables)
@@ -409,7 +409,7 @@ public class JavaScriptLifter: Lifter {
             case .beginObjectLiteralMethod(let op):
                 let vars = w.declareAll(instr.innerOutputs.dropFirst(), usePrefix: "a")
                 let PARAMS = liftParameters(op.parameters, as: vars)
-                let METHOD = op.methodName
+                let METHOD = quoteMethodDefinitionIfNeeded(op.methodName)
                 currentObjectLiteral.beginMethod("\(METHOD)(\(PARAMS)) {", &w)
                 bindVariableToThis(instr.innerOutput(0))
 
@@ -526,7 +526,7 @@ public class JavaScriptLifter: Lifter {
             case .beginClassInstanceMethod(let op):
                 let vars = w.declareAll(instr.innerOutputs.dropFirst(), usePrefix: "a")
                 let PARAMS = liftParameters(op.parameters, as: vars)
-                let METHOD = op.methodName
+                let METHOD = quoteMethodDefinitionIfNeeded(op.methodName)
                 w.emit("\(METHOD)(\(PARAMS)) {")
                 w.enterNewBlock()
                 bindVariableToThis(instr.innerOutput(0))
@@ -596,7 +596,7 @@ public class JavaScriptLifter: Lifter {
             case .beginClassStaticMethod(let op):
                 let vars = w.declareAll(instr.innerOutputs.dropFirst(), usePrefix: "a")
                 let PARAMS = liftParameters(op.parameters, as: vars)
-                let METHOD = op.methodName
+                let METHOD = quoteMethodDefinitionIfNeeded(op.methodName)
                 w.emit("static \(METHOD)(\(PARAMS)) {")
                 w.enterNewBlock()
                 bindVariableToThis(instr.innerOutput(0))
@@ -981,14 +981,14 @@ public class JavaScriptLifter: Lifter {
 
             case .callMethod(let op):
                 let obj = input(0)
-                let method = MemberExpression.new() + obj + "." + op.methodName
+                let method = MemberExpression.new() + obj + (liftMemberAccess(op.methodName))
                 let args = inputs.dropFirst()
                 let expr = CallExpression.new() + method + "(" + liftCallArguments(args) + ")"
                 w.assign(expr, to: instr.output)
 
             case .callMethodWithSpread(let op):
                 let obj = input(0)
-                let method = MemberExpression.new() + obj + "." + op.methodName
+                let method = MemberExpression.new() + obj + (liftMemberAccess(op.methodName))
                 let args = inputs.dropFirst()
                 let expr = CallExpression.new() + method + "(" + liftCallArguments(args, spreading: op.spreads) + ")"
                 w.assign(expr, to: instr.output)
@@ -1152,7 +1152,8 @@ public class JavaScriptLifter: Lifter {
                 w.emit("\(EXPR);")
 
             case .callSuperMethod(let op):
-                let expr = CallExpression.new() + "super.\(op.methodName)(" + liftCallArguments(inputs)  + ")"
+                let method = MemberExpression.new() + "super" + liftMemberAccess(op.methodName)
+                let expr = CallExpression.new() + method + "(" + liftCallArguments(inputs)  + ")"
                 w.assign(expr, to: instr.output)
 
             case .getPrivateProperty(let op):
@@ -1511,9 +1512,9 @@ public class JavaScriptLifter: Lifter {
 
             case .bindMethod(let op):
                 let V = w.declare(instr.output)
-                let OBJECT = input(0)
                 let LET = w.varKeyword
-                w.emit("\(LET) \(V) = Function.prototype.call.bind(\(OBJECT).\(op.methodName));")
+                let method = MemberExpression.new() + input(0) + liftMemberAccess(op.methodName)
+                w.emit("\(LET) \(V) = Function.prototype.call.bind(\(method));")
 
             case .bindFunction(_):
                 let V = w.declare(instr.output)
@@ -1875,6 +1876,21 @@ public class JavaScriptLifter: Lifter {
         }
     }
 
+    func quoteMethodDefinitionIfNeeded(_ name: String) -> String {
+        if environment.isValidNameForMethodDefinition(name) {
+            return name
+        }
+        return "\"\(name)\""
+    }
+
+    private func liftMemberAccess(_ name: String) -> String {
+        if environment.isValidDotNotationName(name){
+            return "." + name
+        }
+        let safeName = environment.isValidPropertyIndex(name) ? name : "\"\(name)\""
+        return "[" + safeName + "]"
+    }
+
     private func liftParameters(_ parameters: Parameters, as variables: [String]) -> String {
         assert(parameters.count == variables.count)
         var paramList = [String]()

Tests/FuzzilliTests/CompilerTests.swift

@@ -39,7 +39,7 @@ class CompilerTests: XCTestCase {
 
         let compiler = JavaScriptCompiler()
 
-        let lifter = JavaScriptLifter(ecmaVersion: .es6)
+        let lifter = JavaScriptLifter(ecmaVersion: .es6, environment: JavaScriptEnvironment())
 
         for testcasePath in enumerateAllTestcases() {
             let testName = URL(fileURLWithPath: testcasePath).lastPathComponent

Tests/FuzzilliTests/CompilerTests/computed_and_indexed_properties.js

@@ -15,6 +15,18 @@ console.log("Computed object method");
   console.log(obj.theAnswerIs());
 })();
 
+console.log("Indexed object method");
+(() => {
+  const obj = { 67() { return 42; } };
+  console.log(obj[67]());
+})();
+
+console.log("String literal object method");
+(() => {
+  const obj = { "?"() { return 42; } };
+  console.log(obj["?"]());
+})();
+
 console.log("Computed class property (field)");
 (() => {
   function classify (name) {
@@ -136,7 +148,6 @@ console.log("Indexed static class property (field)");
   console.log(C[42]);
 })();
 
-/*
 console.log("Indexed class property (method)");
 (() => {
   class C {
@@ -148,9 +159,7 @@ console.log("Indexed class property (method)");
   const c = new C();
   console.log(c[42]());
 })();
-*/
 
-/*
 console.log("Indexed static class property (method)");
 (() => {
   class C {
@@ -161,7 +170,6 @@ console.log("Indexed static class property (method)");
   }
   console.log(C[42]());
 })();
-*/
 
 /*
 console.log("Indexed class property (getter/setter)");
@@ -319,24 +327,24 @@ console.log("String-literal static class property (field)");
 console.log("String-literal class property (method)");
 (() => {
   class C {
-    "theAnswerIs"() {
+    "theAnswerIs?"() {
       console.log("Heavy calculations");
       return 42;
     }
   }
   const c = new C();
-  console.log(c.theAnswerIs());
+  console.log(c["theAnswerIs?"]());
 })();
 
 console.log("String-literal static class property (method)");
 (() => {
   class C {
-    static "theAnswerIs"() {
+    static "theAnswerIs?"() {
       console.log("Heavy calculations");
       return 42;
     }
   }
-  console.log(C.theAnswerIs());
+  console.log(C["theAnswerIs?"]());
 })();
 
 /*