Skip to content

Commit d52e23b

Browse files
authored
Cranelift/Wasmtime: disable fastalloc (single-pass) allocator for now. (#10554)
Unfortunately, as discovered by a recent fuzzbug [1], the single-pass register allocator is not compatible with the approach to callsite defs that exception-handling support has forced us to take. In particular, we needed to move all call return-value defs onto the call instruction itself, so calls could be terminators; this unbounded number of defs is made to be a solvable allocation problem by using `any` constraints, which allow allocation directly into spillslots; but fastalloc appears to error out if it runs out of registers, regardless of this constraint. Long-term, we should fix this, but unfortunately I don't have cycles to dive into fastalloc's internals at the moment, and it's (I think) a tier-3 feature. As such, this PR disables its use for now. I've filed a tracking issue in RA2 [2], and referenced this in the Cranelift configuration option docs at least. To keep from shifting all fuzzbugs / fuzzing corpii by altering the `arbitrary` interpretation, I opted to keep the enum the same in the fuzzing crate, and remap `SinglePass` to `Backtracking` there. I'm happy to take the other approach and remove the option (thus invalidating all fuzzbugs) if we'd prefer that instead. [1]: https://oss-fuzz.com/testcase-detail/5433312476987392 [2]: bytecodealliance/regalloc2#217
1 parent 366f320 commit d52e23b

File tree

8 files changed

+15
-26
lines changed

8 files changed

+15
-26
lines changed

cranelift/codegen/meta/src/shared/settings.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,12 @@ pub(crate) fn define() -> SettingGroup {
3838
3939
- `backtracking`: A backtracking allocator with range splitting; more expensive
4040
but generates better code.
41-
- `single_pass`: A single-pass algorithm that yields quick compilation but
42-
results in code with more register spills and moves.
41+
42+
Note that the `single_pass` option is currently disabled because it does not
43+
have adequate support for the kinds of allocations required by exception
44+
handling (https://github.com/bytecodealliance/regalloc2/issues/217).
4345
"#,
44-
vec!["backtracking", "single_pass"],
46+
vec!["backtracking"],
4547
);
4648

4749
settings.add_enum(

cranelift/codegen/src/machinst/compile.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,8 @@ pub fn compile<B: LowerBackend + TargetIsa>(
6666

6767
options.algorithm = match b.flags().regalloc_algorithm() {
6868
RegallocAlgorithm::Backtracking => Algorithm::Ion,
69-
RegallocAlgorithm::SinglePass => Algorithm::Fastalloc,
69+
// Note: single-pass is currently disabled
70+
// (https://github.com/bytecodealliance/regalloc2/issues/217).
7071
};
7172

7273
regalloc2::run(&vcode, vcode.machine_env(), &options)

crates/cli-flags/src/lib.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1082,7 +1082,6 @@ mod tests {
10821082
// Regalloc algorithm
10831083
for (regalloc_value, expected) in [
10841084
("\"backtracking\"", Some(RegallocAlgorithm::Backtracking)),
1085-
("\"single-pass\"", Some(RegallocAlgorithm::SinglePass)),
10861085
("\"hello\"", None), // should fail
10871086
("3", None), // should fail
10881087
("true", None), // should fail

crates/cli-flags/src/opt.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -486,7 +486,6 @@ impl WasmtimeOptionValue for wasmtime::RegallocAlgorithm {
486486
fn parse(val: Option<&str>) -> Result<Self> {
487487
match String::parse(val)?.as_str() {
488488
"backtracking" => Ok(wasmtime::RegallocAlgorithm::Backtracking),
489-
"single-pass" => Ok(wasmtime::RegallocAlgorithm::SinglePass),
490489
other => bail!(
491490
"unknown regalloc algorithm`{}`, only backtracking,single-pass accepted",
492491
other
@@ -497,7 +496,6 @@ impl WasmtimeOptionValue for wasmtime::RegallocAlgorithm {
497496
fn display(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
498497
match *self {
499498
wasmtime::RegallocAlgorithm::Backtracking => f.write_str("backtracking"),
500-
wasmtime::RegallocAlgorithm::SinglePass => f.write_str("single-pass"),
501499
_ => unreachable!(),
502500
}
503501
}

crates/fuzzing/src/generators/config.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -853,7 +853,13 @@ impl RegallocAlgorithm {
853853
fn to_wasmtime(&self) -> wasmtime::RegallocAlgorithm {
854854
match self {
855855
RegallocAlgorithm::Backtracking => wasmtime::RegallocAlgorithm::Backtracking,
856-
RegallocAlgorithm::SinglePass => wasmtime::RegallocAlgorithm::SinglePass,
856+
// Note: we have disabled `single_pass` for now because of
857+
// its limitations w.r.t. exception handling
858+
// (https://github.com/bytecodealliance/regalloc2/issues/217). To
859+
// avoid breaking all existing fuzzbugs by changing the
860+
// `arbitrary` mappings, we keep the `RegallocAlgorithm`
861+
// enum as it is and remap here to `Backtracking`.
862+
RegallocAlgorithm::SinglePass => wasmtime::RegallocAlgorithm::Backtracking,
857863
}
858864
}
859865
}

crates/wasmtime/src/config.rs

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -279,10 +279,9 @@ impl Config {
279279

280280
// When running under MIRI try to optimize for compile time of wasm
281281
// code itself as much as possible. Disable optimizations by
282-
// default and use the fastest regalloc available to us.
282+
// default.
283283
if cfg!(miri) {
284284
ret.cranelift_opt_level(OptLevel::None);
285-
ret.cranelift_regalloc_algorithm(RegallocAlgorithm::SinglePass);
286285
}
287286
}
288287

@@ -1255,7 +1254,6 @@ impl Config {
12551254
pub fn cranelift_regalloc_algorithm(&mut self, algo: RegallocAlgorithm) -> &mut Self {
12561255
let val = match algo {
12571256
RegallocAlgorithm::Backtracking => "backtracking",
1258-
RegallocAlgorithm::SinglePass => "single_pass",
12591257
};
12601258
self.compiler_config
12611259
.settings
@@ -2841,16 +2839,6 @@ pub enum RegallocAlgorithm {
28412839
/// results in better register utilization, producing fewer spills
28422840
/// and moves, but can cause super-linear compile runtime.
28432841
Backtracking,
2844-
/// Generates acceptable code very quickly.
2845-
///
2846-
/// This algorithm performs a single pass through the code,
2847-
/// guaranteed to work in linear time. (Note that the rest of
2848-
/// Cranelift is not necessarily guaranteed to run in linear time,
2849-
/// however.) It cannot undo earlier decisions, however, and it
2850-
/// cannot foresee constraints or issues that may occur further
2851-
/// ahead in the code, so the code may have more spills and moves as
2852-
/// a result.
2853-
SinglePass,
28542842
}
28552843

28562844
/// Select which profiling technique to support.

tests/all/cli_tests.rs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2200,15 +2200,11 @@ fn config_cli_flag() -> Result<()> {
22002200
br#"
22012201
[optimize]
22022202
opt-level = 2
2203-
regalloc-algorithm = "single-pass"
22042203
signals-based-traps = false
22052204
22062205
[codegen]
22072206
collector = "null"
22082207
2209-
[debug]
2210-
debug-info = true
2211-
22122208
[wasm]
22132209
max-wasm-stack = 65536
22142210

tests/all/stack_overflow.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,6 @@ fn big_stack_works_ok(config: &mut Config) -> Result<()> {
147147
// Disable cranelift optimizations to ensure that this test doesn't take too
148148
// long in debug mode due to the large size of its code.
149149
config.cranelift_opt_level(OptLevel::None);
150-
config.cranelift_regalloc_algorithm(RegallocAlgorithm::SinglePass);
151150
let engine = Engine::new(config)?;
152151

153152
let mut store = Store::new(&engine, ());

0 commit comments

Comments
 (0)