fuzzilli

fuzzilli copied to clipboard

This PR allows generating functions of the form:

function v0([v1,v2,...v3]) {
   // instructions
}

v0([item1, item2, item3, item4])

I've learnt more about JS writing FuzzIL ops than writing actual JS code :D

The inflation of FunctionDefiniton operations. I suppose there'll be another PR for Object destructuring in parameters? I'm wondering if there's a sensible way to group some of these together under a single operation. Maybe the next part is related to this.

There's going to be a PR on Object destructing that I'm working on at the moment, it's based off of this PR. I was going to have two separate PRs so that the code review is a bit easier. Wrt to the inflation of BeginFunction ops, there were two strategies that I thought about:

1. Have a configuration enum that can have one of three values: Default, DestructArray, and DestructObject. We could stick this configuration enum in BeginAnyFunctionDefinition and then all the BeginFunction variants would derive from this. Similarly we could do the same for the variants as well and have a configuration enum such as Async, Generator, etc. I wasn't sure if this was the direction we wanted to go so I decided to stick with the current modelling. If you're happy with the enum approach, I'll re-write this PR. The one challenge with enums though is that we will need additional logic to prevent invalid configurations. For example if we have the DestructArray enum set then we should ignore or null out any property lists and if we have the DestructObject set then a property list must be provided or there must be a rest parameter in the signature.
2. The second approach was to stick with the current modelling and have two subclasses for each variant, one for DestructArray and another for DestructObject. This is the simpler approach but as you pointed out we end up with duplicating code.

With these new operations, the FunctonSignatures don't really make sense for these functions now I think, do they? Ideally, "from the outside" it'd be clear that the destructured parameter should be an array so that e.g. the generative/hybrid engine can produce useful code, while "from the inside" each parameter should have its own type (this'll be more important for Object destructuring).

I think it would still make sense when typing the input types (i.e. what you've described as "from the inside") but we would need a way for the signature to indicate, to potential function callers, that the function expects an array or an object. FunctionSignature doesn't support that now and I'm not sure if it's easier to encode that bit of information in the signature struct itself (perhaps an extension?).

I'm fine with merging as is and keeping a TODO. Let me know if you have any thoughts on this!

Let me know if you want me to re-write this using enums and if I should combine both destruct array and destruct object ops in one PR.

Cool! Yeah I think it makes sense to split array desctructuring and object destruturing into two separate PRs. I guess we'd eventually also want to be able to support something like

function foo([v1, v2, ...v3], v4, v5, {a, b}, v6) { ... }

i.e. mixed function definition with regular parameters, array destructuring, and object destructuring?

We could define additional "flag" types like these here https://github.com/googleprojectzero/fuzzilli/blob/cbac00cb49d24b455ea727b10061348be5fe8dcf/Sources/Fuzzilli/FuzzIL/TypeSystem.swift#L807 which are only used in FunctionSignatures and express that some parameter is a destructured Array, but I don't think it would really work with that, for example due to the different number of inputs to the function call and the outputs in the FunctionDefinition, but also because there's no way to encode the name of the property for object destructuring. Also, those flag types are kind of weird... However, I think it'd still be nice to be able to quickly write out FunctionSignatures like here: https://github.com/googleprojectzero/fuzzilli/blob/cbac00cb49d24b455ea727b10061348be5fe8dcf/Sources/Fuzzilli/Core/JavaScriptEnvironment.swift#L390 And the signature kind of seems like the natural place to express array/object destructuring as well. Then we'd also immediately be able to use array destructuring e.g. in class methods. So maybe we can make the FunctionSignature struct more powerful instead of moving the necessary enums/data structures into the FunctionDefinition operations. I guess we'd need to add a Parameter struct, make the FunctionSignature essentially a let inputs: [Parameter], let output: Type, then somehow express for every parameter whether

• It's part of a destructured array, and if it's a rest param
• It's part of a destructured object, and if so for which property name or whether it's a rest param

Now if we anyway do that, maybe we can later use the same mechanism to also encode

• If it's an optional parameter
• If it's a rest parameter

And then we can maybe get rid of these weird flag types completely: https://github.com/googleprojectzero/fuzzilli/blob/cbac00cb49d24b455ea727b10061348be5fe8dcf/Sources/Fuzzilli/FuzzIL/TypeSystem.swift#L803 I think that'd be pretty nice.

Maybe an enum would do for that:

enum Parameter {
    case plain(Type)
    case optional(Type)
    case rest
    case arrayDestruct(ArrayDestructDescriptor)
    case objectDestruct(ObjectDestructDescriptor)
}

And then those ArrayDestructDescriptor contain the information about which indices to use/skip and if there's a rest param.

Maybe that could work. WDYT?

I've split this into two PRs: #314 replaces inputTypes with paramTypes and gets rid of the additional flag types. This should be merged first. This PR adds array destructing as a new param type and is rebased off of #314

@saelo this PR has now been rebased and is ready for review :)

Yeah I think that would be neat improvement to the current generation that we have. However, I would prefer if we track this as a separate PR. I'm keen to complete the array destruct, object destruct and default param PRs before we take on the FunctionSig generator piece.

Ok that sounds good to me. So then we'd go with new CodeGenerators for now, but then remove them again once we have a generateFunctionSignature since we won't need them anymore then. Correct?

Indeed, that's what I'm aiming for.

I'm closing some seemingly stale PRs, but please reopen if you'd still like to merge this. Thanks!