[PATCH 1/6] d3dcompiler: Get rid of the superfluous "variable" rule.
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/d3dcompiler_43/hlsl.y | 31 ++++++++++++------------------- 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/dlls/d3dcompiler_43/hlsl.y b/dlls/d3dcompiler_43/hlsl.y index 4ece6695514..ba0fd5ceaff 100644 --- a/dlls/d3dcompiler_43/hlsl.y +++ b/dlls/d3dcompiler_43/hlsl.y @@ -913,7 +913,6 @@ static const struct hlsl_ir_function_decl *get_overloaded_func(struct wine_rb_tr BOOL boolval; char *name; DWORD modifiers; - struct hlsl_ir_var *var; struct hlsl_ir_node *instr; struct list *list; struct parse_function function; @@ -1049,7 +1048,6 @@ static const struct hlsl_ir_function_decl *get_overloaded_func(struct wine_rb_tr %type <list> parameters %type <list> param_list %type <instr> expr -%type <var> variable %type <intval> array %type <list> statement %type <list> statement_list @@ -1910,10 +1908,19 @@ primary_expr: C_FLOAT c->v.value.b[0] = $1; $$ = &c->node; } - | variable + | VAR_IDENTIFIER { - struct hlsl_ir_deref *deref = new_var_deref($1); - if (deref) + struct hlsl_ir_deref *deref; + struct hlsl_ir_var *var; + + if (!(var = get_variable(hlsl_ctx.cur_scope, $1))) + { + hlsl_message("Line %d: variable '%s' not declared\n", + hlsl_ctx.line_no, $1); + set_parse_status(&hlsl_ctx.status, PARSE_ERR); + return 1; + } + if ((deref = new_var_deref(var))) { $$ = &deref->node; set_location(&$$->loc, &@1); @@ -1926,20 +1933,6 @@ primary_expr: C_FLOAT $$ = $2; } -variable: VAR_IDENTIFIER - { - struct hlsl_ir_var *var; - var = get_variable(hlsl_ctx.cur_scope, $1); - if (!var) - { - hlsl_message("Line %d: variable '%s' not declared\n", - hlsl_ctx.line_no, $1); - set_parse_status(&hlsl_ctx.status, PARSE_ERR); - return 1; - } - $$ = var; - } - postfix_expr: primary_expr { $$ = $1; -- 2.20.1
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/d3dcompiler_43/d3dcompiler_private.h | 3 ++- dlls/d3dcompiler_43/hlsl.y | 8 +++++++- dlls/d3dcompiler_43/utils.c | 10 ++++++---- 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/dlls/d3dcompiler_43/d3dcompiler_private.h b/dlls/d3dcompiler_43/d3dcompiler_private.h index e165f21897f..d6db46b4536 100644 --- a/dlls/d3dcompiler_43/d3dcompiler_private.h +++ b/dlls/d3dcompiler_43/d3dcompiler_private.h @@ -952,7 +952,8 @@ struct hlsl_ir_constant struct hlsl_ir_constructor { struct hlsl_ir_node node; - struct list *arguments; + struct hlsl_ir_node *args[16]; + unsigned int args_count; }; struct hlsl_scope diff --git a/dlls/d3dcompiler_43/hlsl.y b/dlls/d3dcompiler_43/hlsl.y index ba0fd5ceaff..0f9819062c7 100644 --- a/dlls/d3dcompiler_43/hlsl.y +++ b/dlls/d3dcompiler_43/hlsl.y @@ -2092,6 +2092,7 @@ postfix_expr: primary_expr | var_modifiers type '(' initializer_expr_list ')' { struct hlsl_ir_constructor *constructor; + struct hlsl_ir_node *instr; TRACE("%s constructor.\n", debug_hlsl_type($2)); if ($1) @@ -2120,7 +2121,12 @@ postfix_expr: primary_expr constructor->node.type = HLSL_IR_CONSTRUCTOR; set_location(&constructor->node.loc, &@3); constructor->node.data_type = $2; - constructor->arguments = $4; + constructor->args_count = 0; + LIST_FOR_EACH_ENTRY(instr, $4, struct hlsl_ir_node, entry) + { + constructor->args[constructor->args_count++] = instr; + } + d3dcompiler_free($4); $$ = &constructor->node; } diff --git a/dlls/d3dcompiler_43/utils.c b/dlls/d3dcompiler_43/utils.c index a521eb65382..742b2623ddb 100644 --- a/dlls/d3dcompiler_43/utils.c +++ b/dlls/d3dcompiler_43/utils.c @@ -2122,12 +2122,12 @@ static void debug_dump_ir_expr(const struct hlsl_ir_expr *expr) static void debug_dump_ir_constructor(const struct hlsl_ir_constructor *constructor) { - struct hlsl_ir_node *arg; + unsigned int i; TRACE("%s (", debug_hlsl_type(constructor->node.data_type)); - LIST_FOR_EACH_ENTRY(arg, constructor->arguments, struct hlsl_ir_node, entry) + for (i = 0; i < constructor->args_count; ++i) { - debug_dump_instr(arg); + debug_dump_instr(constructor->args[i]); TRACE(" "); } TRACE(")"); @@ -2349,7 +2349,9 @@ static void free_ir_swizzle(struct hlsl_ir_swizzle *swizzle) static void free_ir_constructor(struct hlsl_ir_constructor *constructor) { - free_instr_list(constructor->arguments); + unsigned int i; + for (i = 0; i < constructor->args_count; ++i) + free_instr(constructor->args[i]); d3dcompiler_free(constructor); } -- 2.20.1
Signed-off-by: Matteo Bruni <mbruni(a)codeweavers.com>
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/d3dcompiler_43/hlsl.y | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/dlls/d3dcompiler_43/hlsl.y b/dlls/d3dcompiler_43/hlsl.y index 0f9819062c7..c18efcc1350 100644 --- a/dlls/d3dcompiler_43/hlsl.y +++ b/dlls/d3dcompiler_43/hlsl.y @@ -387,19 +387,7 @@ static unsigned int initializer_size(struct list *initializer) { count += components_count_type(node->data_type); } - TRACE("Initializer size = %u\n", count); - return count; -} - -static unsigned int components_count_expr_list(struct list *list) -{ - struct hlsl_ir_node *node; - unsigned int count = 0; - - LIST_FOR_EACH_ENTRY(node, list, struct hlsl_ir_node, entry) - { - count += components_count_type(node->data_type); - } + TRACE("Initializer size = %u.\n", count); return count; } @@ -2109,7 +2097,7 @@ postfix_expr: primary_expr set_parse_status(&hlsl_ctx.status, PARSE_ERR); return -1; } - if ($2->dimx * $2->dimy != components_count_expr_list($4)) + if ($2->dimx * $2->dimy != initializer_size($4)) { hlsl_message("Line %u: wrong number of components in constructor.\n", hlsl_ctx.line_no); -- 2.20.1
Signed-off-by: Matteo Bruni <mbruni(a)codeweavers.com>
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/d3dcompiler_43/tests/hlsl.c | 78 ++++++++------------------------ 1 file changed, 19 insertions(+), 59 deletions(-) diff --git a/dlls/d3dcompiler_43/tests/hlsl.c b/dlls/d3dcompiler_43/tests/hlsl.c index e67e68cc1ec..cef9b4d40cb 100644 --- a/dlls/d3dcompiler_43/tests/hlsl.c +++ b/dlls/d3dcompiler_43/tests/hlsl.c @@ -562,93 +562,53 @@ static void test_trig(IDirect3DDevice9 *device, IDirect3DVertexBuffer9 *quad_geo static void test_fail(IDirect3DDevice9 *device, IDirect3DVertexBuffer9 *qquad_geometry, IDirect3DVertexShader9 *vshader_passthru) { - static const char *undefined_variable_shader = + static const char *tests[] = + { "float4 test(float2 pos: TEXCOORD0) : COLOR\n" "{\n" " return y;\n" - "}"; + "}", - static const char *invalid_swizzle_shader = "float4 test(float2 pos: TEXCOORD0) : COLOR\n" "{\n" " float4 x = float4(0, 0, 0, 0);\n" " x.xzzx = float4(1, 2, 3, 4);\n" " return x;\n" - "}"; + "}", - static const char *invalid_conversion_shader = "float4 test(float2 pos: TEXCOORD0) : COLOR\n" "{\n" " float4 x = pos;\n" " return x;\n" - "}"; + "}", - static const char *invalid_syntax_shader = "float4 test(float2 pos, TEXCOORD0) ; COLOR\n" "{\n" " pos = float4 x;\n" " mul(float4(5, 4, 3, 2), mvp) = x;\n" " return float4;\n" - "}"; + "}", - static const char *invalid_identifiers_shader = "float4 563r(float2 45s: TEXCOORD0) : COLOR\n" "{\n" " float2 x = 45s;\n" " return float4(x.x, x.y, 0, 0);\n" - "}"; + "}", + }; - ID3D10Blob *compiled = NULL, *errors = NULL; + ID3D10Blob *compiled, *errors; + unsigned int i; HRESULT hr; - hr = ppD3DCompile(undefined_variable_shader, strlen(undefined_variable_shader), NULL, NULL, NULL, - "test", "ps_2_0", 0, 0, &compiled, &errors); - ok(hr != D3D_OK, "Pixel shader compilation succeeded on shader with undefined variable\n"); - ok(errors != NULL, "No errors returned for a shader with undefined variables\n"); - ok(compiled == NULL, "A shader blob was returned for a shader with undefined variables\n"); - - ID3D10Blob_Release(errors); - errors = NULL; - - hr = ppD3DCompile(invalid_swizzle_shader, strlen(invalid_swizzle_shader), NULL, NULL, NULL, - "test","ps_2_0", 0, 0, &compiled, &errors); - ok(hr != D3D_OK, "Pixel shader compilation succeeded on shader with an invalid swizzle mask\n"); - ok(errors != NULL, "No errors returned for a shader with an invalid swizzle mask\n"); - ok(compiled == NULL, "A shader blob was returned for a shader with an invalid swizzle mask\n"); - - ID3D10Blob_Release(errors); - errors = NULL; - - hr = ppD3DCompile(invalid_conversion_shader, strlen(invalid_conversion_shader), NULL, NULL, NULL, - "test", "ps_2_0", 0, 0, &compiled, &errors); - ok(hr != D3D_OK, "Pixel shader compilation succeeded on shader with an invalid type " - "conversion\n"); - ok(errors != NULL, "No errors returned for a shader with invalid type conversions\n"); - ok(compiled == NULL, "A shader blob was returned for a shader with invalid type conversions\n"); - - ID3D10Blob_Release(errors); - errors = NULL; - - hr = ppD3DCompile(invalid_syntax_shader, strlen(invalid_syntax_shader), NULL, NULL, NULL, "test", - "ps_2_0", 0, 0, &compiled, &errors); - ok(hr != D3D_OK, "Pixel shader compilation succeeded on shader with blatantly invalid " - "syntax\n"); - ok(errors != NULL, "No errors returned for a shader with invalid syntax\n"); - ok(compiled == NULL, "A shader blob was returned for a shader with invalid syntax\n"); - - ID3D10Blob_Release(errors); - errors = NULL; - - hr = ppD3DCompile(invalid_identifiers_shader, strlen(invalid_identifiers_shader), NULL, NULL, - NULL, "test", "ps_2_0", 0, 0, &compiled, &errors); - ok(hr != D3D_OK, "Pixel shader compilation successful on a shader with invalid variable and " - "function names\n"); - ok(errors != NULL, "No errors returned for a shader with invalid variable and function " - "names\n"); - ok(compiled == NULL, "A shader blob was returned for a shader with invalid variable and " - "function names\n"); - - ID3D10Blob_Release(errors); + for (i = 0; i < ARRAY_SIZE(tests); ++i) + { + compiled = errors = NULL; + hr = ppD3DCompile(tests[i], strlen(tests[i]), NULL, NULL, NULL, "test", "ps_2_0", 0, 0, &compiled, &errors); + ok(hr == E_FAIL, "Got unexpected hr %#x.\n", hr); + ok(!!errors, "Expected non-NULL error blob.\n"); + ok(!compiled, "Expected no compiled shader blob.\n"); + ID3D10Blob_Release(errors); + } } static BOOL load_d3dcompiler(void) -- 2.20.1
Signed-off-by: Matteo Bruni <mbruni(a)codeweavers.com>
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/d3dcompiler_43/tests/hlsl.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/dlls/d3dcompiler_43/tests/hlsl.c b/dlls/d3dcompiler_43/tests/hlsl.c index cef9b4d40cb..9f696eebf61 100644 --- a/dlls/d3dcompiler_43/tests/hlsl.c +++ b/dlls/d3dcompiler_43/tests/hlsl.c @@ -594,6 +594,18 @@ static void test_fail(IDirect3DDevice9 *device, IDirect3DVertexBuffer9 *qquad_ge " float2 x = 45s;\n" " return float4(x.x, x.y, 0, 0);\n" "}", + + "float4 test(float2 pos: TEXCOORD0) : COLOR\n" + "{\n" + " struct { int b,c; } x = {0};\n" + " return y;\n" + "}", + + "float4 test(float2 pos: TEXCOORD0) : COLOR\n" + "{\n" + " struct {} x = {};\n" + " return y;\n" + "}", }; ID3D10Blob *compiled, *errors; -- 2.20.1
Signed-off-by: Matteo Bruni <mbruni(a)codeweavers.com>
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/d3dcompiler_43/d3dcompiler_private.h | 8 +- dlls/d3dcompiler_43/hlsl.y | 117 +++++++++++----------- 2 files changed, 67 insertions(+), 58 deletions(-) diff --git a/dlls/d3dcompiler_43/d3dcompiler_private.h b/dlls/d3dcompiler_43/d3dcompiler_private.h index d6db46b4536..b576c2401b6 100644 --- a/dlls/d3dcompiler_43/d3dcompiler_private.h +++ b/dlls/d3dcompiler_43/d3dcompiler_private.h @@ -980,6 +980,12 @@ struct parse_colon_attribute struct reg_reservation *reg_reservation; }; +struct parse_initializer +{ + struct hlsl_ir_node **args; + unsigned int args_count; +}; + struct parse_variable_def { struct list entry; @@ -989,7 +995,7 @@ struct parse_variable_def unsigned int array_size; const char *semantic; struct reg_reservation *reg_reservation; - struct list *initializer; + struct parse_initializer initializer; }; struct parse_function diff --git a/dlls/d3dcompiler_43/hlsl.y b/dlls/d3dcompiler_43/hlsl.y index c18efcc1350..1dedbcb8a24 100644 --- a/dlls/d3dcompiler_43/hlsl.y +++ b/dlls/d3dcompiler_43/hlsl.y @@ -378,14 +378,13 @@ oom: return NULL; } -static unsigned int initializer_size(struct list *initializer) +static unsigned int initializer_size(const struct parse_initializer *initializer) { - unsigned int count = 0; - struct hlsl_ir_node *node; + unsigned int count = 0, i; - LIST_FOR_EACH_ENTRY(node, initializer, struct hlsl_ir_node, entry) + for (i = 0; i < initializer->args_count; ++i) { - count += components_count_type(node->data_type); + count += components_count_type(initializer->args[i]->data_type); } TRACE("Initializer size = %u.\n", count); return count; @@ -488,28 +487,29 @@ static struct hlsl_ir_swizzle *get_swizzle(struct hlsl_ir_node *value, const cha return NULL; } -static void struct_var_initializer(struct list *list, struct hlsl_ir_var *var, struct list *initializer) +static void struct_var_initializer(struct list *list, struct hlsl_ir_var *var, + struct parse_initializer *initializer) { struct hlsl_type *type = var->node.data_type; - struct hlsl_ir_node *node; struct hlsl_struct_field *field; - struct list *cur_node; struct hlsl_ir_node *assignment; struct hlsl_ir_deref *deref; + unsigned int i = 0; if (initializer_size(initializer) != components_count_type(type)) { hlsl_report_message(var->node.loc.file, var->node.loc.line, var->node.loc.col, HLSL_LEVEL_ERROR, "structure initializer mismatch"); - free_instr_list(initializer); + for (i = 0; i < initializer->args_count; ++i) + free_instr(initializer->args[i]); return; } - cur_node = list_head(initializer); - assert(cur_node); - node = LIST_ENTRY(cur_node, struct hlsl_ir_node, entry); + LIST_FOR_EACH_ENTRY(field, type->e.elements, struct hlsl_struct_field, entry) { - if (!cur_node) + struct hlsl_ir_node *node = initializer->args[i]; + + if (i++ >= initializer->args_count) { d3dcompiler_free(initializer); return; @@ -528,19 +528,12 @@ static void struct_var_initializer(struct list *list, struct hlsl_ir_var *var, s } else FIXME("Initializing with \"mismatched\" fields is not supported yet.\n"); - cur_node = list_next(initializer, cur_node); - node = LIST_ENTRY(cur_node, struct hlsl_ir_node, entry); } /* Free initializer elements in excess. */ - while (cur_node) - { - struct list *next = list_next(initializer, cur_node); - free_instr(node); - cur_node = next; - node = LIST_ENTRY(cur_node, struct hlsl_ir_node, entry); - } - d3dcompiler_free(initializer); + for (; i < initializer->args_count; ++i) + free_instr(initializer->args[i]); + d3dcompiler_free(initializer->args); } static struct list *declare_vars(struct hlsl_type *basic_type, DWORD modifiers, struct list *var_list) @@ -593,7 +586,7 @@ static struct list *declare_vars(struct hlsl_type *basic_type, DWORD modifiers, local = FALSE; } - if (var->modifiers & HLSL_MODIFIER_CONST && !(var->modifiers & HLSL_STORAGE_UNIFORM) && !v->initializer) + if (var->modifiers & HLSL_MODIFIER_CONST && !(var->modifiers & HLSL_STORAGE_UNIFORM) && !v->initializer.args_count) { hlsl_report_message(v->loc.file, v->loc.line, v->loc.col, HLSL_LEVEL_ERROR, "const variable without initializer"); @@ -611,10 +604,9 @@ static struct list *declare_vars(struct hlsl_type *basic_type, DWORD modifiers, } TRACE("Declared variable %s.\n", var->name); - if (v->initializer) + if (v->initializer.args_count) { - unsigned int size = initializer_size(v->initializer); - struct hlsl_ir_node *node; + unsigned int size = initializer_size(&v->initializer), i; TRACE("Variable with initializer.\n"); if (type->type <= HLSL_CLASS_LAST_NUMERIC @@ -624,7 +616,9 @@ static struct list *declare_vars(struct hlsl_type *basic_type, DWORD modifiers, { hlsl_report_message(v->loc.file, v->loc.line, v->loc.col, HLSL_LEVEL_ERROR, "'%s' initializer does not match", v->name); - free_instr_list(v->initializer); + for (i = 0; i < v->initializer.args_count; ++i) + free_instr(v->initializer.args[i]); + d3dcompiler_free(v->initializer.args); d3dcompiler_free(v); continue; } @@ -634,43 +628,51 @@ static struct list *declare_vars(struct hlsl_type *basic_type, DWORD modifiers, { hlsl_report_message(v->loc.file, v->loc.line, v->loc.col, HLSL_LEVEL_ERROR, "'%s' initializer does not match", v->name); - free_instr_list(v->initializer); + for (i = 0; i < v->initializer.args_count; ++i) + free_instr(v->initializer.args[i]); + d3dcompiler_free(v->initializer.args); d3dcompiler_free(v); continue; } if (type->type == HLSL_CLASS_STRUCT) { - struct_var_initializer(statements_list, var, v->initializer); + struct_var_initializer(statements_list, var, &v->initializer); d3dcompiler_free(v); continue; } if (type->type > HLSL_CLASS_LAST_NUMERIC) { FIXME("Initializers for non scalar/struct variables not supported yet.\n"); - free_instr_list(v->initializer); + for (i = 0; i < v->initializer.args_count; ++i) + free_instr(v->initializer.args[i]); + d3dcompiler_free(v->initializer.args); d3dcompiler_free(v); continue; } if (v->array_size > 0) { FIXME("Initializing arrays is not supported yet.\n"); - free_instr_list(v->initializer); + for (i = 0; i < v->initializer.args_count; ++i) + free_instr(v->initializer.args[i]); + d3dcompiler_free(v->initializer.args); d3dcompiler_free(v); continue; } - if (list_count(v->initializer) > 1) + if (v->initializer.args_count > 1) { FIXME("Complex initializers are not supported yet.\n"); - free_instr_list(v->initializer); + for (i = 0; i < v->initializer.args_count; ++i) + free_instr(v->initializer.args[i]); + d3dcompiler_free(v->initializer.args); d3dcompiler_free(v); continue; } - node = LIST_ENTRY(list_head(v->initializer), struct hlsl_ir_node, entry); + assignment = make_assignment(&var->node, ASSIGN_OP_ASSIGN, - BWRITERSP_WRITEMASK_ALL, node); + BWRITERSP_WRITEMASK_ALL, v->initializer.args[0]); + d3dcompiler_free(v->initializer.args); list_add_tail(statements_list, &assignment->entry); - d3dcompiler_free(v->initializer); } d3dcompiler_free(v); } @@ -696,6 +698,7 @@ static struct list *gen_struct_fields(struct hlsl_type *type, DWORD modifiers, s struct parse_variable_def *v, *v_next; struct hlsl_struct_field *field; struct list *list; + unsigned int i; list = d3dcompiler_alloc(sizeof(*list)); if (!list) @@ -718,11 +721,13 @@ static struct list *gen_struct_fields(struct hlsl_type *type, DWORD modifiers, s field->name = v->name; field->modifiers = modifiers; field->semantic = v->semantic; - if (v->initializer) + if (v->initializer.args_count) { hlsl_report_message(v->loc.file, v->loc.line, v->loc.col, HLSL_LEVEL_ERROR, "struct field with an initializer.\n"); - free_instr_list(v->initializer); + for (i = 0; i < v->initializer.args_count; ++i) + free_instr(v->initializer.args[i]); + d3dcompiler_free(v->initializer.args); } list_add_tail(list, &field->entry); d3dcompiler_free(v); @@ -905,6 +910,7 @@ static const struct hlsl_ir_function_decl *get_overloaded_func(struct wine_rb_tr struct list *list; struct parse_function function; struct parse_parameter parameter; + struct parse_initializer initializer; struct parse_variable_def *variable_def; struct parse_if_body if_body; enum parse_unary_op unary_op; @@ -1028,8 +1034,8 @@ static const struct hlsl_ir_function_decl *get_overloaded_func(struct wine_rb_tr %type <type> unnamed_struct_spec %type <list> type_specs %type <variable_def> type_spec -%type <list> complex_initializer -%type <list> initializer_expr_list +%type <initializer> complex_initializer +%type <initializer> initializer_expr_list %type <instr> initializer_expr %type <modifiers> var_modifiers %type <list> field @@ -1675,9 +1681,9 @@ var_modifiers: /* Empty */ complex_initializer: initializer_expr { - $$ = d3dcompiler_alloc(sizeof(*$$)); - list_init($$); - list_add_head($$, &$1->entry); + $$.args_count = 1; + $$.args = d3dcompiler_alloc(sizeof(*$$.args)); + $$.args[0] = $1; } | '{' initializer_expr_list '}' { @@ -1695,14 +1701,15 @@ initializer_expr: assignment_expr initializer_expr_list: initializer_expr { - $$ = d3dcompiler_alloc(sizeof(*$$)); - list_init($$); - list_add_head($$, &$1->entry); + $$.args_count = 1; + $$.args = d3dcompiler_alloc(sizeof(*$$.args)); + $$.args[0] = $1; } | initializer_expr_list ',' initializer_expr { $$ = $1; - list_add_tail($$, &$3->entry); + $$.args = d3dcompiler_realloc($$.args, ($$.args_count + 1) * sizeof(*$$.args)); + $$.args[$$.args_count++] = $3; } boolean: KW_TRUE @@ -2080,7 +2087,6 @@ postfix_expr: primary_expr | var_modifiers type '(' initializer_expr_list ')' { struct hlsl_ir_constructor *constructor; - struct hlsl_ir_node *instr; TRACE("%s constructor.\n", debug_hlsl_type($2)); if ($1) @@ -2097,25 +2103,22 @@ postfix_expr: primary_expr set_parse_status(&hlsl_ctx.status, PARSE_ERR); return -1; } - if ($2->dimx * $2->dimy != initializer_size($4)) + if ($2->dimx * $2->dimy != initializer_size(&$4)) { hlsl_message("Line %u: wrong number of components in constructor.\n", hlsl_ctx.line_no); set_parse_status(&hlsl_ctx.status, PARSE_ERR); return -1; } + assert($4.args_count <= ARRAY_SIZE(constructor->args)); constructor = d3dcompiler_alloc(sizeof(*constructor)); constructor->node.type = HLSL_IR_CONSTRUCTOR; set_location(&constructor->node.loc, &@3); constructor->node.data_type = $2; - constructor->args_count = 0; - LIST_FOR_EACH_ENTRY(instr, $4, struct hlsl_ir_node, entry) - { - constructor->args[constructor->args_count++] = instr; - } - d3dcompiler_free($4); - + constructor->args_count = $4.args_count; + memcpy(constructor->args, $4.args, $4.args_count * sizeof(*$4.args)); + d3dcompiler_free($4.args); $$ = &constructor->node; } -- 2.20.1
On Fri, Aug 9, 2019 at 4:53 PM Zebediah Figura <zfigura(a)codeweavers.com> wrote:
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/d3dcompiler_43/d3dcompiler_private.h | 8 +- dlls/d3dcompiler_43/hlsl.y | 117 +++++++++++----------- 2 files changed, 67 insertions(+), 58 deletions(-)
LIST_FOR_EACH_ENTRY(field, type->e.elements, struct hlsl_struct_field, entry) { - if (!cur_node) + struct hlsl_ir_node *node = initializer->args[i]; + + if (i++ >= initializer->args_count) { d3dcompiler_free(initializer); return; @@ -528,19 +528,12 @@ static void struct_var_initializer(struct list *list, struct hlsl_ir_var *var, s } else FIXME("Initializing with \"mismatched\" fields is not supported yet.\n"); - cur_node = list_next(initializer, cur_node); - node = LIST_ENTRY(cur_node, struct hlsl_ir_node, entry); }
/* Free initializer elements in excess. */ - while (cur_node) - { - struct list *next = list_next(initializer, cur_node); - free_instr(node); - cur_node = next; - node = LIST_ENTRY(cur_node, struct hlsl_ir_node, entry); - } - d3dcompiler_free(initializer); + for (; i < initializer->args_count; ++i) + free_instr(initializer->args[i]); + d3dcompiler_free(initializer->args); }
This is a bit suspicious. In the hunk above there's still a d3dcompiler_free(initializer) but here you're removing it. I think removing it is correct but that probably means the one above should also go.
@@ -624,7 +616,9 @@ static struct list *declare_vars(struct hlsl_type *basic_type, DWORD modifiers, { hlsl_report_message(v->loc.file, v->loc.line, v->loc.col, HLSL_LEVEL_ERROR, "'%s' initializer does not match", v->name); - free_instr_list(v->initializer); + for (i = 0; i < v->initializer.args_count; ++i) + free_instr(v->initializer.args[i]); + d3dcompiler_free(v->initializer.args); d3dcompiler_free(v); continue; }
Not exactly critical, but maybe introducing a small function for freeing the initializer would be nice.
| initializer_expr_list ',' initializer_expr { $$ = $1; - list_add_tail($$, &$3->entry); + $$.args = d3dcompiler_realloc($$.args, ($$.args_count + 1) * sizeof(*$$.args)); + $$.args[$$.args_count++] = $3; }
This isn't really something new with your patch, just to start a conversation on the question: is this a memory allocation we should check? Which are those? I think ideally we should check all of them, although that's quite impractical. For this one specifically I think it's reasonable to check it given that it's a realloc for a (small, but potentially not tiny) array. Then there's the question of handling the failure without crashing while completing parsing. IIRC I tried to do that, in places :/ I'd say don't go to crazy lengths to make this (and other similar spots) entirely correct, but when it's not too much of a nuisance it's nice to have. Fixing those is not a priority of course.
On 8/9/19 1:01 PM, Matteo Bruni wrote:
On Fri, Aug 9, 2019 at 4:53 PM Zebediah Figura <zfigura(a)codeweavers.com> wrote:
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/d3dcompiler_43/d3dcompiler_private.h | 8 +- dlls/d3dcompiler_43/hlsl.y | 117 +++++++++++----------- 2 files changed, 67 insertions(+), 58 deletions(-)
LIST_FOR_EACH_ENTRY(field, type->e.elements, struct hlsl_struct_field, entry) { - if (!cur_node) + struct hlsl_ir_node *node = initializer->args[i]; + + if (i++ >= initializer->args_count) { d3dcompiler_free(initializer); return; @@ -528,19 +528,12 @@ static void struct_var_initializer(struct list *list, struct hlsl_ir_var *var, s } else FIXME("Initializing with \"mismatched\" fields is not supported yet.\n"); - cur_node = list_next(initializer, cur_node); - node = LIST_ENTRY(cur_node, struct hlsl_ir_node, entry); }
/* Free initializer elements in excess. */ - while (cur_node) - { - struct list *next = list_next(initializer, cur_node); - free_instr(node); - cur_node = next; - node = LIST_ENTRY(cur_node, struct hlsl_ir_node, entry); - } - d3dcompiler_free(initializer); + for (; i < initializer->args_count; ++i) + free_instr(initializer->args[i]); + d3dcompiler_free(initializer->args); }
This is a bit suspicious. In the hunk above there's still a d3dcompiler_free(initializer) but here you're removing it. I think removing it is correct but that probably means the one above should also go.
Yep, I just missed the one above. Thanks for catching it.
@@ -624,7 +616,9 @@ static struct list *declare_vars(struct hlsl_type *basic_type, DWORD modifiers, { hlsl_report_message(v->loc.file, v->loc.line, v->loc.col, HLSL_LEVEL_ERROR, "'%s' initializer does not match", v->name); - free_instr_list(v->initializer); + for (i = 0; i < v->initializer.args_count; ++i) + free_instr(v->initializer.args[i]); + d3dcompiler_free(v->initializer.args); d3dcompiler_free(v); continue; }
Not exactly critical, but maybe introducing a small function for freeing the initializer would be nice.
| initializer_expr_list ',' initializer_expr { $$ = $1; - list_add_tail($$, &$3->entry); + $$.args = d3dcompiler_realloc($$.args, ($$.args_count + 1) * sizeof(*$$.args)); + $$.args[$$.args_count++] = $3; }
This isn't really something new with your patch, just to start a conversation on the question: is this a memory allocation we should check? Which are those?
I think ideally we should check all of them, although that's quite impractical. For this one specifically I think it's reasonable to check it given that it's a realloc for a (small, but potentially not tiny) array. Then there's the question of handling the failure without crashing while completing parsing. IIRC I tried to do that, in places :/ I'd say don't go to crazy lengths to make this (and other similar spots) entirely correct, but when it's not too much of a nuisance it's nice to have. Fixing those is not a priority of course.
Yeah, that's fair; I just noticed there were places that did both and immediately went with the easier option. But it's not particularly difficult to watch for allocation errors, so I'll do that where practical.
participants (3)
-
Matteo Bruni -
Matteo Bruni -
Zebediah Figura