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

Adjust fuzzing configs to match what's in QuickJS #836

Merged
merged 2 commits into from
Nov 19, 2024

Conversation

jeffcharles
Copy link
Collaborator

Description of the change

Adding some parameters to the Javy library crate's Config so we can set them to non-default values when fuzzing. Also updating the fuzzer to set those parameters. Also throwing any fuzzing inputs with more than 350 { characters.

Note that the fuzzer still crashes with out of memory errors with this change.

Why am I making this change?

We see out of memory errors when running the fuzzer continuously. This updates a few configs to match what's in QuickJS's fuzzer. I also don't try to run strings with more than 350 { characters because that trips the maximum stack size. We should consider investigating that further but I'd like to at least get the fuzzer running continuously.

Checklist

  • I've updated the relevant CHANGELOG files if necessary. Changes to javy-cli and javy-plugin do not require updating CHANGELOG files.
  • I've updated the relevant crate versions if necessary. Versioning policy for library crates
  • I've updated documentation including crate documentation if necessary.

Copy link
Member

@saulecabrera saulecabrera left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding the OOM -- I wonder if the usage of ManuallyDrop inside the runtime is at play here? Have you investigated removing its usage? What I'm getting at is that we never recycle objects that are no longer referenced inside the runtime since the context is behind a ManuallyDrop.

@jeffcharles
Copy link
Collaborator Author

jeffcharles commented Nov 19, 2024

We never drop the runtime while fuzzing since they're inside mutable statics so the drops on the rquickjs runtime and context will never run during fuzzing anyway.

I did try switching to building a new runtime every time and removing the manual drops and that seemed to slow down the memory growth but I was still seeing out of memory errors.

@jeffcharles
Copy link
Collaborator Author

Actually trying that again with this branch, I get a hard crash on Assertion failed: (list_empty(&rt->gc_obj_list)), function JS_FreeRuntime, file quickjs.c, line 2002.. I'm guessing that's from the JSON functions still being in the gc_obj_list since that was what I saw with dump-leaks on the other PR.

@jeffcharles jeffcharles merged commit bfd6176 into main Nov 19, 2024
7 checks passed
@jeffcharles jeffcharles deleted the jc.tweak-fuzzing-config branch November 19, 2024 22:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants