Skip to content

Commit

Permalink
Fix UB and add Miri to Github CI (petgraph#505)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
clubby789 authored and teuron committed Oct 9, 2022
1 parent 15e2aa3 commit 4d67669
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 10 deletions.
9 changes: 9 additions & 0 deletions .github/workflows/ci.yml
Expand Up @@ -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
2 changes: 1 addition & 1 deletion Cargo.toml
Expand Up @@ -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 }
Expand Down
13 changes: 7 additions & 6 deletions src/matrix_graph.rs
Expand Up @@ -853,15 +853,16 @@ fn extend_flat_square_matrix<T: Default>(
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() {
Expand Down
11 changes: 8 additions & 3 deletions tests/iso.rs
Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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));
}

Expand All @@ -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)]);
Expand All @@ -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)]);
Expand Down

0 comments on commit 4d67669

Please sign in to comment.