From 1660a2954797e056caba319c5d6c70b0d4be22fe Mon Sep 17 00:00:00 2001 From: Carl Worth Date: Wed, 23 Jun 2010 18:11:51 -0700 Subject: exec_node: Add new talloc-based new() And fix all callers to use the tallbac-based new for exec_node construction. We make ready use of talloc_parent in order to get valid, (and appropriate) talloc owners for everything we construct without having to add new 'ctx' parameters up and down all the call trees. This closes the majority of the memory leaks in the glsl-orangebook-ch06-bump.frag test: total heap usage: 55,623 allocs, 42,672 frees (was 14,533 frees) Now 76.7% leak-free. Woo-hoo! --- ast_function.cpp | 53 +++++++++++++++++++++++++++++++---------------------- 1 file changed, 31 insertions(+), 22 deletions(-) (limited to 'ast_function.cpp') diff --git a/ast_function.cpp b/ast_function.cpp index ff2dfa502f..866cbc4ecd 100644 --- a/ast_function.cpp +++ b/ast_function.cpp @@ -54,6 +54,8 @@ process_call(exec_list *instructions, ir_function *f, YYLTYPE *loc, exec_list *actual_parameters, struct _mesa_glsl_parse_state *state) { + void *ctx = talloc_parent(state); + const ir_function_signature *sig = f->matching_signature(actual_parameters); @@ -93,7 +95,7 @@ process_call(exec_list *instructions, ir_function *f, /* FINISHME: The list of actual parameters needs to be modified to * FINISHME: include any necessary conversions. */ - return new ir_call(sig, actual_parameters); + return new(ctx) ir_call(sig, actual_parameters); } else { /* FINISHME: Log a better error message here. G++ will show the types * FINISHME: of the actual parameters and the set of candidate @@ -132,6 +134,7 @@ match_function_by_name(exec_list *instructions, const char *name, static ir_rvalue * convert_component(ir_rvalue *src, const glsl_type *desired_type) { + void *ctx = talloc_parent(src); const unsigned a = desired_type->base_type; const unsigned b = src->type->base_type; ir_expression *result = NULL; @@ -149,22 +152,22 @@ convert_component(ir_rvalue *src, const glsl_type *desired_type) case GLSL_TYPE_UINT: case GLSL_TYPE_INT: if (b == GLSL_TYPE_FLOAT) - result = new ir_expression(ir_unop_f2i, desired_type, src, NULL); + result = new(ctx) ir_expression(ir_unop_f2i, desired_type, src, NULL); else { assert(b == GLSL_TYPE_BOOL); - result = new ir_expression(ir_unop_b2i, desired_type, src, NULL); + result = new(ctx) ir_expression(ir_unop_b2i, desired_type, src, NULL); } break; case GLSL_TYPE_FLOAT: switch (b) { case GLSL_TYPE_UINT: - result = new ir_expression(ir_unop_u2f, desired_type, src, NULL); + result = new(ctx) ir_expression(ir_unop_u2f, desired_type, src, NULL); break; case GLSL_TYPE_INT: - result = new ir_expression(ir_unop_i2f, desired_type, src, NULL); + result = new(ctx) ir_expression(ir_unop_i2f, desired_type, src, NULL); break; case GLSL_TYPE_BOOL: - result = new ir_expression(ir_unop_b2f, desired_type, src, NULL); + result = new(ctx) ir_expression(ir_unop_b2f, desired_type, src, NULL); break; } break; @@ -172,12 +175,12 @@ convert_component(ir_rvalue *src, const glsl_type *desired_type) ir_constant *zero = NULL; switch (b) { - case GLSL_TYPE_UINT: zero = new ir_constant(unsigned(0)); break; - case GLSL_TYPE_INT: zero = new ir_constant(int(0)); break; - case GLSL_TYPE_FLOAT: zero = new ir_constant(0.0f); break; + case GLSL_TYPE_UINT: zero = new(ctx) ir_constant(unsigned(0)); break; + case GLSL_TYPE_INT: zero = new(ctx) ir_constant(int(0)); break; + case GLSL_TYPE_FLOAT: zero = new(ctx) ir_constant(0.0f); break; } - result = new ir_expression(ir_binop_nequal, desired_type, src, zero); + result = new(ctx) ir_expression(ir_binop_nequal, desired_type, src, zero); } } @@ -194,6 +197,7 @@ convert_component(ir_rvalue *src, const glsl_type *desired_type) static ir_rvalue * dereference_component(ir_rvalue *src, unsigned component) { + void *ctx = talloc_parent(src); assert(component < src->type->components()); /* If the source is a constant, just create a new constant instead of a @@ -201,12 +205,12 @@ dereference_component(ir_rvalue *src, unsigned component) */ ir_constant *constant = src->as_constant(); if (constant) - return new ir_constant(constant, component); + return new(ctx) ir_constant(constant, component); if (src->type->is_scalar()) { return src; } else if (src->type->is_vector()) { - return new ir_swizzle(src, component, 0, 0, 0, 1); + return new(ctx) ir_swizzle(src, component, 0, 0, 0, 1); } else { assert(src->type->is_matrix()); @@ -215,8 +219,8 @@ dereference_component(ir_rvalue *src, unsigned component) */ const int c = component / src->type->column_type()->vector_elements; const int r = component % src->type->column_type()->vector_elements; - ir_constant *const col_index = new ir_constant(c); - ir_dereference *const col = new ir_dereference_array(src, col_index); + ir_constant *const col_index = new(ctx) ir_constant(c); + ir_dereference *const col = new(ctx) ir_dereference_array(src, col_index); col->type = src->type->column_type(); @@ -306,6 +310,7 @@ constant_record_constructor(const glsl_type *constructor_type, YYLTYPE *loc, exec_list *parameters, struct _mesa_glsl_parse_state *state) { + void *ctx = talloc_parent(state); bool all_parameters_are_constant = true; exec_node *node = parameters->head; @@ -338,7 +343,7 @@ constant_record_constructor(const glsl_type *constructor_type, if (!all_parameters_are_constant) return NULL; - return new ir_constant(constructor_type, parameters); + return new(ctx) ir_constant(constructor_type, parameters); } @@ -440,6 +445,7 @@ ir_rvalue * ast_function_expression::hir(exec_list *instructions, struct _mesa_glsl_parse_state *state) { + void *ctx = talloc_parent(state); /* There are three sorts of function calls. * * 1. contstructors - The first subexpression is an ast_type_specifier. @@ -574,11 +580,13 @@ ast_function_expression::hir(exec_list *instructions, * glsl-vs-constructor-call.shader_test. */ if (result->type->components() >= 1 && !result->as_constant()) { - result_var = new ir_variable(result->type, "constructor_tmp"); + result_var = new(ctx) ir_variable(result->type, + "constructor_tmp"); ir_dereference_variable *lhs; - lhs = new ir_dereference_variable(result_var); - instructions->push_tail(new ir_assignment(lhs, result, NULL)); + lhs = new(ctx) ir_dereference_variable(result_var); + instructions->push_tail(new(ctx) ir_assignment(lhs, + result, NULL)); } /* Process each of the components of the parameter. Dereference @@ -592,7 +600,7 @@ ast_function_expression::hir(exec_list *instructions, ir_rvalue *component; if (result_var) { - ir_dereference *d = new ir_dereference_variable(result_var); + ir_dereference *d = new(ctx) ir_dereference_variable(result_var); component = dereference_component(d, i); } else { component = dereference_component(result, i); @@ -674,7 +682,8 @@ ast_function_expression::hir(exec_list *instructions, */ if (all_parameters_are_constant) { if (components_used >= type_components) - return new ir_constant(sig->return_type, & actual_parameters); + return new(ctx) ir_constant(sig->return_type, + & actual_parameters); assert(sig->return_type->is_vector() || sig->return_type->is_matrix()); @@ -695,9 +704,9 @@ ast_function_expression::hir(exec_list *instructions, generate_constructor_vector(sig->return_type, initializer, &data); - return new ir_constant(sig->return_type, &data); + return new(ctx) ir_constant(sig->return_type, &data); } else - return new ir_call(sig, & actual_parameters); + return new(ctx) ir_call(sig, & actual_parameters); } else { /* FINISHME: Log a better error message here. G++ will show the * FINSIHME: types of the actual parameters and the set of -- cgit v1.2.3 From e01193af325cbdde51b3219c85c58f19d5a87f1b Mon Sep 17 00:00:00 2001 From: Carl Worth Date: Wed, 23 Jun 2010 18:25:04 -0700 Subject: Close memory leak in ir_call::get_error_instruction. By propagating a 'ctx' parameter through these calls. This fix happens to have no impact on glsl-orangebook-ch06-bump.frag, (since it doesn't trigger any errors). --- ast_function.cpp | 32 +++++++++++++++++--------------- hir_field_selection.cpp | 2 +- ir.cpp | 4 +--- ir.h | 4 +++- 4 files changed, 22 insertions(+), 20 deletions(-) (limited to 'ast_function.cpp') diff --git a/ast_function.cpp b/ast_function.cpp index 866cbc4ecd..9550d4d2f0 100644 --- a/ast_function.cpp +++ b/ast_function.cpp @@ -104,7 +104,7 @@ process_call(exec_list *instructions, ir_function *f, */ _mesa_glsl_error(loc, state, "no matching function for call to `%s'", f->name); - return ir_call::get_error_instruction(); + return ir_call::get_error_instruction(ctx); } } @@ -114,11 +114,12 @@ match_function_by_name(exec_list *instructions, const char *name, YYLTYPE *loc, exec_list *actual_parameters, struct _mesa_glsl_parse_state *state) { + void *ctx = talloc_parent(state); ir_function *f = state->symbols->get_function(name); if (f == NULL) { _mesa_glsl_error(loc, state, "function `%s' undeclared", name); - return ir_call::get_error_instruction(); + return ir_call::get_error_instruction(ctx); } /* Once we've determined that the function being called might exist, try @@ -238,6 +239,7 @@ process_array_constructor(exec_list *instructions, YYLTYPE *loc, exec_list *parameters, struct _mesa_glsl_parse_state *state) { + void *ctx = talloc_parent(state); /* Array constructors come in two forms: sized and unsized. Sized array * constructors look like 'vec4[2](a, b)', where 'a' and 'b' are vec4 * variables. In this case the number of parameters must exactly match the @@ -272,7 +274,7 @@ process_array_constructor(exec_list *instructions, "parameter%s", (constructor_type->length != 0) ? "at least" : "exactly", min_param, (min_param <= 1) ? "" : "s"); - return ir_call::get_error_instruction(); + return ir_call::get_error_instruction(ctx); } if (constructor_type->length == 0) { @@ -468,14 +470,14 @@ ast_function_expression::hir(exec_list *instructions, if (constructor_type->is_sampler()) { _mesa_glsl_error(& loc, state, "cannot construct sampler type `%s'", constructor_type->name); - return ir_call::get_error_instruction(); + return ir_call::get_error_instruction(ctx); } if (constructor_type->is_array()) { if (state->language_version <= 110) { _mesa_glsl_error(& loc, state, "array constructors forbidden in GLSL 1.10"); - return ir_call::get_error_instruction(); + return ir_call::get_error_instruction(ctx); } return process_array_constructor(instructions, constructor_type, @@ -525,7 +527,7 @@ ast_function_expression::hir(exec_list *instructions, _mesa_glsl_error(& loc, state, "too few components to construct " "`%s'", constructor_type->name); - return ir_call::get_error_instruction(); + return ir_call::get_error_instruction(ctx); } foreach_list (n, &this->expressions) { @@ -555,14 +557,14 @@ ast_function_expression::hir(exec_list *instructions, _mesa_glsl_error(& loc, state, "too many parameters to `%s' " "constructor", constructor_type->name); - return ir_call::get_error_instruction(); + return ir_call::get_error_instruction(ctx); } if (!result->type->is_numeric() && !result->type->is_boolean()) { _mesa_glsl_error(& loc, state, "cannot construct `%s' from a " "non-numeric data type", constructor_type->name); - return ir_call::get_error_instruction(); + return ir_call::get_error_instruction(ctx); } /* Count the number of matrix and nonmatrix parameters. This @@ -637,7 +639,7 @@ ast_function_expression::hir(exec_list *instructions, _mesa_glsl_error(& loc, state, "cannot construct `%s' from a " "matrix in GLSL 1.10", constructor_type->name); - return ir_call::get_error_instruction(); + return ir_call::get_error_instruction(ctx); } /* From page 50 (page 56 of the PDF) of the GLSL 1.50 spec: @@ -651,7 +653,7 @@ ast_function_expression::hir(exec_list *instructions, _mesa_glsl_error(& loc, state, "for matrix `%s' constructor, " "matrix must be only parameter", constructor_type->name); - return ir_call::get_error_instruction(); + return ir_call::get_error_instruction(ctx); } /* From page 28 (page 34 of the PDF) of the GLSL 1.10 spec: @@ -664,14 +666,14 @@ ast_function_expression::hir(exec_list *instructions, _mesa_glsl_error(& loc, state, "too few components to construct " "`%s'", constructor_type->name); - return ir_call::get_error_instruction(); + return ir_call::get_error_instruction(ctx); } ir_function *f = state->symbols->get_function(constructor_type->name); if (f == NULL) { _mesa_glsl_error(& loc, state, "no constructor for type `%s'", constructor_type->name); - return ir_call::get_error_instruction(); + return ir_call::get_error_instruction(ctx); } const ir_function_signature *sig = @@ -715,11 +717,11 @@ ast_function_expression::hir(exec_list *instructions, */ _mesa_glsl_error(& loc, state, "no matching constructor for `%s'", constructor_type->name); - return ir_call::get_error_instruction(); + return ir_call::get_error_instruction(ctx); } } - return ir_call::get_error_instruction(); + return ir_call::get_error_instruction(ctx); } else { const ast_expression *id = subexpressions[0]; YYLTYPE loc = id->get_location(); @@ -744,5 +746,5 @@ ast_function_expression::hir(exec_list *instructions, &actual_parameters, state); } - return ir_call::get_error_instruction(); + return ir_call::get_error_instruction(ctx); } diff --git a/hir_field_selection.cpp b/hir_field_selection.cpp index 6da14925b9..e2efff60d3 100644 --- a/hir_field_selection.cpp +++ b/hir_field_selection.cpp @@ -77,5 +77,5 @@ _mesa_ast_field_selection_to_hir(const ast_expression *expr, expr->primary_expression.identifier); } - return result ? result : ir_call::get_error_instruction(); + return result ? result : ir_call::get_error_instruction(ctx); } diff --git a/ir.cpp b/ir.cpp index 26cd475552..6286d874e6 100644 --- a/ir.cpp +++ b/ir.cpp @@ -833,10 +833,8 @@ ir_function::ir_function(const char *name) ir_call * -ir_call::get_error_instruction() +ir_call::get_error_instruction(void *ctx) { - /* NULL is wrong and leaks */ - void *ctx = NULL; ir_call *call = new(ctx) ir_call; call->type = glsl_type::error_type; diff --git a/ir.h b/ir.h index 68e90653ed..1c95512fb1 100644 --- a/ir.h +++ b/ir.h @@ -645,8 +645,10 @@ public: /** * Get a generic ir_call object when an error occurs + * + * Any allocation will be performed with 'ctx' as talloc owner. */ - static ir_call *get_error_instruction(); + static ir_call *get_error_instruction(void *ctx); /** * Get an iterator for the set of acutal parameters -- cgit v1.2.3 From 12c411504ca86341f8b96c349c15413ee198cc71 Mon Sep 17 00:00:00 2001 From: Carl Worth Date: Fri, 18 Jun 2010 17:52:59 -0700 Subject: Close memory leaks in glsl_type (constructor and get_array_instance) Add a talloc ctx to both get_array_instance and the glsl_type constructor in order to be able to call talloc_size instead of malloc. This fix now makes glsl-orangebook-ch06-bump.frag 99.99% leak free: total heap usage: 55,623 allocs, 55,615 Only 8 missing frees now. --- ast_function.cpp | 3 ++- ast_to_hir.cpp | 5 +++-- glsl_types.cpp | 11 ++++++----- glsl_types.h | 5 +++-- ir_reader.cpp | 2 +- ir_variable.cpp | 17 ++++++++++------- 6 files changed, 25 insertions(+), 18 deletions(-) (limited to 'ast_function.cpp') diff --git a/ast_function.cpp b/ast_function.cpp index 9550d4d2f0..761af00b95 100644 --- a/ast_function.cpp +++ b/ast_function.cpp @@ -279,7 +279,8 @@ process_array_constructor(exec_list *instructions, if (constructor_type->length == 0) { constructor_type = - glsl_type::get_array_instance(constructor_type->element_type(), + glsl_type::get_array_instance(state, + constructor_type->element_type(), parameter_count); assert(constructor_type != NULL); assert(constructor_type->length == parameter_count); diff --git a/ast_to_hir.cpp b/ast_to_hir.cpp index eafc9e8114..38e344f0d2 100644 --- a/ast_to_hir.cpp +++ b/ast_to_hir.cpp @@ -506,7 +506,8 @@ do_assignment(exec_list *instructions, struct _mesa_glsl_parse_state *state, var->max_array_access); } - var->type = glsl_type::get_array_instance(lhs->type->element_type(), + var->type = glsl_type::get_array_instance(state, + lhs->type->element_type(), rhs->type->array_size()); } } @@ -1409,7 +1410,7 @@ process_array_type(const glsl_type *base, ast_node *array_size, } } - return glsl_type::get_array_instance(base, length); + return glsl_type::get_array_instance(state, base, length); } diff --git a/glsl_types.cpp b/glsl_types.cpp index 7dcb4a4caf..d1b9dc6412 100644 --- a/glsl_types.cpp +++ b/glsl_types.cpp @@ -574,7 +574,7 @@ _mesa_glsl_initialize_constructors(exec_list *instructions, } -glsl_type::glsl_type(const glsl_type *array, unsigned length) : +glsl_type::glsl_type(void *ctx, const glsl_type *array, unsigned length) : base_type(GLSL_TYPE_ARRAY), sampler_dimensionality(0), sampler_shadow(0), sampler_array(0), sampler_type(0), @@ -588,7 +588,7 @@ glsl_type::glsl_type(const glsl_type *array, unsigned length) : * NUL. */ const unsigned name_length = strlen(array->name) + 10 + 3; - char *const n = (char *) malloc(name_length); + char *const n = (char *) talloc_size(ctx, name_length); if (length == 0) snprintf(n, name_length, "%s[]", array->name); @@ -691,9 +691,10 @@ glsl_type::array_key_hash(const void *a) const glsl_type * -glsl_type::get_array_instance(const glsl_type *base, unsigned array_size) +glsl_type::get_array_instance(void *ctx, const glsl_type *base, + unsigned array_size) { - const glsl_type key(base, array_size); + const glsl_type key(ctx, base, array_size); if (array_types == NULL) { array_types = hash_table_ctor(64, array_key_hash, array_key_compare); @@ -701,7 +702,7 @@ glsl_type::get_array_instance(const glsl_type *base, unsigned array_size) const glsl_type *t = (glsl_type *) hash_table_find(array_types, & key); if (t == NULL) { - t = new glsl_type(base, array_size); + t = new glsl_type(ctx, base, array_size); hash_table_insert(array_types, (void *) t, t); } diff --git a/glsl_types.h b/glsl_types.h index 939c173fd4..baec763c42 100644 --- a/glsl_types.h +++ b/glsl_types.h @@ -198,7 +198,8 @@ struct glsl_type { /** * Get the instance of an array type */ - static const glsl_type *get_array_instance(const glsl_type *base, + static const glsl_type *get_array_instance(void *ctx, + const glsl_type *base, unsigned elements); /** @@ -387,7 +388,7 @@ private: /** * Constructor for array types */ - glsl_type(const glsl_type *array, unsigned length); + glsl_type(void *ctx, const glsl_type *array, unsigned length); /** Hash table containing the known array types. */ static struct hash_table *array_types; diff --git a/ir_reader.cpp b/ir_reader.cpp index d6985c4981..7383c42cbc 100644 --- a/ir_reader.cpp +++ b/ir_reader.cpp @@ -138,7 +138,7 @@ read_type(_mesa_glsl_parse_state *st, s_expression *expr) return NULL; } - return glsl_type::get_array_instance(base_type, size->value()); + return glsl_type::get_array_instance(st, base_type, size->value()); } else if (strcmp(type_sym->value(), "struct") == 0) { assert(false); // FINISHME } else { diff --git a/ir_variable.cpp b/ir_variable.cpp index fabd856591..15a4a92f62 100644 --- a/ir_variable.cpp +++ b/ir_variable.cpp @@ -104,7 +104,7 @@ generate_110_uniforms(exec_list *instructions, * FINISHME: for now. */ const glsl_type *const mat4_array_type = - glsl_type::get_array_instance(glsl_type::mat4_type, 4); + glsl_type::get_array_instance(symtab, glsl_type::mat4_type, 4); add_variable("gl_TextureMatrix", ir_var_uniform, -1, mat4_array_type, instructions, symtab); @@ -122,7 +122,8 @@ generate_110_uniforms(exec_list *instructions, * FINISHME: at least 8, so hard-code 8 for now. */ const glsl_type *const light_source_array_type = - glsl_type::get_array_instance(symtab->get_type("gl_LightSourceParameters"), 8); + glsl_type::get_array_instance(symtab, + symtab->get_type("gl_LightSourceParameters"), 8); add_variable("gl_LightSource", ir_var_uniform, -1, light_source_array_type, instructions, symtab); @@ -157,7 +158,7 @@ generate_110_vs_variables(exec_list *instructions, * FINISHME: for now. */ const glsl_type *const vec4_array_type = - glsl_type::get_array_instance(glsl_type::vec4_type, 4); + glsl_type::get_array_instance(symtab, glsl_type::vec4_type, 4); add_variable("gl_TexCoord", ir_var_out, VERT_RESULT_TEX0, vec4_array_type, instructions, symtab); @@ -179,6 +180,7 @@ static void generate_130_vs_variables(exec_list *instructions, glsl_symbol_table *symtab) { + void *ctx = symtab; generate_120_vs_variables(instructions, symtab); for (unsigned i = 0; i < Elements(builtin_130_vs_variables); i++) { @@ -190,7 +192,7 @@ generate_130_vs_variables(exec_list *instructions, * FINISHME: the value of GL_MAX_CLIP_DISTANCES. */ const glsl_type *const clip_distance_array_type = - glsl_type::get_array_instance(glsl_type::float_type, 8); + glsl_type::get_array_instance(ctx, glsl_type::float_type, 8); /* FINISHME: gl_ClipDistance needs a real location assigned. */ add_variable("gl_ClipDistance", ir_var_out, -1, clip_distance_array_type, @@ -240,7 +242,7 @@ generate_110_fs_variables(exec_list *instructions, * FINISHME: for now. */ const glsl_type *const vec4_array_type = - glsl_type::get_array_instance(glsl_type::vec4_type, 4); + glsl_type::get_array_instance(symtab, glsl_type::vec4_type, 4); add_variable("gl_TexCoord", ir_var_in, FRAG_ATTRIB_TEX0, vec4_array_type, instructions, symtab); @@ -256,7 +258,7 @@ generate_ARB_draw_buffers_fs_variables(exec_list *instructions, * FINISHME: at least 1, so hard-code 1 for now. */ const glsl_type *const vec4_array_type = - glsl_type::get_array_instance(glsl_type::vec4_type, 1); + glsl_type::get_array_instance(symtab, glsl_type::vec4_type, 1); ir_variable *const fd = add_variable("gl_FragData", ir_var_out, FRAG_RESULT_DATA0, @@ -279,13 +281,14 @@ static void generate_130_fs_variables(exec_list *instructions, glsl_symbol_table *symtab) { + void *ctx = symtab; generate_120_fs_variables(instructions, symtab); /* FINISHME: The size of this array is implementation dependent based on * FINISHME: the value of GL_MAX_CLIP_DISTANCES. */ const glsl_type *const clip_distance_array_type = - glsl_type::get_array_instance(glsl_type::float_type, 8); + glsl_type::get_array_instance(ctx, glsl_type::float_type, 8); /* FINISHME: gl_ClipDistance needs a real location assigned. */ add_variable("gl_ClipDistance", ir_var_in, -1, clip_distance_array_type, -- cgit v1.2.3