strings: add more tests

cheney_c89: fix memory leak errors

.gitignore

@@ -1,2 +1,3 @@
 *.o
 *.so
+*.test

Makefile

@@ -1,33 +1,21 @@
-.PHONY: test_strings clean
+.PHONY: test clean
 
 COMMON_OBJECTS=uns.o
 CHENEY_C89_OBJECTS=cheney_c89.o c89_relo.o
 
-CFLAGS=-Wall -Wno-overlength-strings -std=c89 -Werror -pedantic -fPIC -g -Iinclude
+CFLAGS=-Wall -std=c89 -Werror -pedantic -fPIC -g -Iinclude
 
 libuniversalservice.so: $(COMMON_OBJECTS) $(CHENEY_C89_OBJECTS)
 	$(CC) -shared $(COMMON_OBJECTS) $(CHENEY_C89_OBJECTS) -o libuniversalservice.so
 .c.o:
 	$(CC) $(CFLAGS) $< -c -o $@
 
-#############################################
+test:
-## Examples
-#############################################
 	
-STRING_TEST_OBJS=examples/string/test_common.o examples/string/uns_string.o
-STRING_TESTS= examples/string/test_small
-
-examples/string/test_small: examples/string/test_small.c $(STRING_TEST_OBJS) libuniversalservice.so
-	$(CC) -I. -Iext $(CFLAGS) examples/string/test_small.c $(STRING_TEST_OBJS) -L. -luniversalservice -o examples/string/test_small
-
-test_strings: $(STRING_TESTS)
-	for i in $(STRING_TESTS); do \
-		LD_LIBRARY_PATH=$$(pwd) valgrind $$i; \
-	done
 
 ###################
 ## Clean
 ##################
 
 clean:
-	rm -f $(STRING_TEST_OBJS) $(STRING_TESTS) $(COMMON_OBJECTS) $(CHENEY_STRING_OBJECTS)
+	rm -f $(COMMON_OBJECTS) $(CHENEY_C89_OBJECTS) libuniversalservice.so

cheney_c89.c

@@ -75,7 +75,7 @@ static unsigned char *relocate(struct uns_gc *gc, unsigned char *p)
 	l = uns_c89_relo_get_len(p);
 	res = raw_alloc(ctx, l, uns_c89_relo_is_record(p));
 	memcpy(res, p, l);
-	gc->after_collection += l;
+	gc->after_collection += l + sizeof(struct uns_c89_relo_hdr);
 
 	uns_c89_relo_set_relo(p, res);
 	return res;
@@ -90,7 +90,7 @@ static void scan_record(struct uns_gc *gc, unsigned char *p)
 
 	for (i = 0; i < l; i++) {
 		newp = relocate(gc, uns_c89_relo_record_get(p, i));
-		memcpy(p, &newp, sizeof(void *));
+		uns_c89_relo_record_set(p, i, newp);
 	}
 }
 
@@ -130,6 +130,10 @@ int uns_cheney_c89_collect(struct uns_gc *gc)
 
 	scanptr = ctx->tospace;
 	while (scanptr != ctx->tospace_alloc) {
+		/* scanptr currently points to the header data that the mutator
+		 * does not usually see.
+		 */
+		scanptr += sizeof(struct uns_c89_relo_hdr);
 		if (uns_c89_relo_is_record(scanptr))
 			scan_record(gc, scanptr);
 		scanptr += uns_c89_relo_get_len(scanptr);
@@ -144,10 +148,12 @@ int uns_cheney_c89_collect(struct uns_gc *gc)
 static void *alloc(struct uns_gc *gc, size_t bytes, int is_record)
 {
 	struct ctx *ctx = gc->ctx;
+	size_t total_bytes = bytes + sizeof(struct uns_c89_relo_hdr);
 
-	if (ctx->tospace_end - ctx->tospace_alloc < bytes)
+	/* Make sure to check for header space when allocating */
+	if (ctx->tospace_end - ctx->tospace_alloc < total_bytes)
 		uns_cheney_c89_collect(gc);
-	if (ctx->tospace_end - ctx->tospace_alloc < bytes)
+	if (ctx->tospace_end - ctx->tospace_alloc < total_bytes)
 		gc->oom(gc);
 
 	return raw_alloc(ctx, bytes, is_record);

examples/string/Makefile

@@ -0,0 +1,22 @@
+.PHONY: test clean
+TESTS=test_small.test test_inefficient.test test_large.test 
+COMMON_OBJS=test_common.o uns_string.o
+.SUFFIXES: .test
+CFLAGS=-Wall -Wno-overlength-strings -std=c89 -Werror -pedantic -fPIC -g -Iinclude
+
+test: $(TESTS) $(COMMON_OBJS)
+	for i in $(TESTS); do \
+		LD_LIBRARY_PATH=$$(pwd)/../../ valgrind ./$$i || exit 1; \
+	done
+
+test_inefficient.test: $(COMMON_OBJS)
+test_small.test: $(COMMON_OBJS)
+test_large.test: $(COMMON_OBJS)
+
+.c.test:
+	$(CC) -I../../include $(CFLAGS) $< $(COMMON_OBJS) -L../../ -luniversalservice -o $@
+.c.o:
+	$(CC) -I../../include $(CFLAGS) $< -c -o $@
+
+clean:
+	rm -f $(TESTS) $(COMMON_OBJS)

examples/string/test_inefficient.c

@@ -0,0 +1,72 @@
+/* Copyright (c) 2024, Peter McGoron
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1) Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2) Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED
+ * TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+ * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include <stdio.h>
+#include "uns.h"
+#include "uns_string.h"
+
+int test(struct uns_gc *gc)
+{
+	struct uns_root_list r = {0};
+	int i;
+
+	/* MistralAI 8x7B 0.1:
+	 * Prompt: "write a sonnet about garbage collectors (the memory management kind)"
+	 *
+	 * This string constant is larger than the standard's minimum. If
+	 * this test doesn't compile, get a better compiler.
+	 */
+	const char s[] =
+		"To the humble garbagemen of code's dark night,\n"
+		"Who tread through bits and bytes with steady hand.\n"
+		"They roam the heap, in dimly lit sight,\n"
+		"And rescue order from chaos unplanned.\n"
+
+		"With algorithms precise and swift they glide,\n"
+		"Through malloc'd arrays and pointers vast.\n"
+		"Their work essential to keep programs wide,\n"
+		"From crashing down upon us too soon passed.\n"
+
+		"Yet often left unsung are these brave knights,\n"
+		"Whose vigilance keeps our software clean.\n"
+		"In shadows cast by CPUs might,\n"
+		"Unseen forces that make sense of unseen.\n"
+
+		"So here's to you, dear Garbage Collectors all,\n"
+		"Thank you for keeping coding dreams enthralled.\n"
+	;
+
+	/* Generate lots of garbage by being inefficient. */
+	uns_string_alloc(gc, &r, 32);
+	uns_root_add(gc, &r);
+	for (i = 0; i < sizeof(s) - 1; i++) {
+		uns_string_append_char(gc, &r, s[i]);
+	}
+	gc->collect(gc);
+	uns_root_remove(gc, &r);
+	gc->collect(gc);
+
+	return 0;
+}

examples/string/test_large.c

@@ -23,13 +23,14 @@
  * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
+#include <stdio.h>
 #include "uns.h"
 #include "uns_string.h"
 
 int test(struct uns_gc *gc)
 {
 	struct uns_root_list r = {0};
-	size_t i;
+	int i;
 
 	/* MistralAI 8x7B 0.1:
 	 * Prompt: "write a sonnet about garbage collectors (the memory management kind)"
@@ -57,21 +58,15 @@ int test(struct uns_gc *gc)
 		"Thank you for keeping coding dreams enthralled.\n"
 	;
 
-	uns_string_alloc_into(&gc, &r);
-
 	/* generate lots of garbage */
-	uns_string_alloc_into(&gc, &r);
+	uns_string_alloc(gc, &r, 32);
-	uns_root_add(&gc, &r);
+	uns_root_add(gc, &r);
-	for (i = 0; i < 100; i++)
+	for (i = 0; i < 100; i++) {
-		uns_string_append_bytes(&gc, &r, s, sizeof(s) - 1);
+		uns_string_append_bytes(gc, &r, s, sizeof(s) - 1);
-	uns_root_remove(&gc, &r);
+	}
-
+	gc->collect(gc);
-	/* Generate lots of garbage by being inefficient. */
+	uns_root_remove(gc, &r);
-	uns_string_alloc_into(&gc, &r);
+	gc->collect(gc);
-	uns_root_add(&gc, &r);
-	for (i = 0; i < sizeof(s) - 1; i++)
-		uns_string_append_char(&gc, &r, s[i]);
-	uns_root_remove(&gc, &r);
 
 	return 0;
 }

examples/string/uns_string.c

@@ -70,27 +70,43 @@ char *uns_string_ptr(struct uns_gc *gc, struct uns_root_list *root)
 
 void uns_string_alloc(struct uns_gc *gc, struct uns_root_list *root, size_t start_len)
 {
+	unsigned char *p;
 	root->p = gc->alloc_record(gc, NUMBER_OF_IND);
 
-	gc->record_set_ptr(root->p, ALLEN_IND, gc->alloc(gc, sizeof(size_t)));
+	/* The following is incorrect:
+	 *   gc->record_set_ptr(root->p, ALLEN_IND, gc->alloc(gc, sizeof(size_t)));
+	 * This is because gc->alloc() can cause a collection, invalidating root->p,
+	 * but root->p may have already been put on the stack.
+	 */
+
+	p = gc->alloc(gc, sizeof(size_t));
+	gc->record_set_ptr(root->p, ALLEN_IND, p);
 	set_allen(gc, root, start_len);
 
-	gc->record_set_ptr(root->p, LEN_IND, gc->alloc(gc, sizeof(size_t)));
+	p = gc->alloc(gc, sizeof(size_t));
+	gc->record_set_ptr(root->p, LEN_IND, p);
 	set_len(gc, root, 0);
 
-	gc->record_set_ptr(root->p, BYTES_IND, gc->alloc(gc, start_len));
+	p = gc->alloc(gc, start_len);
+	gc->record_set_ptr(root->p, BYTES_IND, p);
 }
 
 void uns_string_resize(struct uns_gc *gc, struct uns_root_list *root, size_t newlen)
 {
-	char *tmp;
+	struct uns_root_list tmp_new = {0};
+	size_t old_len = uns_string_len(gc, root);
 
-	tmp = gc->alloc(gc, newlen);
+	uns_root_add(gc, &tmp_new);
-	set_allen(gc, root, newlen);
+	uns_string_alloc(gc, &tmp_new, newlen);
 
-	if (newlen <= uns_string_len(gc, root))
+	if (newlen <= old_len)
-		set_len(gc, root, newlen);
+		set_len(gc, &tmp_new, newlen);
-	memcpy(tmp, uns_string_ptr(gc, root), uns_string_len(gc, root));
+	else
+		set_len(gc, &tmp_new, old_len);
+
+	memcpy(uns_string_ptr(gc, &tmp_new), uns_string_ptr(gc, root), uns_string_len(gc, root));
+	root->p = tmp_new.p;
+	uns_root_remove(gc, &tmp_new);
 }
 
 void uns_string_ensure(struct uns_gc *gc, struct uns_root_list *root, size_t extent)