Skip to content

Commit

Permalink
Cap the maximum basic block padding in Cranelift (#6736)
Browse files Browse the repository at this point in the history
* Cap the maximum basic block padding in Cranelift

This commit is a fix for a fuzz failure that came in the other day where
a module took more than 2GB of memory to compile, triggering the fuzzer
to fail. Investigating locally it looks like the problem lies in the
basic-block padding added recently. Turning on that option increases the
memory usage of Cranelift from ~70M to ~2.5G. The setting in the test
was to add `1<<14` bytes of padding between all basic blocks, and with a
lot of basic blocks that can add up quite quickly.

The solution implemented here is to implement a per-function limit of
the amount of padding that can be injected. I've set it to 1MB for now
which is hopefully enough to continue to stress the various bits of the
`MachBuffer` while not blowing limits accidentally while fuzzing.

After this commit the fuzz test case in question jumps from 70M to 470M with
basic block padding enabled, which while still large is a bit more
modest and sticks within the limits of the fuzzer.

* Review feedback

* Fix typo

* Tweak some constants
  • Loading branch information
alexcrichton authored Jul 19, 2023
1 parent fe69c04 commit 475d1ba
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 10 deletions.
13 changes: 12 additions & 1 deletion cranelift/codegen/src/machinst/vcode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -827,10 +827,11 @@ impl<I: VCodeInst> VCode<I> {
}

let is_forward_edge_cfi_enabled = self.abi.is_forward_edge_cfi_enabled();
let bb_padding = match flags.bb_padding_log2_minus_one() {
let mut bb_padding = match flags.bb_padding_log2_minus_one() {
0 => Vec::new(),
n => vec![0; 1 << (n - 1)],
};
let mut total_bb_padding = 0;

for (block_order_idx, &block) in final_order.iter().enumerate() {
trace!("emitting block {:?}", block);
Expand Down Expand Up @@ -1054,9 +1055,19 @@ impl<I: VCodeInst> VCode<I> {

// Insert padding, if configured, to stress the `MachBuffer`'s
// relocation and island calculations.
//
// Padding can get quite large during fuzzing though so place a
// total cap on it where when a per-function threshold is exceeded
// the padding is turned back down to zero. This avoids a small-ish
// test case generating a GB+ memory footprint in Cranelift for
// example.
if !bb_padding.is_empty() {
buffer.put_data(&bb_padding);
buffer.align_to(I::LabelUse::ALIGN);
total_bb_padding += bb_padding.len();
if total_bb_padding > (150 << 20) {
bb_padding = Vec::new();
}
}
}

Expand Down
35 changes: 26 additions & 9 deletions crates/fuzzing/src/generators/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,15 +207,32 @@ impl Config {
}
}

// Allow at most 1<<16 == 64k of padding between basic blocks. If this
// gets too large then functions actually grow to 4G+ sizes which is not
// really that interesting to test as it's pretty unrealistic at this
// time.
unsafe {
cfg.cranelift_flag_set(
"bb_padding_log2_minus_one",
&(self.wasmtime.bb_padding_log2 & 0xf).to_string(),
);
// Try to configure cranelift to insert padding between basic blocks to
// stress code layout mechanisms within Cranelift. This padding can get
// unwieldy quickly, however, generating a 4G+ function which isn't
// particularly interesting at this time.
//
// To keep this setting under control there are a few limits in place:
//
// * Cranelift will generate at most 150M of padding per-function,
// regardless of how many basic blocks there are.
// * Here it's limited to enable this setting only when there's at most
// 10 functions to ensure that the overhead for all functions is <1G
// ish or otherwise doesn't run away.
// * The `bb_padding_log2_minus_one` setting isn't allowed to be
// larger than 26 as that allows for `1<<25` maximum padding size
// which is 32M.
//
// With all that combined this is intended to still be enabled,
// although perhaps not all the time, and stress enough interesting test
// cases in cranelift.
if self.module_config.config.max_funcs < 5 {
unsafe {
cfg.cranelift_flag_set(
"bb_padding_log2_minus_one",
&(self.wasmtime.bb_padding_log2 % 27).to_string(),
);
}
}

// Vary the memory configuration, but only if threads are not enabled.
Expand Down

0 comments on commit 475d1ba

Please sign in to comment.