esp-rs / rust

Rust for the xtensa architecture. Built in targets for the ESP32 and ESP8266
https://www.rust-lang.org
Other
743 stars 39 forks source link

Default allocator doesn't guarantee allocation bigger than 4? #99

Closed UnTraDe closed 2 years ago

UnTraDe commented 2 years ago

Allocating a type with alignment bigger than 4 bytes sometimes gets allocated on address which is not a multiple of that alignment, causing accesses to fail debug_assert! in slice::from_raw_parts_*

struct Something {
    x: f64,
    y: f32
}

fn main() -> anyhow::Result<()> {
    esp_idf_sys::link_patches();
    esp_idf_svc::log::EspLogger::initialize_default();

    log::info!("align_of::<Something>(): {}", std::mem::align_of::<Something>());
    log::info!("size_of::<Something>(): {}", std::mem::size_of::<Something>());

    loop {
        let r = rand::random::<usize>() % 100;
        let mut v = Vec::with_capacity(r);
        log::info!("pointer: 0x{:x}", v.as_ptr() as usize);
        log::info!("v.as_ptr() % std::mem::align_of::<Something>() == {}", v.as_ptr() as usize % std::mem::align_of::<Something>());
        unsafe { v.set_len(r); };

        log::info!("fill");

        for x in &mut v {
            *x = Something {
                x: 10f64,
                y: 20f32
            };
        }

        log::info!("filled");
        std::thread::sleep(std::time::Duration::from_millis(100));
    }

    Ok(())
}

Note that the alignment of Something is 8, and sometimes the pointer is not a multiple of 8, causing a panic in for x in &mut v { in debug builds.

Happens on ESP32 with Rust STD library, using https://github.com/ivmarkov/rust-esp32-std-demo

MabezDev commented 2 years ago

Thanks for the report, and the helpful testcase!

This should be fixed in this branch with this commit: https://github.com/esp-rs/rust/commit/2e592a919c28f14a0b30bcf6e25536fe25d63f13.

I have also submitted the fix upstream (see the linked PR).

MabezDev commented 2 years ago

This patch is now included in our 1.58 release. It's still not been accepted upstream, but hopefully it will soon!

Closing this now.