From 4d6766968d2549a287a8b6cc6b059b864ab54ee3 Mon Sep 17 00:00:00 2001 From: clubby789 Date: Tue, 27 Sep 2022 14:51:02 +0100 Subject: [PATCH] Fix UB and add Miri to Github CI (#505) * Fix UB reported by Miri by splitting a slice * Add Miri to GitHub CI Disables some tests that take too long to run while under Miri, and adds `features = ["std"]` to the `indexmap` dependency so it compiles correctly under Miri. * Re-add removed comment * Remove unsafe version and enable a test in Miri * Switch back to copy_nonoverlapping --- .github/workflows/ci.yml | 9 +++++++++ Cargo.toml | 2 +- src/matrix_graph.rs | 13 +++++++------ tests/iso.rs | 11 ++++++++--- 4 files changed, 25 insertions(+), 10 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8fb6487a4..fae7a44e1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -72,3 +72,12 @@ jobs: - name: Rustfmt if: matrix.rustfmt run: cargo fmt -- --check + + miri: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + - uses: dtolnay/rust-toolchain@nightly + with: + components: miri + - run: cargo miri test diff --git a/Cargo.toml b/Cargo.toml index e45220218..24ee6c57c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -36,7 +36,7 @@ debug = true [dependencies] fixedbitset = { version = "0.4.0", default-features = false } -indexmap = { version = "1.7" } +indexmap = { version = "1.7", features = ["std"] } quickcheck = { optional = true, version = "0.8", default-features = false } serde = { version = "1.0", optional = true } serde_derive = { version = "1.0", optional = true } diff --git a/src/matrix_graph.rs b/src/matrix_graph.rs index 8b80f26c3..06f37b5bd 100644 --- a/src/matrix_graph.rs +++ b/src/matrix_graph.rs @@ -853,15 +853,16 @@ fn extend_flat_square_matrix( for c in (1..old_node_capacity).rev() { let pos = c * old_node_capacity; let new_pos = c * new_node_capacity; - // Move the slices directly if they do not overlap with their new position if pos + old_node_capacity <= new_pos { - let old = node_adjacencies[pos..pos + old_node_capacity].as_mut_ptr(); - let new = node_adjacencies[new_pos..new_pos + old_node_capacity].as_mut_ptr(); - - // SAFE: new starts at least `old_node_capacity` positions after old. + debug_assert!(pos + old_node_capacity < node_adjacencies.len()); + debug_assert!(new_pos + old_node_capacity < node_adjacencies.len()); + let ptr = node_adjacencies.as_mut_ptr(); + // SAFETY: pos + old_node_capacity <= new_pos, so this won't overlap unsafe { - std::ptr::swap_nonoverlapping(old, new, old_node_capacity); + let old = ptr.add(pos); + let new = ptr.add(new_pos); + core::ptr::swap_nonoverlapping(old, new, old_node_capacity); } } else { for i in (0..old_node_capacity).rev() { diff --git a/tests/iso.rs b/tests/iso.rs index f808830e4..13eb1ca3d 100644 --- a/tests/iso.rs +++ b/tests/iso.rs @@ -294,6 +294,7 @@ fn full_iso() { } #[test] +#[cfg_attr(miri, ignore = "Takes too long to run in Miri")] fn praust_dir_no_iso() { let a = str_to_digraph(PRAUST_A); let b = str_to_digraph(PRAUST_B); @@ -302,6 +303,7 @@ fn praust_dir_no_iso() { } #[test] +#[cfg_attr(miri, ignore = "Takes too long to run in Miri")] fn praust_undir_no_iso() { let a = str_to_graph(PRAUST_A); let b = str_to_graph(PRAUST_B); @@ -465,15 +467,16 @@ fn iso_matching() { #[test] fn iso_100n_100e() { - let g0 = graph_from_file("tests/res/graph_100n_100e.txt"); - let g1 = graph_from_file("tests/res/graph_100n_100e_iso.txt"); + let g0 = str_to_digraph(include_str!("res/graph_100n_100e.txt")); + let g1 = str_to_digraph(include_str!("res/graph_100n_100e_iso.txt")); assert!(petgraph::algo::is_isomorphic(&g0, &g1)); } #[test] +#[cfg_attr(miri, ignore = "Too large for Miri")] fn iso_large() { let g0 = graph_from_file("tests/res/graph_1000n_1000e.txt"); - let g1 = graph_from_file("tests/res/graph_1000n_1000e_iso.txt"); + let g1 = graph_from_file("tests/res/graph_1000n_1000e.txt"); assert!(petgraph::algo::is_isomorphic(&g0, &g1)); } @@ -489,6 +492,7 @@ fn iso_multigraph_failure() { } #[test] +#[cfg_attr(miri, ignore = "Takes too long to run in Miri")] fn iso_subgraph() { let g0 = Graph::<(), ()>::from_edges(&[(0, 1), (1, 2), (2, 0)]); let g1 = Graph::<(), ()>::from_edges(&[(0, 1), (1, 2), (2, 0), (2, 3), (0, 4)]); @@ -497,6 +501,7 @@ fn iso_subgraph() { } #[test] +#[cfg_attr(miri, ignore = "Takes too long to run in Miri")] fn iter_subgraph() { let a = Graph::<(), ()>::from_edges(&[(0, 1), (1, 2), (2, 0)]); let b = Graph::<(), ()>::from_edges(&[(0, 1), (1, 2), (2, 0), (2, 3), (0, 4)]);