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

Overhaul ufuzz.js for ES6 code #14

Open
fabiosantoscode opened this issue Apr 29, 2018 · 10 comments
Open

Overhaul ufuzz.js for ES6 code #14

fabiosantoscode opened this issue Apr 29, 2018 · 10 comments

Comments

@fabiosantoscode
Copy link
Collaborator

I think we need to do some work on ufuzz.js. It doesn't currently generate ES6 code, which could be useful.

@kzc what are your thoughts?

@kzc
Copy link
Contributor

kzc commented Apr 29, 2018

By all means extend ufuzz to support ES6+. I'd be happy to critique it.

Adding things like const, let, classes, class expressions and arrow functions would be fairly straightforward. Destructuring - in all its various forms (declaration, default values, in function parameters and destructuring assignment) could be interesting. Correctly exercising generators would be brutal, for example. The trick is to always generate valid code that produces a result - not as easy as one might think. The other think to watch out for is the frequency of generation of a specific ES6 feature so that a good representative mix of functionality is produced.

The best way to start would be to run uglifyjs -h ast and knock off each node type one at a time.

I'd suggest to avoid generating async/await code until the node VM sandbox can handle it. We currently have no way to run asynchronous tests due to the timeout problem. All tests are synchronous for this reason.

OT - Would it be possible to put such non-trivial work in pull requests? It's easier to review that way.

@kzc
Copy link
Contributor

kzc commented Apr 29, 2018

Expanding on the "representative mix of functionality" remark, we don't want to have destructuring in more than 5% of generated functions, for example. If a certain node type is too frequently generated it will cause various optimizations to be disabled such that it will not exercise the Compressor very well. I suppose a code coverage tool could gauge the effectiveness of ufuzz testing.

@kzc
Copy link
Contributor

kzc commented Apr 29, 2018

BTW, reducing failing ufuzz test cases is somewhat of a black art. It takes me 30 minutes on average per test case to whittle it down to 5 - 10 lines to see whether it's a legitimate minify code generation failure or a false positive. That's why care has to be taken in ufuzz design to always try to generate valid code producing a valid result to reduce the false positive frequency.

@fabiosantoscode
Copy link
Collaborator Author

I'm having fun and found a parser error already :) looks like you can't have a class or function as the body of a for loop without a block.

@kzc
Copy link
Contributor

kzc commented May 1, 2018

Would there be any merit to having an --ecma flag similar to acorn has to optionally restrict fuzzing to a certain ecma level?

@fabiosantoscode
Copy link
Collaborator Author

fabiosantoscode commented May 1, 2018

I think that flag might make the script a lot more complex, when it comes to choosing what to output next you'd have to have two lists of what's possible.


In other news, it looks like since classes make the code inside of them strict mode automatically, ufuzz is outputting a lot of errors now I've added them. This might be a while.

@kzc
Copy link
Contributor

kzc commented May 2, 2018

There's a strict flag - maybe that can help.

      case '--use-strict':
        use_strict = true;

@fabiosantoscode
Copy link
Collaborator Author

Thanks!

@nifgraup
Copy link
Contributor

I made a fuzzer that's able to catch parsing errors in ES6 code, #198. The idea comes from https://github.com/Rich-Harris/butternut/blob/master/test/fuzz-test.js.

@fabiosantoscode
Copy link
Collaborator Author

I like it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants