lancaster-university / codal-microbit-v2

CODAL target for the micro:bit v2.x series of devices
MIT License
41 stars 50 forks source link

Erasing data logging storage in a fiber while plotting a bar graph in the display crashes programme #362

Open microbit-carlos opened 11 months ago

microbit-carlos commented 11 months ago

This programme erases the DAPLink storage in a separate fiber, while the main one updates the display with a bar graph using random numbers.

#include "MicroBit.h"

MicroBit uBit;

static void clearDataLoggingStorage() {
    uBit.log.clear(true);
    uBit.audio.soundExpressions.playAsync("hello");
}

static void plotBarGraph(uint32_t value, int high) {
    float v = (float)value / (float)high;
    float dv = 1.0 / 16.0;
    float k = 0;
    for (int y = 4; y >= 0; --y) {
        for (int x = 0; x < 3; ++x) {
            if (k > v) {
                uBit.display.image.setPixelValue(2 - x, y, 0);
                uBit.display.image.setPixelValue(2 + x, y, 0);
            } else {
                uBit.display.image.setPixelValue(2 - x, y, 255);
                uBit.display.image.setPixelValue(2 + x, y, 255);
            }
            k += dv;
        }
    }
}

int main() {
    uBit.init();

    create_fiber(clearDataLoggingStorage);

    for (int i = 0; i < 500;i++) {
        int value = uBit.random(10);
        uBit.serial.printf("value: %d\n", value);
        plotBarGraph(value, 10);
        // Oddly enough, uBit.display.print instead of plotBarGraph does not crash
        //uBit.display.print(value);
        uBit.sleep(10);
    }

    uBit.display.print("!");
    for (;;)
        uBit.sleep(1000);
}

MICROBIT.hex.zip


Update: Replicable only when when adding "DMESG_ENABLE": 1 to codal.json, and with gcc v10.3 (gcc v12 works).

https://github.com/lancaster-university/codal-microbit-v2/issues/362#issuecomment-1669823809

microbit-carlos commented 11 months ago

Having a quick look with the debugger call stack, the hard fault handler is reached from:

image image image image
microbit-carlos commented 11 months ago

Looks like this is replicable with gcc v10.3, but not with gcc v12. And only when adding "DMESG_ENABLE": 1 to codal.json.

JohnVidler commented 10 months ago

With the merge of PR 165 in codal-core we can now build without spurious compiler warnings on gcc 12 so the recommendation here is to move to this compiler, as the bug looks to be caused by incorect generation of the copy-on-return constructor when returning a ManagedBuffer under very specific circumstances (these, in fact!), and is rather beyond the remit of CODAL to fix.

I'm going to close this for now, but feel free to re-open if this needs to be discussed further.

microbit-carlos commented 10 months ago

If that's the case, can't we code the move constructor and workaround the gcc issue? MakeCode and CODAL users will still be using older GCC versions than v12, so if possible I think it'd be good to try to resolve this issue.

microbit-carlos commented 4 months ago

Could be related: