[PATCH vkd3d 01/12] vkd3d-shader/hlsl: Store the struct fields as an array.

[Francisco Casas]

From: Zebediah Figura

Signed-off-by: Francisco Casas 
---
 libs/vkd3d-shader/hlsl.c         | 123 +++++++++++++++----------------
 libs/vkd3d-shader/hlsl.h         |  10 ++-
 libs/vkd3d-shader/hlsl.y         |  99 ++++++++++++++-----------
 libs/vkd3d-shader/hlsl_codegen.c |  18 +++--
 libs/vkd3d-shader/hlsl_sm1.c     |  13 +++-
 libs/vkd3d-shader/hlsl_sm4.c     |  13 +++-
 6 files changed, 150 insertions(+), 126 deletions(-)

diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c
index 7239b183..b349eb15 100644
--- a/libs/vkd3d-shader/hlsl.c
+++ b/libs/vkd3d-shader/hlsl.c
@@ -175,13 +175,14 @@ static void hlsl_type_calculate_reg_size(struct hlsl_ctx *ctx, struct hlsl_type

case HLSL_CLASS_STRUCT:
         {
-            struct hlsl_struct_field *field;
+            unsigned int i;

type->dimx = 0;
             type->reg_size = 0;

-            LIST_FOR_EACH_ENTRY(field, type->e.elements, struct hlsl_struct_field, entry)
+            for (i = 0; i < type->e.record.field_count; ++i)
             {
+                struct hlsl_struct_field *field = &type->e.record.fields[i];
                 unsigned int field_size = field->type->reg_size;

assert(field_size);
@@ -282,10 +283,12 @@ unsigned int hlsl_compute_component_offset(struct hlsl_ctx *ctx, struct hlsl_typ
         case HLSL_CLASS_STRUCT:
         {
             struct hlsl_struct_field *field;
+            unsigned int elem_comp_count, i;

-            LIST_FOR_EACH_ENTRY(field, type->e.elements, struct hlsl_struct_field, entry)
+            for (i = 0; i < type->e.record.field_count; ++i)
             {
-                unsigned int elem_comp_count = hlsl_type_component_count(field->type);
+                field = &type->e.record.fields[i];
+                elem_comp_count = hlsl_type_component_count(field->type);

if (idx < elem_comp_count)
                 {
@@ -331,7 +334,8 @@ struct hlsl_type *hlsl_new_array_type(struct hlsl_ctx *ctx, struct hlsl_type *ba
     return type;
 }

-struct hlsl_type *hlsl_new_struct_type(struct hlsl_ctx *ctx, const char *name, struct list *fields)
+struct hlsl_type *hlsl_new_struct_type(struct hlsl_ctx *ctx, const char *name,
+        struct hlsl_struct_field *fields, size_t field_count)
 {
     struct hlsl_type *type;

@@ -341,7 +345,8 @@ struct hlsl_type *hlsl_new_struct_type(struct hlsl_ctx *ctx, const char *name, s
     type->base_type = HLSL_TYPE_VOID;
     type->name = name;
     type->dimy = 1;
-    type->e.elements = fields;
+    type->e.record.fields = fields;
+    type->e.record.field_count = field_count;
     hlsl_type_calculate_reg_size(ctx, type);

list_add_tail(&ctx->types, &type->entry);
@@ -401,8 +406,7 @@ struct hlsl_ir_function_decl *hlsl_get_func_decl(struct hlsl_ctx *ctx, const cha

unsigned int hlsl_type_component_count(struct hlsl_type *type)
 {
-    struct hlsl_struct_field *field;
-    unsigned int count = 0;
+    unsigned int count = 0, i;

if (type->type <= HLSL_CLASS_LAST_NUMERIC)
     {
@@ -418,10 +422,8 @@ unsigned int hlsl_type_component_count(struct hlsl_type *type)
         return 0;
     }

-    LIST_FOR_EACH_ENTRY(field, type->e.elements, struct hlsl_struct_field, entry)
-    {
-        count += hlsl_type_component_count(field->type);
-    }
+    for (i = 0; i < type->e.record.field_count; ++i)
+        count += hlsl_type_component_count(type->e.record.fields[i].type);
     return count;
 }

@@ -451,24 +453,22 @@ bool hlsl_types_are_equal(const struct hlsl_type *t1, const struct hlsl_type *t2
         return false;
     if (t1->type == HLSL_CLASS_STRUCT)
     {
-        struct list *t1cur, *t2cur;
-        struct hlsl_struct_field *t1field, *t2field;
+        size_t i;
+
+        if (t1->e.record.field_count != t2->e.record.field_count)
+            return false;

-        t1cur = list_head(t1->e.elements);
-        t2cur = list_head(t2->e.elements);
-        while (t1cur && t2cur)
+        for (i = 0; i < t1->e.record.field_count; ++i)
         {
-            t1field = LIST_ENTRY(t1cur, struct hlsl_struct_field, entry);
-            t2field = LIST_ENTRY(t2cur, struct hlsl_struct_field, entry);
-            if (!hlsl_types_are_equal(t1field->type, t2field->type))
+            const struct hlsl_struct_field *field1 = &t1->e.record.fields[i];
+            const struct hlsl_struct_field *field2 = &t2->e.record.fields[i];
+
+            if (!hlsl_types_are_equal(field1->type, field2->type))
                 return false;
-            if (strcmp(t1field->name, t2field->name))
+
+            if (strcmp(field1->name, field2->name))
                 return false;
-            t1cur = list_next(t1->e.elements, t1cur);
-            t2cur = list_next(t2->e.elements, t2cur);
         }
-        if (t1cur != t2cur)
-            return false;
     }
     if (t1->type == HLSL_CLASS_ARRAY)
         return t1->e.array.elements_count == t2->e.array.elements_count
@@ -480,7 +480,6 @@ bool hlsl_types_are_equal(const struct hlsl_type *t1, const struct hlsl_type *t2
 struct hlsl_type *hlsl_type_clone(struct hlsl_ctx *ctx, struct hlsl_type *old,
         unsigned int default_majority, unsigned int modifiers)
 {
-    struct hlsl_struct_field *old_field, *field;
     struct hlsl_type *type;

if (!(type = hlsl_alloc(ctx, sizeof(*type))))
@@ -512,37 +511,30 @@ struct hlsl_type *hlsl_type_clone(struct hlsl_ctx *ctx, struct hlsl_type *old,

case HLSL_CLASS_STRUCT:
         {
-            if (!(type->e.elements = hlsl_alloc(ctx, sizeof(*type->e.elements))))
+            size_t field_count = old->e.record.field_count, i;
+
+            type->e.record.field_count = field_count;
+
+            if (!(type->e.record.fields = hlsl_alloc(ctx, field_count * sizeof(*type->e.record.fields))))
             {
                 vkd3d_free((void *)type->name);
                 vkd3d_free(type);
                 return NULL;
             }
-            list_init(type->e.elements);
-            LIST_FOR_EACH_ENTRY(old_field, old->e.elements, struct hlsl_struct_field, entry)
+
+            for (i = 0; i < field_count; ++i)
             {
-                if (!(field = hlsl_alloc(ctx, sizeof(*field))))
-                {
-                    LIST_FOR_EACH_ENTRY_SAFE(field, old_field, type->e.elements, struct hlsl_struct_field, entry)
-                    {
-                        vkd3d_free((void *)field->semantic.name);
-                        vkd3d_free((void *)field->name);
-                        vkd3d_free(field);
-                    }
-                    vkd3d_free(type->e.elements);
-                    vkd3d_free((void *)type->name);
-                    vkd3d_free(type);
-                    return NULL;
-                }
-                field->loc = old_field->loc;
-                field->type = hlsl_type_clone(ctx, old_field->type, default_majority, modifiers);
-                field->name = hlsl_strdup(ctx, old_field->name);
-                if (old_field->semantic.name)
+                const struct hlsl_struct_field *src_field = &old->e.record.fields[i];
+                struct hlsl_struct_field *dst_field = &type->e.record.fields[i];
+
+                dst_field->loc = src_field->loc;
+                dst_field->type = hlsl_type_clone(ctx, src_field->type, default_majority, modifiers);
+                dst_field->name = hlsl_strdup(ctx, src_field->name);
+                if (src_field->semantic.name)
                 {
-                    field->semantic.name = hlsl_strdup(ctx, old_field->semantic.name);
-                    field->semantic.index = old_field->semantic.index;
+                    dst_field->semantic.name = hlsl_strdup(ctx, src_field->semantic.name);
+                    dst_field->semantic.index = src_field->semantic.index;
                 }
-                list_add_tail(type->e.elements, &field->entry);
             }
             break;
         }
@@ -911,24 +903,22 @@ static int compare_param_hlsl_types(const struct hlsl_type *t1, const struct hls
         return r;
     if (t1->type == HLSL_CLASS_STRUCT)
     {
-        struct list *t1cur, *t2cur;
-        struct hlsl_struct_field *t1field, *t2field;
+        size_t i;

-        t1cur = list_head(t1->e.elements);
-        t2cur = list_head(t2->e.elements);
-        while (t1cur && t2cur)
+        if (t1->e.record.field_count != t2->e.record.field_count)
+            return t1->e.record.field_count - t2->e.record.field_count;
+
+        for (i = 0; i < t1->e.record.field_count; ++i)
         {
-            t1field = LIST_ENTRY(t1cur, struct hlsl_struct_field, entry);
-            t2field = LIST_ENTRY(t2cur, struct hlsl_struct_field, entry);
-            if ((r = compare_param_hlsl_types(t1field->type, t2field->type)))
+            const struct hlsl_struct_field *field1 = &t1->e.record.fields[i];
+            const struct hlsl_struct_field *field2 = &t2->e.record.fields[i];
+
+            if ((r = compare_param_hlsl_types(field1->type, field2->type)))
                 return r;
-            if ((r = strcmp(t1field->name, t2field->name)))
+
+            if ((r = strcmp(field1->name, field2->name)))
                 return r;
-            t1cur = list_next(t1->e.elements, t1cur);
-            t2cur = list_next(t2->e.elements, t2cur);
         }
-        if (t1cur != t2cur)
-            return t1cur ? 1 : -1;
         return 0;
     }
     if (t1->type == HLSL_CLASS_ARRAY)
@@ -1519,17 +1509,20 @@ void hlsl_replace_node(struct hlsl_ir_node *old, struct hlsl_ir_node *new)

void hlsl_free_type(struct hlsl_type *type)
 {
-    struct hlsl_struct_field *field, *next_field;
+    struct hlsl_struct_field *field;
+    size_t i;

vkd3d_free((void *)type->name);
     if (type->type == HLSL_CLASS_STRUCT)
     {
-        LIST_FOR_EACH_ENTRY_SAFE(field, next_field, type->e.elements, struct hlsl_struct_field, entry)
+        for (i = 0; i < type->e.record.field_count; ++i)
         {
+            field = &type->e.record.fields[i];
+
             vkd3d_free((void *)field->name);
             vkd3d_free((void *)field->semantic.name);
-            vkd3d_free(field);
         }
+        vkd3d_free((void *)type->e.record.fields);
     }
     vkd3d_free(type);
 }
diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h
index 3c1dd08d..ce1f69df 100644
--- a/libs/vkd3d-shader/hlsl.h
+++ b/libs/vkd3d-shader/hlsl.h
@@ -126,7 +126,11 @@ struct hlsl_type
     unsigned int dimy;
     union
     {
-        struct list *elements;
+        struct
+        {
+            struct hlsl_struct_field *fields;
+            size_t field_count;
+        } record;
         struct
         {
             struct hlsl_type *type;
@@ -147,7 +151,6 @@ struct hlsl_semantic

struct hlsl_struct_field
 {
-    struct list entry;
     struct vkd3d_shader_location loc;
     struct hlsl_type *type;
     const char *name;
@@ -752,7 +755,8 @@ struct hlsl_ir_resource_load *hlsl_new_resource_load(struct hlsl_ctx *ctx, struc
 struct hlsl_ir_store *hlsl_new_simple_store(struct hlsl_ctx *ctx, struct hlsl_ir_var *lhs, struct hlsl_ir_node *rhs);
 struct hlsl_ir_store *hlsl_new_store(struct hlsl_ctx *ctx, struct hlsl_ir_var *var, struct hlsl_ir_node *offset,
         struct hlsl_ir_node *rhs, unsigned int writemask, struct vkd3d_shader_location loc);
-struct hlsl_type *hlsl_new_struct_type(struct hlsl_ctx *ctx, const char *name, struct list *fields);
+struct hlsl_type *hlsl_new_struct_type(struct hlsl_ctx *ctx, const char *name,
+        struct hlsl_struct_field *fields, size_t field_count);
 struct hlsl_ir_swizzle *hlsl_new_swizzle(struct hlsl_ctx *ctx, DWORD s, unsigned int components,
         struct hlsl_ir_node *val, const struct vkd3d_shader_location *loc);
 struct hlsl_ir_var *hlsl_new_synthetic_var(struct hlsl_ctx *ctx, const char *name, struct hlsl_type *type,
diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y
index 09a01c6d..f293b82e 100644
--- a/libs/vkd3d-shader/hlsl.y
+++ b/libs/vkd3d-shader/hlsl.y
@@ -28,6 +28,12 @@

#define HLSL_YYLTYPE struct vkd3d_shader_location

+struct parse_fields
+{
+    struct hlsl_struct_field *fields;
+    size_t count, capacity;
+};
+
 struct parse_parameter
 {
     struct hlsl_type *type;
@@ -799,14 +805,15 @@ static bool add_array_load(struct hlsl_ctx *ctx, struct list *instrs, struct hls
     return !!add_load(ctx, instrs, array, index, data_type, loc);
 }

-static struct hlsl_struct_field *get_struct_field(struct list *fields, const char *name)
+static const struct hlsl_struct_field *get_struct_field(const struct hlsl_struct_field *fields,
+        size_t count, const char *name)
 {
-    struct hlsl_struct_field *f;
+    size_t i;

-    LIST_FOR_EACH_ENTRY(f, fields, struct hlsl_struct_field, entry)
+    for (i = 0; i < count; ++i)
     {
-        if (!strcmp(f->name, name))
-            return f;
+        if (!strcmp(fields[i].name, name))
+            return &fields[i];
     }
     return NULL;
 }
@@ -857,31 +864,28 @@ static void free_parse_variable_def(struct parse_variable_def *v)
     vkd3d_free(v);
 }

-static struct list *gen_struct_fields(struct hlsl_ctx *ctx,
-        struct hlsl_type *type, unsigned int modifiers, struct list *fields)
+static bool gen_struct_fields(struct hlsl_ctx *ctx, struct parse_fields *fields,
+        struct hlsl_type *type, unsigned int modifiers, struct list *defs)
 {
     struct parse_variable_def *v, *v_next;
-    struct hlsl_struct_field *field;
-    struct list *list;
+    size_t i = 0;

if (type->type == HLSL_CLASS_MATRIX)
         assert(type->modifiers & HLSL_MODIFIERS_MAJORITY_MASK);

-    if (!(list = make_empty_list(ctx)))
-        return NULL;
-    LIST_FOR_EACH_ENTRY_SAFE(v, v_next, fields, struct parse_variable_def, entry)
-    {
-        unsigned int i;
+    memset(fields, 0, sizeof(*fields));
+    fields->count = list_count(defs);
+    if (!hlsl_array_reserve(ctx, (void **)&fields->fields, &fields->capacity, fields->count, sizeof(*fields->fields)))
+        return false;

-        if (!(field = hlsl_alloc(ctx, sizeof(*field))))
-        {
-            free_parse_variable_def(v);
-            continue;
-        }
+    LIST_FOR_EACH_ENTRY_SAFE(v, v_next, defs, struct parse_variable_def, entry)
+    {
+        struct hlsl_struct_field *field = &fields->fields[i++];
+        unsigned int j;

field->type = type;
-        for (i = 0; i < v->arrays.count; ++i)
-            field->type = hlsl_new_array_type(ctx, field->type, v->arrays.sizes[i]);
+        for (j = 0; j < v->arrays.count; ++j)
+            field->type = hlsl_new_array_type(ctx, field->type, v->arrays.sizes[j]);
         vkd3d_free(v->arrays.sizes);
         field->loc = v->loc;
         field->name = v->name;
@@ -892,11 +896,10 @@ static struct list *gen_struct_fields(struct hlsl_ctx *ctx,
             hlsl_error(ctx, &v->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_SYNTAX, "Illegal initializer on a struct field.");
             free_parse_initializer(&v->initializer);
         }
-        list_add_tail(list, &field->entry);
         vkd3d_free(v);
     }
-    vkd3d_free(fields);
-    return list;
+    vkd3d_free(defs);
+    return true;
 }

static bool add_typedef(struct hlsl_ctx *ctx, DWORD modifiers, struct hlsl_type *orig_type, struct list *list)
@@ -2681,6 +2684,7 @@ static bool add_method_call(struct hlsl_ctx *ctx, struct list *instrs, struct hl
     DWORD modifiers;
     struct hlsl_ir_node *instr;
     struct list *list;
+    struct parse_fields fields;
     struct parse_function function;
     struct parse_parameter parameter;
     struct parse_initializer initializer;
@@ -2809,8 +2813,6 @@ static bool add_method_call(struct hlsl_ctx *ctx, struct list *instrs, struct hl
 %type <list> equality_expr
 %type <list> expr
 %type <list> expr_statement
-%type <list> field
-%type <list> fields_list
 %type <list> initializer_expr
 %type <list> jump_statement
 %type <list> logicand_expr
@@ -2847,6 +2849,9 @@ static bool add_method_call(struct hlsl_ctx *ctx, struct list *instrs, struct hl

%type <colon_attribute> colon_attribute

+%type <fields> field
+%type <fields> fields_list
+
 %type <function> func_declaration
 %type <function> func_prototype

@@ -3002,7 +3007,7 @@ named_struct_spec:
         {
             bool ret;

-            $$ = hlsl_new_struct_type(ctx, $2, $4);
+            $$ = hlsl_new_struct_type(ctx, $2, $4.fields, $4.count);

if (hlsl_get_var(ctx->cur_scope, $2))
             {
@@ -3021,7 +3026,7 @@ named_struct_spec:
 unnamed_struct_spec:
       KW_STRUCT '{' fields_list '}'
         {
-            $$ = hlsl_new_struct_type(ctx, NULL, $3);
+            $$ = hlsl_new_struct_type(ctx, NULL, $3.fields, $3.count);
         }

any_identifier:
@@ -3032,30 +3037,35 @@ any_identifier:
 fields_list:
       %empty
         {
-            if (!($$ = make_empty_list(ctx)))
-                YYABORT;
+            $$.fields = NULL;
+            $$.count = 0;
+            $$.capacity = 0;
         }
     | fields_list field
         {
-            struct hlsl_struct_field *field, *next, *existing;
+            size_t i;

-            $$ = $1;
-            LIST_FOR_EACH_ENTRY_SAFE(field, next, $2, struct hlsl_struct_field, entry)
+            for (i = 0; i < $2.count; ++i)
             {
-                if ((existing = get_struct_field($$, field->name)))
+                const struct hlsl_struct_field *field = &$2.fields[i];
+                const struct hlsl_struct_field *existing;
+
+                if ((existing = get_struct_field($1.fields, $1.count, field->name)))
                 {
-                    hlsl_error(ctx, &@2, VKD3D_SHADER_ERROR_HLSL_REDEFINED,
+                    hlsl_error(ctx, &field->loc, VKD3D_SHADER_ERROR_HLSL_REDEFINED,
                             "Field "%s" is already defined.", field->name);
                     hlsl_note(ctx, &existing->loc, VKD3D_SHADER_LOG_ERROR,
                             "'%s' was previously defined here.", field->name);
-                    vkd3d_free(field);
-                }
-                else
-                {
-                    list_add_tail($$, &field->entry);
                 }
             }
-            vkd3d_free($2);
+
+            if (!hlsl_array_reserve(ctx, (void **)&$1.fields, &$1.capacity, $1.count + $2.count, sizeof(*$1.fields)))
+                YYABORT;
+            memcpy($1.fields + $1.count, $2.fields, $2.count * sizeof(*$2.fields));
+            $1.count += $2.count;
+            vkd3d_free($2.fields);
+
+            $$ = $1;
         }

field_type:
@@ -3079,7 +3089,8 @@ field:
                             "Modifiers '%s' are not allowed on struct fields.", string->buffer);
                 hlsl_release_string_buffer(ctx, string);
             }
-            $$ = gen_struct_fields(ctx, type, modifiers, $3);
+            if (!gen_struct_fields(ctx, &$$, type, modifiers, $3))
+                YYABORT;
         }

func_declaration:
@@ -3936,9 +3947,9 @@ postfix_expr:
             if (node->data_type->type == HLSL_CLASS_STRUCT)
             {
                 struct hlsl_type *type = node->data_type;
-                struct hlsl_struct_field *field;
+                const struct hlsl_struct_field *field;

-                if (!(field = get_struct_field(type->e.elements, $3)))
+                if (!(field = get_struct_field(type->e.record.fields, type->e.record.field_count, $3)))
                 {
                     hlsl_error(ctx, &@3, VKD3D_SHADER_ERROR_HLSL_NOT_DEFINED, "Field "%s" is not defined.", $3);
                     YYABORT;
diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c
index bc7f0a62..484b415d 100644
--- a/libs/vkd3d-shader/hlsl_codegen.c
+++ b/libs/vkd3d-shader/hlsl_codegen.c
@@ -137,10 +137,12 @@ static void prepend_input_copy(struct hlsl_ctx *ctx, struct list *instrs, struct
 static void prepend_input_struct_copy(struct hlsl_ctx *ctx, struct list *instrs, struct hlsl_ir_var *var,
         struct hlsl_type *type, unsigned int field_offset)
 {
-    struct hlsl_struct_field *field;
+    size_t i;

-    LIST_FOR_EACH_ENTRY(field, type->e.elements, struct hlsl_struct_field, entry)
+    for (i = 0; i < type->e.record.field_count; ++i)
     {
+        const struct hlsl_struct_field *field = &type->e.record.fields[i];
+
         if (field->type->type == HLSL_CLASS_STRUCT)
             prepend_input_struct_copy(ctx, instrs, var, field->type, field_offset + field->reg_offset);
         else if (field->semantic.name)
@@ -225,10 +227,12 @@ static void append_output_copy(struct hlsl_ctx *ctx, struct list *instrs, struct
 static void append_output_struct_copy(struct hlsl_ctx *ctx, struct list *instrs, struct hlsl_ir_var *var,
         struct hlsl_type *type, unsigned int field_offset)
 {
-    struct hlsl_struct_field *field;
+    size_t i;

-    LIST_FOR_EACH_ENTRY(field, type->e.elements, struct hlsl_struct_field, entry)
+    for (i = 0; i < type->e.record.field_count; ++i)
     {
+        const struct hlsl_struct_field *field = &type->e.record.fields[i];
+
         if (field->type->type == HLSL_CLASS_STRUCT)
             append_output_struct_copy(ctx, instrs, var, field->type, field_offset + field->reg_offset);
         else if (field->semantic.name)
@@ -859,10 +863,10 @@ static bool split_array_copies(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr,

static bool split_struct_copies(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, void *context)
 {
-    const struct hlsl_struct_field *field;
     const struct hlsl_ir_node *rhs;
     const struct hlsl_type *type;
     struct hlsl_ir_store *store;
+    size_t i;

if (instr->type != HLSL_IR_STORE)
         return false;
@@ -879,8 +883,10 @@ static bool split_struct_copies(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr
         return false;
     }

-    LIST_FOR_EACH_ENTRY(field, type->e.elements, struct hlsl_struct_field, entry)
+    for (i = 0; i < type->e.record.field_count; ++i)
     {
+        const struct hlsl_struct_field *field = &type->e.record.fields[i];
+
         if (!split_copy(ctx, store, hlsl_ir_load(rhs), field->reg_offset, field->type))
             return false;
     }
diff --git a/libs/vkd3d-shader/hlsl_sm1.c b/libs/vkd3d-shader/hlsl_sm1.c
index 0cdd3917..06313a42 100644
--- a/libs/vkd3d-shader/hlsl_sm1.c
+++ b/libs/vkd3d-shader/hlsl_sm1.c
@@ -243,28 +243,33 @@ static void write_sm1_type(struct vkd3d_bytecode_buffer *buffer, struct hlsl_typ
 {
     const struct hlsl_type *array_type = get_array_type(type);
     unsigned int array_size = get_array_size(type);
-    struct hlsl_struct_field *field;
     unsigned int field_count = 0;
     size_t fields_offset = 0;
+    size_t i;

if (type->bytecode_offset)
         return;

if (array_type->type == HLSL_CLASS_STRUCT)
     {
-        LIST_FOR_EACH_ENTRY(field, array_type->e.elements, struct hlsl_struct_field, entry)
+        field_count = array_type->e.record.field_count;
+
+        for (i = 0; i < field_count; ++i)
         {
+            struct hlsl_struct_field *field = &array_type->e.record.fields[i];
+
             field->name_bytecode_offset = put_string(buffer, field->name);
             write_sm1_type(buffer, field->type, ctab_start);
         }

fields_offset = bytecode_get_size(buffer) - ctab_start;

-        LIST_FOR_EACH_ENTRY(field, array_type->e.elements, struct hlsl_struct_field, entry)
+        for (i = 0; i < field_count; ++i)
         {
+            struct hlsl_struct_field *field = &array_type->e.record.fields[i];
+
             put_u32(buffer, field->name_bytecode_offset - ctab_start);
             put_u32(buffer, field->type->bytecode_offset - ctab_start);
-            ++field_count;
         }
     }

diff --git a/libs/vkd3d-shader/hlsl_sm4.c b/libs/vkd3d-shader/hlsl_sm4.c
index 79027169..7758555b 100644
--- a/libs/vkd3d-shader/hlsl_sm4.c
+++ b/libs/vkd3d-shader/hlsl_sm4.c
@@ -355,7 +355,7 @@ static void write_sm4_type(struct hlsl_ctx *ctx, struct vkd3d_bytecode_buffer *b
     const struct hlsl_profile_info *profile = ctx->profile;
     unsigned int field_count = 0, array_size = 0;
     size_t fields_offset = 0, name_offset = 0;
-    struct hlsl_struct_field *field;
+    size_t i;

if (type->bytecode_offset)
         return;
@@ -368,20 +368,25 @@ static void write_sm4_type(struct hlsl_ctx *ctx, struct vkd3d_bytecode_buffer *b

if (array_type->type == HLSL_CLASS_STRUCT)
     {
-        LIST_FOR_EACH_ENTRY(field, array_type->e.elements, struct hlsl_struct_field, entry)
+        field_count = array_type->e.record.field_count;
+
+        for (i = 0; i < field_count; ++i)
         {
+            struct hlsl_struct_field *field = &array_type->e.record.fields[i];
+
             field->name_bytecode_offset = put_string(buffer, field->name);
             write_sm4_type(ctx, buffer, field->type);
         }

fields_offset = bytecode_get_size(buffer);

-        LIST_FOR_EACH_ENTRY(field, array_type->e.elements, struct hlsl_struct_field, entry)
+        for (i = 0; i < field_count; ++i)
         {
+            struct hlsl_struct_field *field = &array_type->e.record.fields[i];
+
             put_u32(buffer, field->name_bytecode_offset);
             put_u32(buffer, field->type->bytecode_offset);
             put_u32(buffer, field->reg_offset);
-            ++field_count;
         }
     }

[Giovanni Mascellani]

Hi,

Il 01/07/22 23:24, Francisco Casas ha scritto:
> @@ -512,37 +511,30 @@ struct hlsl_type *hlsl_type_clone(struct hlsl_ctx *ctx, struct hlsl_type *old,
>       case HLSL_CLASS_STRUCT:
>       {
> -
>        if (!(type->e.elements = hlsl_alloc(ctx, sizeof(*type->e.elements))))
> -
>        size_t field_count = old->e.record.field_count, i;
> -
> -
>        type->e.record.field_count = field_count;
> -
> -
>        if (!(type->e.record.fields = hlsl_alloc(ctx, field_count * sizeof(*type->e.record.fields))))
>         {
>             vkd3d_free((void *)type->name);
>             vkd3d_free(type);
>             return NULL;
>         }
> -
>        list_init(type->e.elements);
> -
>        LIST_FOR_EACH_ENTRY(old_field, old->e.elements, struct hlsl_struct_field, entry)
> -
> -
>        for (i = 0; i < field_count; ++i)
>         {
> -
>            if (!(field = hlsl_alloc(ctx, sizeof(*field))))
> -
>            {
> -
>                LIST_FOR_EACH_ENTRY_SAFE(field, old_field, type->e.elements, struct hlsl_struct_field, entry)
> -
>                {
> -
>                    vkd3d_free((void *)field->semantic.name);
> -
>                    vkd3d_free((void *)field->name);
> -
>                    vkd3d_free(field);
> -
>                }
> -
>                vkd3d_free(type->e.elements);
> -
>                vkd3d_free((void *)type->name);
> -
>                vkd3d_free(type);
> -
>                return NULL;
> -
>            }
> -
>            field->loc = old_field->loc;
> -
>            field->type = hlsl_type_clone(ctx, old_field->type, default_majority, modifiers);
> -
>            field->name = hlsl_strdup(ctx, old_field->name);
> -
>            if (old_field->semantic.name)
> -
>            const struct hlsl_struct_field *src_field = &old->e.record.fields[i];
> -
>            struct hlsl_struct_field *dst_field = &type->e.record.fields[i];
> -
> -
>            dst_field->loc = src_field->loc;
> -
>            dst_field->type = hlsl_type_clone(ctx, src_field->type, default_majority, modifiers);

This is happening also before the patch and in the array branch, but 
shouldn't we detect if hlsl_type_clone() returns NULL and handle the 
error? It's true that the memory error is set anyway, so the compilation 
will eventually fail, but in the meantime dst_file->type is NULL, and 
some other code ran during parsing could trip on that.

Giovanni.

[Francisco Casas]

On 05-07-22 05:22, Giovanni Mascellani wrote:
> > -
> >
> > +                dst_field->loc = src_field->loc;
> > +                dst_field->type = hlsl_type_clone(ctx, 
> > src_field->type, default_majority, modifiers);
>
> This is happening also before the patch and in the array branch, but 
> shouldn't we detect if hlsl_type_clone() returns NULL and handle the 
> error? It's true that the memory error is set anyway, so the compilation 
> will eventually fail, but in the meantime dst_file->type is NULL, and 
> some other code ran during parsing could trip on that.
>
> Giovanni.

Hmm, yes, looking at the implementation of hlsl_type_clone() it makes 
sense to handle the possibility of recursive calls returning NULL.

Given that we also have to fix the HLSL_CLASS_ARRAY branch, I will 
introduce a little new patch to fix these problems; just after this one.

Best regards,
Francisco.

[Zebediah Figura]

On 7/5/22 04:22, Giovanni Mascellani wrote:
> Hi,
>
> Il 01/07/22 23:24, Francisco Casas ha scritto:
> > @@ -512,37 +511,30 @@ struct hlsl_type *hlsl_type_clone(struct 
> > hlsl_ctx *ctx, struct hlsl_type *old,
> >           case HLSL_CLASS_STRUCT:
> >           {
> > -            if (!(type->e.elements = hlsl_alloc(ctx, 
> > sizeof(*type->e.elements))))
> > +            size_t field_count = old->e.record.field_count, i;
> > -
> >
> > +            type->e.record.field_count = field_count;
> > -
> >
> > +            if (!(type->e.record.fields = hlsl_alloc(ctx, field_count
> > - sizeof(*type->e.record.fields))))
> >
> > {
> >                   vkd3d_free((void *)type->name);
> >                   vkd3d_free(type);
> >                   return NULL;
> >               }
> > -            list_init(type->e.elements);
> > -            LIST_FOR_EACH_ENTRY(old_field, old->e.elements, struct 
> > hlsl_struct_field, entry)
> > -
> >
> > +            for (i = 0; i < field_count; ++i)
> >               {
> > -                if (!(field = hlsl_alloc(ctx, sizeof(*field))))
> > -                {
> > -                    LIST_FOR_EACH_ENTRY_SAFE(field, old_field, 
> > type->e.elements, struct hlsl_struct_field, entry)
> > -                    {
> > -                        vkd3d_free((void *)field->semantic.name);
> > -                        vkd3d_free((void *)field->name);
> > -                        vkd3d_free(field);
> > -                    }
> > -                    vkd3d_free(type->e.elements);
> > -                    vkd3d_free((void *)type->name);
> > -                    vkd3d_free(type);
> > -                    return NULL;
> > -                }
> > -                field->loc = old_field->loc;
> > -                field->type = hlsl_type_clone(ctx, old_field->type, 
> > default_majority, modifiers);
> > -                field->name = hlsl_strdup(ctx, old_field->name);
> > -                if (old_field->semantic.name)
> > +                const struct hlsl_struct_field *src_field = 
> > &old->e.record.fields[i];
> > +                struct hlsl_struct_field *dst_field = 
> > &type->e.record.fields[i];
> > -
> >
> > +                dst_field->loc = src_field->loc;
> > +                dst_field->type = hlsl_type_clone(ctx, 
> > src_field->type, default_majority, modifiers);
>
> This is happening also before the patch and in the array branch, but 
> shouldn't we detect if hlsl_type_clone() returns NULL and handle the 
> error? It's true that the memory error is set anyway, so the compilation 
> will eventually fail, but in the meantime dst_file->type is NULL, and 
> some other code ran during parsing could trip on that.

Yes, there are still a few places in the HLSL parser that don't check 
for allocation failure and should.