NomicFoundation / slang

Solidity compiler tooling by @NomicFoundation
https://nomicfoundation.github.io/slang/
MIT License
228 stars 20 forks source link

fuzz testing the parser #440

Open OmarTawfik opened 1 year ago

OmarTawfik commented 1 year ago

Start with fuzz testing, where you pick valid inputs, and insert random mutations to the tree, and see if the output of both parsers will differ. We need to consider negative cases as well. We need to make sure we reject the same set of inputs that solc rejects.

0xalpharush commented 1 year ago

I put together a fairly simple fuzzer here https://github.com/0xalpharush/slang-fuzz/. It found several crashes that I can share, but it'd probably be easiest to fix them one after another and rerun the fuzzer each time to avoid having to deduplicate them.

There's a lot more that can be done as Slang adds functionality (see https://github.com/astral-sh/ruff/pull/4822)

OmarTawfik commented 1 year ago

@0xalpharush thanks a lot for your effort, and for the information. I looked through your repository, and if I understand correctly, it looks like you are testing random inputs, without excluding incorrect ones. Currently our parse tree is only complete for correct inputs (files with no syntax errors). When it encounters errors, it returns the incomplete tree up until that error. I wonder if you saw incorrect roundtrip results for correct inputs?

As we are incrementally working on error recovery, such results will improve over time. It is an important goal, as IDEs often need to respond to queries on incomplete/partial input as the user is typing, and we want Slang to be usable for such cases.

0xalpharush commented 1 year ago

I wonder if you saw incorrect roundtrip results for correct inputs?

I do not believe so.

What I have thus far probably generates mostly invalid syntax but it causes panics (https://github.com/0xalpharush/slang-fuzz/issues/1) which would be undesirable in the IDE. As I said, fixing these will make it easier to continue fuzzing and add assertions for more interesting properties and better test case generation.

I figured this is lower hanging fruit to start with than tackling the problem of generating valid syntax and dealing with the overhead of running solc against the input. If this isn't useful, lmk and I can switch gears

OmarTawfik commented 1 year ago

We should never be panicking on invalid input! so this is really helpful. I copied it to #562 and will update that issue once the fix is out. Thank you for your effort! really appreciated!