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

Topic: meeting-2019-07-25


RalfJ (Jul 25 2019 at 15:17, on Zulip):

Hi @WG-unsafe-code-guidelines !

RalfJ (Jul 25 2019 at 15:17, on Zulip):

let's see who shows up this time... ;)

gnzlbg (Jul 25 2019 at 15:17, on Zulip):

i'm here !

RalfJ (Jul 25 2019 at 15:18, on Zulip):

you didnt melt yet?

gnzlbg (Jul 25 2019 at 15:18, on Zulip):

more like testing how far i can melt

gnzlbg (Jul 25 2019 at 15:19, on Zulip):

munich is only 34C, @rkruppe should have like almost 40C at KIT if they are there

gnzlbg (Jul 25 2019 at 15:19, on Zulip):

pretty sure they are winning the melting competition

RalfJ (Jul 25 2019 at 15:19, on Zulip):

yeah 40C is also announced for Saarbr├╝cken

RalfJ (Jul 25 2019 at 15:20, on Zulip):

right now its around 39

gnzlbg (Jul 25 2019 at 15:20, on Zulip):

lucky thing is I was at a music festival last weekend, at it was like 26 or so

gnzlbg (Jul 25 2019 at 15:21, on Zulip):

one weekend later and it would have been just pure death

rkruppe (Jul 25 2019 at 15:21, on Zulip):

I'm not at KIT, but it's also 39┬░ in Darmstadt where I am :)

RalfJ (Jul 25 2019 at 15:21, on Zulip):

yeah there's a "stadtviertelfest" this weekend here, let's see how that goes

gnzlbg (Jul 25 2019 at 15:21, on Zulip):

ah i thought you were at karlsruhe

RalfJ (Jul 25 2019 at 15:21, on Zulip):

looks like we got a German UCG mtg today? :D

RalfJ (Jul 25 2019 at 15:21, on Zulip):

@rkruppe Darmstadt? I grew up near Wiesbaden. ;)

gnzlbg (Jul 25 2019 at 15:22, on Zulip):

wiesbaden is really nice for some reason

RalfJ (Jul 25 2019 at 15:22, on Zulip):

"for some reason"?^^

gnzlbg (Jul 25 2019 at 15:23, on Zulip):

i was there last year, and I did not expected it to be as nice

RalfJ (Jul 25 2019 at 15:23, on Zulip):

hehe ;)

gnzlbg (Jul 25 2019 at 15:23, on Zulip):

super wide streets and nice buildings, i don't know :D

RalfJ (Jul 25 2019 at 15:23, on Zulip):

well I wasnt actually in the city that often. but yeah the center is pretty.

RalfJ (Jul 25 2019 at 15:23, on Zulip):

@rkruppe now that we got your attention, what are your thoughts on https://github.com/rust-lang/unsafe-code-guidelines/pull/153 ?

rkruppe (Jul 25 2019 at 15:27, on Zulip):

I haven't been able to properly form an opinion (in particular I should read what the existing chapters look like now after @gnzlbg updated them) but my pre-existing opinion is that call ABI probably shouldn't be included in "layout" because of the existing "memory layout" connotations (and existing community practice of omitting the "memory" part) and the fact that the cases where the call ABI matters seem rather niche (sorry not sorry) compared to all the cases where it doesn't matter.

RalfJ (Jul 25 2019 at 15:28, on Zulip):

OTOH (a) it is included in the current def.n in master, and (b) it is included in TyLayout in the compiler

gnzlbg (Jul 25 2019 at 15:29, on Zulip):

I think we discussed somewhere that we often need to distinguish size+align (e.g. for allocating memory for a T), size+align+niche (for allocating memory for an Option<T>), and size+layout+niche+callABI (for FFI)

rkruppe (Jul 25 2019 at 15:30, on Zulip):

"for FFI" - only for by-value FFI, not behind pointers, which is another good example of call ABI rarely mattering

gnzlbg (Jul 25 2019 at 15:30, on Zulip):

yes, by value FFI

gnzlbg (Jul 25 2019 at 15:30, on Zulip):

for memory FFI, we usually only care about size+align, not even niches, since C doesn't have those

rkruppe (Jul 25 2019 at 15:30, on Zulip):

We definitely need terms for all three but if we have "layout" without qualifications, it should roughly match e.g. how everyone talks about transmute being OK or not

RalfJ (Jul 25 2019 at 15:30, on Zulip):

@gnzlbg and fields

gnzlbg (Jul 25 2019 at 15:31, on Zulip):

its hard for me to articulate when do we care about fields

rkruppe (Jul 25 2019 at 15:31, on Zulip):

@gnzlbg Niches don't matter directly/at the top level but the niche of T matters e.g. for whether Option<T> is sensible to pass by value in FFI

RalfJ (Jul 25 2019 at 15:31, on Zulip):

@gnzlbg well we care that they are at the same offset in Rust and C

gnzlbg (Jul 25 2019 at 15:31, on Zulip):

i mean, the C code will need to respect whatever niches are in the Rust memory, if the Rust memory uses those

gnzlbg (Jul 25 2019 at 15:32, on Zulip):

(or the other way around, Rust cannot use type with niches)

gnzlbg (Jul 25 2019 at 15:33, on Zulip):

so for FFI and transmutes in Rust we care about fields and niches

gnzlbg (Jul 25 2019 at 15:33, on Zulip):

size+align, size+align+fields+niches, and ...+callABI for by value FFI

RalfJ (Jul 25 2019 at 15:34, on Zulip):

size+align+niche is what matters for inner types when computing the layout of the outer type

gnzlbg (Jul 25 2019 at 15:34, on Zulip):

and size+align+niche+fields is required to transmute between two types

RalfJ (Jul 25 2019 at 15:35, on Zulip):

niche actually isnt

RalfJ (Jul 25 2019 at 15:35, on Zulip):

like you can transmute an &T to a *const T just fine

gnzlbg (Jul 25 2019 at 15:35, on Zulip):

you can't transmute an Option<&T> into a &T if the value of the option is None

rkruppe (Jul 25 2019 at 15:35, on Zulip):

If you start distingushing the directions, alignment also matters partially

RalfJ (Jul 25 2019 at 15:36, on Zulip):

@rkruppe why that? its by-value

gnzlbg (Jul 25 2019 at 15:36, on Zulip):

alignment doesn't matter for transmute right?

rkruppe (Jul 25 2019 at 15:36, on Zulip):

Oh right, not for transmute, I was thinking of type punning memory in-place

gnzlbg (Jul 25 2019 at 15:37, on Zulip):

like going from a *const T to a *const U where the alignment doesn't match exactly ?

gnzlbg (Jul 25 2019 at 15:37, on Zulip):

(e.g. if the alignment of T is 4 but the alignment of U is 2)

rkruppe (Jul 25 2019 at 15:38, on Zulip):

Yes, stuff like that

gnzlbg (Jul 25 2019 at 15:38, on Zulip):

So what if we just mention in the glossary all layout components and explain them, but in the guarantees we stop using the term layout, and mention exactly what's guaranteed ?

RalfJ (Jul 25 2019 at 15:38, on Zulip):

you can still read_unaligned, but yes

gnzlbg (Jul 25 2019 at 15:39, on Zulip):

e.g. instead of saying the layout of T is the same as that of U, we'll say the size+align+fields+niche+callABI of T is the same as that of U

RalfJ (Jul 25 2019 at 15:39, on Zulip):

I tend to agree, "same layout" is just way too ambiguous as a term

rkruppe (Jul 25 2019 at 15:39, on Zulip):

Agreed.

RalfJ (Jul 25 2019 at 15:39, on Zulip):

I would be open though to having explicitly qualified shortcuts for common combinations

RalfJ (Jul 25 2019 at 15:40, on Zulip):

"full layout" = all of it, "memory layout" = this+that, ...
we shouldn't have too many, but say the two most common or so might make sense

rkruppe (Jul 25 2019 at 15:40, on Zulip):

Seems fine, but this is editorial polish IMO, nothing to prioritize now.

RalfJ (Jul 25 2019 at 15:41, on Zulip):

agreed

gnzlbg (Jul 25 2019 at 15:41, on Zulip):

maybe we can just say layout (SA), or layout (SAFNC) or similar, we can do that later when the correct things are written down

RalfJ (Jul 25 2019 at 15:41, on Zulip):

do we have unique first letters?

RalfJ (Jul 25 2019 at 15:41, on Zulip):

size, align, field, niche, callabi... seems like it. even if we add "(un)inhabitedness" one day.

gnzlbg (Jul 25 2019 at 15:42, on Zulip):

if we wanted to include padding we would use P

RalfJ (Jul 25 2019 at 15:42, on Zulip):

though if we do that we should agree on a consistent ordering

RalfJ (Jul 25 2019 at 15:42, on Zulip):

I dont want to have to scan if CNSAF and SAFNC are the same^^

gnzlbg (Jul 25 2019 at 15:42, on Zulip):

I'll open an issue to track this, documenting what we have discussed here

RalfJ (Jul 25 2019 at 15:42, on Zulip):

or just update the PR? :D

gnzlbg (Jul 25 2019 at 15:43, on Zulip):

i'll update the PR to expand things, but i mean, for the abbreviations, or adding new terms for the common uses

RalfJ (Jul 25 2019 at 15:43, on Zulip):

you already opened an issue to track "pushing through a changed def.n of layout"

RalfJ (Jul 25 2019 at 15:43, on Zulip):

seems like the same issue?

RalfJ (Jul 25 2019 at 15:44, on Zulip):

but either way, seems fine to me

RalfJ (Jul 25 2019 at 15:44, on Zulip):

next oldest PR: https://github.com/rust-lang/unsafe-code-guidelines/pull/154

gnzlbg (Jul 25 2019 at 15:44, on Zulip):

that was more about making niche and call ABI a part of layout, but we are punting the work of coming up with some abbreviations to avoid repeating stuff, I can reuse that issue for that

RalfJ (Jul 25 2019 at 15:44, on Zulip):

I think I'll just merge that one? @rkruppe ?

gnzlbg (Jul 25 2019 at 15:45, on Zulip):

while we could merge this @briansmith raises a good point

gnzlbg (Jul 25 2019 at 15:45, on Zulip):

why is this called "unsafe code guidelines" ?

RalfJ (Jul 25 2019 at 15:45, on Zulip):

because @nikomatsakis called it that way^^

RalfJ (Jul 25 2019 at 15:45, on Zulip):

eventually the hope is we can produce guidelines for unsafe code authors to follow

gnzlbg (Jul 25 2019 at 15:45, on Zulip):

sounds like we wanted to provide some general guidelines for unsafe code to rely on, but this is turned into groundwork for a full blown language spec

RalfJ (Jul 25 2019 at 15:46, on Zulip):

because its hard to give guidelines that we can commit to without any kind of spec...

gnzlbg (Jul 25 2019 at 15:46, on Zulip):

with all the drafts about an aliasing model, memory model, mir model maybe in the future

gnzlbg (Jul 25 2019 at 15:46, on Zulip):

so this is more like a "reference-evolution" working group

gnzlbg (Jul 25 2019 at 15:47, on Zulip):

where we prepare things that we want to specify in the reference, that aren't necessarily new language features per se

gnzlbg (Jul 25 2019 at 15:47, on Zulip):

and prepare RFCs for those, push them through the process, and then update the reference proper

RalfJ (Jul 25 2019 at 15:47, on Zulip):

well maybe at some point someone will join us who enjoys writing documents explaining "do this, dont do that". but other people outside the WG are already doing that (e.g. with MaybeUninit, or RustSec).

RalfJ (Jul 25 2019 at 15:48, on Zulip):

and we are also the go-to place when people ask "can unsafe code do this or rely on that"

RalfJ (Jul 25 2019 at 15:48, on Zulip):

so there's some amount of unsafe code guidance in what we do

gnzlbg (Jul 25 2019 at 15:48, on Zulip):

usually you would ask the spec for that

RalfJ (Jul 25 2019 at 15:49, on Zulip):

sure

gnzlbg (Jul 25 2019 at 15:49, on Zulip):

or the rust reference in this case

RalfJ (Jul 25 2019 at 15:49, on Zulip):

well you'd ask someone who read and understood the spec ;)

gnzlbg (Jul 25 2019 at 15:49, on Zulip):

i don't know, I felt like @briansmith concern that that particular PR has not much to do with unsafe code is correct

rkruppe (Jul 25 2019 at 15:50, on Zulip):

I don't have a strong opinion on https://github.com/rust-lang/unsafe-code-guidelines/pull/154 but let's say I would not have suggested writing this up in UCG if someone has asked me beforehand.

RalfJ (Jul 25 2019 at 15:50, on Zulip):

it is completely in line with the other things in that file though

rkruppe (Jul 25 2019 at 15:50, on Zulip):

But yes it's in line with the other C standard interactions and sizes of types and so on we discuss

rkruppe (Jul 25 2019 at 15:50, on Zulip):

Mostly I think the point is this could be more useful elsewhere. Which doesn't make it useless here, but still a good point

RalfJ (Jul 25 2019 at 15:50, on Zulip):

okay I am going to merge then

RalfJ (Jul 25 2019 at 15:51, on Zulip):

@rkruppe any idea what "elsewhere" might be?

gnzlbg (Jul 25 2019 at 15:51, on Zulip):

ok, so I will open an issue in the reference to track down writing this somewhere else

rkruppe (Jul 25 2019 at 15:51, on Zulip):

Either reference or stdlib docs seems good

RalfJ (Jul 25 2019 at 15:52, on Zulip):

like the docs of the isize/usize type?

RalfJ (Jul 25 2019 at 15:52, on Zulip):

eventually the plan is for the WG to move towards "upstreaming things we agree on into RFCs/docs", I guess that would be part of this

rkruppe (Jul 25 2019 at 15:53, on Zulip):

Yes, on the types. Or perhaps on the trait impls that are justified by this observation.

RalfJ (Jul 25 2019 at 15:53, on Zulip):

seems reasonable

RalfJ (Jul 25 2019 at 15:53, on Zulip):

I personally just dont care enough about those things to push for it ;)

RalfJ (Jul 25 2019 at 15:54, on Zulip):

next PR, and last for today because the mtg is already ~10min over: https://github.com/rust-lang/unsafe-code-guidelines/pull/162

RalfJ (Jul 25 2019 at 15:54, on Zulip):

seems like an obvious statement, I guess my only question is "is this worth stating"^^

rkruppe (Jul 25 2019 at 15:57, on Zulip):

Doesn't hurt, merge it

rkruppe (Jul 25 2019 at 15:58, on Zulip):

At some point we'll have to follow up to the stride!=size matter that broke out in that thread but let's not punish the nice note for that

RalfJ (Jul 25 2019 at 15:58, on Zulip):

yay, we're like.. 25% through @gnzlbg's attempt at DoSing us with a dozen PRs ;)

RalfJ (Jul 25 2019 at 15:58, on Zulip):

At some point we'll have to follow up to the stride!=size matter that broke out in that thread but let's not punish the nice note for that

yeah

gnzlbg (Jul 25 2019 at 15:58, on Zulip):

I think the general feeling is that it will be good to have a chapter somewhere in the layout section summarizing zero-sized types

RalfJ (Jul 25 2019 at 15:58, on Zulip):

I am actually not sure about that any more

gnzlbg (Jul 25 2019 at 15:58, on Zulip):

i think the nomicon has a chapter on exotically sized types, including zero-sized, unsized, etc.

gnzlbg (Jul 25 2019 at 15:59, on Zulip):

I'll open an issue to track removing the stride != size stuff

gnzlbg (Jul 25 2019 at 16:02, on Zulip):

so we are done for today

RalfJ (Jul 25 2019 at 16:03, on Zulip):

we are. this was productive. thanks all!

gnzlbg (Jul 25 2019 at 16:04, on Zulip):

@RalfJ your last PR adds definitions for value / place +- expression . I think it would be nice if more people could review that, e.g., @Nicole Mazzuca and @rkruppe for example.

gnzlbg (Jul 25 2019 at 16:04, on Zulip):

wait why does that PR have 64 comments already?

RalfJ (Jul 25 2019 at 16:07, on Zulip):

because @centril found it^^

rkruppe (Jul 25 2019 at 16:08, on Zulip):

I'll gladly review/approve (took a quick look already and seems good) the glossary changes but it's impossible for me to keep up with the discussion of the other stuff. So if you split out the glossary change, it can be merged much sooner :)

RalfJ (Jul 25 2019 at 16:10, on Zulip):

yeah I was wondering about that

RalfJ (Jul 25 2019 at 16:10, on Zulip):

I should probably do that, though it will lose the comments for that part

gnzlbg (Jul 25 2019 at 16:10, on Zulip):

the split sounds good to me

gnzlbg (Jul 25 2019 at 16:10, on Zulip):

you can keep them in the WIP PR until those changes are merged

RalfJ (Jul 25 2019 at 16:11, on Zulip):

argh is it just me or does GH often time out recently when you click through its UI, and you have to re-load the entire page to get it working again?

RalfJ (Jul 25 2019 at 16:28, on Zulip):

btw @gnzlbg https://github.com/rust-lang/unsafe-code-guidelines/pull/160 has some outstanding comments

gnzlbg (Jul 25 2019 at 16:29, on Zulip):

yep, I don't know what to do about the padding one yet

gnzlbg (Jul 25 2019 at 16:29, on Zulip):

padding is not part of layout

gnzlbg (Jul 25 2019 at 16:30, on Zulip):

right now we informally call the unused space between fields "padding"

gnzlbg (Jul 25 2019 at 16:30, on Zulip):

unions do not have unused space between fields

gnzlbg (Jul 25 2019 at 16:30, on Zulip):

or at least repr(C) unions do not, because all fields start at offset 0

RalfJ (Jul 25 2019 at 16:31, on Zulip):

I'd just drop the new paragraph on padding

gnzlbg (Jul 25 2019 at 16:31, on Zulip):

trailing padding is clear, in that there is unused space after the last field

RalfJ (Jul 25 2019 at 16:32, on Zulip):

and the ZST statement again is of the kind "is that worth stating"? I mean why would they not participate with their alginment, and why would anyone assume that? doesn't warrant 15-20 lines of text IMO.

gnzlbg (Jul 25 2019 at 16:36, on Zulip):

I'd assume that, unless otherwise guaranteed, the behavior of using a ZST field in a repr(C) struct or union is undefined on FFI (and unspecified otherwise)

gnzlbg (Jul 25 2019 at 16:37, on Zulip):

Because the general definition of repr(C) is "do what C does" and in C this cannot happen

RalfJ (Jul 25 2019 at 16:40, on Zulip):

the text is written as if the default assumption was "the compiler just ignores them"

RalfJ (Jul 25 2019 at 16:40, on Zulip):

that is the only way I can interpret "(ZST) fields participate in layout computation". I mean, of course they do.

RalfJ (Jul 25 2019 at 16:40, on Zulip):

https://github.com/rust-lang/unsafe-code-guidelines/pull/161 is also blocked on talking about padding; these PRs are too early IMO, we don't have enough data/information/definitions to form an informed consensus there

centril (Jul 25 2019 at 18:13, on Zulip):

because @centril found it^^

fwiw, I was pointed to it ;)

RalfJ (Jul 25 2019 at 18:41, on Zulip):

ah, who's the traitor? :P

RalfJ (Jul 25 2019 at 19:09, on Zulip):

@gnzlbg I think https://github.com/rust-lang/unsafe-code-guidelines/pull/163/files is basically read to be merged except for that one wording change

gnzlbg (Jul 25 2019 at 21:01, on Zulip):

ah, who's the traitor? :P

I was forced to point it to them because you wrote "operational semantics" in it in a heading and I thought "Centril is into that"

RalfJ (Jul 25 2019 at 22:26, on Zulip):

:+1:

Lokathor (Jul 26 2019 at 00:08, on Zulip):

@RalfJ meeting is over, but I am a person who loves to write the "do this don't do that" documents

Lokathor (Jul 26 2019 at 00:09, on Zulip):

one new unsafe group recently is the Safety Dance https://github.com/rust-secure-code/safety-dance

RalfJ (Jul 26 2019 at 06:59, on Zulip):

@Lokathor great!
I guess the question is which kinds of "do this dont do that" documents are the most useful? The first coming to my mind is basic stuff around unsafe and UB, around obligations and who is trusting whom to get what right -- to avoid discussions like this, and to promote a style where every unsafe block is accompanied by a comment explaining why this is not UB (neither now nor later -- see Vec::set_len, sometimes unsafe operations are bad even if they do not cause immediate UB).
but the nomicon already covers some of this.

Lokathor (Jul 26 2019 at 07:14, on Zulip):

The whole discussion in that thread is in a sense funny because clearly the best way to make 26 blank vecs in an array is to use

fn main() {
    let v: Vec<u8> = Vec::new();
    let v_bytes: [u8; 24] = unsafe { core::mem::transmute(v) };
    let array_of_vecs: [[u8; 24]; 26] = [v_bytes; 26];
    let final_vecs: [Vec<u8>; 26] = unsafe { core::mem::transmute(array_of_vecs) };
    println!("{:?}", final_vecs);
}

And then you don't need uninitialized or MaybeUninit at all.

RalfJ (Jul 26 2019 at 07:26, on Zulip):

btw @Lokathor our regular meeting time is Thursdays 15:15 UTC (anchored to New York time, so we shift by 1h as DST starts/ends there), the intended length is 30min, and the usual idea is to not discuss content but process -- in particular, which PRs to merge, which PRs/issues to close/open.

RalfJ (Jul 26 2019 at 07:28, on Zulip):

@Lokathor I would not call that the best way... also I am not even sure if you are entirely serious^^ but in case you are, (a) this only works on 64bit, (b) this breaks even there when Vec changes size; (c) this is UB if Vec ever has padding.

RalfJ (Jul 26 2019 at 07:29, on Zulip):

(d) this relies on Vec::new implementation details

Lokathor (Jul 26 2019 at 07:31, on Zulip):

It's possible I could be present for the next one if you like. That's around 9am for me, which isn't unreasonable given the rest of my schedule lately. However, I'm not sure I'd be much help at PR/issue triage for UCG.

Lokathor (Jul 26 2019 at 07:34, on Zulip):

As to the vec thing, (a) you could use size_of or you could call it a feature that the code stops compiling when the unexpected happens. (b) the standard library says "Due to its incredibly fundamental nature, Vec makes a lot of guarantees about its design. ... Most fundamentally, Vec is and always will be a (pointer, capacity, length) triplet. No more, no less." (c) I don't understand your meaning. (d) I can live with that, and since Vec::new is on track to even become const I can extra live with that. I wouldn't tell people to do it for every possible type, just for Vecs and other very well understood and specified types.

Lokathor (Jul 26 2019 at 07:36, on Zulip):

I was mildly serious: I would always rather do a transmute dance or a plain and simple zeroed than deal with a single bit of uninit in any form. Perhaps that's just my personal style

RalfJ (Jul 26 2019 at 08:11, on Zulip):

It's possible I could be present for the next one if you like. That's around 9am for me, which isn't unreasonable given the rest of my schedule lately. However, I'm not sure I'd be much help at PR/issue triage for UCG.

up to you, I just mentioned that in case you'd be interested. :)

RalfJ (Jul 26 2019 at 08:13, on Zulip):

(c) is basically also resolved by your answer to (b), but what I mean is that given (u8, u16), a transmute to [u8; 4] is UB because there is an uninitialized byte here: the padding byte.

Last update: Nov 19 2019 at 18:50UTC