Repository: ParkMyCar/compact_str
Pull request #52 (2022-02-11 23:38:43)

fuzz: Combine both fuzz targets into one
This PR combines both fuzz targets into one, where we now generate a random string via multiple methods, and then run a number of actions. It also adds a `CheckSubslice` action which will check a random subslice of both the `CompactStr` and control `String`


Patch:
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 197ff8a7..130407b7 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -72,8 +72,8 @@ jobs:
         run: |
           cargo miri test --all-features --manifest-path=compact_str/Cargo.toml
 
-  fuzz-creation:
-    name: fuzz creation
+  fuzz:
+    name: fuzz
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v2
@@ -91,28 +91,7 @@ jobs:
           version: latest
       - name: Fuzz!
         run: |
-          cargo fuzz run --debug-assertions creation -- -max_total_time=120
-
-  fuzz-modify:
-    name: fuzz modify
-    runs-on: ubuntu-latest
-    steps:
-      - uses: actions/checkout@v2
-        name: Checkout repo
-      - uses: actions-rs/toolchain@v1
-        name: Install Rust and friends
-        with:
-          profile: minimal
-          toolchain: nightly
-          override: true
-      - uses: actions-rs/install@v0.1
-        name: Install cargo-fuzz
-        with:
-          crate: cargo-fuzz
-          version: latest
-      - name: Fuzz!
-        run: |
-          cargo fuzz run --debug-assertions modify -- -max_total_time=120
+          cargo fuzz run --debug-assertions compact_str -- -max_total_time=120
 
   example-bytes:
     name: example - bytes
diff --git a/.github/workflows/fuzz.yml b/.github/workflows/fuzz.yml
index 36c38f77..471a17a4 100644
--- a/.github/workflows/fuzz.yml
+++ b/.github/workflows/fuzz.yml
@@ -6,8 +6,8 @@ on:
 name: Fuzz
 
 jobs:
-  creation_x86_64:
-    name: creation [x86_64]
+  fuzz_x86_64:
+    name: fuzz [x86_64]
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v2
@@ -27,30 +27,7 @@ jobs:
         name: Fuzz
         with:
           command: fuzz
-          args: run --debug-assertions creation -- -max_total_time=1200
-
-  modify_x86_64:
-    name: modify [x86_64]
-    runs-on: ubuntu-latest
-    steps:
-      - uses: actions/checkout@v2
-        name: Checkout compact_str
-      - uses: actions-rs/toolchain@v1
-        name: Install Rust
-        with:
-          profile: minimal
-          toolchain: nightly
-          override: true
-      - uses: actions-rs/install@v0.1
-        name: Install cargo-fuzz
-        with:
-          crate: cargo-fuzz
-          version: latest
-      - uses: actions-rs/cargo@v1
-        name: Fuzz
-        with:
-          command: fuzz
-          args: run --debug-assertions modify -- -max_total_time=1200
+          args: run --debug-assertions compact_str -- -max_total_time=1200
 
   # creation_aarch64:
   #   name: creation [aarch64]
diff --git a/compact_str/src/repr/iter.rs b/compact_str/src/repr/iter.rs
index f983d820..e25f470e 100644
--- a/compact_str/src/repr/iter.rs
+++ b/compact_str/src/repr/iter.rs
@@ -74,16 +74,11 @@ where
     String: core::iter::Extend<S>,
     String: FromIterator<S>,
 {
-    // If there are more strings than we can fit bytes inline, then immediately make a `HeapString`
-    let (size_hint, _) = iter.size_hint();
-    if size_hint > MAX_SIZE {
-        let heap = HeapString::from(iter.collect::<String>());
-        return Repr {
-            heap: ManuallyDrop::new(heap),
-        };
-    }
+    // Note: We don't check the lower bound here like we do in the character iterator because it's
+    // possible for the iterator to be full of empty strings! In which case checking the lower bound
+    // could cause us to heap allocate when there's no need.
 
-    // Otherwise, continuously pull strings from the iterator
+    // Continuously pull strings from the iterator
     let mut curr_len = 0;
     let mut inline_buf = [0u8; MAX_SIZE];
     while let Some(s) = iter.next() {
diff --git a/fuzz/Cargo.toml b/fuzz/Cargo.toml
index cc2402b3..c93594de 100644
--- a/fuzz/Cargo.toml
+++ b/fuzz/Cargo.toml
@@ -1,7 +1,7 @@
 [package]
 name = "compact_str-fuzz"
 version = "0.0.0"
-authors = ["Automatically generated"]
+authors = ["Parker Timmerman <parker@parkertimmerman.com>"]
 publish = false
 edition = "2018"
 
@@ -14,13 +14,7 @@ compact_str = { path = "../compact_str", features = ["bytes"] }
 libfuzzer-sys = "0.4"
 
 [[bin]]
-name = "creation"
-path = "fuzz_targets/creation.rs"
-test = false
-doc = false
-
-[[bin]]
-name = "modify"
-path = "fuzz_targets/modify.rs"
+name = "compact_str"
+path = "fuzz_targets/compact_str.rs"
 test = false
 doc = false
diff --git a/fuzz/fuzz_targets/compact_str.rs b/fuzz/fuzz_targets/compact_str.rs
new file mode 100644
index 00000000..cdcd0e8a
--- /dev/null
+++ b/fuzz/fuzz_targets/compact_str.rs
@@ -0,0 +1,18 @@
+#![no_main]
+
+use compact_str_fuzz::Scenario;
+use libfuzzer_sys::fuzz_target;
+
+fuzz_target!(|scenario: Scenario<'_>| {
+    // Given random creation method, if we can create a string
+    if let Some((mut compact, mut control)) = scenario.creation.create() {
+        // run some actions, asserting properties along the way
+        scenario
+            .actions
+            .into_iter()
+            .for_each(|a| a.perform(&mut control, &mut compact));
+
+        // make sure our strings are the same
+        assert_eq!(compact, control);
+    }
+});
diff --git a/fuzz/fuzz_targets/creation.rs b/fuzz/fuzz_targets/creation.rs
deleted file mode 100644
index bee2ea33..00000000
--- a/fuzz/fuzz_targets/creation.rs
+++ /dev/null
@@ -1,87 +0,0 @@
-#![no_main]
-#![allow(clippy::if_same_then_else)]
-
-use std::io::Cursor;
-
-use arbitrary::Arbitrary;
-use compact_str::CompactStr;
-use libfuzzer_sys::fuzz_target;
-
-const MAX_INLINE_LENGTH: usize = std::mem::size_of::<String>();
-
-#[derive(Arbitrary, Debug)]
-enum CreationMethod<'a> {
-    Bytes(&'a [u8]),
-    IterChar(Vec<char>),
-    IterString(Vec<String>),
-    Word(String),
-}
-
-fuzz_target!(|method: CreationMethod<'_>| {
-    use CreationMethod::*;
-
-    match method {
-        // Create a `CompactStr` from a `String`
-        Word(word) => {
-            let compact = CompactStr::new(&word);
-            assert_eq!(compact, word);
-
-            // assert the CompactStr is properly allocated
-            if word.len() <= MAX_INLINE_LENGTH {
-                assert!(!compact.is_heap_allocated());
-            } else {
-                assert!(compact.is_heap_allocated());
-            }
-        }
-        // Create a `CompactStr` from an iterator of `char`s
-        IterChar(chars) => {
-            let compact: CompactStr = chars.iter().collect();
-            let std_str: String = chars.iter().collect();
-
-            assert_eq!(compact, std_str);
-
-            // assert the CompactStr is properly allocated
-            if std_str.len() <= MAX_INLINE_LENGTH {
-                assert!(!compact.is_heap_allocated());
-            } else {
-                assert!(compact.is_heap_allocated());
-            }
-        }
-        // Create a `CompactStr` from an iterator of `String`s
-        IterString(strings) => {
-            let compact: CompactStr = strings.iter().map::<&str, _>(|s| s.as_ref()).collect();
-            let std_str: String = strings.iter().map::<&str, _>(|s| s.as_ref()).collect();
-
-            assert_eq!(compact, std_str);
-
-            // assert the CompactStr is properly allocated
-            if std_str.len() <= MAX_INLINE_LENGTH {
-                assert!(!compact.is_heap_allocated());
-            } else {
-                assert!(compact.is_heap_allocated());
-            }
-        }
-        // Create a `CompactStr` from a buffer of bytes
-        Bytes(data) => {
-            let mut buffer = Cursor::new(data);
-
-            let compact = CompactStr::from_utf8_buf(&mut buffer);
-            let std_str = std::str::from_utf8(data);
-
-            match (compact, std_str) {
-                (Ok(c), Ok(s)) => {
-                    assert_eq!(c, s);
-
-                    // assert the CompactStr is properly allocated
-                    if s.len() <= MAX_INLINE_LENGTH {
-                        assert!(!c.is_heap_allocated());
-                    } else {
-                        assert!(c.is_heap_allocated());
-                    }
-                }
-                (Err(c_err), Err(s_err)) => assert_eq!(c_err, s_err),
-                _ => panic!("CompactStr and core::str read UTF-8 differently?"),
-            }
-        }
-    }
-});
diff --git a/fuzz/fuzz_targets/modify.rs b/fuzz/fuzz_targets/modify.rs
deleted file mode 100644
index a7a5867e..00000000
--- a/fuzz/fuzz_targets/modify.rs
+++ /dev/null
@@ -1,79 +0,0 @@
-#![no_main]
-
-use arbitrary::Arbitrary;
-use compact_str::CompactStr;
-use libfuzzer_sys::fuzz_target;
-
-#[derive(Arbitrary, Debug)]
-struct Scenario<'a> {
-    word: String,
-    actions: Vec<Modification<'a>>,
-}
-
-fuzz_target!(|scenario: Scenario| {
-    let actions = scenario.actions;
-
-    let mut word = scenario.word;
-    let mut compact = CompactStr::new(&word);
-
-    actions
-        .into_iter()
-        .for_each(|a| a.perform(&mut word, &mut compact));
-});
-
-#[derive(Arbitrary, Debug)]
-enum Modification<'a> {
-    Push(char),
-    // Note: We use a `u16` to limit the number of pops
-    Pop(u16),
-    PushStr(&'a str),
-    ExtendChars(Vec<char>),
-    ExtendStr(Vec<&'a str>),
-}
-
-impl Modification<'_> {
-    pub fn perform(self, control: &mut String, compact: &mut CompactStr) {
-        use Modification::*;
-
-        match self {
-            Push(c) => {
-                control.push(c);
-                compact.push(c);
-
-                assert_eq!(control, compact);
-                assert_eq!(control.len(), compact.len());
-            }
-            Pop(count) => {
-                (0..count).for_each(|_| {
-                    let a = control.pop();
-                    let b = compact.pop();
-                    assert_eq!(a, b);
-                });
-                assert_eq!(control, compact);
-                assert_eq!(control.len(), compact.len());
-                assert_eq!(control.is_empty(), compact.is_empty());
-            }
-            PushStr(s) => {
-                control.push_str(s);
-                compact.push_str(s);
-
-                assert_eq!(control, compact);
-                assert_eq!(control.len(), compact.len());
-            }
-            ExtendChars(chs) => {
-                control.extend(chs.iter());
-                compact.extend(chs.iter());
-
-                assert_eq!(control, compact);
-                assert_eq!(control.len(), compact.len());
-            }
-            ExtendStr(strs) => {
-                control.extend(strs.iter().copied());
-                compact.extend(strs.iter().copied());
-
-                assert_eq!(control, compact);
-                assert_eq!(control.len(), compact.len());
-            }
-        }
-    }
-}
diff --git a/fuzz/src/lib.rs b/fuzz/src/lib.rs
new file mode 100644
index 00000000..212763e5
--- /dev/null
+++ b/fuzz/src/lib.rs
@@ -0,0 +1,179 @@
+use std::io::Cursor;
+
+use arbitrary::Arbitrary;
+use compact_str::CompactStr;
+
+const MAX_INLINE_LENGTH: usize = std::mem::size_of::<String>();
+
+/// A framework to generate a `CompactStr` and control `String`, and then run a series of actions
+/// and assert equality
+///
+/// Used for fuzz testing
+#[derive(Arbitrary, Debug)]
+pub struct Scenario<'a> {
+    pub creation: Creation<'a>,
+    pub actions: Vec<Action<'a>>,
+}
+
+#[derive(Arbitrary, Debug)]
+pub enum Creation<'a> {
+    Bytes(&'a [u8]),
+    IterChar(Vec<char>),
+    IterString(Vec<String>),
+    Word(String),
+}
+
+impl Creation<'_> {
+    pub fn create(self) -> Option<(CompactStr, String)> {
+        use Creation::*;
+
+        match self {
+            // Create a `CompactStr` from a `String`
+            Word(word) => {
+                let compact = CompactStr::new(&word);
+
+                assert_eq!(compact, word);
+                assert_properly_allocated(&compact, &word);
+
+                Some((compact, word))
+            }
+            // Create a `CompactStr` from an iterator of `char`s
+            IterChar(chars) => {
+                let compact: CompactStr = chars.iter().collect();
+                let std_str: String = chars.iter().collect();
+
+                assert_eq!(compact, std_str);
+                assert_properly_allocated(&compact, &std_str);
+
+                Some((compact, std_str))
+            }
+            // Create a `CompactStr` from an iterator of `String`s
+            IterString(strings) => {
+                let compact: CompactStr = strings.iter().map::<&str, _>(|s| s.as_ref()).collect();
+                let std_str: String = strings.iter().map::<&str, _>(|s| s.as_ref()).collect();
+
+                assert_eq!(compact, std_str);
+                assert_properly_allocated(&compact, &std_str);
+
+                Some((compact, std_str))
+            }
+            // Create a `CompactStr` from a buffer of bytes
+            Bytes(data) => {
+                let mut buffer = Cursor::new(data);
+
+                let compact = CompactStr::from_utf8_buf(&mut buffer);
+                let std_str = std::str::from_utf8(data);
+
+                match (compact, std_str) {
+                    // valid UTF-8
+                    (Ok(c), Ok(s)) => {
+                        assert_eq!(c, s);
+                        assert_properly_allocated(&c, s);
+
+                        Some((c, s.to_string()))
+                    }
+                    // non-valid UTF-8
+                    (Err(c_err), Err(s_err)) => {
+                        assert_eq!(c_err, s_err);
+                        None
+                    }
+                    _ => panic!("CompactStr and core::str read UTF-8 differently?"),
+                }
+            }
+        }
+    }
+}
+
+#[derive(Arbitrary, Debug)]
+pub enum Action<'a> {
+    Push(char),
+    // Note: We use a `u8` to limit the number of pops
+    Pop(u8),
+    PushStr(&'a str),
+    ExtendChars(Vec<char>),
+    ExtendStr(Vec<&'a str>),
+    CheckSubslice(u8, u8),
+}
+
+impl Action<'_> {
+    pub fn perform(self, control: &mut String, compact: &mut CompactStr) {
+        use Action::*;
+
+        match self {
+            // push a character
+            Push(c) => {
+                control.push(c);
+                compact.push(c);
+
+                assert_eq!(control, compact);
+                assert_eq!(control.len(), compact.len());
+            }
+            // pop `count` number of characters
+            Pop(count) => {
+                (0..count).for_each(|_| {
+                    let a = control.pop();
+                    let b = compact.pop();
+                    assert_eq!(a, b);
+                });
+                assert_eq!(control, compact);
+                assert_eq!(control.len(), compact.len());
+                assert_eq!(control.is_empty(), compact.is_empty());
+            }
+            // push a `&str`
+            PushStr(s) => {
+                control.push_str(s);
+                compact.push_str(s);
+
+                assert_eq!(control, compact);
+                assert_eq!(control.len(), compact.len());
+            }
+            // extend with a Iterator<Item = char>
+            ExtendChars(chs) => {
+                control.extend(chs.iter());
+                compact.extend(chs.iter());
+
+                assert_eq!(control, compact);
+                assert_eq!(control.len(), compact.len());
+            }
+            // extend with a Iterator<Item = &str>
+            ExtendStr(strs) => {
+                control.extend(strs.iter().copied());
+                compact.extend(strs.iter().copied());
+
+                assert_eq!(control, compact);
+                assert_eq!(control.len(), compact.len());
+            }
+            // check a subslice of bytes is equal
+            CheckSubslice(a, b) => {
+                assert_eq!(control.len(), compact.len());
+
+                // scale a, b to be [0, 1]
+                let c = a as f32 / u8::MAX as f32;
+                let d = b as f32 / u8::MAX as f32;
+
+                // scale c, b to be [0, compact.len()]
+                let e = (c * compact.len() as f32) as usize;
+                let f = (d * compact.len() as f32) as usize;
+
+                let lower = core::cmp::min(e, f);
+                let upper = core::cmp::max(e, f);
+
+                let control_slice = &control.as_bytes()[lower..upper];
+                let compact_slice = &compact.as_bytes()[lower..upper];
+
+                assert_eq!(control_slice, compact_slice);
+            }
+        }
+    }
+}
+
+/// Asserts the provided CompactStr is allocated properly either on the stack or on the heap, using
+/// a "control" `&str` for a reference length.
+fn assert_properly_allocated(compact: &CompactStr, control: &str) {
+    assert_eq!(compact.len(), control.len());
+    if control.len() <= MAX_INLINE_LENGTH {
+        assert!(!compact.is_heap_allocated());
+    } else {
+        assert!(compact.is_heap_allocated());
+    }
+}


Test patch:
diff --git a/compact_str/src/tests.rs b/compact_str/src/tests.rs
index d98baa6e..5b353f20 100644
--- a/compact_str/src/tests.rs
+++ b/compact_str/src/tests.rs
@@ -276,6 +276,19 @@ fn test_pop_empty() {
     assert_eq!(compact, "");
 }
 
+#[test]
+fn test_extend_from_empty_strs() {
+    let strs = vec![
+        "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "",
+        "", "",
+    ];
+    let compact: CompactStr = strs.clone().into_iter().collect();
+
+    assert_eq!(compact, "");
+    assert!(compact.is_empty());
+    assert!(!compact.is_heap_allocated());
+}
+
 #[test]
 fn test_compact_str_is_send_and_sync() {
     fn is_send_and_sync<T: Send + Sync>() {}