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

Topic: Type punning and "active field" of a union


RalfJ (Jun 29 2019 at 10:24, on Zulip):

Some people think we are worse than C when it comes to unions :(
https://www.reddit.com/r/rust/comments/c5w36i/brave_browser_from_the_inventor_of_javascript/es6vpdd/

RalfJ (Jun 29 2019 at 10:25, on Zulip):

they cite in particular https://rust-lang.github.io/rfcs/1444-union.html#unions-and-undefined-behavior as evidence for Rust tracking an "active field" of a union. may ewe could improve the wording there?

gnzlbg (Jun 29 2019 at 11:48, on Zulip):

what does the nomicon say ?

gnzlbg (Jun 29 2019 at 11:48, on Zulip):

AFAICT it's not worth it to update those RFCs, they are historic documents

gnzlbg (Jun 29 2019 at 11:49, on Zulip):

the RFCs get merged into the reference, and that's maintained, so if anything, update the reference / nomicon

RalfJ (Jun 29 2019 at 11:57, on Zulip):

AFAICT it's not worth it to update those RFCs, they are historic documents

I am not sure if that is really clear to everyone

RalfJ (Jun 29 2019 at 11:57, on Zulip):

also the RFC there is not outdated, just badly worded

RalfJ (Jun 29 2019 at 11:58, on Zulip):

the nomicon does not have a section on unions

gnzlbg (Jun 29 2019 at 12:05, on Zulip):

we have updated RFCs in the past

gnzlbg (Jun 29 2019 at 12:06, on Zulip):

but bandwidth-wise if the feature is already in the reference / nomicon it's often most worth it to update it there

Lokathor (Jun 29 2019 at 14:48, on Zulip):

Most people just check the RFCs. It needs to be communicated better that the Reference is the actual authority.

RalfJ (Jun 29 2019 at 15:43, on Zulip):

we did put this into the reference though: https://doc.rust-lang.org/stable/reference/items/unions.html

RalfJ (Jun 29 2019 at 15:43, on Zulip):

Unions have no notion of an "active field".

gnzlbg (Jun 29 2019 at 16:32, on Zulip):

Most people just check the RFCs. It needs to be communicated better that the Reference is the actual authority.

I don't think we have any data about what most people check. But if I had to guess, I'd guess that most people check the book, then the libstd API docs, then nomicon, then reference, and only few people check RFCs (this group might still be hundreds of people though).

gnzlbg (Jun 29 2019 at 16:33, on Zulip):

we did put this into the reference

So the main unanswered question IMO is "why do some users go to the union RFC to learn the feature instead of going to the reference?".

Shnatsel (Jun 29 2019 at 17:09, on Zulip):

Hot take: people check whatever comes up as the first result on google.

Shnatsel (Jun 29 2019 at 17:10, on Zulip):

Which may be any of these resources depending on the query.

RalfJ (Jun 29 2019 at 17:13, on Zulip):

the reference has a disclaimer: "this is all WIP" etc. maybe the rendered RFCs should have similar disclaimer?

RalfJ (Jun 29 2019 at 17:13, on Zulip):

the way it looks now, I'd rather trust the RFC than the ref, as the RFC does not come with any kind of disclaimer^^

Lokathor (Jun 29 2019 at 19:04, on Zulip):

@RalfJ I know you put it in the Reference, my point is that people don't read the reference much so they won't know what you put in it.

@gnzlbg my data is anecdotal at best, but I've generally seen people in Discord cite docs.rs and RFCs, but I've honestly never seen people cite the Reference.

why don't users go to the reference?

I would also guess that it's because of the big red disclaimer. I have never trusted the reference and basically immediately close the tab if a page sends me there.

That disclaimer needs to go, and as soon as possible, if you want people to trust the Reference. That's my take

RalfJ (Jun 29 2019 at 19:14, on Zulip):

funny enough, the nomicon does not have such a disclaimer^^

RalfJ (Jun 29 2019 at 19:15, on Zulip):

so I tend to agree, the reference disclaimer is a bit over-the-top, at least compared to what we do in other "books"

centril (Jun 30 2019 at 12:18, on Zulip):

That disclaimer needs to go, and as soon as possible, if you want people to trust the Reference. That's my take

But we don't want people to trust the reference because it is not normative

Lokathor (Jun 30 2019 at 15:34, on Zulip):

The problem with people not trusting the Reference is that people end up not trusting the Reference

RalfJ (Jun 30 2019 at 15:36, on Zulip):

well the problem is that they trust something else, something that's not really any more normative or up-to-date or accurate than the reference

Lokathor (Jun 30 2019 at 15:41, on Zulip):

I don't think most people want documents that are normative now and forever. I think they usually want best effort descriptions of how things currently work even if it changes later.

Lokathor (Jun 30 2019 at 15:42, on Zulip):

Being set in stone forever is nice and all, but it takes a while to get there (and that's okay).

centril (Jul 01 2019 at 01:54, on Zulip):

I emphatically do not want people to rely on whatever best effort non-normative things we say in the reference, in the UCG, or anywhere else. This ties our hands because eventually someone says "there's too much code to break".

centril (Jul 01 2019 at 01:54, on Zulip):

Explicit non-guarantees are a good thing tho

centril (Jul 01 2019 at 01:55, on Zulip):

as well as the compiler aggressively taking advantage of unspecified behavior as if it were UB

RalfJ (Jul 01 2019 at 17:22, on Zulip):

as well as the compiler aggressively taking advantage of unspecified behavior as if it were UB

that's not at all easy

gnzlbg (Jul 01 2019 at 17:23, on Zulip):

@RalfJ note that C does not have the notion of an active field - only C++ has that

RalfJ (Jul 01 2019 at 17:24, on Zulip):

are you sure? C also needs to rule out type-punning through unions unless the compiler can "see" that there is a union involved

RalfJ (Jul 01 2019 at 17:29, on Zulip):

there's that weird clause about only doing union-based type punning if the union is "in scope" or so. doesnt that require tracking the "active field" to define precisely?

gnzlbg (Jul 01 2019 at 17:30, on Zulip):

C allows type punning through unions

RalfJ (Jul 01 2019 at 17:31, on Zulip):

only in very specific ways

RalfJ (Jul 01 2019 at 17:31, on Zulip):

just wrote this up the other day so I still have the example :D

RalfJ (Jul 01 2019 at 17:31, on Zulip):

this is UB in C:

float transmute_int_to_float_inner(int in, int *x, float *f) {
    *x = in;
    return *f;
}

// In a different file

union TypePun {
    int x;
    float f;
};

float transmute_int_to_float(int in) {
    union TypePun p;
    return transmute_int_to_float_inner(in, &p.x, &p.f);
}
RalfJ (Jul 01 2019 at 17:31, on Zulip):

it has to be UB, or else type-based alias analysis does not work

gnzlbg (Jul 01 2019 at 17:32, on Zulip):

https://port70.net/~nsz/c/c11/n1570.html#note95

RalfJ (Jul 01 2019 at 17:32, on Zulip):

"Warning: Potential Security Risk Ahead"

RalfJ (Jul 01 2019 at 17:32, on Zulip):

looks like they are still using GeoTrust certs

RalfJ (Jul 01 2019 at 17:33, on Zulip):

which means their admin hasnt checked their setup for many months. that's the real security risk^^

gnzlbg (Jul 01 2019 at 17:33, on Zulip):

C does not have type-based alias analysis

RalfJ (Jul 01 2019 at 17:33, on Zulip):

uh. what? yes it does.

gnzlbg (Jul 01 2019 at 17:33, on Zulip):

only C++ has that

RalfJ (Jul 01 2019 at 17:33, on Zulip):

that's what all the effective type stuff is for

RalfJ (Jul 01 2019 at 17:33, on Zulip):

I am very sure that GCC and clang both to TBAA in C

gnzlbg (Jul 01 2019 at 17:33, on Zulip):

ah yes, you are right, i'm misremembering

gnzlbg (Jul 01 2019 at 17:34, on Zulip):

so yes, that code is UB in C

gnzlbg (Jul 01 2019 at 17:34, on Zulip):

those two pointers can be assumed not to alias

gnzlbg (Jul 01 2019 at 17:34, on Zulip):

but that is kind of unrelated to type punning - C does support type punning, but your example has a TBAA violation

RalfJ (Jul 01 2019 at 17:35, on Zulip):

TBAA works by forbidding type punning :D

RalfJ (Jul 01 2019 at 17:35, on Zulip):

see see https://stackoverflow.com/questions/34616086/union-punning-structs-w-common-initial-sequence-why-does-c-99-but-not/34641113

gnzlbg (Jul 01 2019 at 17:35, on Zulip):

In repr(Rust) union that doesn't work either, because creating a reference to a field would be an error - and you need to do that to figure the field offsets

RalfJ (Jul 01 2019 at 17:35, on Zulip):

the standard basically says that type punning through unions only works if the union is "visible"

RalfJ (Jul 01 2019 at 17:35, on Zulip):

nobody knows what that means

gnzlbg (Jul 01 2019 at 17:35, on Zulip):

so repr(Rust) unions don't support type punning at all

RalfJ (Jul 01 2019 at 17:36, on Zulip):

that's entirely off-topic as the reasons are unrelated

gnzlbg (Jul 01 2019 at 17:36, on Zulip):

C supports type punning by using the union as a transmute

gnzlbg (Jul 01 2019 at 17:36, on Zulip):

union U { t }.v

RalfJ (Jul 01 2019 at 17:36, on Zulip):

the common interpretation of the "visible" thing is that you need to do something like u.f or so for the type-changing access -- the place needs to be computed "through" the union type

gnzlbg (Jul 01 2019 at 17:37, on Zulip):

exactly

RalfJ (Jul 01 2019 at 17:37, on Zulip):

but that's a very restricted form of union-based type punning

RalfJ (Jul 01 2019 at 17:37, on Zulip):

the moment you take a pointer, you're out

gnzlbg (Jul 01 2019 at 17:37, on Zulip):

not always

RalfJ (Jul 01 2019 at 17:37, on Zulip):

if there would be an agreed-upon operational semantics for C, that could be made precise; the standard weasels its way around that with this "visible" rule

RalfJ (Jul 01 2019 at 17:38, on Zulip):

but saying "TBAA violation" is the wrong way around. you have to prove TBAA correct by deriving it from what the standard says about effective types.

RalfJ (Jul 01 2019 at 17:38, on Zulip):

and effective types are not enough with unions, because of that clause that says that unions re-interpret the storage at the type used for reading. so this needs to be restricted to enable TBAA.

gnzlbg (Jul 01 2019 at 17:39, on Zulip):

Let's make your example a bit worse:

float transmute_int_to_float_inner(int in, union TypePun* u,  int *x, float *f) {
    // ...
}

Can the compiler assume that u and e.g. x do not alias?

gnzlbg (Jul 01 2019 at 17:39, on Zulip):

I'd say so, because they have different types.

RalfJ (Jul 01 2019 at 17:39, on Zulip):

C++ does it with active fields. C does it with a "visible" rule that is imprecise. a strict reading of the standard would make my code above okay if its all in one file as the union type is "visible" everywhere.

gnzlbg (Jul 01 2019 at 17:40, on Zulip):

i'm much more familiar with the C++ standard than with the C standard

RalfJ (Jul 01 2019 at 17:40, on Zulip):

I mean this discussion is not really interesting for Rust anyway^^ Rust doesnt need any of this mess of a "visible" rule or an active field because we dont want TBAA

gnzlbg (Jul 01 2019 at 17:43, on Zulip):

Not having TBAA removes many headaches

gnzlbg (Jul 01 2019 at 17:44, on Zulip):

But not all of them

gnzlbg (Jul 01 2019 at 17:45, on Zulip):

For example, given:

#[repr(C)] union U { x: SmallType, y: LargeType }
assert!(size_of::<LargeType>() > size_of::<SmallType>());
let mut u: U;
let ptr: *mut SmallType = &mut u.x as *mut _;
for i in 0..size_of::<LargeType>() {
    (ptr as *mut u8).add(i).write(0);
}

Is that ok ? I mean, technically the storage is there, but *mut u8 was derived from a &mut SmallType, and its accessing memory OOB of SmallType. Do we want to optimize based on that not happening?

gnzlbg (Jul 01 2019 at 17:45, on Zulip):

(deleted)

RalfJ (Jul 01 2019 at 17:48, on Zulip):

Stacked Borrows says this is NOT okay because when casting a mutable ref to a raw ptr, only the bytes that this ref actually points to become "raw-accessible"

gnzlbg (Jul 01 2019 at 17:49, on Zulip):

In C++ this is not ok either.

RalfJ (Jul 01 2019 at 17:49, on Zulip):

this is tracked at https://github.com/rust-lang/unsafe-code-guidelines/issues/134

RalfJ (Jul 01 2019 at 17:49, on Zulip):

I dont know yet if this is practical, but relaxing it has to be done very carefully or else it will destroy optimizations

gnzlbg (Jul 01 2019 at 17:49, on Zulip):

There I had to cast it to an u8, but I can make SmallType == u8, and then it's kind of a bit less obvious.

gnzlbg (Jul 01 2019 at 17:50, on Zulip):
#[repr(C)] union U { x: u8, y: LargeType }
assert!(size_of::<LargeType>() > size_of::<u8>());
let mut u: U;
let ptr: *mut SmallType = &mut u.x as *mut _;
for i in 0..size_of::<LargeType>() {
    ptr.add(i).write(0);
}
RalfJ (Jul 01 2019 at 17:50, on Zulip):

no, you should cast the union ptr to raw

RalfJ (Jul 01 2019 at 17:50, on Zulip):

then you can use that for either field

gnzlbg (Jul 01 2019 at 17:50, on Zulip):

Yeah, I know, but it looks like an easy error to make.

gnzlbg (Jul 01 2019 at 17:50, on Zulip):

And then you get UB.

RalfJ (Jul 01 2019 at 17:52, on Zulip):

indeed, that's why I opened that issue

gnzlbg (Jul 01 2019 at 17:55, on Zulip):

If we want to be able to use getelementptr with inbounds I don't see how to avoid being strict there.

RalfJ (Jul 01 2019 at 17:56, on Zulip):

hu? dont see the connection

RalfJ (Jul 01 2019 at 17:56, on Zulip):

we could just say "it makes the entire allocation accessible to raw ptr"

RalfJ (Jul 01 2019 at 17:56, on Zulip):

the hard part is not destroying reference-based reorderings

gnzlbg (Jul 01 2019 at 17:56, on Zulip):

I mean, the reason this rule exists in C++ is to allow using getelementptr with inbounds

gnzlbg (Jul 01 2019 at 17:57, on Zulip):

I thought that was the optimization that you had in mind. Was it something else?

RalfJ (Jul 01 2019 at 17:57, on Zulip):

I mean, the reason this rule exists in C++ is to allow using getelementptr with inbounds

which rule?

gnzlbg (Jul 01 2019 at 17:58, on Zulip):

That a pointer to an object (or sub-object) can only point to the addresses in range of [object_addr, object_addr + size + 1].

RalfJ (Jul 01 2019 at 17:58, on Zulip):

I thought that was the optimization that you had in mind. Was it something else?

something like

fn foo(x: &mut (u8, u8)) -> u8 {
  let raw = &mut x.1 as *mut u8;
  x.0 = 14;
  // lots of code, using raw, but not using x.
  return x.0; // can be optimized to: return 14;
}
RalfJ (Jul 01 2019 at 17:59, on Zulip):

That a pointer to an object (or sub-object) can only point to the addresses in range of [object_addr, object_addr + size + 1].

if they would allow it to "snap" to a surrounding union, that would still permit inbounds

gnzlbg (Jul 01 2019 at 17:59, on Zulip):

I mean, that optimization is doable because you assume that raw cannot be used to access x.0

gnzlbg (Jul 01 2019 at 17:59, on Zulip):

typically this is done by assuming that raw can only be used to access x.1

gnzlbg (Jul 01 2019 at 18:00, on Zulip):

C++ just forbids all pointer arithmetic that results in a pointer address that's not inbounds of x.1

RalfJ (Jul 01 2019 at 18:00, on Zulip):

I wasnt aware C++ forbids this. LLVM allows it.

gnzlbg (Jul 01 2019 at 18:00, on Zulip):

it supports the one-past-end rule, so that you can write `for (; ptr != one_past_end; ++ptr)

RalfJ (Jul 01 2019 at 18:00, on Zulip):

I think de-facto-C++ will have to allow it too

gnzlbg (Jul 01 2019 at 18:00, on Zulip):

LLVM allows it, unless you use getlementptr inbounds

RalfJ (Jul 01 2019 at 18:01, on Zulip):

I would expect many people will do something like "take address of first element of an array / vector / whatever, and use that to access other elements"

RalfJ (Jul 01 2019 at 18:01, on Zulip):

LLVM allows it, unless you use getlementptr inbounds

that is incorrect

RalfJ (Jul 01 2019 at 18:01, on Zulip):

inbounds referes to the bounds of the allocation

RalfJ (Jul 01 2019 at 18:01, on Zulip):

not of the subobject that you are indexing into

RalfJ (Jul 01 2019 at 18:01, on Zulip):

LLVM allows this with inbounds

gnzlbg (Jul 01 2019 at 18:03, on Zulip):

hmmm, am i misremembereing again ?

gnzlbg (Jul 01 2019 at 18:03, on Zulip):

does inbounds allow passing a size ? one option does not, but another option does, let me look

gnzlbg (Jul 01 2019 at 18:04, on Zulip):

@RalfJ inrange <ty> idx

gnzlbg (Jul 01 2019 at 18:05, on Zulip):

If the inrange keyword is present before any index, loading from or storing to any pointer derived from the getelementptr has undefined behavior if the load or store would access memory outside of the bounds of the element selected by the index marked as inrange.

RalfJ (Jul 01 2019 at 18:05, on Zulip):

inrange... never seen that

RalfJ (Jul 01 2019 at 18:05, on Zulip):

interesting. I dont think we emit it anywhere currently?

RalfJ (Jul 01 2019 at 18:06, on Zulip):

and TBH this entire subobject thing is really complicated when you try to do it formally. so I am very happy we don't do it.

gnzlbg (Jul 01 2019 at 18:06, on Zulip):

so with getelementptr inbounds ... inrange <ty> idx you say that at the idx within the allocation there is a <ty>, and that the resulting pointer cannot be used to access the allocation out of that type storage

gnzlbg (Jul 01 2019 at 18:07, on Zulip):

so if we say that in let a [0, 1, 2, 3]; let p = &a[0]; the p and pointers derived from it can only be used to access the first i32, we can emit an inrange i32 0.

gnzlbg (Jul 01 2019 at 18:08, on Zulip):

The result of a pointer comparison or ptrtoint (including ptrtoint-like operations involving memory) involving a pointer derived from a getelementptr with the inrange keyword is undefined, with the exception of comparisons in the case where both operands are in the range of the element selected by the inrange keyword, inclusive of the address one past the end of that element. Note that the inrange keyword is currently only allowed in constant getelementptr expressions.

gnzlbg (Jul 01 2019 at 18:09, on Zulip):

the one past the end there allows the for (; ptr != one_past_end; ++ptr) case

RalfJ (Jul 01 2019 at 18:11, on Zulip):

so if we say that in let a [0, 1, 2, 3]; let p = &a[0]; the p and pointers derived from it can only be used to access the first i32, we can emit an inrange i32 0.

I agree... interesting how Stacked Borrows "replaces" an explicit model of subobjects here

RalfJ (Jul 01 2019 at 18:12, on Zulip):

I wonder if we can similarly use Stacked Borrows to justify inbounds. I mean we already basically do for references. just not for raw pointers.

gnzlbg (Jul 01 2019 at 18:20, on Zulip):

I think that would be really cool

RalfJ (Jul 01 2019 at 18:21, on Zulip):

I suggested for this reason that we dont do inbounds when a raw ptr is used

RalfJ (Jul 01 2019 at 18:21, on Zulip):

but @rkruppe didnt like the idea ;)

rkruppe (Jul 01 2019 at 18:24, on Zulip):

I do like getting rid of the part that corresponds to "stay in the same (sub)object" part. I just don't want to lose the other aspect, that the offset is less than isize::MAX.

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

ah right you were mostly worried about overflows

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

making overflows for field accesses UB is fairly easy I think, Miri probably already does that

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

@rkruppe does inrange allow any interesting optimizations ?

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

but note the part I am worried about is not "stay in the same (sub)object", it's "stay in the same allocation"

rkruppe (Jul 01 2019 at 18:30, on Zulip):

I parenthesized the "sub" part for a reason! And object/allocation is tomayto, tomahto

rkruppe (Jul 01 2019 at 18:30, on Zulip):

@gnzlbg I don't know.

RalfJ (Jul 01 2019 at 18:36, on Zulip):

I parenthesized the "sub" part for a reason! And object/allocation is tomayto, tomahto

oh. not sure if that's how everyone uses the word. ;)

Last update: Nov 19 2019 at 18:35UTC