Rc::ptr_eq should be unsafe because

1

Let’s say a Rc gets fully dropped: that means another Rc may take the same address, so a task that relies on ptr_eq may accidentally treat two different Rcs as pointing to the same.

Or, like JavaScript, Rc::ptr_eq also checks the identity hash so it’s safe?

2

I think you misunderstood, what Rc::ptr_eq() does. It does not compare the references to the Rcs, but the inner pointers, which the Rc points to. This is well documented:

Otoh, if one Rc gets fully dropped, you cannot pass it to Rc::ptr_eq() anyways, because, well, it’s gone.

3 Likes
3

as long the user code doesn't use unsafe (or at least doesn't contain unsound unsafe code), such use cases is a logic error, it should not be a memory safety vulnerability.

unsafe in rust is about memory safety and UB, not logic errors.

4

How do you expect a Rc to be at the same time:

  • fully dropped, so that another Rc can take its address
  • still alive, to be able to pass it to Rc::ptr_eq

Even if this was possible, functions should be marked unsafe only if they can be a source of UB. Just calling Rc::ptr_eq with different Rcs cannot lead to UB, so it should not be marked as unsafe

5 Likes
5

That’s not what I meant. I meant, the old address will may be referenced in a map:

  • HashMap<ByAddress<Rc<Node>>, Thingy>

And then, that means a new node, whereas the equivalent key has been dropped, will be mapped to Thingy wrongly.

—

Wait, TBH you’re right. I don’t think this is possible. This is maybe only possible for Gc depending on how references are managed.

1 Like
6

Note, that Rc::ptr_eq == true guarantees that two instances are equal, but a false value does not guarantee they are not equal. In other words, the function is just a cheap fast-path optimization. In particular, Rc::eq first compares the pointers and falls back to a structural comparison only when the addresses differ.

Also, you can trick HashMap by supplying an incorrect implementation of Eq and Hash for the key object. That would be a bug in your algorithm, but it is not undefined behavior, you simply replace one key-value pair with the other.

1 Like
7

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.