Rust: Const validation rejects references that might be dangling (but could actually be valid at run-time)

[axos88]

Apparently it's undefined behaviour to use:

const gpio: &RegisterBlock = unsafe { (& (*lpc176x5x::GPIO::ptr())) };

but not undefined to use:

fn x() {
  let gpio: &RegisterBlock = unsafe { (& (*lpc176x5x::GPIO::ptr())) };
  (...)
}

with:

    pub const fn ptr() -> *const gpio::RegisterBlock {
        0x2009_c000 as *const _
    }

Error message is:

error[E0080]: it is undefined behavior to use this value
  --> src/rtos.rs:27:1
   |
27 | const gpio: &RegisterBlock = unsafe { (& (*lpc176x5x::GPIO::ptr())) };
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered dangling reference (created from integer)
   |
   = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior

This is happening on in an embedded envrionment, where there is no MMU, and access to that address is correct.
I'm not sure why it would be undefined to use it as a const, but not undefined to use it as a variable value?
Perpahs it has something to do with the borrow checker not being able to track mutable and unmutable references to that value, but this is an immutable access, so it should be fine?

[axos88]

Using *const instead of & fixes the issue with the compiler, but now it's needed to be dereferenced on each access:

const gpio: *const RegisterBlock = unsafe { lpc176x5x::GPIO::ptr() };

unsafe fn x() {
  (*gpio).pin2.write( |w| w.bits(0xDEADBEEF))
}

[Centril]

cc @oli-obk @RalfJung

[RalfJung]

Apparently it's undefined behaviour to use:
[...]
but not undefined to use:

Correction: the rules are the same in both cases, we just don't have a lint catching the latter.

There is no difference between the static version and the let version. It is just much easier to check statics than it is to check the body of a function.

This is happening on in an embedded envrionment, where there is no MMU, and access to that address is correct.

That is an interesting corner case. The reason we error is that in general, it is not correct to just access some random integer cast to a pointer.

This is a very good argument for making that error suppressible, likely by folding it into the deny-by-default const_error.

However, also note that if this is an MMIO block, references are likely not what you want. Also see this long discussion. The gist of it is that under current rules, the compiler is allowed to insert spurious reads from references. Which crate is providing the GPIO module/type here?

[axos88]

All pac crates generated from the svd files for ARM-Cortex (but might be all ARM) microcontrollers use this type of code. The svd2rust generates this code from the honky svd xml files.

It this exact case it's https://github.com/lpc-rs/lpc-pac.

But accessing "random addresses" is THE way to communicate with the hardware when you are on bare metal.

[RalfJung]

accessing "random addresses" is THE way to communicate with the hardware when you are on bare metal.

I understand. We just were not aware people would put those as references into a static, hence the check that rejects "random addresses" in references.

And also see the issues mentioned above with using references. It is incorrect in Rust to have a reference to volatile (MMIO, hardware-register) memory. It does not matter how brief that reference exists, the moment you just create a reference, the compiler is allowed to speculatively read it.

I am aware that this pattern is widely used on embedded, but that doesn't change the fact that it is incorrect, and has always been incorrect since at least Rust 1.0. Unfortunately the people implementing that pattern were not aware of this subtlety. :/
And it also seems like noone is motivated enough to push for an RFC. (Several proposals for fixing this have been made in the various threads.) I guess this is because "it works" even though it is incorrect and might break when LLVM optimizes code the wrong way.

The alternative using raw pointers mostly avoids that problem, though I cannot easily tell if it does not also locally create references.

---

So the question for rustc now is, do we want to stop erroring for this because of embedded devices? The one usecase we have here is not actually correct for references, but for reasons entirely unrelated to what this is checking. I guess it is conceivable that non-MMIO memory might also sit at a fixed address on embedded, and then references are perfectly fine.

[cramertj]

@RalfJung

It is incorrect in Rust to have a reference to volatile (MMIO, hardware-register) memory. It does not matter how brief that reference exists, the moment you just create a reference, the compiler is allowed to speculatively read it.

I know you know this, but it seems worth re-mentioning this library exists and is heavily used in the rust embedded ecosystem, and references to values of this type are often obtained by transmuting integers to references.

There are other places, too, where int->ptr occurs where the integer does not originate from a ptr->int cast, such as the address returned by the zx_vmar_map syscall (and presumably other similar calls that map something into memory and return an address pointing to the resulting mapping).

[RalfJung]

@cramertj yes, that is a problem. :/ I mean not the part where integers are transmuted, that's fine, but the part where embedded programs use &VolatileCell.

such as the address returned by the zx_vmar_map syscall

I doubt we will ever permit calling that syscall in CTFE. ;) The bug here is strictly about the checks we do on the value computed for a static or const. We do not consider it UB in general to have a raw integer in a reference (assuming that the pointer created this way is actually dereferencable).

[oli-obk]

It is incorrect in Rust to have a reference to volatile (MMIO, hardware-register) memory. It does not matter how brief that reference exists, the moment you just create a reference, the compiler is allowed to speculatively read it.

In the embedded case, that is totally fine, except for MMIO where reading mutates hardware. We don't ever read the VolatileCell<T> that is behind the reference, instead we read_volatile(&self as *const VolatileCell<T> as *const T), which is the only place we actually care about the read. So if ther are spurious reads whose value is unused inserted around that, it doesn't matter except for performance (and the already mentioned "read mutates hardware" case).

But all that seems orthogonal to the issue at hand. We don't permit references (in constants and statics) that can't be dereferenced during const eval. We should not change this behaviour. Also: we don't permit &Cell<T> in constants, why should we permit &VolatileCell<T>?

@axos88 why do you want these values stored inside statics? It seems to go against Rust's owernship scheme to have global state unless absolutely necessary. If you want that function to have access to that hardware register, pass the register down via arguments.

[RalfJung]

We don't ever read the VolatileCell that is behind the reference, instead we read_volatile(&self as *const VolatileCell as *const T), which is the only place we actually care about the read. So if ther are spurious reads whose value is unused inserted around that, it doesn't matter except for performance (and the already mentioned "read mutates hardware" case).

The general expectation with volatile is that only the accesses explicitly written by the programmer happen, nothing else. This expectation can be violated with &VolatileCell.

But all that seems orthogonal to the issue at hand.

Agreed.

Also: we don't permit &Cell in constants, why should we permit &VolatileCell?

Hm, true. OTOH, we do permit &Cell in statics, but the code above won't work for statics either.

It seems to go against Rust's owernship scheme to have global state unless absolutely necessary.

In this case the hardware is global state. Just forcing people to use local definitions does not make it any less global. Sure Rust discourages global state, but I see no reason to pretend something isn't global when in fact it is.

[oli-obk]

In this case the hardware is global state. Just forcing people to use local definitions does not make it any less global. Sure Rust discourages global state, but I see no reason to pretend something isn't global when in fact it is.

I disagree strongly, but I'm also heavily involved with https://github.com/embed-rs/stm32f7-discovery which treats hardware in an ownership based way, allowing us to use Send and Sync to prevent interrupts from modifying hardware in an uncontrolled manner. Also amongst other things, the ownership semantics make sure that we don't accidentally give multiple high level hardware drivers access to the same registers.

Anyway, independently of any opinions on ownership of hardware, the question that needs to be answered in order for this issue to be resolved is whether

const X: u32 = *Y;

should always compile successfully for any Y with &u32 type.

[nagisa]

Synthetic (or “synthesized”) pointers (pointers materialized out of “thin air”) are valid and should have their own operational semantics with respect to e.g. aliasing analysis specified, just like they do in LLVM/C/C++/etc.. Even making a reference out of them should be fine as long as the requirements for references can be satisfied. Synthetic MMIO pointers only rarely do satisfy the requirements, but they sometimes do, and those cases where they do should have defined semantics.

I believe that we should allow reborrowing data under synthetic pointers as references some way, but I’d be fine with rust making it hard(-er than usual) to achieve.

---

treats hardware in an ownership based way

Hardware registers are still global state, ownership semantics these crates add are an abstraction over this global state. A perfectly reasonable alternate design would be to have these register blocks be behind some sort of a mutex, which makes globality of these register blocks more apparent.

should always compile successfully for any Y with &u32 type.

This would have non-local semantics when X is used, so I’m against it. But it is worth noting that this is a different kind of operation compared to &*Y, and therefore perhaps not too relevant to this discussion.

[oli-obk]

Reborrows are already fine in const eval. It's only when you have a reference in a final constant that we prevent "invalid" (according to an on-purpose-restrictive definition). We can lift restrictions if there's a good argument for that.

Allowing undereferencable references in constants is OK, promoteds can't deref anyway. At least I hope we also prevent implicit derefs in field accesses

[RalfJung]

Synthetic (or “synthesized”) pointers (pointers materialized out of “thin air”) are valid and should have their own operational semantics with respect to e.g. aliasing analysis specified, just like they do in LLVM/C/C++/etc.. [..]
I believe that we should allow reborrowing data under synthetic pointers as references some way, but I’d be fine with rust making it hard(-er than usual) to achieve.

To reiterate, this is not at all about the dynamic semantics of Rust.

This is entirely about what checks we are making on the value that we compute (at compile-time) for a const or static of reference type. These values can be used by other statics/constants and even promoteds, so we want to be a bit careful here, in particular for the latter.

[axos88]

It seems to go against Rust's owernship scheme to have global state unless absolutely necessary. If you want that function to have access to that hardware register, pass the register down via arguments.

I also disagree with this statement. Yes, there are cases where this makes sense (such as a serial port, or an I2C bus, where the hardware has complex state). But in other cases, such as driving a few LEDs for debuging purposes, it's not.

I want to be able to change my debug LEDs from all over the place without having to pass down the reference to them and fight rusts ownership system, and then possibly remove them in production code (imagine the complexity conditional compilation would bring when I need to have them on in debug mode, and off in production). Also you could think about those blinking LEDs as the most simplistic stdout, and this IS global even in std, isn't it?
So in some cases yes, it makes sense to pass down references to pieces of hardware to ensure only one piece of code can manipulate it, but in other cases I want to be in control, because I believe I can ensure that nothing bad will happen(TM). If that makes for unsafe code, that's totally fine, but it needs to be possible

[oli-obk]

Stdout in libstd uses a mutable static that is initialized at runtime. This would work for these references that can't be dereferenced at compile-time, because you only have the odd value at runtime. At the same time thisvwould fit into the ownership scheme, because you then lose access to that pin, so no other driver may attempt to use it. Many boards allow remapping pins, so "just an LED" is not a necessarily universally true statement.

You can always create a global function that generates the value for you and call that instead of using a constant.

Note that I'm not against changing the behaviour here just because I disagree with the way ppl are using hardware on embedded. I won't block us making these rules less strict, as long as we're clear on what such a change entails. Right now, any reference in a constant's value can be dereferenced in another constant. We'd lose this guarantee. As @nagisa said, it's not necessarily a guarantee that makes sense or is desirable, but we should be aware that we'd change it and that this may have far ranging consequences. E.g. it would forever forbid us from allowing dereferencing in promoteds, even though that would be sane to allow right now. If we had allowed it on stable, then it would be a breaking change to allow undereferencable references in constants

[RalfJung]

I am not worried about restricting what new features can be added to promoteds. They are already too powerful anyway and extensions should IMO happen in a more explicit mechanism (such as const blocks).

[Centril]

At any rate, I don't think this is something that should be changed by issue or PR here since it has wider implications. I would like to see an elaborated RFC if something is to be changed here. (But please continue discussing...)

[RalfJung]

To be fair, we introduced and strengthened those checks without much ado. So it seems weird to require an RFC for weakening them.

[Centril]

Well we could also just FCP in one way (my current preference is to keep things as-is) but it seems like there's a debate here which is why an RFC seems appropriate.

[oli-obk]

There's no debate. There's just the "is everyone aware of and OK with the consequences?". So far no one has argued against doing it. I'm mostly arguing that the use case is invalid, but that's just a personal opinion not founded in any evidence.

I guess the only thing speaking against it is that you can't tell from a constant's type whether a deref is OK, but we already can't tell whether subtracting 1 from a usize constant is ok

[RalfJung]

we already can't tell whether subtracting 1 from a usize constant is ok

In which sense? The MIR subtraction operations (checked and unchecked) are AFAIK guaranteed to be okay on a usize. (We do check that they are not pointers.)

[oli-obk]

Well.. 0usize stored in a constant FOO will abort const eval if you do 1/FOO. So the type itself is no guarantee that the evaluation will work

[RalfJung]

That's panicking, which is very different (IMO) from raising another kind of evaluation error.

[oli-obk]

That difference is only relevant when we're talking about implicit promotion, because there the difference to the runtime behaviour is noticable. It is very different conceptually, but not in how it's noticable from a user perspective

[RalfJung]

I don't understand what you are saying. The fact is, promoting / is no problem even though it can fail on valid constants; promoting * would be a problem if it can fail on valid constants.

My explanation for why that is is that we actually promote the infallible checked_div operation. So long as we don't promote fallible operations, we are fine.

[oli-obk]

Ah. No. What I mean is that if we make this change, we can't make deref promotable just like we can't make ptr to int casts promotable

[RalfJung]

I agree with that.

[axos88]

What is the use case for dereferencing references in a const?

[oli-obk]

We'll need dereferencing if we ever want to support iterators or user-written operator trait impls like Add or PartialEq for custom types. Many things that aren't just simple construction of values but require some more complex computation will need dereferencing somewhere

[RalfJung]

https://github.com/rust-lang/rust/issues/63359 is another instance of this, with an int cast to a function pointer in that case.

We'll need dereferencing if we ever want to support iterators or user-written operator trait impls like Add or PartialEq for custom types. Many things that aren't just simple construction of values but require some more complex computation will need dereferencing somewhere

Agreed. But I don't think we should implicitly promote any of these things.

[oli-obk]

Yea. Let's allow these values.

[RalfJung]

@axos88 is it correct that you are using a nightly compiler? On stable, you cannot deref raw pointers in consts the way youa re doing in the OP.
When you have nightly features enabled, please always mention that in the issue.

[RalfJung]

So looks like this will need lang team discussion / FCP. Nominating.

@rust-lang/lang the issue here is about code that does things like

#[repr(C)]
union Transmute {
    int: usize,
    ptr: &'static i32
}

const GPIO: &i32 = unsafe { Transmute { int: 0x800 }.ptr };

(This is the stable-Rust version of what the OP is doing.)

Right now, this code gets rejected by our "const safety" check. That check runs on the final value of every static and const, and tries to make sure it is const safe. This check serves two purposes:

• It helps to make sure people do not create bogus values, like a NULL reference. The check does not have to be perfect to be of value here. (And indeed we do not check references pointing to statics defined in other crates, mostly to avoid making the check too expensive.)
• The check is also crucial for promotion of run-time code to run at compile-time (to give things like &(3+4) static lifetime). When promoting things like &(4 * CONST), we have to know that all operations happening there are const-safe. If CONST e.g. is the result of a ptr-to-int cast and we promoted this multiplication, evaluating it at compile-time would fail, meaning we broke code that would have worked fine had we not decided to promote.

The issue here is that in embedded environments, sometimes you actually know that some usable memory (code or data) resides at a given fixed address. So maybe you want to just cast that integer address to a reference, put the result in a const, and use that. This results in a run-time-safe but not-const-safe reference / function pointer, and that currently gets rejected by the check.

The proposal is to accept such cases in the future. Concretely, this would mean accepting any sufficiently aligned and non-null integer as a reference or function pointer when doing const safety checking.

The impact on promotion is that we can no longer rely on references and function pointers to be const safe. But as of right now, that is okay because we do not allow dereferencing references or calling function pointers when looking for promotion candidates. The decision to make here is whether we are okay with permanently enshrining this limitation, by actually allowing people to produce non-const-safe references and function pointers in constants.

For function pointers that seems like a no-brainer (we already allow people to create function pointers to run-time functions, calls to which we definitely cannot promote). For references, I feel like making sure that all references in existence point to recursively const-safe data is a hopeless undertaking. I think we should never promote dereferencing a reference (code like &(3 + *FOO)); the things we do promote already cause us enough trouble. Instead, to enable such code to produce static lifetimes I think we should move towards explicit promotion (something like const { &(3 + *FOO) }, in expression position). With explicit promotion, non-const-safe operations are okay because the user asked for evaluation at compile-time; if that errors, they can just fix their code (e.g. by removing the const).

[axos88]

Yeah, it was nightly rust, I'll keep that in mind in the future.

[axos88]

I'm not sure I entirely follow your last message, however I am getting a feeling that statics in rust are conflating two things, what you call promotion: compile-time consts and run-time constants. The former being a compile-time only thing that never reaches the end binary and the latter being an entry in the .rodata section, living in the actual memory space of the end application, that is read from that particular memory space whenever its value is required. Oh, and then there is the mutable values, that would end up in .data, that can be modified, which are denoted static mut in rust. My code would obviously not uphold the requirements of the compile-time constants, but I don't see anything that should disallow them becoming run-time constants.

Wouldn't the clean solution be to separate these concepts? Say

• static mut (.data)
• static readonly (.rodata)
• static const (compile-time only, guaranteed to be inlined and optimized away. Maybe just #[inline]?)

Wouldn't this make it much easier to reason what can and cannot become static const?
In some cases (again in embedded) I DO want to have control over which constants actually end up in .rodata instead of being optimized away, because I may want to have access to a memory space where that value is stored. Can't remember the exact use case, but I vaguely remember having to use a static mut instead of a static once to overcome this.

[axos88]

In this particular storing dangling references would be a no-no for static consts, but would be okay in static readonlys. Dereferencing them would be disallowed, since it can't happen at compile time.

[oli-obk]

I think we could add #[link-section=".rodata"] to statics and make that have special semantics (you can't use them from other statics and they are allowed a few extra things)

[axos88]

@oli-obk why not define those special semantics with a language level keyword, since imho they are separate concepts. Also I'm not entirely sure, but the name of the .rodata section is heavily dependent on the linker script afaik, and could easily be named nowritedata in some architectures / special cases, that we couldn't really predict.

[oli-obk]

It could be an explicit and new attribute that will generate the appropriate link-section hint.

[RalfJung]

compile-time consts and run-time constants. The former being a compile-time only thing that never reaches the end binary and the latter being an entry in the .rodata section, living in the actual memory space of the end application, that is read from that particular memory space whenever its value is required.

"promotion" is unfortunately a heavily overloaded term, but the one I am talking about here is the automatic creation of .rodata entries. See this RFC and also this document for further details.

Compile-time constants ("constant propagation") are entirely irrelevant for the discussion in this issue.

Oh, and then there is the mutable values, that would end up in .data, that can be modified, which are denoted static mut in rust.

static FOO: Mutex<i32> also ends up in .data.

[RalfJung]

@oli-obk

I think we could add #[link-section=".rodata"] to statics and make that have special semantics (you can't use them from other statics and they are allowed a few extra things)

Not sure what that would solve...?

[oli-obk]

Well... you could have all of the below under the assumption that #[final_static] statics cannot be read from other statics, but can only be declared ... and while writing this, I realized, that's true even true right now... so ignore me.

#[final_static]
static A: &u32 = unsafe { &*(42 as *const i32) }; // ok
static B: &u32 = unsafe { &*(42 as *const i32) }; // error
static C: &u32 = &10; // ok
static D: u32 = *A; // error
static E: u32 = *C; // ok

[nikomatsakis]

We discussed this in our 2019-10-10 meeting -- we considered a proposal by @RalfJung. We didn't reach any firm decisions but at least some people thought that this seemed reasonable:

• const <expr> — explicit const-promoted expression form (“anonymous constant”)
• forever decide not to “promote” *<expr>
• loosen the validation rules so that &T doesn’t have to be a valid pointer

As an immediate step, @ecstatic-morse has been working towards better specifying the const promotion rules, and that seems like an important part of the puzzle to work out first.

[RalfJung]

Btw @axos88 I think we never got to the bottom of whether RegisterBlock contains MMIO memory? Because if it does, this

const gpio: &RegisterBlock = unsafe { (& (*lpc176x5x::GPIO::ptr())) };

is wrong. &RegisterBlock gives the compiler permission to perform arbitrary reads from the memory the reference points to, even if the code would never read from that memory. Last time I tried to discuss this, we got derailed.

So, what is that RegisterBlock type? It sure sounds like something that would describe MMIO memory.

[axos88]

It is MMIO, but in case of the processor we are coding for there is no MPU, so code can always read from that memory region.

[axos88]

Well... Except if the peripheral is powered off, but i'm not sure that's something we would want to catch at the compiler level.

One could argue that because of that caveat this should be wrapped in unsafe code, but it should be able to compile. The user of said reference should then be responsible for checking if accessing that memory area is permissible in that context, which is basically the definition of unsafe code, is it not?

[RalfJung]

@axos88 the thing with references is that they can get accessed by the compiler even if the code does not access them. That's what it means for them to be "dereferencable". For example, if you have

if foo { let val = *bar; }

then if bar is a reference the compiler is permitted to move that read out of the if.

[RalfJung]

The "wrapping in unsafe code" is exactly what using raw pointers here achieves.

[oli-obk]

If there are no dereferences of a reference anywhere in the code, is the compiler still permitted to insert a read?

[RalfJung]

For locals, certainly:

fn test(x: &i32) {
}

is equivalent to

fn test(x: &i32) {
  if false { let _val = *x; }
}

and the latter can again have the read moved out of the if.

For references in global variables... well, I don't know. But I'd say to be on the safe side, we should say that the same applies there.

[ewoudje]

Has there been any change? Or a way to disable the compiler for checking it?

I have this problem with a custom lazy static implementation. It makes static values with an UnsafeCell but the value in UnsafeCell are all zero's (wich later get initialized). It works until i add a reference in the struct then it stops working and says there is a NULL reference.

pub struct Initializer<T> {
    data: UnsafeCell<T>,
    initialized: RwLock<bool>
}

[RalfJung]

It is UB to have a NULL reference, so that error sounds correct to me.

If you work with uninitialized memory, you need to use MaybeUninit.

[ewoudje]

It is UB to have a NULL reference, so that error sounds correct to me.

If you work with uninitialized memory, you need to use MaybeUninit.

Oops my bad didn't know that existed...
Thanks for the help!