language-servers/rust-analyzer
1
0
Fork 0
mirror of https://github.com/rust-lang/rust-analyzer.git synced 2025-11-13 09:11:51 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

Improve soundness a bit by making TaggedArcPtr::try_as_arc_owned() unsafe

Browse source 
Since the `ManuallyDrop` it returns can be safely used to consume the `Arc`, which is can cause UB if done incorrectly. See #18499.
This commit is contained in:
Chayim Refael Friedman 2024-12-04 03:38:37 +02:00
parent e6276c8b64
commit 3aeb5e66c4
1 changed files with 17 additions and 13 deletions
Download patch file Download diff file Expand all files Collapse all files

30
crates/intern/src/symbol.rs
View file

@ -77,8 +77,12 @@ impl TaggedArcPtr {
} }
/// Retrieves the tag. /// Retrieves the tag.
///
/// # Safety
///
/// You can only drop the `Arc` if the instance is dropped.
#[inline] #[inline]
pub(crate) fn try_as_arc_owned(self) -> Option<ManuallyDrop<Arc<Box<str>>>> { pub(crate) unsafe fn try_as_arc_owned(self) -> Option<ManuallyDrop<Arc<Box<str>>>> {
// Unpack the tag from the alignment niche // Unpack the tag from the alignment niche
let tag = Strict::addr(self.packed.as_ptr()) & Self::BOOL_BITS; let tag = Strict::addr(self.packed.as_ptr()) & Self::BOOL_BITS;
if tag != 0 { if tag != 0 {
@ -245,16 +249,14 @@ impl Symbol {
} }
} }
ManuallyDrop::into_inner( let ptr = match shard.raw_entry_mut().from_key_hashed_nocheck::<str>(hash, arc.as_ref()) {
match shard.raw_entry_mut().from_key_hashed_nocheck::<str>(hash, arc.as_ref()) { RawEntryMut::Occupied(occ) => occ.remove_entry(),
RawEntryMut::Occupied(occ) => occ.remove_entry(), RawEntryMut::Vacant(_) => unreachable!(),
RawEntryMut::Vacant(_) => unreachable!(), }
} .0
.0 .0;
.0 // SAFETY: We're dropping, we have ownership.
.try_as_arc_owned() ManuallyDrop::into_inner(unsafe { ptr.try_as_arc_owned().unwrap() });
.unwrap(),
);
debug_assert_eq!(Arc::count(arc), 1); debug_assert_eq!(Arc::count(arc), 1);
// Shrink the backing storage if the shard is less than 50% occupied. // Shrink the backing storage if the shard is less than 50% occupied.
@ -267,7 +269,8 @@ impl Symbol {
impl Drop for Symbol { impl Drop for Symbol {
#[inline] #[inline]
fn drop(&mut self) { fn drop(&mut self) {
let Some(arc) = self.repr.try_as_arc_owned() else { // SAFETY: We're dropping, we have ownership.
let Some(arc) = (unsafe { self.repr.try_as_arc_owned() }) else {
return; return;
}; };
// When the last `Ref` is dropped, remove the object from the global map. // When the last `Ref` is dropped, remove the object from the global map.
@ -288,7 +291,8 @@ impl Clone for Symbol {
} }
fn increase_arc_refcount(repr: TaggedArcPtr) -> TaggedArcPtr { fn increase_arc_refcount(repr: TaggedArcPtr) -> TaggedArcPtr {
let Some(arc) = repr.try_as_arc_owned() else { // SAFETY: We're not dropping the `Arc`.
let Some(arc) = (unsafe { repr.try_as_arc_owned() }) else {
return repr; return repr;
}; };
// increase the ref count // increase the ref count