Multiple panic safety issues

Hi there, we (Rust group @sslab-gatech) are scanning crates on crates.io for potential soundness bugs. We noticed a few panic safety issues in this library.

clone_from double-frees if T::clone panics

https://github.com/andrewhickman/id-map/blob/a2fa8d4a554dea2f9ea2ec6c3d06793576f8e7c0/src/lib.rs#L370-L380

The current values in the map are dropped and the ids are updated up front. This means that if other.values.get_unchecked(id).clone() panics, it can cause the previously dropped values to drop again.

get_or_insert double frees if insertion function f panics

https://github.com/andrewhickman/id-map/blob/a2fa8d4a554dea2f9ea2ec6c3d06793576f8e7c0/src/lib.rs#L169-L180

Since this reserves space for the value before calling ptr::write(space, f());, if f panics here, it can drop an already freed value.

remove_set double frees if drop panics

https://github.com/andrewhickman/id-map/blob/a2fa8d4a554dea2f9ea2ec6c3d06793576f8e7c0/src/lib.rs#L192-L203

This code goes over to the ids to remove and calls drop_in_place on them. However if the drop function for the type panics, the element gets dropped again when the IdMap is dropped.

Code to recrate these problems is here:

#![forbid(unsafe_code)]

use id_map::IdMap;
use id_set::IdSet;

struct DropDetector(u32);

impl Drop for DropDetector {
    fn drop(&mut self) {
        println!("Dropping {}", self.0);
    }
}
impl Clone for DropDetector {
    fn clone(&self) -> Self { panic!("Panic on clone!"); }
}

fn main() {
    //clone_from_panic_will_drop_invalid_memory();
    //get_or_insert_with_leaves_state_inconsistent();
    remove_set_leaves_state_inconsistent_if_drop_panics();
}

fn clone_from_panic_will_drop_invalid_memory() {
    let mut map = IdMap::new();
    map.insert(DropDetector(1));

    let mut map_2 = IdMap::new();
    map_2.insert(DropDetector(2));
    map_2.clone_from(&map);
}

fn get_or_insert_with_leaves_state_inconsistent() {
    let mut map : IdMap<DropDetector> = IdMap::with_capacity(0);
    map.get_or_insert_with(0, || panic!("Panic in insertion function!"));
}

struct PanicsOnDrop(u32, bool);

impl Drop for PanicsOnDrop {
    fn drop(&mut self) {
        println!("Dropping {}", self.0);

        if (self.1) {
            self.1 = false;
            panic!("Panicking on drop");
        }
    }
}

fn remove_set_leaves_state_inconsistent_if_drop_panics() {
    let mut map = IdMap::new();
    map.insert(PanicsOnDrop(1, true));

    map.remove_set(&IdSet::new_filled(1));
}

andrewhickman / id-map