fuzz: randomize block lowering order (#6254)

* fuzz: randomize block lowering order

Co-authored-by: Moritz Waser <mzrw.dev@pm.me>
Co-authored-by: Remo Senekowitsch <contact@remlse.dev>

* fix block lowering order randomization

Co-authored-by: Falk Zwimpfer <24669719+FalkZ@users.noreply.github.com>
Co-authored-by: Moritz Waser <mzrw.dev@pm.me>

* simplify control plane internals

Co-authored-by: Falk Zwimpfer <24669719+FalkZ@users.noreply.github.com>
Co-authored-by: Moritz Waser <mzrw.dev@pm.me>

* avoid unnecessary allocations

Co-authored-by: Falk Zwimpfer <24669719+FalkZ@users.noreply.github.com>
Co-authored-by: Moritz Waser <mzrw.dev@pm.me>

* remove unused change_order function

Co-authored-by: Falk Zwimpfer <24669719+FalkZ@users.noreply.github.com>
Co-authored-by: Moritz Waser <mzrw.dev@pm.me>

* add arbitrary 1.3.0 to cargo vet imports lock

Co-authored-by: Falk Zwimpfer <24669719+FalkZ@users.noreply.github.com>
Co-authored-by: Moritz Waser <mzrw.dev@pm.me>

* optimize ControlPlane::shuffle

Co-authored-by: Falk Zwimpfer <24669719+FalkZ@users.noreply.github.com>
Co-authored-by: Moritz Waser <mzrw.dev@pm.me>

* clarify shuffle being a noop without chaos mode

Co-authored-by: Falk Zwimpfer <24669719+FalkZ@users.noreply.github.com>
Co-authored-by: Moritz Waser <mzrw.dev@pm.me>

* reorder only direct successors of a block

Co-authored-by: Falk Zwimpfer <24669719+FalkZ@users.noreply.github.com>
Co-authored-by: Moritz Waser <mzrw.dev@pm.me>

* rename get_permutation -> shuffled

Co-authored-by: Falk Zwimpfer <24669719+FalkZ@users.noreply.github.com>
Co-authored-by: Moritz Waser <mzrw.dev@pm.me>

---------

Co-authored-by: Falk Zwimpfer <24669719+FalkZ@users.noreply.github.com>
Co-authored-by: Moritz Waser <mzrw.dev@pm.me>

Cargo.lock

@@ -92,9 +92,9 @@ checksum = "216261ddc8289130e551ddcd5ce8a064710c0d064a4d2895c67151c92b5443f6"
 
 [[package]]
 name = "arbitrary"
-version = "1.1.0"
+version = "1.3.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "c38b6b6b79f671c25e1a3e785b7b82d7562ffc9cd3efdc98627e5668a2472490"
+checksum = "e2d098ff73c1ca148721f37baad5ea6a465a13f9573aba8641fbbbae8164a54e"
 dependencies = [
  "derive_arbitrary",
 ]
@@ -1036,9 +1036,9 @@ dependencies = [
 
 [[package]]
 name = "derive_arbitrary"
-version = "1.1.0"
+version = "1.3.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "98e23c06c035dac87bd802d98f368df73a7f2cb05a66ffbd1f377e821fac4af9"
+checksum = "f3cdeb9ec472d588e539a818b2dee436825730da08ad0017c4b1a17676bdc8b7"
 dependencies = [
  "proc-macro2",
  "quote",

cranelift/codegen/src/isa/aarch64/mod.rs

@@ -58,11 +58,12 @@ impl AArch64Backend {
         &self,
         func: &Function,
         domtree: &DominatorTree,
+        ctrl_plane: &mut ControlPlane,
     ) -> CodegenResult<(VCode<inst::Inst>, regalloc2::Output)> {
         let emit_info = EmitInfo::new(self.flags.clone());
         let sigs = SigSet::new::<abi::AArch64MachineDeps>(func, &self.flags)?;
         let abi = abi::AArch64Callee::new(func, self, &self.isa_flags, &sigs)?;
-        compile::compile::<AArch64Backend>(func, domtree, self, abi, emit_info, sigs)
+        compile::compile::<AArch64Backend>(func, domtree, self, abi, emit_info, sigs, ctrl_plane)
     }
 }
 
@@ -74,7 +75,7 @@ impl TargetIsa for AArch64Backend {
         want_disasm: bool,
         ctrl_plane: &mut ControlPlane,
     ) -> CodegenResult<CompiledCodeStencil> {
-        let (vcode, regalloc_result) = self.compile_vcode(func, domtree)?;
+        let (vcode, regalloc_result) = self.compile_vcode(func, domtree, ctrl_plane)?;
 
         let emit_result = vcode.emit(
             &regalloc_result,

cranelift/codegen/src/isa/riscv64/mod.rs

@@ -58,11 +58,12 @@ impl Riscv64Backend {
         &self,
         func: &Function,
         domtree: &DominatorTree,
+        ctrl_plane: &mut ControlPlane,
     ) -> CodegenResult<(VCode<inst::Inst>, regalloc2::Output)> {
         let emit_info = EmitInfo::new(self.flags.clone(), self.isa_flags.clone());
         let sigs = SigSet::new::<abi::Riscv64MachineDeps>(func, &self.flags)?;
         let abi = abi::Riscv64Callee::new(func, self, &self.isa_flags, &sigs)?;
-        compile::compile::<Riscv64Backend>(func, domtree, self, abi, emit_info, sigs)
+        compile::compile::<Riscv64Backend>(func, domtree, self, abi, emit_info, sigs, ctrl_plane)
     }
 }
 
@@ -74,7 +75,7 @@ impl TargetIsa for Riscv64Backend {
         want_disasm: bool,
         ctrl_plane: &mut ControlPlane,
     ) -> CodegenResult<CompiledCodeStencil> {
-        let (vcode, regalloc_result) = self.compile_vcode(func, domtree)?;
+        let (vcode, regalloc_result) = self.compile_vcode(func, domtree, ctrl_plane)?;
 
         let want_disasm = want_disasm || log::log_enabled!(log::Level::Debug);
         let emit_result = vcode.emit(

cranelift/codegen/src/isa/s390x/mod.rs

@@ -58,11 +58,12 @@ impl S390xBackend {
         &self,
         func: &Function,
         domtree: &DominatorTree,
+        ctrl_plane: &mut ControlPlane,
     ) -> CodegenResult<(VCode<inst::Inst>, regalloc2::Output)> {
         let emit_info = EmitInfo::new(self.isa_flags.clone());
         let sigs = SigSet::new::<abi::S390xMachineDeps>(func, &self.flags)?;
         let abi = abi::S390xCallee::new(func, self, &self.isa_flags, &sigs)?;
-        compile::compile::<S390xBackend>(func, domtree, self, abi, emit_info, sigs)
+        compile::compile::<S390xBackend>(func, domtree, self, abi, emit_info, sigs, ctrl_plane)
     }
 }
 
@@ -75,7 +76,7 @@ impl TargetIsa for S390xBackend {
         ctrl_plane: &mut ControlPlane,
     ) -> CodegenResult<CompiledCodeStencil> {
         let flags = self.flags();
-        let (vcode, regalloc_result) = self.compile_vcode(func, domtree)?;
+        let (vcode, regalloc_result) = self.compile_vcode(func, domtree, ctrl_plane)?;
 
         let emit_result = vcode.emit(
             &regalloc_result,

cranelift/codegen/src/isa/x64/mod.rs

@@ -51,13 +51,14 @@ impl X64Backend {
         &self,
         func: &Function,
         domtree: &DominatorTree,
+        ctrl_plane: &mut ControlPlane,
     ) -> CodegenResult<(VCode<inst::Inst>, regalloc2::Output)> {
         // This performs lowering to VCode, register-allocates the code, computes
         // block layout and finalizes branches. The result is ready for binary emission.
         let emit_info = EmitInfo::new(self.flags.clone(), self.x64_flags.clone());
         let sigs = SigSet::new::<abi::X64ABIMachineSpec>(func, &self.flags)?;
         let abi = abi::X64Callee::new(func, self, &self.x64_flags, &sigs)?;
-        compile::compile::<Self>(func, domtree, self, abi, emit_info, sigs)
+        compile::compile::<Self>(func, domtree, self, abi, emit_info, sigs, ctrl_plane)
     }
 }
 
@@ -69,7 +70,7 @@ impl TargetIsa for X64Backend {
         want_disasm: bool,
         ctrl_plane: &mut ControlPlane,
     ) -> CodegenResult<CompiledCodeStencil> {
-        let (vcode, regalloc_result) = self.compile_vcode(func, domtree)?;
+        let (vcode, regalloc_result) = self.compile_vcode(func, domtree, ctrl_plane)?;
 
         let emit_result = vcode.emit(
             &regalloc_result,

cranelift/codegen/src/machinst/blockorder.rs

@@ -145,7 +145,11 @@ impl LoweredBlock {
 
 impl BlockLoweringOrder {
     /// Compute and return a lowered block order for `f`.
-    pub fn new(f: &Function, domtree: &DominatorTree) -> BlockLoweringOrder {
+    pub fn new(
+        f: &Function,
+        domtree: &DominatorTree,
+        ctrl_plane: &mut ControlPlane,
+    ) -> BlockLoweringOrder {
         trace!("BlockLoweringOrder: function body {:?}", f);
 
         // Step 1: compute the in-edge and out-edge count of every block.
@@ -195,7 +199,15 @@ impl BlockLoweringOrder {
 
             if block_out_count[block] > 1 {
                 let range = block_succ_range[block].clone();
-                for (succ_ix, lb) in block_succs[range].iter_mut().enumerate() {
+
+                // If chaos-mode is enabled in the control plane, iterate over
+                // the successors in an arbtirary order, which should have no
+                // impact on correctness. The order of the blocks is generally
+                // relevant: Uses must be seen before defs for dead-code
+                // elimination.
+                let succs = ctrl_plane.shuffled(block_succs[range].iter_mut().enumerate());
+
+                for (succ_ix, lb) in succs {
                     let succ = lb.orig_block().unwrap();
                     if block_in_count[succ] > 1 {
                         // Mutate the successor to be a critical edge, as `block` has multiple
@@ -364,7 +376,7 @@ mod test {
         cfg.compute(&func);
         let dom_tree = DominatorTree::with_function(&func, &cfg);
 
-        BlockLoweringOrder::new(&func, &dom_tree)
+        BlockLoweringOrder::new(&func, &dom_tree, &mut Default::default())
     }
 
     #[test]

cranelift/codegen/src/machinst/compile.rs

@@ -18,11 +18,12 @@ pub fn compile<B: LowerBackend + TargetIsa>(
     abi: Callee<<<B as LowerBackend>::MInst as MachInst>::ABIMachineSpec>,
     emit_info: <B::MInst as MachInstEmit>::Info,
     sigs: SigSet,
+    ctrl_plane: &mut ControlPlane,
 ) -> CodegenResult<(VCode<B::MInst>, regalloc2::Output)> {
     let machine_env = b.machine_env();
 
     // Compute lowered block order.
-    let block_order = BlockLoweringOrder::new(f, domtree);
+    let block_order = BlockLoweringOrder::new(f, domtree, ctrl_plane);
 
     // Build the lowering context.
     let lower = crate::machinst::Lower::new(f, machine_env, abi, emit_info, block_order, sigs)?;

cranelift/control/Cargo.toml

@@ -10,7 +10,7 @@ keywords = ["fuzz", "test"]
 edition.workspace = true
 
 [dependencies]
-arbitrary = { version = "1.1.0" }
+arbitrary = { version = "1.3.0" }
 
 [features]
 

cranelift/control/src/chaos.rs

@@ -1,16 +1,21 @@
-use arbitrary::Arbitrary;
+use arbitrary::{Arbitrary, Unstructured};
 
 /// The control plane of chaos mode.
 /// Please see the [crate-level documentation](crate).
 #[derive(Debug, Clone, Default)]
 pub struct ControlPlane {
-    data: Vec<bool>,
+    data: Vec<u8>,
+    /// This is used as a little optimization to avoid additional heap
+    /// allocations when using `Unstructured` internally. See the source of
+    /// [`ControlPlane::shuffle`] for an example.
+    tmp: Vec<u8>,
 }
 
 impl Arbitrary<'_> for ControlPlane {
     fn arbitrary<'a>(u: &mut arbitrary::Unstructured<'a>) -> arbitrary::Result<Self> {
         Ok(Self {
             data: u.arbitrary()?,
+            tmp: Vec::new(),
         })
     }
 }
@@ -23,6 +28,51 @@ impl ControlPlane {
     /// pseudo-random data is exhausted or the control plane was constructed
     /// with `default`.
     pub fn get_decision(&mut self) -> bool {
-        self.data.pop().unwrap_or_default()
+        self.data.pop().unwrap_or_default() & 1 == 1
+    }
+
+    /// Shuffles the items in the slice into a pseudo-random permutation if
+    /// the control plane was constructed with `arbitrary`.
+    ///
+    /// The default operation, to leave the slice unchanged, will always be
+    /// performed if the pseudo-random data is exhausted or the control
+    /// plane was constructed with `default`.
+    pub fn shuffle<T>(&mut self, slice: &mut [T]) {
+        if self.data.is_empty() {
+            return;
+        }
+        let mut u = Unstructured::new(&self.data);
+
+        // adapted from:
+        // https://docs.rs/arbitrary/1.3.0/arbitrary/struct.Unstructured.html#examples-1
+        let mut to_permute = &mut slice[..];
+
+        while to_permute.len() > 1 {
+            if let Ok(idx) = u.choose_index(to_permute.len()) {
+                to_permute.swap(0, idx);
+                to_permute = &mut to_permute[1..];
+            } else {
+                break;
+            }
+        }
+
+        // take remaining bytes
+        let rest = u.take_rest();
+        self.tmp.resize(rest.len(), 0); // allocates once per control plane
+        self.tmp.copy_from_slice(rest);
+        std::mem::swap(&mut self.data, &mut self.tmp);
+    }
+
+    /// Returns a new iterator over the same items as the input iterator in
+    /// a pseudo-random order if the control plane was constructed with
+    /// `arbitrary`.
+    ///
+    /// The default value, an iterator with an unchanged order, will always
+    /// be returned if the pseudo-random data is exhausted or the control
+    /// plane was constructed with `default`.
+    pub fn shuffled<T>(&mut self, iter: impl Iterator<Item = T>) -> impl Iterator<Item = T> {
+        let mut slice: Vec<_> = iter.collect();
+        self.shuffle(&mut slice);
+        slice.into_iter()
     }
 }

cranelift/control/src/zero_sized.rs

@@ -25,4 +25,18 @@ impl ControlPlane {
     pub fn get_decision(&mut self) -> bool {
         false
     }
+
+    /// Shuffles the items in the slice into a pseudo-random permutation.
+    /// This variant is used when chaos mode is disabled. It doesn't do
+    /// anything.
+    #[inline]
+    pub fn shuffle<T>(&mut self, _slice: &mut [T]) {}
+
+    /// Returns a new iterator over the same items as the input iterator in
+    /// a pseudo-random order. This variant is used when chaos mode is
+    /// disabled. It always returns the same order.
+    #[inline]
+    pub fn shuffled<T>(&mut self, iter: impl Iterator<Item = T>) -> impl Iterator<Item = T> {
+        iter
+    }
 }

supply-chain/imports.lock

@@ -1,6 +1,13 @@
 
 # cargo-vet imports lock
 
+[[publisher.arbitrary]]
+version = "1.3.0"
+when = "2023-03-13"
+user-id = 696
+user-login = "fitzgen"
+user-name = "Nick Fitzgerald"
+
 [[publisher.bumpalo]]
 version = "3.12.0"
 when = "2023-01-17"
@@ -8,6 +15,13 @@ user-id = 696
 user-login = "fitzgen"
 user-name = "Nick Fitzgerald"
 
+[[publisher.derive_arbitrary]]
+version = "1.3.0"
+when = "2023-03-13"
+user-id = 696
+user-login = "fitzgen"
+user-name = "Nick Fitzgerald"
+
 [[publisher.wasm-mutate]]
 version = "0.2.23"
 when = "2023-04-13"