Stream: t-lang/wg-unsafe-code-guidelines

Topic: things from Stack Overflow


Jake Goulding (Sep 28 2018 at 13:12, on Zulip):

Why does modifying a mutable reference's value through a raw pointer not violate Rust's aliasing rules?

/cc @RalfJ

RalfJ (Sep 30 2018 at 13:31, on Zulip):

@Jake Goulding Seems like someone answered :)

Jake Goulding (Sep 30 2018 at 13:33, on Zulip):

@RalfJ an important thing about any source is to evaluate it yourself. Voting to affect the score would be useful, but you are knowledgable and recognized (and even mentioned in the answer), so adding a comment that says you agree/disagree or an alternate answer would of course be welcomed

Jake Goulding (Sep 30 2018 at 13:34, on Zulip):

aside: it always amazes me how many people get to a SO question and read the top answer, immediately stopping. They never read the subsequent (often better) answers.

RalfJ (Sep 30 2018 at 13:35, on Zulip):

ideally the top answer would be the best one...

RalfJ (Sep 30 2018 at 13:37, on Zulip):

I added a comment. If it was my answer I'd also add another example of a piece of code that is NOT okay... what would be the best way to do that?

RalfJ (Sep 30 2018 at 13:40, on Zulip):

@Jake Goulding e.g. this code is not okay:

let x = &mut [1, 2, 4];
let x_ptr = x.as_mut_ptr();

unsafe {
    for i in 0..x.len() {
        *x_ptr.offset(i as isize) += 2;
    }
}
assert_eq!(x, &[3, 4, 6]);

unsafe { *x_ptr += 1; } // BÄM! Used no-longer-valid raw ptr
Jake Goulding (Sep 30 2018 at 13:52, on Zulip):

It's a narrow world view to assume that there can even be a "best".

Jake Goulding (Sep 30 2018 at 13:53, on Zulip):

And there are plenty of arguments around what should even be at the top of the answers (accepted by OP, most votes, most recently created, etc.)

Jake Goulding (Sep 30 2018 at 13:54, on Zulip):

There's a small amount of sorting you can do on the answers, if so inclined

Jake Goulding (Sep 30 2018 at 13:55, on Zulip):

I'd also add another example of a piece of code that is NOT okay... what would be the best way to do that?

Again, assuming there's a best :wink:

I see a few options. You could provide that code in a playground link and add it and text in a comment (optionally encouraging OP to include it in the answer), you could edit it into the answer, or you could provide an answer yourself

Jake Goulding (Sep 30 2018 at 13:56, on Zulip):

Matthieu M. is a pretty frequent contributor, so I'd expect any communication in comments would be responded to in a short amount of time

Jake Goulding (Sep 30 2018 at 14:00, on Zulip):

Thank you for commenting, @RalfJ !

RalfJ (Sep 30 2018 at 14:03, on Zulip):

I dont think I can edit other people's stuff, so I'll try the playground link

RalfJ (Oct 05 2018 at 16:28, on Zulip):

@Jake Goulding Uh, I am thoroughly confused by https://stackoverflow.com/questions/52601731/how-to-create-unsafecellc-void-safely. But all the other participants seem to be as well.

RalfJ (Oct 05 2018 at 16:28, on Zulip):

I feel like at least 2, probably more, issues are conflated there -- so I am not even sure where to begin answering and/or correcting things^^

RalfJ (Oct 05 2018 at 16:30, on Zulip):

But to begin, when you have an opaque FFI pointer, you don't have to worry about UnsafeCell or interior mutability or anything like that. Rust doesnt care about stuff behind a raw pointer.

Jake Goulding (Oct 05 2018 at 22:39, on Zulip):

Ugh. That question is a mess

Jake Goulding (Oct 05 2018 at 22:55, on Zulip):

I think " Rust doesnt care about stuff behind a raw pointer." is the most important aspect; is that already codified in whatever unsafe things exist?

RalfJ (Oct 06 2018 at 08:10, on Zulip):

I think " Rust doesnt care about stuff behind a raw pointer." is the most important aspect; is that already codified in whatever unsafe things exist?

I am not sure how explicitly it is said -- it is the entire reason raw ptrs even exist, so it may be one of those things that didnt even seem worth saying.^^ but I'd guess it is somewhere

RalfJ (Oct 06 2018 at 08:11, on Zulip):

e.g. https://doc.rust-lang.org/nightly/nomicon/ffi.html

raw pointers fall outside of Rust's safe memory model.

RalfJ (Oct 06 2018 at 08:11, on Zulip):

and there is https://doc.rust-lang.org/book/2018-edition/ch19-01-unsafe-rust.html#dereferencing-a-raw-pointer

Jake Goulding (Oct 06 2018 at 13:43, on Zulip):

Nice, things I can link to always make an answer better

Jake Goulding (Oct 06 2018 at 13:43, on Zulip):

@RalfJ do you think that defining "interior mutability" is something that this WG should be concerned with?

RalfJ (Oct 07 2018 at 08:28, on Zulip):

@RalfJ do you think that defining "interior mutability" is something that this WG should be concerned with?

In principle yes, however -- is there an open question there? Interior mutabilitiy is about mutating memory where there also exists a live shared reference non-transitively pointing to the same memory

RalfJ (Oct 07 2018 at 08:29, on Zulip):

at least in my mind interior mutability is pretty much settleed

Jake Goulding (Oct 07 2018 at 16:00, on Zulip):

That's fine then. I'll submit a PR to the glossary with that and you can approve it ;-)

Jake Goulding (Oct 22 2018 at 19:01, on Zulip):

Is transmuting PhantomData markers safe?

rkruppe (Oct 22 2018 at 19:09, on Zulip):

we do not currently guarantee any such thing for repr(Rust) structs. in the UCG repo discussion seems to be favorable towards guaranteeing it at least for newtypes (exact definition TBD but might include that type). guaranteeing it for something like { Vec<T>, i32, PhantomData<_> } seems controversial though personally I think we really should

Jake Goulding (Oct 22 2018 at 19:11, on Zulip):

Doing my due diligence — should I stick this core question somewhere to ultimately be a part of the Grand Unified Unsafe Theory?

rkruppe (Oct 22 2018 at 19:16, on Zulip):

i don't think there's an actively developed and consulted collection of "stuff we need to settle" any more (there used to be), but with a little massaging this could be an example in an issue comment arguing for ignoring align=1,size=0 fields for repr(Rust) type layout (e.g. here: https://github.com/rust-rfcs/unsafe-code-guidelines/issues/35). i don't think the "PhantomData for phantom types that should be possible to change without copying" use case was explicitly spelled out yet

nikomatsakis (Oct 22 2018 at 19:20, on Zulip):

it'd be good @Jake Goulding to have links to questions that arise in the wild, imo

nikomatsakis (Oct 22 2018 at 19:20, on Zulip):

I also agree with @rkruppe that I think we should make said guarantee

Jake Goulding (Oct 22 2018 at 19:22, on Zulip):

@nikomatsakis do you agree with adding a comment to the issue that @rkruppe linked me then?

nikomatsakis (Oct 22 2018 at 19:24, on Zulip):

I do

RalfJ (Oct 23 2018 at 07:12, on Zulip):

I now responded saying they should use repr(C). seems like the safest choice IMO, and what people should do when playing tricks with transmute and struct layout. you should have good reasons to do what with repr(Rust).

rkruppe (Oct 23 2018 at 07:53, on Zulip):

The downside of repr(C), particularly for non-newtypes, is that you lose all the nice repr(Rust) optimizations that do make sense for your type. While we could introduce a complex hierarchy of attributes to gradually lower the undefinedness of repr(Rust) layout, I think for very niche or speculative optimizations it's more economical to give nice guarantees about repr(Rust) and instead introduce new reprs for optimizations that go beyond that if and when they are implemented

RalfJ (Oct 23 2018 at 08:13, on Zulip):

repr(C) kills niches?

rkruppe (Oct 23 2018 at 08:14, on Zulip):

it doesn't kill niches, but it kills reordering the non-align1 fields to minimize padding

RalfJ (Oct 23 2018 at 08:16, on Zulip):

Yeah it doesnt: https://play.rust-lang.org/?version=stable&mode=debug&edition=2015&gist=6264c79e56afe2b77c6f51d04c0b6059

RalfJ (Oct 23 2018 at 08:25, on Zulip):

for newtypes the reordering doesnt do anything... and I'd rather be cautious and not reorder fields automatically when doing such tricks

RalfJ (Oct 23 2018 at 08:26, on Zulip):

"I cannot always obtain the ideal field order with repr(C)" might be an argument to use repr(Rust), but IMO one should first see whether repr(C) isn't enough

rkruppe (Oct 23 2018 at 08:27, on Zulip):

if you use PhantomData for actual phantom types and so the type in the phantom data is the only thing you change when transmuting, everything's fine

RalfJ (Oct 23 2018 at 08:27, on Zulip):

maybe it is. so far I dont even see this suggested in the UCG discussion, aside from Gankro asking for something stronger that would imply what you said

RalfJ (Oct 23 2018 at 08:27, on Zulip):

I agree it is reasonable though

rkruppe (Oct 23 2018 at 08:28, on Zulip):

I am kicking myself for not bringing it up before because, duh, obviously

RalfJ (Oct 23 2018 at 08:29, on Zulip):

;)

Andrew Poelstra (Oct 23 2018 at 15:27, on Zulip):

https://queue.acm.org/detail.cfm?id=3212479 suggests that C's struct guarantees do actually kill performance (preventing reordering, requiring preservation of padding)

RalfJ (Oct 23 2018 at 15:32, on Zulip):

@Andrew Poelstra that's a very interesting article, thanks for sharing!

rkruppe (Oct 23 2018 at 15:36, on Zulip):

@Andrew Poelstra I'm not sure what exactly you're responding to but two things to keep in mind in this discussion: 1. nobody except joshtriplett wants repr(Rust) struct layout to be as locked down as repr(C) layout, it's just a question of defaults for certain specialized optimizations, and 2. where hard data exists on the effectiveness of certain layout optimizations, it's usually either limited to minimizing padding (which we already do and want to keep doing) or not distinguishing between speedups caused by optimizations that we already have one hand and more advanced and tricky optimizations on the other hand (because they apply them all in one go to C code) which makes it hard to infer how useful the second kind of optimization would be to add to Rust

Andrew Poelstra (Oct 23 2018 at 15:39, on Zulip):

@rkruppe Ok, cool. I was posted the article mostly because it surprised me with the extent of the optimizations that exist and talk of locking down repr(Rust) makes me nervous that people are trying to make programmers' lives easier for certain niche struct-hacking use cases without considering the extent of the performance loss. But I think I misread the room.

rkruppe (Oct 23 2018 at 15:39, on Zulip):

btw, some of the transformations described/wishlisted in that article specifically, such as AoS<->SoA, aren't even possible in safe Rust

RalfJ (Oct 24 2018 at 13:09, on Zulip):

@Jake Goulding thanks for editing :)

Jake Goulding (Oct 24 2018 at 18:11, on Zulip):

no problem; you know how I enjoy it

Jake Goulding (Oct 27 2018 at 15:35, on Zulip):

Is it UB to cast *mut T to *mut ManuallyDrop<T>?

simulacrum (Oct 27 2018 at 15:40, on Zulip):

Isn't pointer-casting always "safe" since only the dereference is potentially unsafe?

simulacrum (Oct 27 2018 at 15:41, on Zulip):

(it can be done in safe code...)

Jake Goulding (Oct 27 2018 at 15:55, on Zulip):

heh, that's my kind of pedantry :wink:

Jake Goulding (Oct 27 2018 at 15:55, on Zulip):

I assume they mean to then use the dereferenced value.

simulacrum (Oct 27 2018 at 15:55, on Zulip):

But more specifically, I think this is perfectly valid -- because ManuallyDrop is #[repr(transparent)]

simulacrum (Oct 27 2018 at 15:57, on Zulip):

Though I might say that let foo: ManuallyDrop<T> = ptr::read(v: *mut T); is how I'd do it

simulacrum (Oct 27 2018 at 15:57, on Zulip):

Though I guess you can't do that directly because read requires *const T -> T

simulacrum (Oct 27 2018 at 15:57, on Zulip):

but yeah this seems fine to me

Jake Goulding (Oct 27 2018 at 15:59, on Zulip):

repr(transparent) was my rationale as well.

Jake Goulding (Oct 27 2018 at 16:00, on Zulip):

but then there's the question of the value of a pointer to a ManuallyDrop in the first place

Jake Goulding (Oct 27 2018 at 16:00, on Zulip):

If you don't want to drop it, don't read it — convert to a reference instead.

simulacrum (Oct 27 2018 at 16:04, on Zulip):

That's true -- or ptr::write to the value

simulacrum (Oct 27 2018 at 16:04, on Zulip):

I guess if you want to write to it "normally" though ManuallyDrop makes &mut ManuallyDrop<T> a "safe" conversion

simulacrum (Oct 27 2018 at 16:04, on Zulip):

(given that it's dereferenceable)

Jake Goulding (Jan 01 2019 at 20:33, on Zulip):

@RalfJ your name was mentioned in the comments, so on the off-chance you might be interested...

Is using an immutable borrow after an aliasing mutable borrow is created but before it is used actually dangerous?

RalfJ (Jan 02 2019 at 15:03, on Zulip):

thanks! Note that in your example, f should also get restrict. it applies to all mutable references as well as shared refs without immediate (non-transitive) interior mutability.

Last update: Nov 19 2019 at 18:50UTC