Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should raw pointer deref/projections have to be in-bounds? #319

Closed
RalfJung opened this issue Feb 23, 2022 · 32 comments
Closed

Should raw pointer deref/projections have to be in-bounds? #319

RalfJung opened this issue Feb 23, 2022 · 32 comments

Comments

@RalfJung
Copy link
Member

RalfJung commented Feb 23, 2022

Currently, when you write addr_of!((*ptr).field), ptr has to be a sufficiently aligned and dereferencable pointer (meaning it has to point to actually allocated memory), with the size/alignment determined by the pointee type.

Formally speaking, this stems from the fact that the * operator, which turns a value (of pointer or reference type) into a place, requires that the pointer be aligned and dereferencable. This is true regardless of the context in which the * operator is used, i.e., regardless of what happens to this place after it has been constructed.

Pragmatically speaking, this reflects the fact that addr_of!((*ptr).field) is lowered to a getelementptr inbounds in LLVM. That said, the rules in the reference go further than what is required for codegen -- they require ptr to be dereferencable for the full size of the type (not just the field), and that even if we do addr_of!(*ptr).

This situation frequently causes confusion, so maybe we want to change it. At least it'd be good to have a central place for discussion and reference, so I created this issue.
Cc @eddyb @joshtriplett @thomcc @Lokathor @cuviper rust-lang/reference#1000
Instance(s) of confusion in practice: rust-lang/rust#93459 (comment)

So, what could we do? Let me collect some proposals that have come up.

Remove restriction from * entirely

The most radical option is to entirely remove the clause which imposes a rule on *, and instead just require that when a place is actually read from (via move/copy) or stored to (via =), then it has to be sufficiently aligned and dereferencable as determined by the type of the place. (We currently do not need such a rule, since as an invariant all places that are constructed are aligned and dereferencable.)

This means we have to remove inbounds from getelementptr in place projection lowering when the "source" of this place projection is a raw pointer. We can still add inbounds for references or other places (locals/statics) since we know those to be dereferencable for other reasons (except for this twist). (We can not add it in cases like &(*ptr).field since ptr might be e.g. 4 bytes before the beginning of the allocation and .field offsets the ptr by 4.)

This has the potential of making LLVM alias analysis and other optimizations less effective. In particular, getelementptr might not just go out-of-bounds, it would even be allowed to overflow, so there are some tricks LLVM might not be able to pull any more.

It is certainly my favorite option, because it maximally reduces the amount of UB -- the remaining rule (about actual loads/stores) is pretty much unavoidable and typically expected by programmers.

Still prevent overflows

We could also say that *ptr on a raw pointer is UB if adding size_of_val_raw(ptr) to ptr would overflow, but allocation bounds do not matter. This would let us emit a hypothetical getelementptr nowrap, if LLVM were to add support for such a flag in the future. I doubt this is any less surprising than our current rules, TBH, but it is less likely to be violated by the typical kind of code people write. It is also more complicated, since we still have to explicitly add the rule which says that loading from and storing to a place requires it to be dereferencable and aligned.

Check inbounds on place projection

If we want to keep codegen unchanged, we could still relax our rules: we could allow addr_of!(*ptr), and we could make it so that addr_of!((*ptr).field) only needs to be inbounds for the actual offset that is performed.

For example, on top of the "remove restrictions" proposal, we could have an extra rule for each place projection which says that this projection needs to be in-bounds.

This has the advantage of keeping our current codegen and quite obviously aligning with the intuition many people seem to have for place projections. It also has some disadvantages though:

  • addr_of!((*ptr).field) continues to be UB if ptr is entirely dangling, so e.g. people wanting to write offset_of-style code still have to navigate around a footgun.
  • Our rules become more complicated. (Basically, we would replace one bullet in the "Behavior considered undefined" list by two bullets.)

Magic?

There's also the option (which I'm aware you aren't a fan of) of allowing addr_of!((*p).field) to violate the inbounds rule but still requiring &(*ptr).field to uphold it.

#319 (comment)

New syntax

What you really just want to be able to really ergonomically talk about field offsets.

#319 (comment)

@thomcc
Copy link
Member

thomcc commented Feb 23, 2022

There's also the option (which I'm aware you aren't a fan of) of allowing addr_of!((*p).field) to violate the inbounds rule but still requiring &(*ptr).field to uphold it.

For better or worse, I think most peoples intuition is that addr_of! is somewhat magic. The reasons for this are numerous, but aside from it being a macro (and macros often being somewhat magic), "places" are not a first class concept in the language and thus most rust programmers who are not compiler engineers or have experience in this area do not think in terms of them, or are even aware they exist.

(I think loosing inbounds on raw pointers everywhere would probably be unfortunate (it's hard to say), but I think loosing it just on addr_of! probably is not a big deal -- it may disallow folding the address calculation into the load, but in practice that's not as a big deal on modern machines. My bigger concern is around this possibly being applied more broadly)

@hkratz
Copy link

hkratz commented Feb 23, 2022

I am wondering why dereferencing of dirent * does not seem to cause problems in C where de->d_name is also compiled down to getelementptr inbounds- Godbolt. I don't think I have ever seen C code that avoids dereferencing it. In the POSIX readdir sample they definitely do not.

@thomcc
Copy link
Member

thomcc commented Feb 23, 2022

C does the equivalent of option 3 there.

@RalfJung
Copy link
Member Author

RalfJung commented Feb 23, 2022

aside from it being a macro (and macros often being somewhat magic)

That is IMO a bad argument -- the only reason it is a macro is that we have not found a nice syntax we wanted to commit to. And certainly macros are less magic than "built-in syntax".

"places" are not a first class concept in the language and thus most rust programmers who are not compiler engineers or have experience in this area do not think in terms of them, or are even aware they exist.

In C, I hear programmers talk about lvalues quite a lot. So I think that similarly, in Rust, programmers are generally aware of the concept of a place. I think the rest can be solved with better documentation and teaching -- IMO most explanations of these concepts and how they affect program execution are rather bad, since they don't actually follow the syntactic structure of the program.

There's also the option (which I'm aware you aren't a fan of) of allowing addr_of!((*p).field) to violate the inbounds rule but still requiring &(*ptr).field to uphold it.

The question is how to even systematically specify the language (e.g. in a way that can be implemented in Miri) to achieve this. I think we would need to basically distinguish safe from unsafe places and then, conceptually, use a form of type inference so that at the *p, we know which kind of place is required.

I don't think that is a good idea. If people think addr_of is magic, the answer should be better documentation and teaching, not doubling down on the magic. I don't think we can expect people to reliably write correct unsafe code if they treat a core operator in their code as magic.

@eddyb
Copy link
Member

eddyb commented Feb 23, 2022

One reason I prefer the first option is because it allows us to make this kind of code safe (as in, not requiring the unsafe keyword) in the future:

addr_of!((*ptr::null::<Struct>()).field)

Whether or not that's a good way to implement offset_of! is another matter, but removing the need for unsafe would signal addr_of!((*ptr).field) can be treated as doing pretty much just this:

(ptr as *const u8).wrapping_add(offset_of!(Struct, field)) as *const FieldType

The unsafe operations, specific to raw pointers and the unsafe places based on them, would then be:

  • loading from/storing to an unsafe place
  • borrowing an unsafe place as a (safe) reference

I'm not sure if also removing autoderef from (*ptr).field and (*ptr)[idx] (i.e. when ptr is a raw pointer to a type that implements Deref, be it &T, Box<T>, Vec<T>, etc. - though this could also happen in the second half of (*ptr).field[idx] or (*ptr).field1.field2) would be backwards-compatible, but at the very least they would still require unsafe even when used inside addr_of!, and we could lint against them.

I believe the first time such an idea came up was in the context of youngspe/project-uninit#1 - that kind of thing feels like it has to "fight the language" to avoid hazards while doing something on behalf of the user.

@RalfJung
Copy link
Member Author

@eddyb at least with the first option, "unsafe place" here is a concept that only exists for unsafety checking, but not for the actual dynamic behavior of the program or whether code has UB. I like that. :D

I believe the first time such an idea came up was in the context of youngspe/project-uninit#1 - that kind of thing feels like it has to "fight the language" to avoid hazards while doing something on behalf of the user.

I am pretty sure I have seen this discussed (and discussed it myself) way earlier, in 2019 when https://github.com/Gilnaa/memoffset was moved towards "the canonical offset_of in the ecosystem".

@eddyb
Copy link
Member

eddyb commented Feb 23, 2022

I am pretty sure I have seen this discussed (and discussed it myself) way earlier

Ehm, sorry, my phrasing was ambiguous. For me, specifically, the idea of making some raw-pointer-based field offseting safe (and banning the existing autoderef, or least keeping it unsafe) only came more recently, when seeing this repeat in situations other than offset_of!.

Maybe it's also because for offset_of! it seems reasonable to limit it to one field, whereas project-uninit was allowing whole "field paths" like .a.b.c etc.

@Gankra
Copy link

Gankra commented Feb 23, 2022

It is my assertion that worrying about the precise semantics of *raw_ptr is treating a symptom and not the actual issue, which is that using * to temporarily enter "path mode" and then only actually specifying what you're talking about at the end with addr_of is a huge confusing mess. What you really just want to be able to really ergonomically talk about field offsets. As a complete strawman syntax that is ignoring parsing concerns:

// Applied to a raw pointer value.

// It's ok if this syntax isn't transactional and we're semantically
// GEP inboundsing to both `field` and `subfield` because subfields
// advance the offset monotonically, and GEP inbounds effectively
// asserts the start and end are in the same allocation and that the
// allocation spans the range between them.
my_ptr~field~subfield.write(val);
*my_ptr~field~subfield += 1;

and

// Applied to a type

// Same as above but caching the offset, possibly as a const.
let subfield_offset = MyType::<T>~field~subfield;
my_ptr.offset(subfield_offset).read()

Once you have this ergonomic syntax that doesn't mess around with places, it's perfectly fine for * to be some enormously powerful assertion about validity because no one will use it like that anymore. At most they'll use it at the very end of a series of offsets, where you do in fact want to assert validity of that final value.

Why won't they use it anymore? Because it is already syntactic salt! (*ptr).field is already super awkward and annoying, if we give them a better alternative (and maybe add lints...) they will happily move off of that horrible syntax!

@Gankra
Copy link

Gankra commented Feb 23, 2022

Like you can argue all you want about the precise semantics of *raw_ptr but I can't actually teach any of these models to a random person reading the rustonomicon. places/lvalues are deeply compilerese and very confusing when you're trying to very precisely write some hairy unsafe code.

@elichai
Copy link

elichai commented Feb 23, 2022

Because the main reason for violating the current rules is for calculating offsets, maybe the right solution is to provide an offset_of macro in the standard library that does exactly that? (and can be implemented via say an intrinsic).
Or a more sophisticated mechanism that allows the user to get the full layout of a type.

That might be enough to allow the current rules to stay as is (as there's no really any reason that I know of to use an invalid pointer other than calculating offsets)

@thomcc
Copy link
Member

thomcc commented Feb 23, 2022

I don't think the case in rust-lang/rust#93459 would be solved by an offset_of macro, I think it's a broader issue than just offset_of (although that is a component).

@RalfJung
Copy link
Member Author

places/lvalues are deeply compilerese and very confusing when you're trying to very precisely write some hairy unsafe code.

I am not convinced that is true. I was confused by places for a long time but I think I have reached "place enlightenment" some time in 2018 and I think this is something that can be shared -- none of the explanations for this that I saw out there were satisfying, including the one they gave me at university in my first semester.

Or maybe I am just delirious and stared into the sun too long, not sure. I certainly should actually write such a "comprehensible explanation of places" before making such big claims, which I haven't done yet, so maybe it is harder than I think. But I also think if places, which are a rather fundamental concept to Rust, are so hard to explain, then we have bigger problems.

@Gankra
Copy link

Gankra commented Feb 24, 2022

Places are like autoderef/autoref. You can and must specify them somewhere but no one using rust actually needs to understand them really. It's magic that the compiler does to make your code work, and it's fine that it's magic because the borrowchecker will catch anything that goes wrong (in my experience).

But we don't do autoref/autoderef for raw pointers because in that context wacky-wild-magic semantics are terrifying and not something we can help you use correctly. I regard places as much the same thing.

Now you can't completely eliminate places, but they should definitely be minimized. Most unsafe code has no need to ever actually use places, and just needs to offset and read/write. Code that does want to use places generally only really cares about applying them to the leaf of the access (what my strawman does) and not be the root of the access (what the current system does).


Aside:

Arguably you also want a postfix deref operator but once you are restricting the place to a leaf, you hardly need anything better than fn manual_deref_mut(self) -> &mut T on raw pointers. That specific API probably runs afoul of the carveouts for (*ptr).int_field = 3 being ok even if int_field is uninitialized. But at this point you want to just unconditionally do this anyway:

ptr~field1.write(1);
ptr~field2.write(true);
ptr~field3.write(my_struct);

Because it's more ergonomic and much more advisable because it's mindlessly correct compared to:

*ptr~field1 = 1;               // I am very clever
*ptr~field2 = true;            // and know this is safe
ptr~field3.write(my_struct);   // but carefully initialize this part with a dtor

I really want to be able to tell unsafe rust programmers:

"Here's this totally braindead and mindlessly correct way to do everything, and it's also the most ergonomic way to do it, so you'll want to do it anyway. (And now here's where you can be a clever smartass if you insist...)"

@RalfJung
Copy link
Member Author

I really want to be able to tell unsafe rust programmers:

"Here's this totally braindead and mindlessly correct way to do everything, and it's also the most ergonomic way to do it, so you'll want to do it anyway.

Fully agreed on that part. :)

@Lokathor
Copy link
Contributor

Lokathor commented Feb 24, 2022

Can't we just make offset_of! be a totally separate operation from addr_of! and then tell people: "Use addr_of! with live pointers and use offset_of! if you just want to calculate the offset". And then offset_of! can be the most magical merlin wizard thing ever, and also fully safe, and it's all fine because it's just doing a compile time calculation, it's not ever generating runtime code that becomes UB.

Because people only want to use addr_of! on null/dangling pointers because they want to try and implement offset_of!. Just making a properly blessed offset_of! in core seems like how we can keep all the magic in one spot, and also give people the tool they actually want.

EDIT: I'm a goof, elichai actually already mostly said this 8 hours ago.

@comex
Copy link

comex commented Feb 24, 2022

I rather like the "magic" option.

Speaking from a C perspective: I know what an lvalue is, but I intuitively think of it as sort of a 'legal fiction'. That is, formalistically speaking,

    int x = foo.bar;

and

    int *x = &foo.bar;

are both doing the same operation on lvalues (going from the lvalue foo to the lvalue foo.bar), followed by either letting it decay to an rvalue, or converting it to a pointer. But in my personal mental model, 'field projection to rvalue' and 'field projection to pointer' are two separate primitive operations.

That's in part because they generate different assembly instructions on most architectures; in part because I find it more intuitive to think of them that way than to imagine the invisible decay operation.

But also, there are circumstances where a direct access is valid but taking a pointer is not:

  • If foo is declared as a register variable;
  • If foo's type is an __attribute__((packed)) struct;
  • If foo's type is a union, and I previously stored to a different field of the union with a different type (in which case, taking a pointer isn't technically illegal by itself, but the pointer is UB to dereference).

Particularly in the latter two cases, the lvalue has to "remember" the path it was accessed through, which compromises the independence of the two parts of the operation (i.e. lvalue projection and lvalue-to-rvalue decay).

@bjorn3
Copy link
Member

bjorn3 commented Feb 24, 2022

That's in part because they generate different assembly instructions on most architectures

Only the decay to rvalue vs converting to a pointer results in different assembly. The decay to rvalue would be loading from the pointer calculated by the lvalue expression. Converting to a pointer directly takes the calculated pointer and does nothing. In both cases each part of the lvalue expression results in a pointer calculation.

But also, there are circumstances where a direct access is valid but taking a pointer is not:

  • If foo is declared as a register variable;

Rust doesn't have register variables.

  • If foo's type is an attribute((packed)) struct;

Taking a raw pointer using addr_of! is valid in rust as they don't have to be aligned. Taking a reference isn't valid though due to alignment requirements on references.

  • If foo's type is a union, and I previously stored to a different field of the union with a different type (in which case, taking a pointer isn't technically illegal by itself, but the pointer is UB to dereference).

Doesn't apply to rust. We require -fno-strict-aliasing.

@comex
Copy link

comex commented Feb 24, 2022

Only the decay to rvalue vs converting to a pointer results in different assembly. The decay to rvalue would be loading from the pointer calculated by the lvalue expression. Converting to a pointer directly takes the calculated pointer and does nothing. In both cases each part of the lvalue expression results in a pointer calculation.

Yes, but most architectures have a single "load from (pointer plus immediate offset)" instruction rather than needing a "add immediate offset to pointer" instruction followed by a "load from pointer" instruction. I fully realize that these are separate operations for most of the compilation pipeline, and aren't always combined into one assembly instruction. But I was specifically describing intuition as opposed to formalism.

Rust doesn't have register variables.

Doesn't apply to rust. We require -fno-strict-aliasing.

I was talking about C. Upon second look, I guess I misread Ralf's post; for some reason I thought that "In C, I hear programmers talk about lvalues quite a lot." was suggesting that Rust's rules should try to match intuition for programmers coming from C. Instead he is just saying that Rust programmers can learn about places analogously to how C programmers learn about lvalues. That said, my anecdote that I haven't learned to think of C lvalues as 'real' still runs against that argument. And for that matter, I think raw pointers in particular should be designed to have as intuitive semantics as possible to Rust programmers coming from C.

Also, one of the cases I mentioned applies to Rust as well:

Taking a raw pointer using addr_of! is valid in rust as they don't have to be aligned. Taking a reference isn't valid though due to alignment requirements on references.

But if you take a raw pointer, then loading from that pointer is still UB without explicitly using read_unaligned. Yet loading directly from the field is not UB. Thus, here too, the lvalue/place "remembers" the path.

To be fair, I do think this is evidence for repr(packed) being badly designed, as if its ability to produce undefined behavior without unsafe weren't enough evidence already. It would have been better if the unaligned-ness were somehow part of the type, if only because that would have allowed the full featureset of unaligned fields (including taking references to fields) to be usable with zero unsafe. But that's another topic.

@thomcc
Copy link
Member

thomcc commented Feb 24, 2022

Yes, but most architectures have a single "load from (pointer plus immediate offset)" instruction rather than needing a "add immediate offset to pointer" instruction followed by a "load from pointer" instruction

It's worth noting that in practice on most CPUs the difference from these is often very minor, both because they get fused into one by the processor even if they are separate instructions, and because the overhead from the memory access dominates. That said, it's not nothing, and I suppose this may also allow other optimizations in other points (I don't know).

That said, my anecdote that I haven't learned to think of C lvalues as 'real' still runs against that argument. And for that matter, I think raw pointers in particular should be designed to have as intuitive semantics as possible to Rust programmers coming from C.

Yes, I have nontrivial C experience (although it's been a while) and don't think of lvalues as real... Honestly, @RalfJung's assertion that "In C, I hear programmers talk about lvalues quite a lot." is quite different than my experience.

I suspect it's just a different crowd, but I've never heard a C programmer who wasn't a compiler engineer or language designer talk about lvalues... Perhaps with the exception of someone referencing the C spec, but that's not typically how most folks think about programming IME.

@comex
Copy link

comex commented Feb 24, 2022

It's worth noting that in practice on most CPUs the difference from these is often very minor, both because they get fused into one by the processor even if they are separate instructions, and because the overhead from the memory access dominates. That said, it's not nothing, and I suppose this may also allow other optimizations in other points (I don't know).

I'm straying off-topic, but to me it's not about performance but literally just "it looks different". I often have to manually match up C code to generated assembly for one reason or another, so that's relevant to me...

@RalfJung
Copy link
Member Author

RalfJung commented Mar 2, 2022

Speaking from a C perspective: I know what an lvalue is, but I intuitively think of it as sort of a 'legal fiction'.

That is also pretty much how lvalues were explained to me, and I think it is a terrible explanation.
Instead I would rather think of Rust surface syntax as being convenient sugar where a certain operation (place-to-value conversion, "load" or "copy" or "move" or whatever you want to call it) does not have to be written out but is added implicitly by the compiler. (Not unlike auto-deref and other kinds of syntactic sugar.)

The full Rust program including explicit "load"s can then be explained entirely structurally, where the behavior of statements and both place and value expressions is all defined entirely in terms of evaluating their operands in a certain order, and then doing something with the result. Everything is nicely compositional, no need for "magic" or "legal fiction". I think this is also the only reasonable way that places can be treated systematically for use in (automated or interactive) formal verification.

But also, there are circumstances where a direct access is valid but taking a pointer is not:
[...]
Particularly in the latter two cases, the lvalue has to "remember" the path it was accessed through, which compromises the independence of the two parts of the operation (i.e. lvalue projection and lvalue-to-rvalue decay).

Rust doesn't have register, and the union case does not apply either: each load (place-to-value conversion) simply interprets the bytes in memory at the given place, using the given type. Unions are not treated any differently than any other kind of place. [But this was already pointed out.]

Packed structs do complicate things, that is true. Specifically, the result of a place expression is not just a location in memory, but also an "expected alignment", and while that alignment is almost always equal to the alignment indicated by the type of the place, when computing a place to a field of a packed struct we lower this alignment to account for packed. This is not pretty I will admit, but I do not think it is worse than explaining packed accesses in the "legal fiction" approach, it maintains compositionality, and it can be explained as an "extension" to the system after having explained the normal case not involving packed structs.
This is implemented in Miri, and Miri does not "remember" the path that lead to the computation of a place, so we know such a memory is not necessary to precisely specify Rust behavior.
(Actually, strictly speaking the expected alignment should be part of the type of the place, since it needs to be known at compile-time for most backends. But I see little benefit in actually treating it like that, and it surely complicates everything enormously.)

Yes, but most architectures have a single "load from (pointer plus immediate offset)" instruction rather than needing a "add immediate offset to pointer" instruction followed by a "load from pointer" instruction. I fully realize that these are separate operations for most of the compilation pipeline, and aren't always combined into one assembly instruction. But I was specifically describing intuition as opposed to formalism.

I don't know basically anything about which combined offset-and-load instructions hardware provides, and I would assume that is true for most Rust programmers. I think it would be a mistake if we decided how to teach (let alone specify) Rust based on things like the details of various ISAs.

@HeroicKatora
Copy link

HeroicKatora commented Mar 2, 2022

If we treat dereferencable pointers, pointers, and places explicitly as separate objects (using &, *T and T syntax respectively), in diagram form it really looks like the simpler morphism is missing:


&T ..???..> &U
|           ^
| (*)       | (&)
|           |
v  .field   |
T --------> U 

On the risk of only re-iterating the comment earlier with minor rewording, but imho this situation alone is confusing enough to warrant a new operator. Such a new operator is only unecessary if we assert the diagram would commute with it. But this seems unecessarily restrictive, and concerns three operations being coupled by some rather abstract requirement. (This is at least one root cause for the difficulty in finding good * semantics). In addition, obviously, we also would want that same operation to act as *T -> *U but tying its semantics to the other operators makes it even more awkward to decompose the semantics to these operators. Both mentally and, I imagine, in compiler implementation (be it MIR or llvm semantics).

I'd rather see this diagram, with independent semantics of (the two potentially different) projection operations and only involving one of dereference and reference at a time (Without regard to syntax, though I find ~ cute due to previous history. Making addr_of syntactic magic for it is maybe okay?).

   ~field 
*T ------> *U
^           ^
|  ~field  |
&T .......> &U
^           ^
| (&)       | (&)
|           |
|  .field   |
T --------> U 

(The middle operator being based on ~ only operating 'within' T). For dereferencable pointers require the reverse diagram, too. This very simply implies the initially confusing mixed form above but without ever having to mix it as one big overarching requirement.

    ~field 
&T ------> &U
|           |
| (*)       | (*)
|           |
v  .field   v
T --------> U 

@RalfJung
Copy link
Member Author

In the OP I wrote this:

We can still add inbounds for references or other places (locals/statics) since we know those to be dereferencable for other reasons.

There is actually a twist here: what about projections into unsized types? I would prefer to avoid making the memory model dynamically compute the 'true' size of a type... but I guess to justify the inbounds when accessing an unsized type, that is the only option. Well, that or checking the projection -- so this is an argument in favor of "inbounds check at place projection time".

@mxk
Copy link

mxk commented Mar 23, 2022

I found this issue while trying to figure out whether the following code is UB:

fn typed_slice<T, const N: usize>(b: &[u8], _: *const [T; N]) -> &[T] {
    ...
}

const P: *const T = NonNull::dangling().as_ptr();
typed_slice(b, addr_of!((*P).field));

I take it that the current answer is that yes, it is UB, and I need to use a MaybeUninit temporary value for P?

My use case is not just finding the offset of a struct field, but also getting its type. In this example, T represents a C struct with a flexible array member. I want to convert the final field it to a slice, but without having to manually specify the type of that field. I'm currently using a vlen!(Struct, field) macro to hide all the addr_of details, but having to pass those pointers as function arguments to get automatic type inference is also a bit ugly. I'm wondering if in addition to a separate offset_of macro there is a need for a type_of macro to simplify this kind of code.

@RalfJung
Copy link
Member Author

I take it that the current answer is that yes, it is UB, and I need to use a MaybeUninit temporary value for P?

Yeah, the *P is UB (unless T is a ZST).

@RalfJung
Copy link
Member Author

Also see #350 where I argue that the only UB we should have in the realm of raw ptr deref and projections is that the offset computation must not wrap the address space.

@RalfJung
Copy link
Member Author

Another knob to tweak that came up in past LLVM discussions is whether getelementptr inbounds needs the pointer to stay inbounds of a live object, or whether it could be a deallocated already. (LLVM probably wants to allow "inbounds of dead object" to ensure that GEPi can be moved down across possible deallocations.)

@jamesmunns
Copy link
Member

jamesmunns commented Feb 26, 2024

I just want to add another value in the counter of "people who have been confused by this", as was explained to me on this zulip thread.

I think I was confused by the partial magic of addr_of! (allowing access to allocated but unaligned values), but not complete magic (place expressions must be in-bounds, e.g. a valid allocation). edit: probably still a bit confused, heading back to Zulip.

As Lokathor mentioned, a main motivator of me falling in this trap was doing things that are better done with offset_of! and wrapping_byte_add (which I now know).

@RalfJung
Copy link
Member Author

The main issue in your mental model seems to be thinking of addr_of! as having any magic at all. It really doesn't. It takes a place expression, evaluates it, and returns the result as a raw pointer -- done.

@jamesmunns
Copy link
Member

Ah, you're right, and I'll edit my comment.

@VorpalBlade
Copy link

What happened to @Gankra 's suggestion to use ~ (syntax bikeahedding aside)? It seems like it would be useful by in many more situations as well. Is anyone working on that? An RFC perhaps?

@RalfJung
Copy link
Member Author

RalfJung commented Mar 2, 2024

This issue is about our existing .field operations; discussion of fundamentally new alternative operations is out-of-scope. Please post your question on Zulip. (But I am not aware of anyone working on it.)

In fact this issue should probably be closed since the summary is outdated -- the * operator no longer has "inbounds" requirements. The one remaining potential degree of freedom is to weaken .field to be "nowrap"; that's tracked in #350.

@RalfJung RalfJung closed this as completed Mar 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests