Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix miri #25

Merged
merged 5 commits into from
Jan 10, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions src/lfu/wtinylfu.rs
Original file line number Diff line number Diff line change
Expand Up @@ -725,7 +725,6 @@ mod test {
}

#[test]
#[cfg_attr(miri, ignore)]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it has been fixed, so remove the ignore tag

fn test_wtinylfu_custom_hasher() {
let mut cache: WTinyLFUCache<
u64,
Expand Down Expand Up @@ -823,7 +822,6 @@ mod test {
}

#[test]
#[cfg_attr(miri, ignore)]
fn test_wtinylfu() {
let mut cache = WTinyLFUCache::with_sizes(1, 2, 2, 5).unwrap();
assert_eq!(cache.cap(), 5);
Expand Down Expand Up @@ -908,7 +906,6 @@ mod test {
}

#[test]
#[cfg_attr(miri, ignore)]
fn test_wtinylfu_clone() {
let mut cache = WTinyLFUCache::with_sizes(1, 2, 2, 5).unwrap();
assert_eq!(cache.cap(), 5);
Expand Down
2 changes: 1 addition & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ mod macros;

cfg_not_std!(
/// Re-export for DefaultHashBuilder
pub type DefaultHashBuilder = hashbrown::hash_map::DefaultHashBuilder;
pub type DefaultHashBuilder = hashbrown::DefaultHashBuilder;
);

cfg_std!(
Expand Down
1 change: 1 addition & 0 deletions src/lru/adaptive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1764,6 +1764,7 @@ mod test {
use rand::seq::SliceRandom;
use rand::{thread_rng, Rng};

#[cfg_attr(miri, ignore)]
#[test]
fn test_arc_cache_random_ops() {
let size = 128;
Expand Down
56 changes: 26 additions & 30 deletions src/lru/raw.rs
Original file line number Diff line number Diff line change
Expand Up @@ -501,7 +501,7 @@ impl<K: Hash + Eq, V, E: OnEvictCallback, S: BuildHasher> Cache<K, V> for RawLRU
{
match self.map.get_mut(KeyWrapper::from_ref(k)) {
None => None,
Some(node) => Some(unsafe { &mut *node.as_mut().val.as_mut_ptr() }),
Some(node) => Some(unsafe { &mut *(*node.as_ptr()).val.as_mut_ptr() }),
}
}

Expand Down Expand Up @@ -557,10 +557,10 @@ impl<K: Hash + Eq, V, E: OnEvictCallback, S: BuildHasher> Cache<K, V> for RawLRU
Some(old_node) => unsafe {
let node_ptr = &mut *old_node.as_ptr();
self.detach(node_ptr);
let node = *Box::from_raw(old_node.as_ptr());
let mut node = *Box::from_raw(old_node.as_ptr());
let val = node.val.assume_init();
self.cb(&*node_ptr.key.as_ptr(), &val);
ptr::drop_in_place(node_ptr.key.as_mut_ptr());
self.cb(&*node.key.as_ptr(), &val);
ptr::drop_in_place(node.key.assume_init_mut());
Some(val)
},
}
Expand Down Expand Up @@ -1363,7 +1363,7 @@ impl<K: Hash + Eq, V, E: OnEvictCallback, S: BuildHasher> RawLRU<K, V, E, S> {
{
match self.map.get_mut(KeyWrapper::from_ref(k)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you may refer to https://github.com/jeromefroe/lru-rs/blob/2d18d2d333cee29057a23f065533d3f2b8dc0ae0/src/lib.rs#L1031.
this kind of operation shall all be accessed via pointers than references

None => None,
Some(node) => Some(unsafe { &mut *node.as_mut().val.as_mut_ptr() }),
Some(node) => Some(unsafe { &mut *(*node.as_ptr()).val.as_mut_ptr() }),
}
}

Expand All @@ -1375,7 +1375,7 @@ impl<K: Hash + Eq, V, E: OnEvictCallback, S: BuildHasher> RawLRU<K, V, E, S> {
{
self.map
.get(KeyWrapper::from_ref(k))
.map(|node| unsafe { &*node.as_ref().val.as_ptr() })
.map(|node| unsafe { &*(*node.as_ptr()).val.as_ptr() })
}

// used for avoiding borrow checker issues
Expand Down Expand Up @@ -1414,22 +1414,22 @@ impl<K: Hash + Eq, V, E: OnEvictCallback, S: BuildHasher> RawLRU<K, V, E, S> {
}
}

pub(crate) fn put_nonnull(&mut self, mut bks: NonNull<EntryNode<K, V>>) -> PutResult<K, V> {
// let old_key = KeyRef {
// k: unsafe { &(*(*(*self.tail).prev).key.as_ptr()) },
// };
// let old_node = self.map.remove(&old_key).unwrap();
pub(crate) fn put_nonnull(&mut self, bks: NonNull<EntryNode<K, V>>) -> PutResult<K, V> {
if self.len() >= self.cap() {
unsafe {
// Safety: the cache length is not zero, so the cache must have a tail node.
let node = ((*self.tail).prev).as_mut().unwrap();
self.detach(node);

let old_key = KeyRef {
k: &(*(*(*self.tail).prev).key.as_ptr()),
};
// Safety: the node is in cache, so the cache map must have the node.
let node = self
.map
.remove(&KeyRef {
k: node.key.as_ptr(),
})
.unwrap();

self.attach(bks.as_mut());
let node = self.map.remove(&old_key).unwrap();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

get the actual node by map.remove, so the value shall be valid

self.detach(node.as_ptr());

self.attach(bks.as_ptr());
self.map.insert(
KeyRef {
k: bks.as_ref().key.as_ptr(),
Expand Down Expand Up @@ -1464,23 +1464,19 @@ impl<K: Hash + Eq, V, E: OnEvictCallback, S: BuildHasher> RawLRU<K, V, E, S> {

pub(crate) fn put_or_evict_nonnull(
&mut self,
mut bks: NonNull<EntryNode<K, V>>,
bks: NonNull<EntryNode<K, V>>,
) -> Option<NonNull<EntryNode<K, V>>> {
if self.len() >= self.cap() {
unsafe {
// Safety: the cache length is not zero, so the cache must have a tail node.
let node = ((*self.tail).prev).as_mut().unwrap();
self.detach(node);

let old_key = KeyRef {
k: &(*(*(*self.tail).prev).key.as_ptr()),
};
// Safety: the node is in cache, so the cache map must have the node.
let node = self
.map
.remove(&KeyRef {
k: node.key.as_ptr(),
})
.unwrap();

self.attach(bks.as_mut());
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the same

let node = self.map.remove(&old_key).unwrap();
self.detach(node.as_ptr());

self.attach(bks.as_ptr());
self.map.insert(
KeyRef {
k: bks.as_ref().key.as_ptr(),
Expand Down
1 change: 1 addition & 0 deletions src/lru/two_queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1618,6 +1618,7 @@ mod test {
use rand::{thread_rng, Rng};
use std::format;

#[cfg_attr(miri, ignore)]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the loop is costly, just ignore

#[test]
fn test_2q_cache_random_ops() {
let size = 128;
Expand Down
Loading