mbv / ssd1680

Apache License 2.0
4 stars 4 forks source link

Allocating memory on stack might lead to corrupting stack #1

Closed georgik closed 8 months ago

georgik commented 10 months ago

There is one problem which becomes visible when turning on esp-wifi, together with this crate. esp-wifi will get corrupted stack, because we run out of the memory. Framebuffer is allocated on the stack. For ESP32 and boards with PSRAM one solution could be to use allocator and pull the memory from PSRAM. For ESP32-C3 probably the only solution is to shink the buffer.

georgik commented 10 months ago

Here's one idea how to solve the problem: https://github.com/georgik/ssd1680/commit/6b82e9cd863c7020f076b10d9678458d34965a42 Basically the buffer is comming from main app, where I can use allocator and pull it from SRAM or PSRAM, like here: https://github.com/georgik/esp32-rust-lilygo-t5-epaper/blob/bug/random-failure/src/main.rs#L127

What's your opinion @mbv ?

mbv commented 10 months ago

@georgik I am good with your changes, Could you create PR?

georgik commented 10 months ago

Sure. I'm investigating the problem little bit more and it might have different root cause. It seems that displays allocates only 4000 bytes per buffer, while it should be ~4039. Basically this funnction returns incorrect data:

const fn buffer_len(width: usize, height: usize) -> usize {
    (width + 7) / 8 * height
}

So it happens that ssd1680.display_frame(&mut spi, &mut delay).unwrap(); basically does something beyond the scope of buffer.

georgik commented 10 months ago

Again this might not be correct theory, but I believe that it has to do something with the end of buffer.

mbv commented 10 months ago

yep, that version sounds more logical to me too. I don't think the 4kb we use for buffer can affect the wifi stack. I'll review the driver documentation and how it is used here again

mbv commented 9 months ago

So it happens that ssd1680.display_frame(&mut spi, &mut delay).unwrap(); basically does something beyond the scope of buffer.

This method just write the buffer to spi.

We should double check the setting of the buffer values: let (index, bit) = find_position(point.x as u32, point.y as u32, width, height, rotation); https://github.com/mbv/ssd1680/blob/main/src/graphics.rs#L80C9-L80C99

georgik commented 9 months ago

Thank you for the suggestion.

Meanwhile we tracked down problem which seems to be related to memory allocation on the stack when working with esp-wifi, specific ESP32 (not ESP32-C3 since it has different memory/rom layout). The working version is here: https://github.com/georgik/esp32-rust-lilygo-t5-epaper/tree/feature/working-wifi To get non-working version it was sufficient to move rx, and tx buffers (1.5k) into the loop. The issues with buffer for display is probably not away, it's just hidden under different buffer. So for ESP32 users we have kind of workaround.

georgik commented 8 months ago

The root cause of the problem was identified. It was caused by call of incorrect function in esp-hal. Instead of ESP32Reset, there was function Reset, which has differnet behavior. The problem manifested only in certain corner cases. Solution is to upgrade to esp-hal 0.16.0. The example now works without problem: https://github.com/georgik/esp32-rust-lilygo-t5-epaper

I'm closing the issue.