From 26afacd565bff3a016b769cb7ee644b3b75b809d Mon Sep 17 00:00:00 2001 From: Peter McGoron Date: Wed, 12 Jun 2024 08:56:49 -0400 Subject: [PATCH] strings: add more tests cheney_c89: fix memory leak errors --- .gitignore | 1 + Makefile | 22 +++------ cheney_c89.c | 14 ++++-- examples/string/Makefile | 22 +++++++++ examples/string/test_inefficient.c | 72 ++++++++++++++++++++++++++++++ examples/string/test_large.c | 25 +++++------ examples/string/uns_string.c | 34 ++++++++++---- 7 files changed, 145 insertions(+), 45 deletions(-) create mode 100644 examples/string/Makefile create mode 100644 examples/string/test_inefficient.c diff --git a/.gitignore b/.gitignore index 9d22eb4..b1ba5a6 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,3 @@ *.o *.so +*.test diff --git a/Makefile b/Makefile index 6a20cbb..86991e5 100644 --- a/Makefile +++ b/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 $@ -############################################# -## 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 +test: + ################### ## Clean ################## clean: - rm -f $(STRING_TEST_OBJS) $(STRING_TESTS) $(COMMON_OBJECTS) $(CHENEY_STRING_OBJECTS) + rm -f $(COMMON_OBJECTS) $(CHENEY_C89_OBJECTS) libuniversalservice.so diff --git a/cheney_c89.c b/cheney_c89.c index bb66973..e6aee3a 100644 --- a/cheney_c89.c +++ b/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); diff --git a/examples/string/Makefile b/examples/string/Makefile new file mode 100644 index 0000000..25352a8 --- /dev/null +++ b/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) diff --git a/examples/string/test_inefficient.c b/examples/string/test_inefficient.c new file mode 100644 index 0000000..fb96684 --- /dev/null +++ b/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 +#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; +} diff --git a/examples/string/test_large.c b/examples/string/test_large.c index dddbbff..68964ef 100644 --- a/examples/string/test_large.c +++ b/examples/string/test_large.c @@ -23,13 +23,14 @@ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ +#include #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_root_add(&gc, &r); - for (i = 0; i < 100; i++) - uns_string_append_bytes(&gc, &r, s, sizeof(s) - 1); - uns_root_remove(&gc, &r); - - /* Generate lots of garbage by being inefficient. */ - uns_string_alloc_into(&gc, &r); - 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); + uns_string_alloc(gc, &r, 32); + uns_root_add(gc, &r); + for (i = 0; i < 100; i++) { + uns_string_append_bytes(gc, &r, s, sizeof(s) - 1); + } + gc->collect(gc); + uns_root_remove(gc, &r); + gc->collect(gc); return 0; } diff --git a/examples/string/uns_string.c b/examples/string/uns_string.c index e510fcf..219df89 100644 --- a/examples/string/uns_string.c +++ b/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); - set_allen(gc, root, newlen); + uns_root_add(gc, &tmp_new); + uns_string_alloc(gc, &tmp_new, newlen); - if (newlen <= uns_string_len(gc, root)) - set_len(gc, root, newlen); - memcpy(tmp, uns_string_ptr(gc, root), uns_string_len(gc, root)); + if (newlen <= old_len) + set_len(gc, &tmp_new, newlen); + 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)