From 83fb5222a25361d8078053b5324452956c904240 Mon Sep 17 00:00:00 2001 From: Kayne Ruse Date: Sun, 10 May 2026 15:39:04 +1000 Subject: [PATCH] Found and fixed a memory leak in the rope strings Also fixed a bit manipulation error in the GC. --- repl/bucket_inspector.c | 82 +++++++++++++++++++++++++++++++++++++++ repl/bucket_inspector.h | 6 +++ repl/main.c | 13 +++++++ scripts/hello_world.toy | 10 ++++- source/toy_bucket.c | 2 +- source/toy_scope.c | 2 + source/toy_string.c | 8 ++++ source/toy_vm.c | 3 +- tests/units/test_string.c | 4 +- 9 files changed, 124 insertions(+), 6 deletions(-) create mode 100644 repl/bucket_inspector.c create mode 100644 repl/bucket_inspector.h diff --git a/repl/bucket_inspector.c b/repl/bucket_inspector.c new file mode 100644 index 0000000..66d0fee --- /dev/null +++ b/repl/bucket_inspector.c @@ -0,0 +1,82 @@ +#include "bucket_inspector.h" +// #include + +#include + +int inspect_bucket(Toy_Bucket** bucketHandle) { + int depth = 0; + + //for each bucket + for (Toy_Bucket* iter = (*bucketHandle); iter != NULL; iter = iter->next) { + int occupied = 0; + int released = 0; + unsigned char* ptr = iter->data; + + + while ((ptr - iter->data < iter->count) && *((int*)ptr) != 0) { //for each partition + if ( ( *((int*)ptr) & 1) == 0) { //is this partition still in use? + occupied++; + } + else { + released++; + } + + //jump distance: ((*((int*)ptr) | 1) ^ 1) + 4 + // printf(" jump %d, ", ((*((int*)ptr) | 1) ^ 1) + 4); + ptr += ((*((int*)ptr) | 1) ^ 1) + 4; //OR + XOR to remove the 'free' flag from the size + } + + printf("Bucket link %d: count %u, %d occupied, %d released\n", depth, iter->count, occupied, released); + + depth++; + } + + printf("\n"); + + return depth; +} + +/* +int inspect_bucket_for_strings(Toy_Bucket** bucketHandle) { + int depth = 0; + + //for each bucket + for (Toy_Bucket* iter = (*bucketHandle); iter != NULL; iter = iter->next) { + int occupied = 0; + int released = 0; + unsigned char* ptr = iter->data; + + + while ((ptr - iter->data < iter->count) && *((int*)ptr) != 0) { //for each partition + if ( ( *((int*)ptr) & 1) == 0) { //is this partition still in use? + occupied++; + + //try to print as a string if possible + Toy_String* str = (void*)(ptr + 4); + + if (str->info.type == TOY_STRING_LEAF && str->info.length < 255) { + printf("String Leaf (%d bytes, %d refCount): %.*s\n", *((int*)ptr), str->info.refCount, str->info.length, str->leaf.data); + } + else if (str->info.type == TOY_STRING_NODE) { + printf("String Node (%d bytes, %d refCount): ...\n", *((int*)ptr), str->info.refCount); + } + } + else { + released++; + } + + //jump distance: ((*((int*)ptr) | 1) ^ 1) + 4 + // printf(" jump %d, ", ((*((int*)ptr) | 1) ^ 1) + 4); + ptr += ((*((int*)ptr) | 1) ^ 1) + 4; //OR + XOR to remove the 'free' flag from the size + } + + printf("Bucket link %d: count %u, %d occupied, %d released\n", depth, iter->count, occupied, released); + + depth++; + } + + printf("\n"); + + return depth; +} +*/ \ No newline at end of file diff --git a/repl/bucket_inspector.h b/repl/bucket_inspector.h new file mode 100644 index 0000000..c0932fb --- /dev/null +++ b/repl/bucket_inspector.h @@ -0,0 +1,6 @@ +#pragma once + +#include "toy_bucket.h" + +int inspect_bucket(Toy_Bucket** bucketHandle); +// int inspect_bucket_for_strings(Toy_Bucket** bucketHandle); \ No newline at end of file diff --git a/repl/main.c b/repl/main.c index efb71df..a11ce66 100644 --- a/repl/main.c +++ b/repl/main.c @@ -1,5 +1,6 @@ #include "ast_inspector.h" #include "bytecode_inspector.h" +#include "bucket_inspector.h" #include "toy_console_colors.h" @@ -380,14 +381,26 @@ int repl(const char* filepath, bool verbose) { //run Toy_runVM(&vm); + int depthBeforeGC = 0; + int depthAfterGC = 0; + //print the debug info if (verbose) { debugStackPrint(vm.stack); debugScopePrint(vm.scope, 0); + + depthBeforeGC = inspect_bucket(&vm.memoryBucket); } //free the memory, and leave the VM ready for the next loop Toy_resetVM(&vm, true, true); + + if (verbose) { + depthAfterGC = inspect_bucket(&vm.memoryBucket); + + printf("GC Report: %d -> %d\n", depthBeforeGC, depthAfterGC); + } + free(bytecode); printf("%s> ", prompt); //shows the terminal prompt diff --git a/scripts/hello_world.toy b/scripts/hello_world.toy index 44e245b..320e2cb 100644 --- a/scripts/hello_world.toy +++ b/scripts/hello_world.toy @@ -1,5 +1,5 @@ - +/* fn swap(a, b) { return b, a; } @@ -10,4 +10,10 @@ var c; var d; //BUG: still causes a segfault -c, d = swap(a, b); \ No newline at end of file +c, d = swap(a, b); + +*/ + + +{ var str = "Hello"; var i = 0; while (i < 10_000) { str = str .. " World"; i++; } } + diff --git a/source/toy_bucket.c b/source/toy_bucket.c index ae42a69..14b4dee 100644 --- a/source/toy_bucket.c +++ b/source/toy_bucket.c @@ -88,7 +88,7 @@ TOY_API void Toy_collectBucketGarbage(Toy_Bucket** bucketHandle) { gc = false; break; } - ptr += (*((int*)(ptr)) ^ 1) + 4; //XOR to remove the 'free' flag from the size + ptr += ((*((int*)ptr) | 1) ^ 1) + 4; //OR + XOR to remove the 'free' flag from the size } //free this link, if its been entirely released diff --git a/source/toy_scope.c b/source/toy_scope.c index af8b544..02f3c7c 100644 --- a/source/toy_scope.c +++ b/source/toy_scope.c @@ -193,6 +193,8 @@ void Toy_assignScope(Toy_Scope* scope, Toy_String* key, Toy_Value value) { return; } + Toy_freeValue(entryPtr->value); + entryPtr->value = value; } diff --git a/source/toy_string.c b/source/toy_string.c index d44dada..8ee4dd2 100644 --- a/source/toy_string.c +++ b/source/toy_string.c @@ -20,10 +20,18 @@ static void incrementRefCount(Toy_String* str) { static void decrementRefCount(Toy_String* str) { str->info.refCount--; if (str->info.type == TOY_STRING_NODE) { + //the parent of this node triggered a decrement across the whole tree decrementRefCount(str->node.left); decrementRefCount(str->node.right); } if (str->info.refCount == 0) { + if (str->info.type == TOY_STRING_NODE) { + //THIS node has triggered the decrement, so run this again + decrementRefCount(str->node.left); + decrementRefCount(str->node.right); + } + + //mark this memory as unused Toy_releaseBucketPartition((void*)str); } } diff --git a/source/toy_vm.c b/source/toy_vm.c index b5ec19f..d719a32 100644 --- a/source/toy_vm.c +++ b/source/toy_vm.c @@ -210,7 +210,7 @@ static void processAssign(Toy_VM* vm) { Toy_Value name = Toy_popStack(&vm->stack); //assign it - Toy_assignScope(vm->scope, TOY_VALUE_AS_STRING(name), Toy_copyValue(&vm->memoryBucket, value)); //scope now owns the value, doesn't need to be freed + Toy_assignScope(vm->scope, TOY_VALUE_AS_STRING(name), Toy_copyValue(&vm->memoryBucket, value)); //in case of chaining, leave a copy on the stack bool chainedAssignment = READ_BYTE(vm); @@ -220,6 +220,7 @@ static void processAssign(Toy_VM* vm) { //cleanup Toy_freeValue(name); + Toy_freeValue(value); } static void processAssignCompound(Toy_VM* vm) { diff --git a/tests/units/test_string.c b/tests/units/test_string.c index ca03d3a..faca24a 100644 --- a/tests/units/test_string.c +++ b/tests/units/test_string.c @@ -192,8 +192,8 @@ int test_string_concatenation(void) { free(buffer); Toy_freeString(result); - Toy_freeString(first); - Toy_freeString(second); + // Toy_freeString(first); //these do NOT need to be freed manually + // Toy_freeString(second); Toy_freeBucket(&bucket); }