Added valgrind to the CI, fixed tests

This exposed an issue with my dev environment, which I had to patch.

Fixed #153

.github/workflows/continuous-integration-v2.yml

@@ -60,3 +60,22 @@ jobs:
       - name: run the tests
         if: (matrix.commands.gdb == true && matrix.platforms.gdb_enabled == false) != true
         run: ${{ matrix.commands.exec }}
+
+  run-test-valgrind:
+    needs: run-test-integrations
+    continue-on-error: true
+    strategy:
+      matrix:
+        platforms:
+          - { os: ubuntu-latest, preinstall: sudo apt-get install valgrind }
+        commands:
+          - { exec: make tests-valgrind }
+
+    runs-on: ${{ matrix.platforms.os }}
+    steps:
+      - uses: actions/checkout@v4
+      - name: Preinstall dependencies
+        run: ${{ matrix.platforms.preinstall }}
+      - name: run the tests
+        run: ${{ matrix.commands.exec }}
+

.notes/raspberry-pi-issues.txt

@@ -0,0 +1,8 @@
+The default version of GCC that ships on Raspian has an issue. The file '/usr/lib/arm-linux-gnueabihf/libarmmem-v7l.so' has a faulty implementation of 'memcpy()' and possibly 'memset()'. Changing to the newer versions doens't work, as they're just symlinks to v7.
+
+To resolve this, download and build this shared object:
+
+https://github.com/simonjhall/copies-and-fills
+
+Then, set the linker's preload value to point to that '.so' file (You may need to edit '/etc/ld.so.preload')
+

repl/main.c

@@ -9,9 +9,9 @@
 	strncpy((dest) + (strlen(dest)), (src), strlen((src)) + 1);
 
 #if defined(_WIN32) || defined(_WIN64)
-	#define FLIPSLASH(str) for (int i = 0; str[i]; i++) str[i] = str[i] == '/' ? '\\' : str[i];
+	#define FLIPSLASH(str) for (int i = 0; str != NULL && str[i]; i++) str[i] = str[i] == '/' ? '\\' : str[i];
 #else
-	#define FLIPSLASH(str) for (int i = 0; str[i]; i++) str[i] = str[i] == '\\' ? '/' : str[i];
+	#define FLIPSLASH(str) for (int i = 0; str != NULL && str[i]; i++) str[i] = str[i] == '\\' ? '/' : str[i];
 #endif
 
 unsigned char* readFile(char* path, int* size) {
@@ -219,8 +219,9 @@ CmdLine parseCmdLine(int argc, const char* argv[]) {
 				i++;
 
 				//total space to reserve - it's actually longer than needed, due to the exe name being removed
-				cmd.infileLength = strlen(argv[0]) + strlen(argv[i]);
-				cmd.infile = malloc(cmd.infileLength + 1);
+				cmd.infileLength = strlen(argv[0]) + strlen(argv[i]) + 1;
+				cmd.infileLength = (cmd.infileLength + 3) & ~3; //BUGFIX: align to  word size for malloc()
+				cmd.infile = malloc(cmd.infileLength);
 
 				if (cmd.infile == NULL) {
 					fprintf(stderr, TOY_CC_ERROR "ERROR: Failed to allocate space while parsing the command line, exiting\n" TOY_CC_RESET);

source/toy_bucket.c

@@ -33,9 +33,7 @@ void* Toy_partitionBucket(Toy_Bucket** bucketHandle, unsigned int amount) {
 	}
 
 	//BUGFIX: the endpoint must be aligned to the word size, otherwise you'll get a bus error from moving pointers
-	if (amount % 4 != 0) {
-		amount += 4 - (amount % 4); //ceil
-	}
+	amount = (amount + 3) & ~3;
 
 	//if you try to allocate too much space
 	if ((*bucketHandle)->capacity < amount) {

source/toy_bytecode.c

@@ -68,6 +68,8 @@ static void writeBytecodeBody(Toy_Bytecode* bc, Toy_Ast* ast) {
 	memcpy(bc->ptr + bc->count, module, len);
 	bc->count += len;
 	bc->moduleCount++;
+
+	free(module);
 }
 
 //exposed functions

source/toy_parser.c

@@ -379,7 +379,8 @@ static Toy_AstFlag literal(Toy_Bucket** bucketHandle, Toy_Parser* parser, Toy_As
 			} while (parser->previous.lexeme[o++] && i < parser->previous.length);
 
 			buffer[i] = '\0';
-			Toy_private_emitAstValue(bucketHandle, rootHandle, TOY_VALUE_FROM_STRING(Toy_createStringLength(bucketHandle, buffer, i - escapeCounter)));
+			unsigned int len = ((i - escapeCounter) + 3) & ~3;
+			Toy_private_emitAstValue(bucketHandle, rootHandle, TOY_VALUE_FROM_STRING(Toy_createStringLength(bucketHandle, buffer, len)));
 
 			return TOY_AST_FLAG_NONE;
 		}

source/toy_scope.c

@@ -17,6 +17,10 @@ static void incrementRefCount(Toy_Scope* scope) {
 static void decrementRefCount(Toy_Scope* scope) {
 	for (Toy_Scope* iter = scope; iter; iter = iter->next) {	
 		iter->refCount--;
+		if (iter->refCount == 0 && iter->table != NULL) {
+			Toy_freeTable(iter->table);
+			iter->table = NULL;
+		}
 	}
 }
 
@@ -64,12 +68,6 @@ Toy_Scope* Toy_popScope(Toy_Scope* scope) {
 	}
 
 	decrementRefCount(scope);
-
-	if (scope->refCount == 0) {
-		Toy_freeTable(scope->table);
-		scope->table = NULL;
-	}
-
 	return scope->next;
 }
 
@@ -86,7 +84,7 @@ Toy_Scope* Toy_deepCopyScope(Toy_Bucket** bucketHandle, Toy_Scope* scope) {
 	//forcibly copy the contents
 	for (int i = 0; i < scope->table->capacity; i++) {
 		if (!TOY_VALUE_IS_NULL(scope->table->data[i].key)) {
-			Toy_insertTable(&newScope->table, scope->table->data[i].key, scope->table->data[i].value);
+			Toy_insertTable(&newScope->table, Toy_copyValue(scope->table->data[i].key), Toy_copyValue(scope->table->data[i].value));
 		}
 	}
 

source/toy_string.c

@@ -229,7 +229,13 @@ char* Toy_getStringRawBuffer(Toy_String* str) {
 		exit(-1);
 	}
 
-	char* buffer = malloc(str->length + 1);
+	//BUGFIX: Make sure it's aligned, and there's space for the null
+	int len = (str->length + 3) & ~3;
+	if (len == str->length) {
+		len += 4;
+	}
+
+	char* buffer = malloc(len);
 
 	deepCopyUtil(buffer, str);
 	buffer[str->length] = '\0';

source/toy_vm.c

@@ -651,12 +651,17 @@ void Toy_bindVMToModule(Toy_VM* vm, unsigned char* module) {
 		vm->subsAddr = READ_UNSIGNED_INT(vm);
 	}
 
-	//allocate the stack, scope, and memory
-	vm->stringBucket = Toy_allocateBucket(TOY_BUCKET_IDEAL);
-	vm->scopeBucket = Toy_allocateBucket(TOY_BUCKET_SMALL);
-	vm->stack = Toy_allocateStack();
+	//allocate the stack, scope, and memory (skip if already in use)
+	if (vm->stringBucket == NULL) {
+		vm->stringBucket = Toy_allocateBucket(TOY_BUCKET_IDEAL);
+	}
+	if (vm->scopeBucket == NULL) {
+		vm->scopeBucket = Toy_allocateBucket(TOY_BUCKET_SMALL);
+	}
+	if (vm->stack == NULL) {
+		vm->stack = Toy_allocateStack();
+	}
 	if (vm->scope == NULL) {
-		//only allocate a new top-level scope when needed, otherwise REPL will break
 		vm->scope = Toy_pushScope(&vm->scopeBucket, NULL);
 	}
 }

tests/cases/makefile

@@ -81,4 +81,4 @@ build-run-valgrind: $(addprefix $(TEST_OUTDIR)/,$(notdir $(TEST_CASESFILES:%.c=%
 
 .PRECIOUS: $(TEST_OUTDIR)/%.run-valgrind
 $(TEST_OUTDIR)/%.run-valgrind: $(TEST_OUTDIR)/%.exe
-	valgrind $< --leak-check=full --show-leak-kinds=all --track-origins=yes --verbose
+	valgrind --leak-check=full --show-leak-kinds=all --track-origins=yes $<

tests/cases/test_string.c

@@ -210,7 +210,7 @@ int test_string_concatenation() {
 
 		//check the refcounts
 		if (strlen(buffer) != 11 ||
-			strcmp(buffer, "Hello world") != 0)
+			strncmp(buffer, "Hello world", 11) != 0)
 		{
 			fprintf(stderr, TOY_CC_ERROR "ERROR: Failed to get the raw buffer from concatenated string\n" TOY_CC_RESET);
 			free(buffer);
@@ -267,7 +267,7 @@ int test_string_with_stressed_bucket() {
 		//grab the buffer
 		char* buffer = Toy_getStringRawBuffer(str);
 
-		if (strcmp(buffer, "thequickbrownfoxjumpedoverthelazydog") != 0 ||
+		if (strncmp(buffer, "thequickbrownfoxjumpedoverthelazydog", 36) != 0 ||
 			strlen(buffer) != 36)
 		{
 			fprintf(stderr, TOY_CC_ERROR "ERROR: Unexpected state of the raw buffer after string stress test: '%s'\n" TOY_CC_RESET, buffer);

tests/integrations/makefile

@@ -49,7 +49,7 @@ valgrind: source repl run-valgrind
 run-valgrind: $(TEST_SCRIPTFILES:.toy=.toy-valgrind)
 
 %.toy-valgrind: %.toy
-	valgrind --leak-check=full --show-leak-kinds=all --track-origins=yes --verbose $(TEST_OUTDIR)/$(TEST_REPLNAME) -f ../$< --verbose
+	valgrind --leak-check=full --show-leak-kinds=all --track-origins=yes $(TEST_OUTDIR)/$(TEST_REPLNAME) -f ../$< --verbose
 
 #compile the source and repl first
 source: $(TEST_OBJDIR) $(TEST_OUTDIR)