py, gc: Zero out newly allocated blocks in the GC.

Also add some more debugging output to gc_dump_alloc_table().

Now that newly allocated heap is always zero'd, maybe we just make this
a policy for the uPy API to keep it simple (ie any new implementation of
memory allocation must zero all allocations).  This follows the D
language philosophy.

Before this patch, a previously used memory block which had pointers in
it may still retain those pointers if the new user of that block does
not actually use the entire block.  Eg, if I want 5 blocks worth of
heap, I actually get 8 (round up to nearest 4).  Then I never use the
last 3, so they keep their old values, which may be pointers pointing to
the heap, hence preventing GC.

In rare (or maybe not that rare) cases, this leads to long, unintentional
"linked lists" within the GC'd heap, filling it up completely.  It's
pretty rare, because you have to reuse exactly that memory which is part
of this "linked list", and reuse it in just the right way.

This should fix issue #522, and might have something to do with
issue #510.

1 files changed, 26 insertions, 3 deletions

diff --git a/py/gc.c b/py/gc.c
index 9a5f9d89bf..d0ba26ddd3 100644
--- a/py/gc.c
+++ b/py/gc.c
@@ -366,10 +366,17 @@ found:
     // get pointer to first block
     void *ret_ptr = (void*)(gc_pool_start + start_block * WORDS_PER_BLOCK);
 
+    // zero out the newly allocated blocks
+    // This is needed because the blocks may have previously held pointers
+    // to the heap and will not be set to something else if the caller
+    // doesn't actually use the entire block.  As such they will continue
+    // to point to the heap and may prevent other blocks from being reclaimed.
+    memset(ret_ptr, 0, (end_block - start_block + 1) * BYTES_PER_BLOCK);
+
 #if MICROPY_ENABLE_FINALISER
     if (has_finaliser) {
-        // clear type pointer in case it is never set
-        ((mp_obj_base_t*)ret_ptr)->type = MP_OBJ_NULL;
+        // clear type pointer in case it is never set (now done above in memset)
+        //((mp_obj_base_t*)ret_ptr)->type = MP_OBJ_NULL;
         // set mp_obj flag only if it has a finaliser
         FTB_SET(start_block);
     }
@@ -523,6 +530,10 @@ void *gc_realloc(void *ptr_in, machine_uint_t n_bytes) {
             assert(ATB_GET_KIND(bl) == AT_FREE);
             ATB_FREE_TO_TAIL(bl);
         }
+
+        // zero out the newly allocated blocks (see comment above in gc_alloc)
+        memset(ptr_in + n_blocks * BYTES_PER_BLOCK, 0, (new_blocks - n_blocks) * BYTES_PER_BLOCK);
+
         return ptr_in;
     }
 
@@ -556,7 +567,7 @@ void gc_dump_info() {
 }
 
 void gc_dump_alloc_table(void) {
-    printf("GC memory layout:");
+    printf("GC memory layout; from %p:", gc_pool_start);
     for (machine_uint_t bl = 0; bl < gc_alloc_table_byte_len * BLOCKS_PER_ATB; bl++) {
         if (bl % 64 == 0) {
             printf("\n%04x: ", (uint)bl);
@@ -565,6 +576,18 @@ void gc_dump_alloc_table(void) {
         switch (ATB_GET_KIND(bl)) {
             case AT_FREE: c = '.'; break;
             case AT_HEAD: c = 'h'; break;
+            /* this prints the uPy object type of the head block
+            case AT_HEAD: {
+                machine_uint_t *ptr = gc_pool_start + bl * WORDS_PER_BLOCK;
+                if (*ptr == (machine_uint_t)&mp_type_tuple) { c = 'T'; }
+                else if (*ptr == (machine_uint_t)&mp_type_list) { c = 'L'; }
+                else if (*ptr == (machine_uint_t)&mp_type_dict) { c = 'D'; }
+                else if (*ptr == (machine_uint_t)&mp_type_float) { c = 'F'; }
+                else if (*ptr == (machine_uint_t)&mp_type_fun_bc) { c = 'B'; }
+                else { c = 'h'; }
+                break;
+            }
+            */
             case AT_TAIL: c = 't'; break;
             case AT_MARK: c = 'm'; break;
         }