breakintoprogram / agon-vdp

Official AGON QUARK Firmware: ESP32 VDP
MIT License
76 stars 27 forks source link

vdu_sys_sprites heap corruption #42

Open HarmonicBrick opened 1 year ago

HarmonicBrick commented 1 year ago

Agon crashes after reusing a bitmap, e.g. after running a program twice. VDP 1.03, FabGL 1.0.8

12:49:06.331 -> vdu_sys_sprites: reset
12:49:06.331 -> vdu_sys_sprites: bitmap 0 selected
12:49:06.331 -> CORRUPT HEAP: Bad head at 0x3ffae894. Expected 0xabba1234 got 0x3ffae6f4
12:49:06.331 -> 
12:49:06.331 -> assert failed: multi_heap_free multi_heap_poisoning.c:253 (head != NULL)
12:49:06.331 -> 
12:49:06.331 -> 
12:49:06.331 -> Backtrace: 0x4008788d:0x3ffb2020 0x4008ddc1:0x3ffb2040 0x4009392d:0x3ffb2060 0x4009359f:0x3ffb2190 0x40087c2d:0x3ffb21b0 0x4009395d:0x3ffb21d0 0x400d3317:0x3ffb21f0 0x400d3d28:0x3ffb2230 0x400d3f71:0x3ffb2250 0x400d436a:0x3ffb2270 0x400eb9ad:0x3ffb2290
12:49:06.331 -> 
12:49:06.331 -> 
12:49:06.331 -> 
12:49:06.331 -> 
12:49:06.331 -> ELF file SHA256: 54ce0c32490349e1
12:49:06.331 -> 
12:49:06.597 -> Rebooting...

Fix for me is to let fabgl::~Bitmap() handle heap free.

diff --git a/video.ino b/video.ino
index 3845f84..5c2c61d 100644
--- a/video.ino
+++ b/video.ino
@@ -1383,10 +1383,6 @@ void vdu_sys_sprites(void) {
            width = rw;
            height = rh;
            //
-           // Clear out any old data first
-           //
-           free(bitmaps[current_bitmap].data);
-           //
            // Allocate new heap data
            //
            dataptr = (void *)heap_caps_malloc(sizeof(uint32_t)*width*height, MALLOC_CAP_8BIT | MALLOC_CAP_INTERNAL);
@@ -1411,7 +1407,9 @@ void vdu_sys_sprites(void) {
                // Create bitmap structure
                //
                bitmaps[current_bitmap] = Bitmap(width,height,dataptr,PixelFormat::RGBA8888);
-               bitmaps[current_bitmap].dataAllocated = false;
+               bitmaps[current_bitmap].dataAllocated = true;
+        const size_t free_heap = heap_caps_get_free_size(MALLOC_CAP_8BIT | MALLOC_CAP_INTERNAL);
+        debug_log("vdu_sys_sprites: free heap %lu\n\r", free_heap);
            }
            else {
                for(n = 0; n < width*height; n++) readLong_b(); // discard incoming data
nihirash commented 1 year ago

I've found that brokes heap vdp_sprite_reset command(I've tried use it for freeing bitmaps).