From a79795216de32fea21aee4ef84bf1733372b5cf5 Mon Sep 17 00:00:00 2001 From: Jeff Charles Date: Tue, 19 Nov 2024 16:48:52 -0500 Subject: [PATCH 1/2] Adjust fuzzing configs to match what's in QuickJS --- Cargo.lock | 2 +- Cargo.toml | 2 +- crates/javy/CHANGELOG.md | 4 ++++ crates/javy/Cargo.toml | 2 +- crates/javy/src/config.rs | 32 +++++++++++++++++++++++++ crates/javy/src/runtime.rs | 5 ++++ fuzz/fuzz_targets/json_differential.rs | 33 ++++++++++++++++++++++---- 7 files changed, 73 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9fa54d89..0ba84c1e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1533,7 +1533,7 @@ dependencies = [ [[package]] name = "javy" -version = "3.0.3-alpha.1" +version = "3.1.0-alpha.1" dependencies = [ "anyhow", "bitflags", diff --git a/Cargo.toml b/Cargo.toml index 4d7083ac..8c1e8a2f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,7 +23,7 @@ wasmtime = "23" wasmtime-wasi = "23" wasi-common = "23" anyhow = "1.0" -javy = { path = "crates/javy", version = "3.0.3-alpha.1" } +javy = { path = "crates/javy", version = "3.1.0-alpha.1" } tempfile = "3.13.0" uuid = { version = "1.11", features = ["v4"] } serde = { version = "1.0", default-features = false } diff --git a/crates/javy/CHANGELOG.md b/crates/javy/CHANGELOG.md index cd643474..6958999d 100644 --- a/crates/javy/CHANGELOG.md +++ b/crates/javy/CHANGELOG.md @@ -8,6 +8,10 @@ Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Added + +- `gc_threshold`, `memory_limit`, and `max_stack_size` properties for `Config`. + ## [3.0.2] - 2024-11-12 ### Changed diff --git a/crates/javy/Cargo.toml b/crates/javy/Cargo.toml index ee76d1de..7cda152c 100644 --- a/crates/javy/Cargo.toml +++ b/crates/javy/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "javy" -version = "3.0.3-alpha.1" +version = "3.1.0-alpha.1" authors.workspace = true edition.workspace = true license.workspace = true diff --git a/crates/javy/src/config.rs b/crates/javy/src/config.rs index 6521aca7..a3efbb29 100644 --- a/crates/javy/src/config.rs +++ b/crates/javy/src/config.rs @@ -59,6 +59,14 @@ pub struct Config { /// This setting requires the `JSON` intrinsic to be enabled, and the `json` /// crate feature to be enabled as well. pub(crate) simd_json_builtins: bool, + /// The threshold to trigger garbage collection. Default is usize::MAX. + pub(crate) gc_threshold: usize, + /// The limit on the max amount of memory the runtime will use. Default is + /// unlimited. + pub(crate) memory_limit: usize, + /// The limit on the max size of stack the runtime will use. Default is + /// 256 * 1024. + pub(crate) max_stack_size: usize, } impl Default for Config { @@ -71,6 +79,9 @@ impl Default for Config { javy_intrinsics: JavyIntrinsics::empty(), redirect_stdout_to_stderr: false, simd_json_builtins: false, + gc_threshold: usize::MAX, + memory_limit: usize::MAX, + max_stack_size: 256 * 1024, // from rquickjs } } } @@ -198,6 +209,27 @@ impl Config { self } + /// The number of bytes to use to trigger garbage collection. + /// The default is usize::MAX. + pub fn gc_threshold(&mut self, bytes: usize) -> &mut Self { + self.gc_threshold = bytes; + self + } + + /// The limit on the max amount of memory the runtime will use. Default is + /// unlimited. + pub fn memory_limit(&mut self, bytes: usize) -> &mut Self { + self.memory_limit = bytes; + self + } + + /// The limit on the max size of stack the runtime will use. Default is + /// 256 * 1024. + pub fn max_stack_size(&mut self, bytes: usize) -> &mut Self { + self.max_stack_size = bytes; + self + } + pub(crate) fn validate(self) -> Result { if self.simd_json_builtins && !self.intrinsics.contains(JSIntrinsics::JSON) { bail!("JSON Intrinsic is required to override JSON.parse and JSON.stringify"); diff --git a/crates/javy/src/runtime.rs b/crates/javy/src/runtime.rs index 1559666a..94edfd0f 100644 --- a/crates/javy/src/runtime.rs +++ b/crates/javy/src/runtime.rs @@ -54,6 +54,11 @@ impl Runtime { let cfg = cfg.validate()?; let intrinsics = &cfg.intrinsics; let javy_intrinsics = &cfg.javy_intrinsics; + + rt.set_gc_threshold(cfg.gc_threshold); + rt.set_memory_limit(cfg.memory_limit); + rt.set_max_stack_size(cfg.max_stack_size); + // We always set Random given that the principles around snapshotting and // random are applicable when using Javy from the CLI (the usage of // Wizer from the CLI is not optional). diff --git a/fuzz/fuzz_targets/json_differential.rs b/fuzz/fuzz_targets/json_differential.rs index 7abb0324..b30e93df 100644 --- a/fuzz/fuzz_targets/json_differential.rs +++ b/fuzz/fuzz_targets/json_differential.rs @@ -17,13 +17,19 @@ static SETUP: Once = Once::new(); fuzz_target!(|data: ArbitraryValue| { SETUP.call_once(|| { + let mut ref_config = Config::default(); + setup_config(&mut ref_config); + let mut config = Config::default(); + setup_config(&mut config); config.simd_json_builtins(true).javy_json(true); unsafe { RT = Some(Runtime::new(std::mem::take(&mut config)).expect("Runtime to be created")); - REF_RT = - Some(Runtime::new(Config::default()).expect("Reference runtime to be created")); + REF_RT = Some( + Runtime::new(std::mem::take(&mut ref_config)) + .expect("Reference runtime to be created"), + ); }; }); @@ -36,9 +42,18 @@ fn exec(data: &ArbitraryValue) -> Result<()> { let mut output: Option = None; let mut ref_output: Option = None; + let input = data.to_string(); + let brace_count = input.chars().filter(|&c| c == '{').count(); + // Higher numbers of braces tends to cause a stack overflow (more braces + // use more stack space). + if brace_count > 350 { + return Ok(()); + } + rt.context().with(|cx| { let globals = cx.globals(); - globals.set("INPUT", JSString::from_str(cx.clone(), &data.to_string())?)?; + + globals.set("INPUT", JSString::from_str(cx.clone(), &input)?)?; let result: Result<(), _> = cx.eval(JSON_PROGRAM); @@ -54,7 +69,7 @@ fn exec(data: &ArbitraryValue) -> Result<()> { ref_rt.context().with(|cx| { let globals = cx.globals(); - globals.set("INPUT", JSString::from_str(cx.clone(), &data.to_string())?)?; + globals.set("INPUT", JSString::from_str(cx.clone(), &input)?)?; let result: Result<(), _> = cx.eval(JSON_PROGRAM); @@ -72,3 +87,13 @@ fn exec(data: &ArbitraryValue) -> Result<()> { Ok(()) } + +fn setup_config(config: &mut Config) { + config + // https://github.com/bellard/quickjs/blob/6e2e68fd0896957f92eb6c242a2e048c1ef3cae0/quickjs.c#L1644 + .gc_threshold(256 * 1024) + // https://github.com/bellard/quickjs/blob/6e2e68fd0896957f92eb6c242a2e048c1ef3cae0/fuzz/fuzz_common.c#L33 + .memory_limit(0x4000000) + // https://github.com/bellard/quickjs/blob/6e2e68fd0896957f92eb6c242a2e048c1ef3cae0/fuzz/fuzz_common.c#L35 + .max_stack_size(0x10000); +} From 424c55436b478cba8bef6a3096e82b6bf7641662 Mon Sep 17 00:00:00 2001 From: Jeff Charles Date: Tue, 19 Nov 2024 17:19:08 -0500 Subject: [PATCH 2/2] Forgot we set the GC here. It gets overriden by the later call. --- crates/javy/src/runtime.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/crates/javy/src/runtime.rs b/crates/javy/src/runtime.rs index 94edfd0f..81011bb4 100644 --- a/crates/javy/src/runtime.rs +++ b/crates/javy/src/runtime.rs @@ -44,8 +44,6 @@ impl Runtime { pub fn new(config: Config) -> Result { let rt = ManuallyDrop::new(QRuntime::new()?); - // See comment above about configuring GC behaviour. - rt.set_gc_threshold(usize::MAX); let context = Self::build_from_config(&rt, config)?; Ok(Self { inner: rt, context }) }