Skip to content

iddqd 0.5.0

Latest

Choose a tag to compare

@github-actions github-actions released this 10 Sep 16:38
· 2 commits to main since this release

This release fixes a number of soundness holes, mostly identified by Claude Fable 5.1 and GPT-6 Astra, plus Google's unsafe_rust_review_experimental agent skill. All identified soundness holes require significantly contrived code, e.g. a Hash impl that stashes the passed-in reference into a thread-local or internal Cell.

Overall, iddqd now has significantly less unsafe code than before, though due to Rust compiler limitations it asks slightly more of trait implementers (such as IdOrdItem::Key now requiring Hash for change detection). We hope to relax these requirements in the future as the Rust compiler improves.

Thanks to the authors of the Google agent skill.

Added

  • debug_with_keys methods on IdOrdMap, IdHashMap, BiHashMap, and TriHashMap. These return a value whose Debug output is the previous {key: item, ...} form, and require the key types to be Debug for the lifetime of the borrow.

  • IdOrdMap's IntoIter now implements ExactSizeIterator and FusedIterator, matching the other maps' owning iterators.

Changed

  • Breaking: The mutable-borrow lookup methods now take the key by value, as T::Key<'_>, rather than any Q: Equivalent<T::Key<'_>> (or Comparable). This affects:

    • IdHashMap and IdOrdMap: get_mut and remove.
    • BiHashMap: get1_mut, get2_mut, remove1, remove2, get_mut_unique, and remove_unique.
    • TriHashMap: get1_mut, get2_mut, get3_mut, remove1, remove2, remove3, get_mut_unique, and remove_unique.

    The shared-borrow lookups (get, contains_key, and their numbered variants) still accept any Q.

    See the Mutable lookups take owned keys section in the crate docs for more information, and the "Fixed" entry below for the soundness hole this closes.

  • The Debug impl for IdHashMap no longer requires S: Clone + BuildHasher, matching BiHashMap and TriHashMap.

  • The Debug impls for IdOrdMap, IdHashMap, BiHashMap, and TriHashMap now format items only, as a set ({item, ...}), and require just T: Debug. Previously they formatted {key: item, ...} and also required the key types to be Debug. Use debug_with_keys for the previous form. The Debug impls for the daft Diff and MapLeaf types likewise no longer require the key types to be Debug.

  • Breaking: IdOrdItem::Key now requires Hash in addition to Ord. Any Hash impl that is generic over the key lifetime, including derived ones, satisfies the new bound.

    IdOrdMap's RefMut has always needed Hash to detect key changes, but the bound used to be on each method that hands out a RefMut. Moving it onto the trait removes those per-method bounds.

    As a result, RefMut::reborrow no longer requires the item type to be 'static. (The 0.3.10 changelog claimed this already worked, but it did not.)

Fixed

  • Fixed a soundness hole in the mutable-borrow lookup methods listed under "Changed". Their signatures, such as fn get_mut<'a, Q: Equivalent<T::Key<'a>>>(&'a mut self, key: &Q), let caller code copy a reference out of that key into a Cell<Option<&'a str>> and read it after the map had mutated or dropped the item.

    These APIs have been changed to take T::Key<'_> directly, which closes this soundness hole.

  • The Iter, IterMut, and IntoIter types now report an exact size_hint. Previously, they returned (0, None). This violated the ExactSizeIterator contract, resulting in calls like .take(...).len() panicking on a non-empty map.

  • Fixed a soundness hole in IdOrdMap's RefMut. Within Entry::and_modify and IdOrdMap::retain, the item can be removed while the RefMut's borrow lifetime 'a is still live. In a contrived scenario where:

    • A map is held for 'static, e.g. with Box::leak; and,
    • A Hash impl was written only for Key<'static>,

    The Hash impl could observe a key that wasn't valid for 'static. The new Hash bound on IdOrdItem::Key rejects a 'static-only Hash impl at compile time.

  • Fixed a soundness hole in the Debug impls for IdOrdMap, IdHashMap, BiHashMap, and TriHashMap. A contrived scenario where a Debug impl was written only for Key<'static> could observe a 'static key that actually borrowed from the map. The impls no longer format keys, and the internal lifetime-extending transmute is gone. (This is why the Debug output changed; see above.)

  • Fixed a soundness hole in the serialize functions of IdOrdMapAsMap, IdHashMapAsMap, BiHashMapAsMap, and TriHashMapAsMap. The lifetime 'a in T::Key<'a>: Serialize was not tied to the borrow of the map, so a contrived scenario where a Serialize impl was written only for Key<'static> could observe a key that actually borrowed from the map.

    This is a breaking change only for Serialize impls that exist solely for a 'static key type. In most cases, impls are generic over the key lifetime — those are unaffected.

  • Fixed a soundness hole in IdHashMap, BiHashMap, and TriHashMap with custom allocators (via the allocator-api2 feature). The maps now correctly call the allocator's grow, grow_zeroed, shrink, and allocate_zeroed methods.

    Previously, only allocate and deallocate were called, and the others fell through to the trait's default implementations -- those implementations allocate a new block, copy the memory, and then call deallocate on the old one. In the unlikely case that the last deallocate freed the block and then panicked, a map resize could leave the map holding a freed pointer, and dropping the map would free it again.

    Allocators that don't implement grow and shrink still get the trait's default grow and shrink. For those allocators, deallocate should not unwind after freeing. (This is a pre-existing limitation in the allocator-api2 crate.)

    IdOrdMap does not support custom allocators and is not affected. Unsoundness on allocator panics is a widespread problem in the Rust ecosystem, which is why the soon-to-be-stabilized standard library allocator API bans panicking within the allocator.