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: Do not consider mutable usage of deref to *mut T as deref_mut #18252

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ShoyuVanilla
Copy link
Member

Fixes #15799

We are doing some heuristics for deciding whether the given deref is deref or deref_mut here;

Expr::UnaryOp { expr, op: UnaryOp::Deref } => {
if let Some((f, _)) = self.result.method_resolutions.get_mut(&tgt_expr) {
if mutability == Mutability::Mut {
if let Some(deref_trait) = self
.db
.lang_item(self.table.trait_env.krate, LangItem::DerefMut)
.and_then(|l| l.as_trait())
{
if let Some(deref_fn) = self
.db
.trait_data(deref_trait)
.method_by_name(&Name::new_symbol_root(sym::deref_mut.clone()))
{
*f = deref_fn;
}
}
}
}
self.infer_mut_expr(*expr, mutability);

But this heuristic is erroneous if we are dereferencing to a mut ptr and normally those cases are filtered out here as builtin;

Expr::UnaryOp { expr, op: hir_def::hir::UnaryOp::Deref } => {
let is_builtin = match self.expr_ty_without_adjust(*expr).kind(Interner) {
TyKind::Ref(..) | TyKind::Raw(..) => true,
TyKind::Adt(id, _) => {
if let Some(lang_item) = self.db.lang_attr(id.0.into()) {
lang_item == LangItem::OwnedBox
} else {
false
}
}
_ => false,
};
if !is_builtin {

Howerver, this works not so well if the given dereferencing is double dereferencings like the case in the #15799.

struct WrapPtr(*mut u32);

impl core::ops::Deref for WrapPtr {
    type Target = *mut u32;
    fn deref(&self) -> &Self::Target {
        &self.0
    }
}

fn main() {
    let mut x = 0u32;
    let wrap = WrapPtr(&mut x);
    unsafe {
        **wrap = 6;
    }
}

Here are two - outer and inner - dereferences here, and the outer dereference is marked as deref_mut because there is an assignment operation.
And this deref_mut marking is propagated into the inner dereferencing.
In the later MIR lowering, the outer dereference is filtered out as it's expr type is *mut u32, but the expr type in the inner dereference is an ADT, so this false-mutablility is not filtered out.

This PR cuts propagation of this false mutablilty chain if the expr type is mut ptr.
Since this happens before the resolve_all, it may have some limitations when the expr type is determined as mut ptr at the very end of inferencing, but I couldn't find simple fix for it 🤔

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mutating data with Deref to raw pointer is reported as an error
2 participants