-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Fix fuzzer runner #19676
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
Fix fuzzer runner #19676
Conversation
We must take into account the calling convention of the tailcall vm.
b9ad891 to
b00ef4e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
Oh, however, opline->handler will not return immediately - it calls the next handler instead, so it will bypass the step limit.
I suggest to fetch the stepping-friendly handler func with zend_get_opcode_handler_func(), like in https://github.com/arnaud-lb/php-src/blob/73b98a385848f4800cf81392b09d648e8cd5a698/ext/opcache/jit/zend_jit_trace.c#L8921.
Okay done that now, but I wonder how expensive the hash+array lookup is :|
Is the step limit still necessary? Can't we use set_time_limit() or rely on the fuzzer's own time limit?
Otherwise, if the lookup is too slow, you can disable the TAILCALL VM by setting php_cv_preverve_none=no during configure.
Is the step limit still necessary? Can't we use set_time_limit() or rely on the fuzzer's own time limit?
I'm not sure, I suppose this is more graceful handling the way it is now. Are there scenarios when the time limit won't work (e.g. a bug somewhere causing infinite loops)?
Otherwise, if the lookup is too slow, you can disable the TAILCALL VM by setting php_cv_preverve_none=no during configure.
I compared the performance of TAILCALL+HT vs CALL+NO-HT. I found that after running a while on an i7-4790 I get around ~12000 exec/s for TAILCALL+HT and ~13500 exec/s for CALL+NO-HT.
We must take into account the calling convention of the tailcall vm.