Coercing &mut to *const should not create a shared reference

@eddyb

It has long been a rule in Rust that you must not mutate through a shared reference, or a raw pointer obtained from a shared reference.

Unfortunately, that rule currently forbids the following code:

fn direct_mut_to_const_raw() {
    let x = &mut 0;
    let y: *const i32 = x;
    unsafe { *(y as *mut i32) = 1; }
    assert_eq!(*x, 1);
}

The reason for this is that coercing &mut T to *const T implicitly first creates a shared reference and then coerces that to *const T, meaning y in the example above is technically a raw pointer obtained from a shared reference.

We should fix our coercion logic to no longer create this intermediate shared reference.

See #56161 for how we uncovered this problem.

Cc @eddyb @nikomatsakis

I believe the relevant code might be

rust/src/librustc_typeck/check/coercion.rs

Lines 734 to 752 in f504d3f

    
           coerce_mutbls(mt_a.mutbl, mutbl_b)?; 
        
           // Although references and unsafe ptrs have the same 
        
           // representation, we still register an Adjust::DerefRef so that 
        
           // regionck knows that the region for `a` must be valid here. 
        
           if is_ref { 
        
               self.unify_and(a_unsafe, b, |target| { 
        
                   vec![Adjustment { 
        
                       kind: Adjust::Deref(None), 
        
                       target: mt_a.ty 
        
                   }, Adjustment { 
        
                       kind: Adjust::Borrow(AutoBorrow::RawPtr(mutbl_b)), 
        
                       target 
        
                   }] 
        
               }) 
        
           } else if mt_a.mutbl != mutbl_b { 
        
               self.unify_and(a_unsafe, b, simple(Adjust::MutToConstPointer)) 
        
           } else { 
        
               self.unify_and(a_unsafe, b, identity) 
        
           }

but I am not sure because this is type checking. I have no idea where the code that decides about lowering of coercions to reborrows/casts lives.

I have no idea where the code that decides about lowering of coercions to reborrows/casts lives.

What do you mean? That code is generating a deref and a borrow, each of those gets turned into the equivalent MIR later, and the result is as if the user wrote &* explicitly.

@nikomatsakis

@nikomatsakis I believe the comment above if is_ref { has been outdated for quite a while (and makes no sense for NLL), can you confirm?

@eddyb

@eddyb that code generates "adjustments". I have no idea what that means. I found no use of AutoBorrow::RawPtr that would turn this into casts or reborrows or so.

@nikomatsakis

Oh I got confused as to what this is doing, I missed the AutoBorrow::RawPtr bit, looks like "borrow" and "cast reference to raw pointer" are fused into one adjustment. The lowering is here, btw:

rust/src/librustc_mir/hair/cx/expr.rs

Lines 152 to 180 in 1c3236a

    
           Adjust::Borrow(AutoBorrow::RawPtr(m)) => { 
        
               // Convert this to a suitable `&foo` and 
        
               // then an unsafe coercion. Limit the region to be just this 
        
               // expression. 
        
               let region = ty::ReScope(region::Scope { 
        
                   id: hir_expr.hir_id.local_id, 
        
                   data: region::ScopeData::Node 
        
               }); 
        
               let region = cx.tcx.mk_region(region); 
        
               expr = Expr { 
        
                   temp_lifetime, 
        
                   ty: cx.tcx.mk_ref(region, 
        
                                     ty::TypeAndMut { 
        
                                       ty: expr.ty, 
        
                                       mutbl: m, 
        
                                     }), 
        
                   span, 
        
                   kind: ExprKind::Borrow { 
        
                       region, 
        
                       borrow_kind: m.to_borrow_kind(), 
        
                       arg: expr.to_ref(), 
        
                   }, 
        
               }; 
        
               let cast_expr = Expr { 
        
                   temp_lifetime, 
        
                   ty: adjustment.target, 
        
                   span, 
        
                   kind: ExprKind::Cast { source: expr.to_ref() } 
        
               };

So looking again at the testcase, y as *mut i32 still goes through the reference->raw pointer coercion logic, but it does so for coercing &mut T to *mut T which does a mutable reborrow instead of an immutable reborrow (and then there's a separate *mut T to *const T coercion).

Changing AutoBorrow::RawPtr(mutbl_b) to AutoBorrow::RawPtr(mt_a.mutbl) would work, but you then also need to push Adjust::MutToConstPointer to that vec![...], if mt_a.mutbl != mutbl_b.

EDIT: this may be backwards incompatible if the mutable reference being coerced was also already immutably borrowed, since you'd be introducing a mutable reborrow.

Maybe we can avoid reborrows altogether here? But I'd leave that to @nikomatsakis.

Changing AutoBorrow::RawPtr(mutbl_b) to AutoBorrow::RawPtr(mt_a.mutbl) would work, but you then also need to push Adjust::MutToConstPointer to that vec![...], if mt_a.mutbl != mutbl_b.

I did the first but not the last and tests seem to still pass...^^

But yeah, there is a compatibility problem with outstanding shared references. Ouch.

Without Adjust::MutToConstPointer the MIR may end up malformed (using *mut T where *const T is expected), I'm surprised you don't get any errors!

using *mut T where *const T is expected

Yeah I figured. run-pass and ui tests all pass, so either the change did nothing or nothing checks the MIR^^

For the backwards compatibility issue:
I think my inclination is that we don't want to reborrow on a cast-to-raw. That would also make testing Stacked Borrows in miri easier, these implicit unavoidable reborrows are a pain. :P We'd have to make sure though that the borrow checker understands that after a (x: &mut T) as *mut T, x is still there -- it hasn't been moved away.

I guess the tricky bit with making it a cast is that what we have to work with is Operand::{Move,Copy}.

Instead, we could just add to the MIR a sort of "borrow to raw pointer" (a lot like AutoBorrow::RawPtr, really), and then we just need to make sure the old borrowck doesn't treat even a mutable AutoBorrow::RawPtr like a real borrow.

"borrow to raw pointer"

You mean like rust-lang/rfcs#2582? :D

@RalfJung

@RalfJung Yupp, that's what I mean, that seems like the perfect solution here.

(Sorry, that was the wrong button.)

Notice that the lint discussed in rust-lang/rfcs#2582, at least when implemented on the MIR, would actually flag these reborrows that are in our way now: The new reference is created just to turn it into a raw pointer, it has no other use.

Nominating as a replacement for #129653 (comment) , to bring up the more general situation.

Rollup merge of rust-lang#129653 - RalfJung:addr-of-read-only, r=scot…

Unrolled build for rust-lang#129653

Rollup merge of #129653 - RalfJung:addr-of-read-only, r=scottmcm

Navigation Menu

Repository navigation

Coercing &mut to *const should not create a shared reference #56604

54 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	coerce_mutbls(mt_a.mutbl, mutbl_b)?;
	// Although references and unsafe ptrs have the same
	// representation, we still register an Adjust::DerefRef so that
	// regionck knows that the region for `a` must be valid here.
	if is_ref {
	self.unify_and(a_unsafe, b, \|target\| {
	vec![Adjustment {
	kind: Adjust::Deref(None),
	target: mt_a.ty
	}, Adjustment {
	kind: Adjust::Borrow(AutoBorrow::RawPtr(mutbl_b)),
	target
	}]
	})
	} else if mt_a.mutbl != mutbl_b {
	self.unify_and(a_unsafe, b, simple(Adjust::MutToConstPointer))
	} else {
	self.unify_and(a_unsafe, b, identity)
	}

	Adjust::Borrow(AutoBorrow::RawPtr(m)) => {
	// Convert this to a suitable `&foo` and
	// then an unsafe coercion. Limit the region to be just this
	// expression.
	let region = ty::ReScope(region::Scope {
	id: hir_expr.hir_id.local_id,
	data: region::ScopeData::Node
	});
	let region = cx.tcx.mk_region(region);
	expr = Expr {
	temp_lifetime,
	ty: cx.tcx.mk_ref(region,
	ty::TypeAndMut {
	ty: expr.ty,
	mutbl: m,
	}),
	span,
	kind: ExprKind::Borrow {
	region,
	borrow_kind: m.to_borrow_kind(),
	arg: expr.to_ref(),
	},
	};
	let cast_expr = Expr {
	temp_lifetime,
	ty: adjustment.target,
	span,
	kind: ExprKind::Cast { source: expr.to_ref() }
	};

Coercing &mut to *const should not create a shared reference #56604

Description

Activity

RalfJung commented on Dec 7, 2018

eddyb commented on Dec 7, 2018

eddyb commented on Dec 7, 2018

RalfJung commented on Dec 7, 2018

eddyb commented on Dec 7, 2018

RalfJung commented on Dec 7, 2018

eddyb commented on Dec 7, 2018

RalfJung commented on Dec 7, 2018

eddyb commented on Dec 7, 2018

RalfJung commented on Dec 8, 2018

eddyb commented on Dec 8, 2018

RalfJung commented on Dec 9, 2018

54 remaining items

scottmcm commented on Sep 4, 2024

RalfJung commented on Jul 15, 2026

RodBurman commented on Jul 15, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions