py/gc: Ensure a gap of one byte after the ATB.

Prior to this fix the follow crash occurred.  With a GC layout of:

    GC layout:
      alloc table at 0x3fd80428, length 32001 bytes, 128004 blocks
      finaliser table at 0x3fd88129, length 16001 bytes, 128008 blocks
      pool at 0x3fd8bfc0, length 2048064 bytes, 128004 blocks

Block 128003 is an AT_HEAD and eventually is passed to gc_mark_subtree.
This causes gc_mark_subtree to call ATB_GET_KIND(128004).  When block 1 is
created with a finaliser, the first byte of the finaliser table becomes
0x2, but ATB_GET_KIND(128004) reads these bits as AT_TAIL, and then
gc_mark_subtree references past the end of the heap, which happened to be
past the end of PSRAM on the esp32-s2.

The fix in this commit is to ensure there is a one-byte gap after the ATB
filled permanently with AT_FREE.

Fixes issue micropython#7116.

See also adafruit#5021

Signed-off-by: Jeff Epler <[email protected]>
Signed-off-by: Damien George <[email protected]>

Author: jepler

py/gc.c

@@ -95,6 +95,12 @@
 #define BLOCK_FROM_PTR(area, ptr) (((byte *)(ptr) - area->gc_pool_start) / BYTES_PER_BLOCK)
 #define PTR_FROM_BLOCK(area, block) (((block) * BYTES_PER_BLOCK + (uintptr_t)area->gc_pool_start))
 
+// After the ATB, there must be a byte filled with AT_FREE so that gc_mark_tree
+// cannot erroneously conclude that a block extends past the end of the GC heap
+// due to bit patterns in the FTB (or first block, if finalizers are disabled)
+// being interpreted as AT_TAIL.
+#define ALLOC_TABLE_GAP_BYTE (1)
+
 #if MICROPY_ENABLE_FINALISER
 // FTB = finaliser table byte
 // if set, then the corresponding block may have a finaliser
@@ -123,16 +129,16 @@ STATIC void gc_setup_area(mp_state_mem_area_t *area, void *start, void *end) {
     // => T = A * (1 + BLOCKS_PER_ATB / BLOCKS_PER_FTB + BLOCKS_PER_ATB * BYTES_PER_BLOCK)
     size_t total_byte_len = (byte *)end - (byte *)start;
     #if MICROPY_ENABLE_FINALISER
-    area->gc_alloc_table_byte_len = total_byte_len * MP_BITS_PER_BYTE / (MP_BITS_PER_BYTE + MP_BITS_PER_BYTE * BLOCKS_PER_ATB / BLOCKS_PER_FTB + MP_BITS_PER_BYTE * BLOCKS_PER_ATB * BYTES_PER_BLOCK);
+    area->gc_alloc_table_byte_len = (total_byte_len - ALLOC_TABLE_GAP_BYTE) * MP_BITS_PER_BYTE / (MP_BITS_PER_BYTE + MP_BITS_PER_BYTE * BLOCKS_PER_ATB / BLOCKS_PER_FTB + MP_BITS_PER_BYTE * BLOCKS_PER_ATB * BYTES_PER_BLOCK);
     #else
-    area->gc_alloc_table_byte_len = total_byte_len / (1 + MP_BITS_PER_BYTE / 2 * BYTES_PER_BLOCK);
+    area->gc_alloc_table_byte_len = (total_byte_len - ALLOC_TABLE_GAP_BYTE) / (1 + MP_BITS_PER_BYTE / 2 * BYTES_PER_BLOCK);
     #endif
 
     area->gc_alloc_table_start = (byte *)start;
 
     #if MICROPY_ENABLE_FINALISER
     size_t gc_finaliser_table_byte_len = (area->gc_alloc_table_byte_len * BLOCKS_PER_ATB + BLOCKS_PER_FTB - 1) / BLOCKS_PER_FTB;
-    area->gc_finaliser_table_start = area->gc_alloc_table_start + area->gc_alloc_table_byte_len;
+    area->gc_finaliser_table_start = area->gc_alloc_table_start + area->gc_alloc_table_byte_len + ALLOC_TABLE_GAP_BYTE;
     #endif
 
     size_t gc_pool_block_len = area->gc_alloc_table_byte_len * BLOCKS_PER_ATB;
@@ -143,12 +149,12 @@ STATIC void gc_setup_area(mp_state_mem_area_t *area, void *start, void *end) {
     assert(area->gc_pool_start >= area->gc_finaliser_table_start + gc_finaliser_table_byte_len);
     #endif
 
-    // clear ATBs
-    memset(area->gc_alloc_table_start, 0, area->gc_alloc_table_byte_len);
-
     #if MICROPY_ENABLE_FINALISER
-    // clear FTBs
-    memset(area->gc_finaliser_table_start, 0, gc_finaliser_table_byte_len);
+    // clear ATBs and FTBs
+    memset(area->gc_alloc_table_start, 0, gc_finaliser_table_byte_len + area->gc_alloc_table_byte_len + ALLOC_TABLE_GAP_BYTE);
+    #else
+    // clear ATBs
+    memset(area->gc_alloc_table_start, 0, area->gc_alloc_table_byte_len + ALLOC_TABLE_GAP_BYTE);
     #endif
 
     area->gc_last_free_atb_index = 0;