From be83eb8671e7789cbe5ca1fc8d3f5d133e2e7014 Mon Sep 17 00:00:00 2001 From: Carl Worth Date: Wed, 23 Jun 2010 13:34:05 -0700 Subject: glsl2 main: Use talloc to allocate whole_program struct. This way, whole_program can be our top-level talloc context object, allowing us to free the lot with a single talloc_free in the end. --- main.cpp | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) (limited to 'main.cpp') diff --git a/main.cpp b/main.cpp index 17f25d741e..78169d257d 100644 --- a/main.cpp +++ b/main.cpp @@ -200,20 +200,21 @@ main(int argc, char **argv) if (argc <= optind) usage_fail(argv[0]); - struct glsl_program whole_program; - memset(&whole_program, 0, sizeof(whole_program)); + struct glsl_program *whole_program; + + whole_program = talloc_zero (NULL, struct glsl_program); + assert(whole_program != NULL); for (/* empty */; argc > optind; optind++) { - whole_program.Shaders = (struct glsl_shader **) - realloc(whole_program.Shaders, - sizeof(struct glsl_shader *) * (whole_program.NumShaders + 1)); - assert(whole_program.Shaders != NULL); + whole_program->Shaders = (struct glsl_shader **) + realloc(whole_program->Shaders, + sizeof(struct glsl_shader *) * (whole_program->NumShaders + 1)); + assert(whole_program->Shaders != NULL); - /* talloc context should probably be whole_program */ - struct glsl_shader *shader = talloc_zero(NULL, glsl_shader); + struct glsl_shader *shader = talloc_zero(whole_program, glsl_shader); - whole_program.Shaders[whole_program.NumShaders] = shader; - whole_program.NumShaders++; + whole_program->Shaders[whole_program->NumShaders] = shader; + whole_program->NumShaders++; const unsigned len = strlen(argv[optind]); if (len < 6) @@ -245,9 +246,11 @@ main(int argc, char **argv) } if ((status == EXIT_SUCCESS) && do_link) { - link_shaders(&whole_program); - status = (whole_program.LinkStatus) ? EXIT_SUCCESS : EXIT_FAILURE; + link_shaders(whole_program); + status = (whole_program->LinkStatus) ? EXIT_SUCCESS : EXIT_FAILURE; } + talloc_free(whole_program); + return status; } -- cgit v1.2.3 From 2d2561ef9696aa5ff0c1a85e3a4a95475f927935 Mon Sep 17 00:00:00 2001 From: Carl Worth Date: Wed, 23 Jun 2010 15:43:38 -0700 Subject: glsl2 main: Use talloc to allocate _mesa_glsl_parse_state This is a short-lived object. It exists only for the duration of the compile_shader() function, (as opposed to the shader and whole_program which live longer). The state is created with the same talloc parent as the shader, so that other allocation can be done with talloc_parent(state) as the owner in order to attach to a long-lived object. --- main.cpp | 57 ++++++++++++++++++++++++++++++--------------------------- 1 file changed, 30 insertions(+), 27 deletions(-) (limited to 'main.cpp') diff --git a/main.cpp b/main.cpp index 78169d257d..f56e6f6142 100644 --- a/main.cpp +++ b/main.cpp @@ -106,38 +106,39 @@ const struct option compiler_opts[] = { void compile_shader(struct glsl_shader *shader) { - struct _mesa_glsl_parse_state state; + struct _mesa_glsl_parse_state *state; + + state = talloc_zero(talloc_parent(shader), struct _mesa_glsl_parse_state); - memset(& state, 0, sizeof(state)); switch (shader->Type) { - case GL_VERTEX_SHADER: state.target = vertex_shader; break; - case GL_FRAGMENT_SHADER: state.target = fragment_shader; break; - case GL_GEOMETRY_SHADER: state.target = geometry_shader; break; + case GL_VERTEX_SHADER: state->target = vertex_shader; break; + case GL_FRAGMENT_SHADER: state->target = fragment_shader; break; + case GL_GEOMETRY_SHADER: state->target = geometry_shader; break; } - state.scanner = NULL; - state.translation_unit.make_empty(); - state.symbols = new glsl_symbol_table; - state.info_log = talloc_strdup(shader, ""); - state.error = false; - state.temp_index = 0; - state.loop_or_switch_nesting = NULL; - state.ARB_texture_rectangle_enable = true; + state->scanner = NULL; + state->translation_unit.make_empty(); + state->symbols = new glsl_symbol_table; + state->info_log = talloc_strdup(shader, ""); + state->error = false; + state->temp_index = 0; + state->loop_or_switch_nesting = NULL; + state->ARB_texture_rectangle_enable = true; /* Create a new context for the preprocessor output. Ultimately, this * should probably be the parser context, but there isn't one yet. */ const char *source = shader->Source; - state.error = preprocess(shader, &source, &state.info_log); + state->error = preprocess(shader, &source, &state->info_log); - if (!state.error) { - _mesa_glsl_lexer_ctor(& state, source); - _mesa_glsl_parse(& state); - _mesa_glsl_lexer_dtor(& state); + if (!state->error) { + _mesa_glsl_lexer_ctor(state, source); + _mesa_glsl_parse(state); + _mesa_glsl_lexer_dtor(state); } if (dump_ast) { - foreach_list_const(n, &state.translation_unit) { + foreach_list_const(n, &state->translation_unit) { ast_node *ast = exec_node_data(ast_node, n, link); ast->print(); } @@ -145,13 +146,13 @@ compile_shader(struct glsl_shader *shader) } shader->ir.make_empty(); - if (!state.error && !state.translation_unit.is_empty()) - _mesa_ast_to_hir(&shader->ir, &state); + if (!state->error && !state->translation_unit.is_empty()) + _mesa_ast_to_hir(&shader->ir, state); validate_ir_tree(&shader->ir); /* Optimization passes */ - if (!state.error && !shader->ir.is_empty()) { + if (!state->error && !shader->ir.is_empty()) { bool progress; do { progress = false; @@ -171,17 +172,19 @@ compile_shader(struct glsl_shader *shader) validate_ir_tree(&shader->ir); /* Print out the resulting IR */ - if (!state.error && dump_lir) { - _mesa_print_ir(&shader->ir, &state); + if (!state->error && dump_lir) { + _mesa_print_ir(&shader->ir, state); } - shader->symbols = state.symbols; - shader->CompileStatus = !state.error; + shader->symbols = state->symbols; + shader->CompileStatus = !state->error; if (shader->InfoLog) talloc_free(shader->InfoLog); - shader->InfoLog = state.info_log; + shader->InfoLog = state->info_log; + + talloc_free(state); return; } -- cgit v1.2.3 From f961e4458f1e894ca782c1627b69cdee993a16f8 Mon Sep 17 00:00:00 2001 From: Carl Worth Date: Wed, 23 Jun 2010 15:47:04 -0700 Subject: glsl_symbol_table: Add new talloc-based new() We take advantage of overloading of the new operator (with an additional parameter!) to make this look as "C++ like" as possible. This closes 507 memory leaks when compiling glsl-orangebook-ch06-bump.frag when measured with: valgrind ./glsl glsl-orangebook-ch06-bump.frag as seen here: total heap usage: 55,623 allocs, 14,389 frees (was 13,882 frees before) --- glsl_symbol_table.h | 33 +++++++++++++++++++++++++++++++++ ir.h | 4 ++++ main.cpp | 6 +----- 3 files changed, 38 insertions(+), 5 deletions(-) (limited to 'main.cpp') diff --git a/glsl_symbol_table.h b/glsl_symbol_table.h index 26b90fdb7c..ae2fd3f4f1 100644 --- a/glsl_symbol_table.h +++ b/glsl_symbol_table.h @@ -26,6 +26,8 @@ #ifndef GLSL_SYMBOL_TABLE #define GLSL_SYMBOL_TABLE +#include + #include "symbol_table.h" #include "ir.h" #include "glsl_types.h" @@ -44,7 +46,38 @@ private: glsl_function_name_space = 2 }; + static int + _glsl_symbol_table_destructor (glsl_symbol_table *table) + { + table->~glsl_symbol_table(); + + return 0; + } + public: + /* Callers of this talloc-based new need not call delete. It's + * easier to just talloc_free 'ctx' (or any of its ancestors). */ + static void* operator new(size_t size, void *ctx) + { + void *table; + + table = talloc_size(ctx, size); + assert(table != NULL); + + talloc_set_destructor(table, (int (*)(void*)) _glsl_symbol_table_destructor); + + return table; + } + + /* If the user *does* call delete, that's OK, we will just + * talloc_free in that case. Here, C++ will have already called the + * destructor so tell talloc not to do that again. */ + static void operator delete(void *table) + { + talloc_set_destructor(table, NULL); + talloc_free(table); + } + glsl_symbol_table() { table = _mesa_symbol_table_ctor(); diff --git a/ir.h b/ir.h index 9277f76204..68e90653ed 100644 --- a/ir.h +++ b/ir.h @@ -29,6 +29,10 @@ #include #include +extern "C" { +#include +} + #include "list.h" #include "ir_visitor.h" #include "ir_hierarchical_visitor.h" diff --git a/main.cpp b/main.cpp index f56e6f6142..dcd4b8f672 100644 --- a/main.cpp +++ b/main.cpp @@ -29,10 +29,6 @@ #include #include -extern "C" { -#include -} - #include "ast.h" #include "glsl_parser_extras.h" #include "glsl_parser.h" @@ -118,7 +114,7 @@ compile_shader(struct glsl_shader *shader) state->scanner = NULL; state->translation_unit.make_empty(); - state->symbols = new glsl_symbol_table; + state->symbols = new(shader) glsl_symbol_table; state->info_log = talloc_strdup(shader, ""); state->error = false; state->temp_index = 0; -- cgit v1.2.3 From a9696e79fb3afc6a4724bd16ee1ccdfebebfd0fd Mon Sep 17 00:00:00 2001 From: Carl Worth Date: Fri, 18 Jun 2010 17:37:02 -0700 Subject: main: Close memory leak of shader string from load_text_file. Could have just added a call to free() to main, but since we're using talloc everywhere else, we might as well just use it here too. So pass a new 'ctx' argument to load_text_file. This removes a single memory leak from all invocations of the standalone glsl compiler. --- main.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'main.cpp') diff --git a/main.cpp b/main.cpp index dcd4b8f672..dfed4a30e8 100644 --- a/main.cpp +++ b/main.cpp @@ -36,9 +36,9 @@ #include "ir_print_visitor.h" #include "program.h" - +/* Returned string will have 'ctx' as its talloc owner. */ static char * -load_text_file(const char *file_name, size_t *size) +load_text_file(void *ctx, const char *file_name, size_t *size) { char *text = NULL; struct stat st; @@ -51,7 +51,7 @@ load_text_file(const char *file_name, size_t *size) } if (fstat(fd, & st) == 0) { - text = (char *) malloc(st.st_size + 1); + text = (char *) talloc_size(ctx, st.st_size + 1); if (text != NULL) { do { ssize_t bytes = read(fd, text + total_read, @@ -229,7 +229,8 @@ main(int argc, char **argv) else usage_fail(argv[0]); - shader->Source = load_text_file(argv[optind], &shader->SourceLen); + shader->Source = load_text_file(whole_program, + argv[optind], &shader->SourceLen); if (shader->Source == NULL) { printf("File \"%s\" does not exist.\n", argv[optind]); exit(EXIT_FAILURE); -- cgit v1.2.3 From 26bbfb7917a71d46d9227bbf960606cb673636d3 Mon Sep 17 00:00:00 2001 From: Carl Worth Date: Wed, 23 Jun 2010 19:09:56 -0700 Subject: glsl2 main: Switch from realloc to talloc_realloc to construct program source. This closes 1 leak in the glsl-orangebook-ch06-bump.frag test leaving 4 to go, (all of which are inside hash_table.c). --- main.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'main.cpp') diff --git a/main.cpp b/main.cpp index dfed4a30e8..dcd9bd69c0 100644 --- a/main.cpp +++ b/main.cpp @@ -206,8 +206,8 @@ main(int argc, char **argv) for (/* empty */; argc > optind; optind++) { whole_program->Shaders = (struct glsl_shader **) - realloc(whole_program->Shaders, - sizeof(struct glsl_shader *) * (whole_program->NumShaders + 1)); + talloc_realloc(whole_program, whole_program->Shaders, + struct glsl_shader *, whole_program->NumShaders + 1); assert(whole_program->Shaders != NULL); struct glsl_shader *shader = talloc_zero(whole_program, glsl_shader); -- cgit v1.2.3