Endless copy loop: Incorrect implementation or wrongly compiled?

[skoe]

First of all: I'm sorry if this is the wrong place to ask, but at least it's related to the book :)

While reading the Embedonomicon I wrote some bare metal stuff without using any crate. Just for learning how everything works together. I didn't find out why this source code:

    // Copy DATA
    let mut sdata = &mut _sdata as *mut u32;
    let edata = &mut _edata as *mut u32;
    let mut sidata = &_sidata as *const u32;
    while sdata < edata {
        ptr::write(sdata, ptr::read(sidata));
        sdata = sdata.offset(1);
        sidata = sidata.offset(1);
    }

was compiled to this assembly (copied from gdb):

│   0x8000026 <rustymc::run+18>     ldmia   r1!, {r2}                                                │
│   0x8000028 <rustymc::run+20>     stmia   r0!, {r2}                                                │
│  >0x800002a <rustymc::run+22>     b.n     0x8000026 <rustymc::run+18>                              │

An endless loop which will write to invalid memory sooner or later.

I boiled it down to a minimal program which is attached. I used rustc 1.47.0 and cargo run --release.

What's wrong with my code?

v3mini.tar.gz

[TeXitoi]

I suspect that's undefined behavior. Miri doesn't like this adapted variant:

https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=38db33b0eb42056ce16f8f4f2161f732

[skoe]

@TeXitoi Thank's for the hint. With some printf-debugging added (https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=2b5c580d4c288e3b78d36d4b2b97c484) it becomes clearer why Miri complains: the range sdata to edata is larger than the memory at sidata. As one could expect, the first word is copied without problems and the second one failed.

I'm not sure whether this is the right track but I'll check the addresses in the project attached tonight. However, if it is UB there too, the question is why https://github.com/rust-embedded/cortex-m-rt/blob/master/src/lib.rs together with https://github.com/rust-embedded/r0/blob/master/src/lib.rs are different.

[skoe]

Update:
I checked the pointers: sidata = 0x0800_003c is in flash, sdata = 0x2000_0000, edata = 0x2000_0000 in RAM. That's correct.

The issue does not happen if I move the two loops into a separate module and call it from the run function:

pub unsafe fn init(mut sbss: *mut u32,
            ebss: *const u32,
            mut sdata: *mut u32,
            edata: *mut u32,
            mut sidata: *const u32) {
    // Initialize BSS
    while sbss < ebss {
        ptr::write(sbss, 0);
        sbss = sbss.offset(1);
    }

    // Copy DATA
    while sdata < edata {
        ptr::write(sdata, ptr::read(sidata));
        sdata = sdata.offset(1);
        sidata = sidata.offset(1);
    }    
}

In this case the copy loop works as expected.

[Dirbaio]

It may be due to "pointer provenance": the compiler is allowed to assume that pointers derived from one "allocation" can never be manipulated to point outside of it. Maybe it's considering _sdata and _edata to be "allocations" of maybe 4? maybe 0? bytes. This may lead the optimizer to assume a pointer created from one can never cross a pointer created from the other, so the loop never ends.

https://rust-lang.github.io/unsafe-code-guidelines/glossary.html#pointer-provenance

[jonas-schievink]

I suspect that's undefined behavior. Miri doesn't like this adapted variant:

https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=38db33b0eb42056ce16f8f4f2161f732

Miri cannot be used to validate code that relies on a linker script for soundness. The statics in that program can be placed at arbitrary locations, with unrelated statics between them, so overwriting the entire range causes UB even when ignoring the out-of-bounds pointers and provenance and aliasing violations.

[skoe]

Just for the records: The reason this stripped down minimal version of the init code was a wrong optimization (rust-lang/rust#28728). With nightly or with the loop replaced with something else it runs as intended.

However, the code relies on several properties that are considered undefined behavior (mainly out of bound access pointers pointing to a single u32, aliasing with the mutable statics that will be there once main() is called, pointers to (apparently) different allocated objects are compared (pointer provenance)). So the code works well but relies on things that are not guaranteed to work by the language Rust.

A proposal for "less undefined behavior" can be found here: https://gist.github.com/skoe/dbd3add2fc3baa600e9ebc995ddf0302
Another (partial) solution would be to use #[linkage = "external"] to import pointers instead of u32.

As the Embedonomicon does not describe how the init code is implemented, this issue can be closed. The bug in the original init implementation is analysed there: rust-embedded/cortex-m-rt#300