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

Topic: Meeting 2019-08-01


RalfJ (Aug 01 2019 at 15:13, on Zulip):

Hi @WG-unsafe-code-guidelines !

DutchGhost (Aug 01 2019 at 15:16, on Zulip):

hello! is this like a public meeting? sorry if I interrupted otherwise

RalfJ (Aug 01 2019 at 15:17, on Zulip):

it's public, otherwise it wouldn't be on Zulip. ;) we usually do some triage, basically, and think about which topics might be most interesting for the WG to look at next.

RalfJ (Aug 01 2019 at 15:17, on Zulip):

if you have things related to that, feel free to chime in!

RalfJ (Aug 01 2019 at 15:18, on Zulip):

otherwise please just open another topic here on zulip

RalfJ (Aug 01 2019 at 15:18, on Zulip):

if it's just me in terms of WG people I guess there is no meeting anyway.^^

RalfJ (Aug 01 2019 at 15:20, on Zulip):

so in terms of triage: I think the current state of our PRs is that https://github.com/rust-lang/unsafe-code-guidelines/pull/180 is read to be merged. I was just going to wait a bit if any other suggestions come up, and was hoping @gnzlbg or @rkruppe or so would be here to give me the green light. ;)
My other PR also hasn't seen comments in a bit, but it's a lot of text so I'll just let it sit for a bit mote.

RalfJ (Aug 01 2019 at 15:20, on Zulip):

@gnzlbg 's PRs AFAIK are all waiting for them to do some edits.

gnzlbg (Aug 01 2019 at 15:21, on Zulip):

i'm here

RalfJ (Aug 01 2019 at 15:21, on Zulip):

so, do you think we can merge https://github.com/rust-lang/unsafe-code-guidelines/pull/180 ?

gnzlbg (Aug 01 2019 at 15:22, on Zulip):

@RalfJ i was hoping to hear @Nicole Mazzuca opinion on that as well. But since the reference kind of uses those terms already and they haven't chimed in the discussion, I think we can merge this.

gnzlbg (Aug 01 2019 at 15:22, on Zulip):

Maybe they are not getting zulip notifications on RL got in their way. I can try pinging them on Discord

RalfJ (Aug 01 2019 at 15:22, on Zulip):

same here, but I also have not heard from them for a bit

RalfJ (Aug 01 2019 at 15:22, on Zulip):

having @nikomatsakis look at this would also be nice

RalfJ (Aug 01 2019 at 15:23, on Zulip):

and also I am using basically their proposed terms here ("value" and "place") so I dont expect objection to that, but they probably have opinions on how I defined those terms.

gnzlbg (Aug 01 2019 at 15:24, on Zulip):

merging its ok for me, I can't find nicole on discord either

RalfJ (Aug 01 2019 at 15:24, on Zulip):

okay, going to merge. it's not like that makes it immutable forever anyway.

RalfJ (Aug 01 2019 at 15:24, on Zulip):

but with this we have completed the TODO items that were added to the glossary long ago :)

RalfJ (Aug 01 2019 at 15:25, on Zulip):

@gnzlbg is there one of your PRs that you think I should look at again or that we should talk about? last time I checked there was outstanding feedback for all of them.

RalfJ (Aug 01 2019 at 15:26, on Zulip):

(no pressure, just trying to make sure I'm not missing things)

gnzlbg (Aug 01 2019 at 15:27, on Zulip):

I don't recall, had a paper deadline this week, and have been a bit disconnected

gnzlbg (Aug 01 2019 at 15:27, on Zulip):

we can just skim over the list and see if any is ready?

RalfJ (Aug 01 2019 at 15:27, on Zulip):

sure

gnzlbg (Aug 01 2019 at 15:27, on Zulip):

so i have 5 PR open

gnzlbg (Aug 01 2019 at 15:28, on Zulip):

what's blocking https://github.com/rust-lang/unsafe-code-guidelines/pull/153 ?

RalfJ (Aug 01 2019 at 15:29, on Zulip):

we talked about that one last weet at the mtg

RalfJ (Aug 01 2019 at 15:29, on Zulip):

and I left a note there about what we decided :D

RalfJ (Aug 01 2019 at 15:29, on Zulip):

https://github.com/rust-lang/unsafe-code-guidelines/pull/153#pullrequestreview-266745247

gnzlbg (Aug 01 2019 at 15:29, on Zulip):

https://github.com/rust-lang/unsafe-code-guidelines/issues/184

gnzlbg (Aug 01 2019 at 15:30, on Zulip):

i've punted the work of making subsection consistent

RalfJ (Aug 01 2019 at 15:30, on Zulip):

right but the PR still defines "layout includes these things"

RalfJ (Aug 01 2019 at 15:30, on Zulip):

which we decided is not how we wanted to approach this

gnzlbg (Aug 01 2019 at 15:30, on Zulip):

ok, so that PR is not ready, that still needs to happen

gnzlbg (Aug 01 2019 at 15:30, on Zulip):

I think I'll do the work in that PR

RalfJ (Aug 01 2019 at 15:31, on Zulip):

and the PR does change some parts in some sections; at least those it should use the new terminology for then

RalfJ (Aug 01 2019 at 15:31, on Zulip):

also to see if it really works

RalfJ (Aug 01 2019 at 15:31, on Zulip):

doing the sweeping change then can be left as future work (it should be rather clear at that point which parts are "old" and which are "new")

gnzlbg (Aug 01 2019 at 15:32, on Zulip):

i think i will do both there

gnzlbg (Aug 01 2019 at 15:32, on Zulip):

it's hard to know from the local change only whether this makes sense everywhere

gnzlbg (Aug 01 2019 at 15:32, on Zulip):

only when updating the rest did we realize that spelling everything out was a mouthful, and decided to do something else

RalfJ (Aug 01 2019 at 15:32, on Zulip):

yes

gnzlbg (Aug 01 2019 at 15:32, on Zulip):

we can then decide whether that's really what we want, and whether it should be split in different PRs

gnzlbg (Aug 01 2019 at 15:33, on Zulip):

https://github.com/rust-lang/unsafe-code-guidelines/pull/159 is blocked on clarifiying a bit field less and data-carrying enums, and on removing the &! bits

gnzlbg (Aug 01 2019 at 15:34, on Zulip):

i'd rather open an issue to clarify field-less and data-carrying enums a bit more, remove the &! parts, and see if we can merge the rest

gnzlbg (Aug 01 2019 at 15:34, on Zulip):

I'll split that PR in two

RalfJ (Aug 01 2019 at 15:35, on Zulip):

I also have some open comments in there

RalfJ (Aug 01 2019 at 15:35, on Zulip):

like not using Rust code to demonstrate layout equality things that cannot actually be demonstrated in code

gnzlbg (Aug 01 2019 at 15:35, on Zulip):

yep, I remove that

RalfJ (Aug 01 2019 at 15:35, on Zulip):

and "inhabited" is a bombshell that you dropped there^^

gnzlbg (Aug 01 2019 at 15:35, on Zulip):

you mention the comment was confusing, I don't have an opinion either way

gnzlbg (Aug 01 2019 at 15:35, on Zulip):

I did not forsee it would be so controversial

RalfJ (Aug 01 2019 at 15:36, on Zulip):

I think "inhabited" is a term not to be used without a precise definition

gnzlbg (Aug 01 2019 at 15:36, on Zulip):

I still think that this would hold for whatever definition of inhabited / uninhabited that we pick, or maybe put in another way, we should pick a definition such that it holds

RalfJ (Aug 01 2019 at 15:36, on Zulip):

I have been involved in long and heated discussions on the rust forums about that term^^

gnzlbg (Aug 01 2019 at 15:36, on Zulip):

but since we haven't done that, its wrong to force that here

gnzlbg (Aug 01 2019 at 15:37, on Zulip):

it should be discussed whether this is a constraint worth having when trying to define inhabited

RalfJ (Aug 01 2019 at 15:37, on Zulip):

IMO this story should start with a glossary definition of "uninhabited"

gnzlbg (Aug 01 2019 at 15:38, on Zulip):

yep, let's punt that to later and just remove those parts

gnzlbg (Aug 01 2019 at 15:38, on Zulip):

maybe open an issue to track defining uninhabited ?

RalfJ (Aug 01 2019 at 15:38, on Zulip):

'd rather open an issue to clarify field-less and data-carrying enums a bit more

I made a concrete small proposal for clarification. I dont think an issue is warranted. if you dont want to implement that proposal in your PR I can just make a PR for it.

RalfJ (Aug 01 2019 at 15:39, on Zulip):

maybe open an issue to track defining uninhabited ?

we have https://github.com/rust-lang/unsafe-code-guidelines/issues/165

RalfJ (Aug 01 2019 at 15:39, on Zulip):

I'd say that's the same discussion

gnzlbg (Aug 01 2019 at 15:39, on Zulip):

I'll update all PRs after the meeting, and we can discuss what to do inline (can't remember how small or large the changes were)

gnzlbg (Aug 01 2019 at 15:39, on Zulip):

sounds good

gnzlbg (Aug 01 2019 at 15:39, on Zulip):

https://github.com/rust-lang/unsafe-code-guidelines/pull/160#discussion_r307866029 is blocked on me rephrasing that, or removing it, will check later

RalfJ (Aug 01 2019 at 15:40, on Zulip):

yes

RalfJ (Aug 01 2019 at 15:40, on Zulip):

also that was the PR with the "pictures", right?

gnzlbg (Aug 01 2019 at 15:41, on Zulip):

yes, i'd prefer to add pictures afterwards to the struct an union sections

RalfJ (Aug 01 2019 at 15:41, on Zulip):

yes: https://github.com/rust-lang/unsafe-code-guidelines/pull/160#discussion_r307714990

RalfJ (Aug 01 2019 at 15:41, on Zulip):

sure, I can open an issue

gnzlbg (Aug 01 2019 at 15:41, on Zulip):

at some point, I think we should try to go through the book, and "summarize" stuff

RalfJ (Aug 01 2019 at 15:42, on Zulip):

https://github.com/rust-lang/unsafe-code-guidelines/pull/161 has some open comments

RalfJ (Aug 01 2019 at 15:42, on Zulip):

and not much discussion yet

gnzlbg (Aug 01 2019 at 15:42, on Zulip):

reading the struct and enum chapters feels like some simple things are explained with a lot of words

RalfJ (Aug 01 2019 at 15:42, on Zulip):

so I guess you can just go over them and see

RalfJ (Aug 01 2019 at 15:42, on Zulip):

reading the struct and enum chapters feels like some simple things are explained with a lot of words

that's what I was trying to fight in some of your PRs as well ;)

RalfJ (Aug 01 2019 at 15:43, on Zulip):

OTOH, precisely stating the obvious is an art on its own

gnzlbg (Aug 01 2019 at 15:43, on Zulip):

I think we should summarize in PRs that are "non-functional changes". As in, that PR cannot change semantics in any way.


https://github.com/rust-lang/unsafe-code-guidelines/pull/161
I think we can now slowly try to resolve those. Your PR with a definition for value representation is already merged?

gnzlbg (Aug 01 2019 at 15:43, on Zulip):

Instead of using padding, I think we can word #161 to just mention something about the bytes of the value representation

RalfJ (Aug 01 2019 at 15:43, on Zulip):

yes we can, but the mtg has 2 more minutes and then my bus comes, so lets do that in the PR :)

RalfJ (Aug 01 2019 at 15:44, on Zulip):

"bytes of the value rep" is not a term that has been defined anywhere yet

RalfJ (Aug 01 2019 at 15:44, on Zulip):

and I honestly dont know what it means

RalfJ (Aug 01 2019 at 15:44, on Zulip):

unless its a weird way to say "non-padding"

RalfJ (Aug 01 2019 at 15:45, on Zulip):

https://github.com/rust-lang/unsafe-code-guidelines/pull/164 should be updated to the "1-ZST" terminology. Otherwise this seems uncontroversial.

gnzlbg (Aug 01 2019 at 15:45, on Zulip):

the value relationship is a 1:1 relationship, not many to one

gnzlbg (Aug 01 2019 at 15:45, on Zulip):

or something like that

RalfJ (Aug 01 2019 at 15:45, on Zulip):

the relationship is between values and lists of bytes

RalfJ (Aug 01 2019 at 15:45, on Zulip):

so that doesnt even typecheck what you just said

gnzlbg (Aug 01 2019 at 15:45, on Zulip):

so saying that this relationship is injective

gnzlbg (Aug 01 2019 at 15:46, on Zulip):

would be a way to say that there is no padding

RalfJ (Aug 01 2019 at 15:46, on Zulip):

its a relation not a function and we use it both ways so "injective" isnt clear...

RalfJ (Aug 01 2019 at 15:47, on Zulip):

you mean the bytelist-to-val mapping is injective? different lists -> different values?

RalfJ (Aug 01 2019 at 15:47, on Zulip):

not sure if padding is the only cause of ambiguity that we could have there... like, I deliberately did not define the values for f32 ;)

gnzlbg (Aug 01 2019 at 15:47, on Zulip):

yes, if we use it both ways it would be bijective and invertible

gnzlbg (Aug 01 2019 at 15:48, on Zulip):

well for f32 it wouldn't be bijective right ?

RalfJ (Aug 01 2019 at 15:48, on Zulip):

and anyway that would still only help to define types with no padding at all, ut doesnt help to define what the "bytes of the value rep" are or what a "padding byte" is

gnzlbg (Aug 01 2019 at 15:49, on Zulip):

do we want to define what a padding byte is at all ?

RalfJ (Aug 01 2019 at 15:49, on Zulip):

well for f32 it wouldn't be bijective right ?

no idea. do we guarantee to preserve NaN bits?

gnzlbg (Aug 01 2019 at 15:49, on Zulip):

no we don't

gnzlbg (Aug 01 2019 at 15:49, on Zulip):

or at least, not yet, LLVM does not preserve them, so we currently cannot

RalfJ (Aug 01 2019 at 15:49, on Zulip):

does everyone know that?^^ seems like a huge footfun

gnzlbg (Aug 01 2019 at 15:49, on Zulip):

it should be in the reference somewhere

RalfJ (Aug 01 2019 at 15:49, on Zulip):

do we want to define what a padding byte is at all ?

I would prefer if we did not have to talk about this in the spec, ever, and it just arose naturally from the "typed copy" rules

RalfJ (Aug 01 2019 at 15:50, on Zulip):

but with the mess that is unions passed by value, I am not sure if we have that luxury

gnzlbg (Aug 01 2019 at 15:50, on Zulip):

so we can define padding as those bytes that are not copied on typed copies :D

RalfJ (Aug 01 2019 at 15:50, on Zulip):

that's way too informal, typed copies dont copy bytes

RalfJ (Aug 01 2019 at 15:50, on Zulip):

they perform two value-bytelist-conversions

RalfJ (Aug 01 2019 at 15:50, on Zulip):

anyway I got to go, cu!

gnzlbg (Aug 01 2019 at 15:51, on Zulip):

cu, we should continue that discussion in that issue

gnzlbg (Aug 01 2019 at 15:52, on Zulip):

we can try to find a way to specify that with the terms what we have, and if we can't, we shall identify which new definitions we need, and work towards adding those first, and then re-evaluate

RalfJ (Aug 01 2019 at 16:03, on Zulip):

and meanwhile the rest of that PR can proceed without this

Tom Phinney (Aug 01 2019 at 16:40, on Zulip):

Couldn't we just state "Padding consists of all bytes in the layout of a struct, or the layout of an enum with associated data, that were added by the compiler to provide required alignment for the explicitly-specified fields of the struct or enum." ?

RalfJ (Aug 01 2019 at 16:42, on Zulip):

@gnzlbg this is odd: https://github.com/rust-lang/unsafe-code-guidelines/blob/master/reference/src/layout/structs-and-tuples.md#zero-sized-structs
it talks about all repr's but is inside the "repr Rust" section

RalfJ (Aug 01 2019 at 16:44, on Zulip):

@Tom Phinney that would be an intensional definition of padding -- in terms of when it gets added, not how it behaves. That could also work.
You have to mention extra trailing padding though that has to be added sometimes with repr(align) to make the size a multiple of the alignment.

RalfJ (Aug 01 2019 at 16:44, on Zulip):

this def.n would basically delegate it to struct/enum/union to say what their respective padding bytes are

gnzlbg (Aug 01 2019 at 16:52, on Zulip):

this is odd
we can move this somewhere else

Tom Phinney (Aug 01 2019 at 16:57, on Zulip):

@RalfJ I believe that we have to define "padding" before we can specify its behavior. I had wondered about the trailing padding; some of the UCG discussions I've seen include it but most do not. That's probably because it's inclusion seems mandatory only for items that are in an array (though I would like to consider it to also be needed to align with cache lines, etc). Personally I believe that trailing padding needs to be included.

Tom Phinney (Aug 01 2019 at 17:06, on Zulip):

Another way to define padding could be "Padding consists of all bytes in an actual layout that were added by the compiler to satisfy type alignment constraints." That definition implies that, in general, the programmer cannot predict exactly how much padding, if any, will be added. That situation is particularly apparent when rustc is free to reorder fields within a layout: (uint8, int16, int8) could have two bytes of padding if laid out in that order, or no bytes if laid out as (int8, uint8, int16).

RalfJ (Aug 01 2019 at 17:08, on Zulip):

@gnzlbg yeah, we should likey have a section on ### Stuff that is true for all repr's

RalfJ (Aug 01 2019 at 17:09, on Zulip):

believe that we have to define "padding" before we can specify its behavior

I disagree. See https://github.com/rust-lang/unsafe-code-guidelines/issues/183 for a definition.
Most of the behavior of padding (at least the part where padding bytes are not preserved on copis) is actually specified in https://github.com/rust-lang/unsafe-code-guidelines/pull/175, and the beauty is that the spec dosnt even need to talk about padding to specify that. :D

RalfJ (Aug 01 2019 at 17:09, on Zulip):

@Tom Phinney trailing padding is also mandatory for #[repr(align(8)] struct Foo(u8);

RalfJ (Aug 01 2019 at 17:10, on Zulip):

That definition implies that, in general, the programmer cannot predict exactly how much padding, if any, will be added.

I quite disagree, in fact it says very precisely that if type alignment constraints are met, no padding will be added.
there are proposals for having padding in unions in a way that has nothing to do with type alignment constraints.

Tom Phinney (Aug 01 2019 at 17:38, on Zulip):

^^ In that case the type size constraint needs to include both any initial padding of the first byte of the type and any terminal padding after the last byte of the type. Otherwise the prefix padding for the second element of an array would include the implied suffix padding for the first element, and the array as a whole could have unspecified suffix padding. (Sorry if I'm using "wrong" terminology here. I'm just trying to convey what I see as the issue.)

As to https://github.com/rust-lang/unsafe-code-guidelines/issues/183, I don't feel bad about not knowing about an issue that was opened only yesterday.

RalfJ (Aug 01 2019 at 18:31, on Zulip):

I didnt say you should feel bad. :) just pointed out that padding can indeed be defined "extensionally", based on behavior that the Rust Abstract Machine has anyway.

Tom Phinney (Aug 01 2019 at 18:39, on Zulip):

Obviously the UCG team has decided that the Rust Abstract Machine (RustAM) should model non-trapping NotAValues (NaVs) as 0xUU Should the RustAM also model Itanium-style trapping NaVs, or is the UCG presuming that no future ISA will impose a similar poorly-though-out "feature"?

RalfJ (Aug 01 2019 at 18:40, on Zulip):

nothing around 0xUU is decided. that's all just my personal proposals so far. But it is the only really concrete proposal we got.

RalfJ (Aug 01 2019 at 18:41, on Zulip):

I have read about trapping "NaVs" on Itanium but do not know much about them and do not know what it would take for the spec to account for them

RalfJ (Aug 01 2019 at 18:42, on Zulip):

I think if we declared let x: u8 = mem::uninitialized() allowed we might actually be able to handle itanium. or maybe not.

Tom Phinney (Aug 01 2019 at 19:06, on Zulip):

My understanding is that the Itanium trapping NaVs are not in primary memory, but only in registers and (probably) cache lines. If so, it's more a compiler problem that the compiler can predict. Any load of a cache line from primary memory will have 0xUUs rather than trapping NaVs. Of course I may be incorrect; I've never had to work with the Itanium ISA and, at the moment, have to run so I can't research it before posting this reply.

gnzlbg (Aug 01 2019 at 20:51, on Zulip):

@Tom Phinney i don't think Rust support Itanium, at least, not yet

gnzlbg (Aug 01 2019 at 20:52, on Zulip):

if it does, the current proposal means that copying NaVs is ok

gnzlbg (Aug 01 2019 at 20:54, on Zulip):

i don't know if that would mean that Rust wouldn't be able to target Itanium

Tom Phinney (Aug 02 2019 at 09:06, on Zulip):

Okay, I've done a very brief investigation of the IA64 (also known as Intel Itanium) Instruction Set Architecture (ISA), based on https://www.intel.com/content/dam/www/public/us/en/documents/manuals/itanium-architecture-vol-3-manual.pdf.

Disclaimer: The following analysis might be in error.

Most architecture registers have an associated NaT (Not a Thing) status flag, whose primary purpose is to invalidate speculative execution results that need to be discarded because the execution trace took a different path. These bits are not programmer-visible in other out-of-order architecture implementations, but in IA64 they are programmer visible. Conclusion 1: rustc is unlikely to encounter this "feature" in any future ISA.

If rustc is ported to IA64, which seems not very likely since the IA64 product line is considered obsolete, then there are two IA64 extensions of the per-architecture-register NaT flags that will need to be considered. First, it is possible to tag pages in the memory map as NaT pages, so that any load from such a page propagates the NaT status to the architecture register containing the loaded value. Second, and much more important, there is an IA64 architecture register that contains a configurable floating-point NaN value (called NaTVal) which, when encountered, propagates preemptively through all IA64 floating-point operations. That permits floating-point values whose status is still speculative to be spilled to memory and later reloaded without losing their NaT status. Conclusion 2: The RustAM should not bother to model this IA64 peculiarity.

RalfJ (Aug 02 2019 at 09:14, on Zulip):

The RustAM should not bother to model this IA64 peculiarity

I like this conclusion ;)

Thanks @Tom Phinney !

nagisa (Aug 02 2019 at 12:29, on Zulip):

I can see other WLIW architectures using a similar mechanism TBQH, but I think that making NaT-like thing visible would throw a wrench not only into Rust’s wheels but also every other language’s as well.

Last update: Nov 19 2019 at 19:05UTC