[PATCH vkd3d v6 5/8] vkd3d-shader/hlsl: Properly free parse_variable_def memory.
Signed-off-by: Francisco Casas <fcasas(a)codeweavers.com> Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- v6: - Using 'continue' instead of 'return false' when iterating lists of variable definitions to correctly free the following fields. By the way, maybe we should also make hlsl_new_array_type() free the memory of the basic_type argument on failure. Also we are assuming that hlsl_new_array_type() can't fail in some parts of the code that call it. However, I suggest fixing that in the future, to avoid drowning in a glass of water now. Signed-off-by: Francisco Casas <fcasas(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.y | 42 +++++++++++++++++++++++++++------------- 1 file changed, 29 insertions(+), 13 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 016f1001..477185f3 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -668,6 +668,15 @@ static struct hlsl_type *apply_type_modifiers(struct hlsl_ctx *ctx, struct hlsl_ return new_type; } +static void free_parse_variable_def(struct parse_variable_def *v) +{ + free_parse_initializer(&v->initializer); + vkd3d_free(v->arrays.sizes); + vkd3d_free(v->name); + vkd3d_free((void *)v->semantic.name); + vkd3d_free(v); +} + static struct list *gen_struct_fields(struct hlsl_ctx *ctx, struct hlsl_type *type, struct list *fields) { struct parse_variable_def *v, *v_next; @@ -685,13 +694,14 @@ static struct list *gen_struct_fields(struct hlsl_ctx *ctx, struct hlsl_type *ty if (!(field = hlsl_alloc(ctx, sizeof(*field)))) { - vkd3d_free(v); - return list; + free_parse_variable_def(v); + continue; } field->type = type; for (i = 0; i < v->arrays.count; ++i) field->type = hlsl_new_array_type(ctx, field->type, v->arrays.sizes[i]); + vkd3d_free(v->arrays.sizes); field->loc = v->loc; field->name = v->name; field->semantic = v->semantic; @@ -719,18 +729,29 @@ static bool add_typedef(struct hlsl_ctx *ctx, DWORD modifiers, struct hlsl_type if (!v->arrays.count) { if (!(type = hlsl_type_clone(ctx, orig_type, 0, modifiers))) - return false; + { + free_parse_variable_def(v); + continue; + } } else { type = orig_type; } + ret = true; for (i = 0; i < v->arrays.count; ++i) { if (!(type = hlsl_new_array_type(ctx, type, v->arrays.sizes[i]))) - return false; + { + free_parse_variable_def(v); + ret = false; + break; + } } + if (!ret) + continue; + vkd3d_free(v->arrays.sizes); vkd3d_free((void *)type->name); type->name = v->name; @@ -747,6 +768,7 @@ static bool add_typedef(struct hlsl_ctx *ctx, DWORD modifiers, struct hlsl_type if (!ret) hlsl_error(ctx, &v->loc, VKD3D_SHADER_ERROR_HLSL_REDEFINED, "Type '%s' is already defined.", v->name); + free_parse_initializer(&v->initializer); vkd3d_free(v); } vkd3d_free(list); @@ -1470,15 +1492,6 @@ static void struct_var_initializer(struct hlsl_ctx *ctx, struct list *list, stru vkd3d_free(initializer->args); } -static void free_parse_variable_def(struct parse_variable_def *v) -{ - free_parse_initializer(&v->initializer); - vkd3d_free(v->arrays.sizes); - vkd3d_free(v->name); - vkd3d_free((void *)v->semantic.name); - vkd3d_free(v); -} - static struct list *declare_vars(struct hlsl_ctx *ctx, struct hlsl_type *basic_type, DWORD modifiers, struct list *var_list) { @@ -1510,6 +1523,7 @@ static struct list *declare_vars(struct hlsl_ctx *ctx, struct hlsl_type *basic_t type = basic_type; for (i = 0; i < v->arrays.count; ++i) type = hlsl_new_array_type(ctx, type, v->arrays.sizes[i]); + vkd3d_free(v->arrays.sizes); if (type->type != HLSL_CLASS_MATRIX) check_invalid_matrix_modifiers(ctx, modifiers, v->loc); @@ -1569,6 +1583,7 @@ static struct list *declare_vars(struct hlsl_ctx *ctx, struct hlsl_type *basic_t hlsl_error(ctx, &v->loc, VKD3D_SHADER_ERROR_HLSL_MISSING_INITIALIZER, "Const variable \"%s\" is missing an initializer.", var->name); hlsl_free_var(var); + free_parse_initializer(&v->initializer); vkd3d_free(v); continue; } @@ -1581,6 +1596,7 @@ static struct list *declare_vars(struct hlsl_ctx *ctx, struct hlsl_type *basic_t "Variable \"%s\" was already declared in this scope.", var->name); hlsl_note(ctx, &old->loc, VKD3D_SHADER_LOG_ERROR, "\"%s\" was previously declared here.", old->name); hlsl_free_var(var); + free_parse_initializer(&v->initializer); vkd3d_free(v); continue; } -- 2.25.1
On 3/14/22 13:52, Francisco Casas wrote:
By the way, maybe we should also make hlsl_new_array_type() free the memory of the basic_type argument on failure. Also we are assuming that hlsl_new_array_type() can't fail in some parts of the code that call it. However, I suggest fixing that in the future, to avoid drowning in a glass of water now.
There's no need to do that; new types are stored in the "ctx->types" array, and will be freed when the context is.
Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com> Il 14/03/22 19:52, Francisco Casas ha scritto:
Signed-off-by: Francisco Casas <fcasas(a)codeweavers.com> Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>
--- v6: - Using 'continue' instead of 'return false' when iterating lists of variable definitions to correctly free the following fields.
By the way, maybe we should also make hlsl_new_array_type() free the memory of the basic_type argument on failure. Also we are assuming that hlsl_new_array_type() can't fail in some parts of the code that call it. However, I suggest fixing that in the future, to avoid drowning in a glass of water now.
Signed-off-by: Francisco Casas <fcasas(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.y | 42 +++++++++++++++++++++++++++------------- 1 file changed, 29 insertions(+), 13 deletions(-)
diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 016f1001..477185f3 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -668,6 +668,15 @@ static struct hlsl_type *apply_type_modifiers(struct hlsl_ctx *ctx, struct hlsl_ return new_type; }
+static void free_parse_variable_def(struct parse_variable_def *v) +{ + free_parse_initializer(&v->initializer); + vkd3d_free(v->arrays.sizes); + vkd3d_free(v->name); + vkd3d_free((void *)v->semantic.name); + vkd3d_free(v); +} + static struct list *gen_struct_fields(struct hlsl_ctx *ctx, struct hlsl_type *type, struct list *fields) { struct parse_variable_def *v, *v_next; @@ -685,13 +694,14 @@ static struct list *gen_struct_fields(struct hlsl_ctx *ctx, struct hlsl_type *ty
if (!(field = hlsl_alloc(ctx, sizeof(*field)))) { - vkd3d_free(v); - return list; + free_parse_variable_def(v); + continue; }
field->type = type; for (i = 0; i < v->arrays.count; ++i) field->type = hlsl_new_array_type(ctx, field->type, v->arrays.sizes[i]); + vkd3d_free(v->arrays.sizes); field->loc = v->loc; field->name = v->name; field->semantic = v->semantic; @@ -719,18 +729,29 @@ static bool add_typedef(struct hlsl_ctx *ctx, DWORD modifiers, struct hlsl_type if (!v->arrays.count) { if (!(type = hlsl_type_clone(ctx, orig_type, 0, modifiers))) - return false; + { + free_parse_variable_def(v); + continue; + } } else { type = orig_type; }
+ ret = true; for (i = 0; i < v->arrays.count; ++i) { if (!(type = hlsl_new_array_type(ctx, type, v->arrays.sizes[i]))) - return false; + { + free_parse_variable_def(v); + ret = false; + break; + } } + if (!ret) + continue; + vkd3d_free(v->arrays.sizes);
vkd3d_free((void *)type->name); type->name = v->name; @@ -747,6 +768,7 @@ static bool add_typedef(struct hlsl_ctx *ctx, DWORD modifiers, struct hlsl_type if (!ret) hlsl_error(ctx, &v->loc, VKD3D_SHADER_ERROR_HLSL_REDEFINED, "Type '%s' is already defined.", v->name); + free_parse_initializer(&v->initializer); vkd3d_free(v); } vkd3d_free(list); @@ -1470,15 +1492,6 @@ static void struct_var_initializer(struct hlsl_ctx *ctx, struct list *list, stru vkd3d_free(initializer->args); }
-static void free_parse_variable_def(struct parse_variable_def *v) -{ - free_parse_initializer(&v->initializer); - vkd3d_free(v->arrays.sizes); - vkd3d_free(v->name); - vkd3d_free((void *)v->semantic.name); - vkd3d_free(v); -} - static struct list *declare_vars(struct hlsl_ctx *ctx, struct hlsl_type *basic_type, DWORD modifiers, struct list *var_list) { @@ -1510,6 +1523,7 @@ static struct list *declare_vars(struct hlsl_ctx *ctx, struct hlsl_type *basic_t type = basic_type; for (i = 0; i < v->arrays.count; ++i) type = hlsl_new_array_type(ctx, type, v->arrays.sizes[i]); + vkd3d_free(v->arrays.sizes);
if (type->type != HLSL_CLASS_MATRIX) check_invalid_matrix_modifiers(ctx, modifiers, v->loc); @@ -1569,6 +1583,7 @@ static struct list *declare_vars(struct hlsl_ctx *ctx, struct hlsl_type *basic_t hlsl_error(ctx, &v->loc, VKD3D_SHADER_ERROR_HLSL_MISSING_INITIALIZER, "Const variable \"%s\" is missing an initializer.", var->name); hlsl_free_var(var); + free_parse_initializer(&v->initializer); vkd3d_free(v); continue; } @@ -1581,6 +1596,7 @@ static struct list *declare_vars(struct hlsl_ctx *ctx, struct hlsl_type *basic_t "Variable \"%s\" was already declared in this scope.", var->name); hlsl_note(ctx, &old->loc, VKD3D_SHADER_LOG_ERROR, "\"%s\" was previously declared here.", old->name); hlsl_free_var(var); + free_parse_initializer(&v->initializer); vkd3d_free(v); continue; }
participants (3)
-
Francisco Casas -
Giovanni Mascellani -
Zebediah Figura