search

ise-uiuc / nnsmith

Automatic DNN generation for fuzzing and more
https://pypi.org/project/nnsmith/
Apache License 2.0
118 stars 25 forks source link

Finding a better solution instead of simply using `check_call` #3

Closed ganler closed 2 years ago

ganler commented 2 years ago

https://github.com/ise-uiuc/nnsmith/blob/2a487d58e276c080ef173b03a9b77c53311e6e20/nnsmith/graph_input_gen.py#L93

It is not a good workaround to use check_call here which:

We can discuss better solutions in this thread.

Previously my implementation makes the generation phase in-process which works well. It seems there are some issues regarding timeout and generation failure.

We can discuss more in this thread.

ganler commented 2 years ago

i am working on a fix to at least get rid of check_call by using fork and IPC. tho it might not have to be this complicated.

ganler commented 2 years ago

https://github.com/ise-uiuc/nnsmith/blob/main/nnsmith/graph_input_gen.py#L38

Made a commit to use real fork.

Still need to understand the reason of hangs to further improve this to a single-process mode for best efficiency and engineering convenience.

BTW, inconsistency in code is bad so we might want to stop using/migrate gen_model_and_range into the new one. @lazycal

lazycal commented 2 years ago

Though not sure what functionality you desire, one alternative I tried is to use multiprocess (you can toggle it by copy-pasting forked function from https://gist.github.com/schlamar/2311116#gistcomment-3932763 into util.py and uncomment this line https://github.com/ise-uiuc/nnsmith/blob/2a487d58e276c080ef173b03a9b77c53311e6e20/nnsmith/graph_input_gen.py#L28). This solves your initialization problem, not sure about the stateful thing though. I forfeit this solution main because of

  1. It is more difficult to reproduce a generation failure, since it is different from directly calling graph_gen.py.
  2. Preivously I found it still hanged, so I thought the forked process crashed and the main process failed to detect it (this is indeed possible based on that forked function)
  3. Now I think the hang issue is more likely because of z3. check_call is convenient in that it has a timeout option as well. Not sure about multiprocess.

So if what you meant by fork and IPC is what I described, then the 3 problems above may be worth noting.

ganler commented 2 years ago

@lazycal What I mean by "stateful" is that: say coverage guided fuzzing, we need to let the generation function in the subprocess know the coverage change. But it is not quite good to pass the coverage information through command lines (i.e., check_call)

ganler commented 2 years ago

@lazycal Yeah. my new implementation has the exact functionality of a new process. e.g., timeout, etc.

Check here. https://github.com/ise-uiuc/nnsmith/blob/main/nnsmith/graph_input_gen.py#L66

But it becomes much easier as you can simply dispatch a subprocess to execute a python function with real forking (e.g., copy-on-write, isolation, IPC, etc.).

ganler commented 2 years ago

Data communication can be done using shared variables so that we don't have to ship everything into the disk which is slow and inconvenient.

Check here: https://github.com/ise-uiuc/nnsmith/blob/main/nnsmith/graph_input_gen.py#L58

lazycal commented 2 years ago

Just read your code. It looks like you are doing the same thing as the forked function I cited, but yours fixes Problem 2 && 3. Cool! If we don't worry about reproducibility then it's fine.

ganler commented 2 years ago

Regarding reproducibility you mean seed? I think I also adapted that but not sure if it is accurately done.

ganler commented 2 years ago

BTW, it is important to accurately locate why it hangs.

If it is an issue of our operators, then we might need to come up with some ideas to solve it.

We should definitely report it to z3 community if you believe it is a bug by z3.

From my last hang experience, it is due to the operators (i.e., Reshape5D).

You can try to diagnose it by printing the constraint expressions and try to write some manual and simple z3 expressions including the patterns to see if it is slow.

Last time I tried "abcd = ef*g" which is very slow that I thought it was not an issue from z3 but the expressions themselves.

lazycal commented 2 years ago

Regarding reproducibility you mean seed? I think I also adapted that but not sure if it is accurately done.

Probably not beacause of seed, but z3's problem. When I tried the fork approach, I find there are some generation failures (timeout) and when I fed the same seed into graph_gen.py they usually got passed. (Note that I can reproduce for those successful generations, so it's not seed issue). Maybe it's because the import order or something? Or maybe it's no longer an issue. Let's see.

BTW, it is important to accurately locate why it hangs.

If it is an issue of our operators, then we might need to come up with some ideas to solve it.

We should definitely report it to z3 community if you believe it is a bug by z3.

From my last hang experience, it is due to the operators (i.e., Reshape5D).

You can try to diagnose it by printing the constraint expressions and try to write some manual and simple z3 expressions including the patterns to see if it is slow.

Last time I tried "a_b_c_d = e_f*g" which is very slow that I thought it was not an issue from z3 but the expressions themselves.

I think z3 is really not stable. I found many weird z3 problems before, so in my mind hang in z3 is not surprising and a must to be handled. That's also why I am more inclined to believe it's a z3 issue and didn't prioritize locating hangs... I will probably do it next week though.

ganler commented 2 years ago

I understood that z3 is unstable if we use some uncommon features (z3.set_param(...)) like setting timeout or so. If the hang still exists without the timeout setting, then I really doubt if it is the fault of z3 because I used to have such kind of feeling but eventually found that the root cause is something else. It is not likely that z3 will produce wrong results and unreasonable hanging under the simplest use case (or say if we don't set any other things).

I do think this is important to fix as any of our results will not be reliable if the code logic from ours or z3 is wrong. I will also help investigate this case to see if it is a z3 issue or some lagging operators.