Merge pull request #33 from alexcrichton/miri-and-fuzz-and-update

Perform some long-needed maintenance here

Author: alexcrichton

.github/workflows/main.yml

@@ -9,7 +9,7 @@ jobs:
       matrix:
         rust: [stable, beta, nightly]
     steps:
-    - uses: actions/checkout@master
+    - uses: actions/checkout@v4
     - name: Install Rust (
       run: rustup update ${{ matrix.rust }} && rustup default ${{ matrix.rust }}
     - run: cargo test
@@ -24,7 +24,7 @@ jobs:
     name: Rustfmt
     runs-on: ubuntu-latest
     steps:
-    - uses: actions/checkout@master
+    - uses: actions/checkout@v4
     - name: Install Rust
       run: rustup update stable && rustup default stable && rustup component add rustfmt
     - run: cargo fmt -- --check
@@ -33,7 +33,7 @@ jobs:
     name: WebAssembly
     runs-on: ubuntu-latest
     steps:
-    - uses: actions/checkout@master
+    - uses: actions/checkout@v4
     - name: Install Rust
       run: rustup update stable && rustup default stable && rustup target add wasm32-unknown-unknown
     - run: cargo build --target wasm32-unknown-unknown
@@ -43,7 +43,32 @@ jobs:
     name: external-platform
     runs-on: ubuntu-latest
     steps:
-    - uses: actions/checkout@master
+    - uses: actions/checkout@v4
     - name: Install Rust
       run: rustup update stable && rustup default stable && rustup target add x86_64-fortanix-unknown-sgx
     - run: cargo build --target x86_64-fortanix-unknown-sgx
+
+  fuzz:
+    name: Build Fuzzers
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@v4
+    - name: Install Rust
+      run: rustup update nightly && rustup default nightly
+    - run: cargo install cargo-fuzz
+    - run: cargo fuzz build --dev
+
+  miri:
+    name: Miri
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@v4
+    - name: Install Miri
+      run: |
+        rustup toolchain install nightly --component miri
+        rustup override set nightly
+        cargo miri setup
+    - name: Test with Miri
+      run: cargo miri test
+      env:
+        MIRIFLAGS: -Zmiri-tree-borrows

Cargo.toml

@@ -10,6 +10,13 @@ documentation = "https://docs.rs/dlmalloc"
 description = """
 A Rust port of the dlmalloc allocator
 """
+edition.workspace = true
+
+[workspace]
+members = ['fuzz']
+
+[workspace.package]
+edition = '2021'
 
 [package.metadata.docs.rs]
 features = ['global']
@@ -27,7 +34,8 @@ core = { version = '1.0.0', optional = true, package = 'rustc-std-workspace-core
 compiler_builtins = { version = '0.1.0', optional = true }
 
 [dev-dependencies]
-rand = "0.3"
+arbitrary = "1.3.2"
+rand = { version = "0.8", features = ['small_rng'] }
 
 [profile.release]
 debug-assertions = true

fuzz/.gitignore

@@ -0,0 +1,2 @@
+corpus
+artifacts

fuzz/Cargo.toml

@@ -0,0 +1,19 @@
+[package]
+name = "dlmalloc-fuzz"
+version = "0.0.1"
+publish = false
+edition.workspace = true
+
+[package.metadata]
+cargo-fuzz = true
+
+[dependencies]
+arbitrary = "1.3.2"
+dlmalloc = { path = '..' }
+libfuzzer-sys = "0.4.7"
+
+[[bin]]
+name = "alloc"
+path = "fuzz_targets/alloc.rs"
+test = false
+bench = false

fuzz/fuzz_targets/alloc.rs

@@ -0,0 +1,8 @@
+#![no_main]
+
+use arbitrary::Unstructured;
+use libfuzzer_sys::fuzz_target;
+
+fuzz_target!(|bytes: &[u8]| {
+    let _ = dlmalloc_fuzz::run(&mut Unstructured::new(bytes));
+});

fuzz/src/lib.rs

@@ -0,0 +1,108 @@
+use arbitrary::{Result, Unstructured};
+use dlmalloc::Dlmalloc;
+use std::cmp;
+
+const MAX_ALLOCATED: usize = 100 << 20; // 100 MB
+
+pub fn run(u: &mut Unstructured<'_>) -> Result<()> {
+    let mut a = Dlmalloc::new();
+    let mut ptrs = Vec::new();
+    let mut allocated = 0;
+    unsafe {
+        while u.arbitrary()? {
+            // If there are pointers to free then have a chance of deallocating
+            // a pointer. Try not to deallocate things until there's a "large"
+            // working set but afterwards give it a 50/50 chance of allocating
+            // or deallocating.
+            let free = match ptrs.len() {
+                0 => false,
+                0..=10_000 => u.ratio(1, 3)?,
+                _ => u.arbitrary()?,
+            };
+            if free {
+                let idx = u.choose_index(ptrs.len())?;
+                let (ptr, size, align) = ptrs.swap_remove(idx);
+                allocated -= size;
+                a.free(ptr, size, align);
+                continue;
+            }
+
+            // 1/100 chance of reallocating a pointer to a different size.
+            if ptrs.len() > 0 && u.ratio(1, 100)? {
+                let idx = u.choose_index(ptrs.len())?;
+                let (ptr, size, align) = ptrs.swap_remove(idx);
+
+                // Arbitrarily choose whether to make this allocation either
+                // twice as large or half as small.
+                let new_size = if u.arbitrary()? {
+                    u.int_in_range(size..=size * 2)?
+                } else if size > 10 {
+                    u.int_in_range(size / 2..=size)?
+                } else {
+                    continue;
+                };
+                if allocated + new_size - size > MAX_ALLOCATED {
+                    ptrs.push((ptr, size, align));
+                    continue;
+                }
+                allocated -= size;
+                allocated += new_size;
+
+                // Perform the `realloc` and assert that all bytes were copied.
+                let mut tmp = Vec::new();
+                for i in 0..cmp::min(size, new_size) {
+                    tmp.push(*ptr.offset(i as isize));
+                }
+                let ptr = a.realloc(ptr, size, align, new_size);
+                assert!(!ptr.is_null());
+                for (i, byte) in tmp.iter().enumerate() {
+                    assert_eq!(*byte, *ptr.offset(i as isize));
+                }
+                ptrs.push((ptr, new_size, align));
+            }
+
+            // Aribtrarily choose a size to allocate as well as an alignment.
+            // Enable small sizes with standard alignment happening a fair bit.
+            let size = if u.arbitrary()? {
+                u.int_in_range(1..=128)?
+            } else {
+                u.int_in_range(1..=128 * 1024)?
+            };
+            let align = if u.ratio(1, 10)? {
+                1 << u.int_in_range(3..=8)?
+            } else {
+                8
+            };
+
+            if size + allocated > MAX_ALLOCATED {
+                continue;
+            }
+            allocated += size;
+
+            // Choose arbitrarily between a zero-allocated chunk and a normal
+            // allocated chunk.
+            let zero = u.ratio(1, 50)?;
+            let ptr = if zero {
+                a.calloc(size, align)
+            } else {
+                a.malloc(size, align)
+            };
+            for i in 0..size {
+                if zero {
+                    assert_eq!(*ptr.offset(i as isize), 0);
+                }
+                *ptr.offset(i as isize) = 0xce;
+            }
+            ptrs.push((ptr, size, align));
+        }
+
+        // Deallocate everythign when we're done.
+        for (ptr, size, align) in ptrs {
+            a.free(ptr, size, align);
+        }
+
+        a.destroy();
+    }
+
+    Ok(())
+}

src/dlmalloc.rs

@@ -7,7 +7,7 @@ use core::cmp;
 use core::mem;
 use core::ptr;
 
-use Allocator;
+use crate::Allocator;
 
 pub struct Dlmalloc<A> {
     smallmap: u32,
@@ -91,22 +91,22 @@ impl<A> Dlmalloc<A> {
         Dlmalloc {
             smallmap: 0,
             treemap: 0,
-            smallbins: [0 as *mut _; (NSMALLBINS + 1) * 2],
-            treebins: [0 as *mut _; NTREEBINS],
+            smallbins: [ptr::null_mut(); (NSMALLBINS + 1) * 2],
+            treebins: [ptr::null_mut(); NTREEBINS],
             dvsize: 0,
             topsize: 0,
-            dv: 0 as *mut _,
-            top: 0 as *mut _,
+            dv: ptr::null_mut(),
+            top: ptr::null_mut(),
             footprint: 0,
             max_footprint: 0,
             seg: Segment {
-                base: 0 as *mut _,
+                base: ptr::null_mut(),
                 size: 0,
-                next: 0 as *mut _,
+                next: ptr::null_mut(),
                 flags: 0,
             },
             trim_check: 0,
-            least_addr: 0 as *mut _,
+            least_addr: ptr::null_mut(),
             release_checks: 0,
             system_allocator,
         }
@@ -201,7 +201,7 @@ impl<A: Allocator> Dlmalloc<A> {
     }
 
     fn top_foot_size(&self) -> usize {
-        self.align_offset_usize(Chunk::mem_offset() as usize)
+        self.align_offset_usize(Chunk::mem_offset())
             + self.pad_request(mem::size_of::<Segment>())
             + self.min_chunk_size()
     }
@@ -799,7 +799,7 @@ impl<A: Allocator> Dlmalloc<A> {
             let nextp = Chunk::plus_offset(p, mem::size_of::<usize>());
             (*p).head = Chunk::fencepost_head();
             nfences += 1;
-            if (&(*nextp).head as *const usize as *mut u8) < old_end {
+            if ptr::addr_of!((*nextp).head).cast::<u8>() < old_end {
                 p = nextp;
             } else {
                 break;
@@ -940,13 +940,15 @@ impl<A: Allocator> Dlmalloc<A> {
     }
 
     unsafe fn smallbin_at(&mut self, idx: u32) -> *mut Chunk {
-        debug_assert!(((idx * 2) as usize) < self.smallbins.len());
-        &mut *self.smallbins.get_unchecked_mut((idx as usize) * 2) as *mut *mut Chunk as *mut Chunk
+        let idx = usize::try_from(idx * 2).unwrap();
+        debug_assert!(idx < self.smallbins.len());
+        self.smallbins.as_mut_ptr().add(idx).cast()
     }
 
     unsafe fn treebin_at(&mut self, idx: u32) -> *mut *mut TreeChunk {
-        debug_assert!((idx as usize) < self.treebins.len());
-        &mut *self.treebins.get_unchecked_mut(idx as usize)
+        let idx = usize::try_from(idx).unwrap();
+        debug_assert!(idx < self.treebins.len());
+        self.treebins.as_mut_ptr().add(idx)
     }
 
     fn compute_tree_index(&self, size: usize) -> u32 {
@@ -1642,6 +1644,26 @@ impl<A: Allocator> Dlmalloc<A> {
     unsafe fn traverse_and_check(&self) -> usize {
         0
     }
+
+    pub unsafe fn trim(&mut self, pad: usize) -> bool {
+        self.sys_trim(pad)
+    }
+
+    pub unsafe fn destroy(mut self) -> usize {
+        let mut freed = 0;
+        let mut sp = &mut self.seg as *mut Segment;
+        while !sp.is_null() {
+            let base = (*sp).base;
+            let size = (*sp).size;
+            let can_free = !base.is_null() && !Segment::is_extern(sp);
+            sp = (*sp).next;
+
+            if can_free && self.system_allocator.free(base, size) {
+                freed += size;
+            }
+        }
+        freed
+    }
 }
 
 const PINUSE: usize = 1 << 0;
@@ -1719,19 +1741,19 @@ impl Chunk {
     }
 
     unsafe fn plus_offset(me: *mut Chunk, offset: usize) -> *mut Chunk {
-        (me as *mut u8).offset(offset as isize) as *mut Chunk
+        me.cast::<u8>().add(offset).cast()
     }
 
     unsafe fn minus_offset(me: *mut Chunk, offset: usize) -> *mut Chunk {
-        (me as *mut u8).offset(-(offset as isize)) as *mut Chunk
+        me.cast::<u8>().offset(-(offset as isize)).cast()
     }
 
     unsafe fn to_mem(me: *mut Chunk) -> *mut u8 {
-        (me as *mut u8).offset(Chunk::mem_offset())
+        me.cast::<u8>().add(Chunk::mem_offset())
     }
 
-    fn mem_offset() -> isize {
-        2 * (mem::size_of::<usize>() as isize)
+    fn mem_offset() -> usize {
+        2 * mem::size_of::<usize>()
     }
 
     unsafe fn from_mem(mem: *mut u8) -> *mut Chunk {
@@ -1750,7 +1772,7 @@ impl TreeChunk {
     }
 
     unsafe fn chunk(me: *mut TreeChunk) -> *mut Chunk {
-        &mut (*me).chunk
+        ptr::addr_of_mut!((*me).chunk)
     }
 
     unsafe fn next(me: *mut TreeChunk) -> *mut TreeChunk {
@@ -1782,14 +1804,14 @@ impl Segment {
     }
 
     unsafe fn top(seg: *mut Segment) -> *mut u8 {
-        (*seg).base.offset((*seg).size as isize)
+        (*seg).base.add((*seg).size)
     }
 }
 
 #[cfg(test)]
 mod tests {
     use super::*;
-    use System;
+    use crate::System;
 
     // Prime the allocator with some allocations such that there will be free
     // chunks in the treemap
@@ -1817,6 +1839,7 @@ mod tests {
     }
 
     #[test]
+    #[cfg(not(miri))]
     // Test allocating the maximum request size with a non-empty treemap
     fn treemap_alloc_max() {
         let mut a = Dlmalloc::new(System::new());

src/dummy.rs

@@ -1,5 +1,5 @@
+use crate::Allocator;
 use core::ptr;
-use Allocator;
 
 pub struct System {
     _priv: (),