Limit the number of frames in backtrace collection#12542
Limit the number of frames in backtrace collection#12542fitzgen merged 8 commits intobytecodealliance:mainfrom
Conversation
Add `Config::wasm_backtrace_max_frames` option to limit the number of frames collected in backtraces and set the default at 20. This helps prevent expensive work from very deep call stacks. Setting the value to 0 is the same as disabling backtraces and so this change deprecates `Config::wasm_backtrace`. Addresses bytecodealliance#5052
fitzgen
left a comment
There was a problem hiding this comment.
Thanks! A couple nitpicks inline below before we merge this.
Label Messager: wasmtime:configIt looks like you are changing Wasmtime's configuration options. Make sure to
DetailsTo modify this label's message, edit the To add new label messages or remove existing label messages, edit the |
|
Regarding the note on fuzzing - I guess we don't need to do anything because fuzzing always used the default of enabling backtraces and that isn't changing. |
|
@fitzgen Looks like there was a fuzz testing failure and a docs failure when merging. They should be fixed now. |
Subscribe to Label Actioncc @fitzgen DetailsThis issue or pull request has been labeled: "fuzzing", "wasmtime:api", "wasmtime:config"Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
fitzgen
left a comment
There was a problem hiding this comment.
Thanks! Just one more small thing before we land this. Thanks for your patience!
crates/fuzzing/src/oracles/stacks.rs
Outdated
| // Default configuration will only allow the host to see at most 20 frames. | ||
| let trace_limit = 20; |
There was a problem hiding this comment.
Rather than hard-coding a second copy of the default, which means this test will start failing if we ever change the default, can we explicitly set a limit and rely on that in the test here? Even better would be to generate an arbitrary limit as part of the wasmtime_fuzzing::generators::Stacks struct, so we fuzz different stack trace limits as well.
Should just require adding a pub limit: Option<NonZeroUsize>, field here:
wasmtime/crates/fuzzing/src/generators/stacks.rs
Lines 17 to 23 in f641800
And then adding something like let limit = NonZeroUsize::new(u.int_in_range(0..=256)?); inside here:
wasmtime/crates/fuzzing/src/generators/stacks.rs
Lines 40 to 47 in f641800
And finally creating the config with the limit and an engine from that config inside this current function, rather than using a default config and default engine:
let mut config = Config::new();
config.wasm_backtrace_max_frames(stacks.limit);
let engine = Engine::new(&config)?;There was a problem hiding this comment.
No problem, I made those changes.
|
Thanks @fitzgen |
Add
Config::wasm_backtrace_max_framesoption to limit the number of frames collected in backtraces and set the default at 20. This helps prevent expensive work from very deep call stacks.Setting the value to 0 is the same as disabling backtraces and so this change deprecates
Config::wasm_backtrace.Addresses #5052