[PATCH v4 0/7] MR259: vkd3d-shader/dxil: Read the DXIL module.
-- v4: vkd3d-shader/dxil: Read function bodies. vkd3d-shader/dxil: Read numeric constants. vkd3d-shader/dxil: Read global function declarations. vkd3d-shader/dxil: Validate the module format version. vkd3d-shader/dxil: Read the value symbol table. vkd3d-shader/dxil: Read the type table. vkd3d-shader/dxil: Use size_t where applicable. https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/259
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- libs/vkd3d-shader/dxil.c | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/libs/vkd3d-shader/dxil.c b/libs/vkd3d-shader/dxil.c index 67dcd26a0..8fc8dba90 100644 --- a/libs/vkd3d-shader/dxil.c +++ b/libs/vkd3d-shader/dxil.c @@ -628,10 +628,10 @@ static enum vkd3d_result dxil_block_read(struct dxil_block *parent, struct sm6_p return VKD3D_ERROR_INVALID_SHADER; } -static unsigned int sm6_parser_compute_global_abbrev_count_for_block_id(struct sm6_parser *sm6, +static size_t sm6_parser_compute_global_abbrev_count_for_block_id(struct sm6_parser *sm6, unsigned int block_id) { - unsigned int i, count; + size_t i, count; for (i = 0, count = 0; i < sm6->abbrev_count; ++i) count += sm6->abbrevs[i]->block_id == block_id; @@ -641,7 +641,7 @@ static unsigned int sm6_parser_compute_global_abbrev_count_for_block_id(struct s static void dxil_block_destroy(struct dxil_block *block) { - unsigned int i; + size_t i; for (i = 0; i < block->record_count; ++i) vkd3d_free(block->records[i]); @@ -663,7 +663,7 @@ static void dxil_block_destroy(struct dxil_block *block) static enum vkd3d_result dxil_block_init(struct dxil_block *block, const struct dxil_block *parent, struct sm6_parser *sm6) { - unsigned int i, abbrev_count = 0; + size_t i, abbrev_count = 0; enum vkd3d_result ret; block->parent = parent; @@ -705,9 +705,9 @@ static enum vkd3d_result dxil_block_init(struct dxil_block *block, const struct return ret; } -static void dxil_global_abbrevs_cleanup(struct dxil_global_abbrev **abbrevs, unsigned int count) +static void dxil_global_abbrevs_cleanup(struct dxil_global_abbrev **abbrevs, size_t count) { - unsigned int i; + size_t i; for (i = 0; i < count; ++i) vkd3d_free(abbrevs[i]); @@ -735,11 +735,12 @@ static enum vkd3d_result sm6_parser_init(struct sm6_parser *sm6, const uint32_t { const struct vkd3d_shader_location location = {.source_name = source_name}; uint32_t version_token, dxil_version, token_count, magic; - unsigned int count, length, chunk_offset, chunk_size; + unsigned int chunk_offset, chunk_size; enum bitcode_block_abbreviation abbr; struct vkd3d_shader_version version; struct dxil_block *block; enum vkd3d_result ret; + size_t count, length; count = byte_code_size / sizeof(*byte_code); if (count < 6) @@ -757,9 +758,9 @@ static enum vkd3d_result sm6_parser_init(struct sm6_parser *sm6, const uint32_t if (token_count < 6 || count < token_count) { - WARN("Invalid token count %u (word count %u).\n", token_count, count); + WARN("Invalid token count %u (word count %zu).\n", token_count, count); vkd3d_shader_error(message_context, &location, VKD3D_SHADER_ERROR_DXIL_INVALID_CHUNK_SIZE, - "DXIL chunk token count %#x is invalid (word count %u).", token_count, count); + "DXIL chunk token count %#x is invalid (word count %zu).", token_count, count); return VKD3D_ERROR_INVALID_SHADER; } @@ -847,17 +848,17 @@ static enum vkd3d_result sm6_parser_init(struct sm6_parser *sm6, const uint32_t length = sm6->ptr - sm6->start - block->start; if (length != block->length) { - WARN("Invalid block length %u; expected %u.\n", length, block->length); + WARN("Invalid block length %zu; expected %u.\n", length, block->length); vkd3d_shader_parser_warning(&sm6->p, VKD3D_SHADER_WARNING_DXIL_INVALID_BLOCK_LENGTH, - "Root block ends with length %u but indicated length is %u.", length, block->length); + "Root block ends with length %zu but indicated length is %u.", length, block->length); } if (sm6->ptr != sm6->end) { - unsigned int expected_length = sm6->end - sm6->start; + size_t expected_length = sm6->end - sm6->start; length = sm6->ptr - sm6->start; - WARN("Invalid module length %u; expected %u.\n", length, expected_length); + WARN("Invalid module length %zu; expected %zu.\n", length, expected_length); vkd3d_shader_parser_warning(&sm6->p, VKD3D_SHADER_WARNING_DXIL_INVALID_MODULE_LENGTH, - "Module ends with length %u but indicated length is %u.", length, expected_length); + "Module ends with length %zu but indicated length is %zu.", length, expected_length); } dxil_block_destroy(&sm6->root_block); -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/259
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- include/private/vkd3d_common.h | 5 + libs/vkd3d-shader/dxil.c | 412 +++++++++++++++++++++++ libs/vkd3d-shader/vkd3d_shader_private.h | 1 + 3 files changed, 418 insertions(+) diff --git a/include/private/vkd3d_common.h b/include/private/vkd3d_common.h index 6be3cee8f..46b7676b4 100644 --- a/include/private/vkd3d_common.h +++ b/include/private/vkd3d_common.h @@ -173,6 +173,11 @@ static inline bool vkd3d_bound_range(size_t start, size_t count, size_t limit) #endif } +static inline bool vkd3d_object_range_overflow(size_t start, size_t count, size_t size) +{ + return (~(size_t)0 - start) / size < count; +} + static inline uint16_t vkd3d_make_u16(uint8_t low, uint8_t high) { return low | ((uint16_t)high << 8); diff --git a/libs/vkd3d-shader/dxil.c b/libs/vkd3d-shader/dxil.c index 8fc8dba90..756b57e3e 100644 --- a/libs/vkd3d-shader/dxil.c +++ b/libs/vkd3d-shader/dxil.c @@ -62,6 +62,86 @@ enum bitcode_abbrev_type ABBREV_BLOB = 5, }; +enum bitcode_address_space +{ + ADDRESS_SPACE_DEFAULT, + ADDRESS_SPACE_DEVICEMEM, + ADDRESS_SPACE_CBUFFER, + ADDRESS_SPACE_GROUPSHARED, +}; + +enum bitcode_type_code +{ + TYPE_CODE_NUMENTRY = 1, + TYPE_CODE_VOID = 2, + TYPE_CODE_FLOAT = 3, + TYPE_CODE_DOUBLE = 4, + TYPE_CODE_LABEL = 5, + TYPE_CODE_INTEGER = 7, + TYPE_CODE_POINTER = 8, + TYPE_CODE_HALF = 10, + TYPE_CODE_ARRAY = 11, + TYPE_CODE_VECTOR = 12, + TYPE_CODE_METADATA = 16, + TYPE_CODE_STRUCT_ANON = 18, + TYPE_CODE_STRUCT_NAME = 19, + TYPE_CODE_STRUCT_NAMED = 20, + TYPE_CODE_FUNCTION = 21, +}; + +struct sm6_pointer_info +{ + const struct sm6_type *type; + enum bitcode_address_space addr_space; +}; + +struct sm6_struct_info +{ + const char *name; + unsigned int elem_count; + const struct sm6_type *elem_types[]; +}; + +struct sm6_function_info +{ + const struct sm6_type *ret_type; + unsigned int param_count; + const struct sm6_type *param_types[]; +}; + +struct sm6_array_info +{ + unsigned int count; + const struct sm6_type *elem_type; +}; + +enum sm6_type_class +{ + TYPE_CLASS_VOID, + TYPE_CLASS_INTEGER, + TYPE_CLASS_FLOAT, + TYPE_CLASS_POINTER, + TYPE_CLASS_STRUCT, + TYPE_CLASS_FUNCTION, + TYPE_CLASS_VECTOR, + TYPE_CLASS_ARRAY, + TYPE_CLASS_LABEL, + TYPE_CLASS_METADATA, +}; + +struct sm6_type +{ + enum sm6_type_class class; + union + { + unsigned int width; + struct sm6_pointer_info pointer; + struct sm6_struct_info *struc; + struct sm6_function_info *function; + struct sm6_array_info array; + } u; +}; + struct dxil_record { unsigned int code; @@ -106,6 +186,9 @@ struct sm6_parser size_t abbrev_capacity; size_t abbrev_count; + struct sm6_type *types; + size_t type_count; + struct vkd3d_shader_parser p; }; @@ -714,6 +797,321 @@ static void dxil_global_abbrevs_cleanup(struct dxil_global_abbrev **abbrevs, siz vkd3d_free(abbrevs); } +static const struct dxil_block *sm6_parser_get_level_one_block(const struct sm6_parser *sm6, + enum bitcode_block_id id, bool *is_unique) +{ + const struct dxil_block *block, *found = NULL; + size_t i; + + for (i = 0, *is_unique = true; i < sm6->root_block.child_block_count; ++i) + { + block = sm6->root_block.child_blocks[i]; + if (block->id != id) + continue; + + if (!found) + found = block; + else + *is_unique = false; + } + + return found; +} + +static char *dxil_record_to_string(const struct dxil_record *record) +{ + unsigned int i; + char *str; + + if (!(str = vkd3d_calloc(record->operand_count + 1, 1))) + return NULL; + + for (i = 0; i < record->operand_count; ++i) + str[i] = record->operands[i]; + + return str; +} + +#define TYPE_RECORD_VALIDATE_OPERAND_MIN_COUNT(min_count) do {\ + if (record->operand_count < (min_count))\ + {\ + WARN("Invalid operand count %u for type code %u, id %zu.\n", record->operand_count, record->code, type_index);\ + return VKD3D_ERROR_INVALID_SHADER;\ + } } while (false) + +#define TYPE_RECORD_VALIDATE_OPERAND_MAX_COUNT(max_count) do {\ + if (record->operand_count > (max_count))\ + WARN("Ignoring %u extra operands for type code %u, id %zu.\n", record->operand_count - (max_count),\ + record->code, type_index); } while (false) + +#define TYPE_RECORD_VALIDATE_OPERAND_COUNT(min_count, max_count) do {\ + TYPE_RECORD_VALIDATE_OPERAND_MIN_COUNT(min_count);\ + TYPE_RECORD_VALIDATE_OPERAND_MAX_COUNT(max_count); } while (false) + +static enum vkd3d_result sm6_parser_type_table_init(struct sm6_parser *sm6) +{ + const struct dxil_record *record; + size_t i, type_count, type_index; + const struct dxil_block *block; + char *struct_name = NULL; + unsigned int j, count; + struct sm6_type *type; + uint64_t type_id; + bool is_unique; + + if (!(block = sm6_parser_get_level_one_block(sm6, TYPE_BLOCK, &is_unique))) + { + WARN("No type definitions found.\n"); + return VKD3D_OK; + } + if (!is_unique) + WARN("Ignoring invalid extra type table(s).\n"); + + type_count = 0; + for (i = 0; i < block->record_count; ++i) + type_count += block->records[i]->code != TYPE_CODE_NUMENTRY && block->records[i]->code != TYPE_CODE_STRUCT_NAME; + + /* The type array must not be relocated. */ + if (!(sm6->types = vkd3d_calloc(type_count, sizeof(*sm6->types)))) + { + ERR("Failed to allocate type array.\n"); + return VKD3D_ERROR_OUT_OF_MEMORY; + } + + for (i = 0; i < block->record_count; ++i) + { + record = block->records[i]; + + type = &sm6->types[sm6->type_count]; + type_index = sm6->type_count; + + switch (record->code) + { + case TYPE_CODE_ARRAY: + case TYPE_CODE_VECTOR: + TYPE_RECORD_VALIDATE_OPERAND_COUNT(2, 2); + + type->class = record->code == TYPE_CODE_ARRAY ? TYPE_CLASS_ARRAY : TYPE_CLASS_VECTOR; + + if (!(type->u.array.count = record->operands[0])) + { + TRACE("Setting unbounded for type %zu.\n", type_index); + type->u.array.count = UINT_MAX; + } + + if ((type_id = record->operands[1]) >= type_count) + { + WARN("Invalid contained type id %"PRIu64" for type %zu.\n", type_id, type_index); + return VKD3D_ERROR_INVALID_SHADER; + } + type->u.array.elem_type = &sm6->types[type_id]; + break; + + case TYPE_CODE_DOUBLE: + TYPE_RECORD_VALIDATE_OPERAND_MAX_COUNT(0); + type->class = TYPE_CLASS_FLOAT; + type->u.width = 64; + break; + + case TYPE_CODE_FLOAT: + TYPE_RECORD_VALIDATE_OPERAND_MAX_COUNT(0); + type->class = TYPE_CLASS_FLOAT; + type->u.width = 32; + break; + + case TYPE_CODE_FUNCTION: + TYPE_RECORD_VALIDATE_OPERAND_MIN_COUNT(2); + if (record->operands[0]) + FIXME("Unhandled vararg function type %zu.\n", type_index); + + type->class = TYPE_CLASS_FUNCTION; + + if ((type_id = record->operands[1]) >= type_count) + { + WARN("Invalid return type id %"PRIu64" for type %zu.\n", type_id, type_index); + return VKD3D_ERROR_INVALID_SHADER; + } + + count = record->operand_count - 2; + if (vkd3d_object_range_overflow(sizeof(type->u.function), count, sizeof(type->u.function->param_types[0])) + || !(type->u.function = vkd3d_malloc(offsetof(struct sm6_function_info, param_types[count])))) + { + ERR("Failed to allocate function parameter types.\n"); + return VKD3D_ERROR_OUT_OF_MEMORY; + } + + type->u.function->ret_type = &sm6->types[type_id]; + type->u.function->param_count = count; + for (j = 0; j < count; ++j) + { + if ((type_id = record->operands[j + 2]) >= type_count) + { + WARN("Invalid parameter type id %"PRIu64" for type %zu.\n", type_id, type_index); + vkd3d_free(type->u.function); + return VKD3D_ERROR_INVALID_SHADER; + } + type->u.function->param_types[j] = &sm6->types[type_id]; + } + break; + + case TYPE_CODE_HALF: + TYPE_RECORD_VALIDATE_OPERAND_MAX_COUNT(0); + type->class = TYPE_CLASS_FLOAT; + type->u.width = 16; + break; + + case TYPE_CODE_INTEGER: + { + uint64_t width; + + TYPE_RECORD_VALIDATE_OPERAND_COUNT(1, 1); + + type->class = TYPE_CLASS_INTEGER; + + switch ((width = record->operands[0])) + { + case 1: + case 8: + case 16: + case 32: + case 64: + break; + default: + WARN("Invalid integer width %"PRIu64" for type %zu.\n", width, type_index); + return VKD3D_ERROR_INVALID_SHADER; + } + type->u.width = width; + break; + } + + case TYPE_CODE_LABEL: + type->class = TYPE_CLASS_LABEL; + break; + + case TYPE_CODE_METADATA: + type->class = TYPE_CLASS_METADATA; + break; + + case TYPE_CODE_NUMENTRY: + continue; + + case TYPE_CODE_POINTER: + TYPE_RECORD_VALIDATE_OPERAND_COUNT(1, 2); + + type->class = TYPE_CLASS_POINTER; + + if ((type_id = record->operands[0]) >= type_count) + { + WARN("Invalid pointee type id %"PRIu64" for type %zu.\n", type_id, type_index); + return VKD3D_ERROR_INVALID_SHADER; + } + type->u.pointer.type = &sm6->types[type_id]; + type->u.pointer.addr_space = (record->operand_count > 1) ? record->operands[1] : ADDRESS_SPACE_DEFAULT; + break; + + case TYPE_CODE_STRUCT_ANON: + case TYPE_CODE_STRUCT_NAMED: + TYPE_RECORD_VALIDATE_OPERAND_MIN_COUNT(2); + if (record->code == TYPE_CODE_STRUCT_NAMED && !struct_name) + { + WARN("Missing struct name before struct type %zu.\n", type_index); + return VKD3D_ERROR_INVALID_SHADER; + } + + type->class = TYPE_CLASS_STRUCT; + + count = record->operand_count - 1; + if (vkd3d_object_range_overflow(sizeof(type->u.struc), count, sizeof(type->u.struc->elem_types[0])) + || !(type->u.struc = vkd3d_malloc(offsetof(struct sm6_struct_info, elem_types[count])))) + { + ERR("Failed to allocate struct element types.\n"); + return VKD3D_ERROR_OUT_OF_MEMORY; + } + + if (record->operands[0]) + FIXME("Ignoring struct packed attribute.\n"); + + type->u.struc->elem_count = count; + for (j = 0; j < count; ++j) + { + if ((type_id = record->operands[j + 1]) >= type_count) + { + WARN("Invalid contained type id %"PRIu64" for type %zu.\n", type_id, type_index); + vkd3d_free(type->u.struc); + return VKD3D_ERROR_INVALID_SHADER; + } + type->u.struc->elem_types[j] = &sm6->types[type_id]; + } + + if (record->code == TYPE_CODE_STRUCT_ANON) + { + type->u.struc->name = NULL; + break; + } + + type->u.struc->name = struct_name; + struct_name = NULL; + break; + + case TYPE_CODE_STRUCT_NAME: + if (!(struct_name = dxil_record_to_string(record))) + { + ERR("Failed to allocate struct name.\n"); + return VKD3D_ERROR_OUT_OF_MEMORY; + } + if (!struct_name[0]) + WARN("Struct name is empty for type %zu.\n", type_index); + continue; + + case TYPE_CODE_VOID: + TYPE_RECORD_VALIDATE_OPERAND_MAX_COUNT(0); + type->class = TYPE_CLASS_VOID; + break; + + default: + FIXME("Unhandled type %u at index %zu.\n", record->code, type_index); + return VKD3D_ERROR_INVALID_SHADER; + } + ++sm6->type_count; + } + + assert(sm6->type_count == type_count); + + if (struct_name) + { + WARN("Unused struct name %s.\n", struct_name); + vkd3d_free(struct_name); + } + + return VKD3D_OK; +} + +static void sm6_type_table_cleanup(struct sm6_type *types, size_t count) +{ + size_t i; + + if (!types) + return; + + for (i = 0; i < count; ++i) + { + switch (types[i].class) + { + case TYPE_CLASS_STRUCT: + vkd3d_free((void *)types[i].u.struc->name); + vkd3d_free(types[i].u.struc); + break; + case TYPE_CLASS_FUNCTION: + vkd3d_free(types[i].u.function); + break; + default: + break; + } + } + + vkd3d_free(types); +} + static void sm6_parser_destroy(struct vkd3d_shader_parser *parser) { struct sm6_parser *sm6 = sm6_parser(parser); @@ -721,6 +1119,7 @@ static void sm6_parser_destroy(struct vkd3d_shader_parser *parser) dxil_block_destroy(&sm6->root_block); dxil_global_abbrevs_cleanup(sm6->abbrevs, sm6->abbrev_count); shader_instruction_array_destroy(&parser->instructions); + sm6_type_table_cleanup(sm6->types, sm6->type_count); free_shader_desc(&parser->shader_desc); vkd3d_free(sm6); } @@ -861,6 +1260,19 @@ static enum vkd3d_result sm6_parser_init(struct sm6_parser *sm6, const uint32_t "Module ends with length %zu but indicated length is %zu.", length, expected_length); } + if ((ret = sm6_parser_type_table_init(sm6)) < 0) + { + if (ret == VKD3D_ERROR_OUT_OF_MEMORY) + vkd3d_shader_error(message_context, &location, VKD3D_SHADER_ERROR_DXIL_OUT_OF_MEMORY, + "Out of memory parsing DXIL type table."); + else if (ret == VKD3D_ERROR_INVALID_SHADER) + vkd3d_shader_error(message_context, &location, VKD3D_SHADER_ERROR_DXIL_INVALID_TYPE_TABLE, + "DXIL type table is invalid."); + else + vkd3d_unreachable(); + return ret; + } + dxil_block_destroy(&sm6->root_block); return VKD3D_OK; diff --git a/libs/vkd3d-shader/vkd3d_shader_private.h b/libs/vkd3d-shader/vkd3d_shader_private.h index 398139c3e..ade0bbd02 100644 --- a/libs/vkd3d-shader/vkd3d_shader_private.h +++ b/libs/vkd3d-shader/vkd3d_shader_private.h @@ -155,6 +155,7 @@ enum vkd3d_shader_error VKD3D_SHADER_ERROR_DXIL_INVALID_CHUNK_OFFSET = 8002, VKD3D_SHADER_ERROR_DXIL_INVALID_CHUNK_SIZE = 8003, VKD3D_SHADER_ERROR_DXIL_INVALID_BITCODE = 8004, + VKD3D_SHADER_ERROR_DXIL_INVALID_TYPE_TABLE = 8005, VKD3D_SHADER_WARNING_DXIL_UNKNOWN_MAGIC_NUMBER = 8300, VKD3D_SHADER_WARNING_DXIL_UNKNOWN_SHADER_TYPE = 8301, -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/259
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- libs/vkd3d-shader/dxil.c | 118 ++++++++++++++++++++++- libs/vkd3d-shader/vkd3d_shader_private.h | 1 + 2 files changed, 114 insertions(+), 5 deletions(-) diff --git a/libs/vkd3d-shader/dxil.c b/libs/vkd3d-shader/dxil.c index 756b57e3e..dda6c2c75 100644 --- a/libs/vkd3d-shader/dxil.c +++ b/libs/vkd3d-shader/dxil.c @@ -89,6 +89,12 @@ enum bitcode_type_code TYPE_CODE_FUNCTION = 21, }; +enum bitcode_value_symtab_code +{ + VST_CODE_ENTRY = 1, + VST_CODE_BBENTRY = 2, +}; + struct sm6_pointer_info { const struct sm6_type *type; @@ -149,6 +155,12 @@ struct dxil_record uint64_t operands[]; }; +struct sm6_symbol +{ + unsigned int id; + const char *name; +}; + struct dxil_block { const struct dxil_block *parent; @@ -189,6 +201,9 @@ struct sm6_parser struct sm6_type *types; size_t type_count; + struct sm6_symbol *global_symbols; + size_t global_symbol_count; + struct vkd3d_shader_parser p; }; @@ -818,16 +833,17 @@ static const struct dxil_block *sm6_parser_get_level_one_block(const struct sm6_ return found; } -static char *dxil_record_to_string(const struct dxil_record *record) +static char *dxil_record_to_string(const struct dxil_record *record, unsigned int offset) { unsigned int i; char *str; - if (!(str = vkd3d_calloc(record->operand_count + 1, 1))) + assert(offset <= record->operand_count); + if (!(str = vkd3d_calloc(record->operand_count - offset + 1, 1))) return NULL; - for (i = 0; i < record->operand_count; ++i) - str[i] = record->operands[i]; + for (i = offset; i < record->operand_count; ++i) + str[i - offset] = record->operands[i]; return str; } @@ -1054,7 +1070,7 @@ static enum vkd3d_result sm6_parser_type_table_init(struct sm6_parser *sm6) break; case TYPE_CODE_STRUCT_NAME: - if (!(struct_name = dxil_record_to_string(record))) + if (!(struct_name = dxil_record_to_string(record, 0))) { ERR("Failed to allocate struct name.\n"); return VKD3D_ERROR_OUT_OF_MEMORY; @@ -1086,6 +1102,75 @@ static enum vkd3d_result sm6_parser_type_table_init(struct sm6_parser *sm6) return VKD3D_OK; } +static int global_symbol_compare(const void *a, const void *b) +{ + return vkd3d_u32_compare(((const struct sm6_symbol *)a)->id, ((const struct sm6_symbol *)b)->id); +} + +static enum vkd3d_result sm6_parser_symtab_init(struct sm6_parser *sm6) +{ + const struct dxil_record *record; + const struct dxil_block *block; + struct sm6_symbol *symbol; + size_t i, count; + bool is_unique; + + if (!(block = sm6_parser_get_level_one_block(sm6, VALUE_SYMTAB_BLOCK, &is_unique))) + { + /* There should always be at least one symbol: the name of the entry point function. */ + WARN("No value symtab block found.\n"); + return VKD3D_ERROR_INVALID_SHADER; + } + if (!is_unique) + FIXME("Ignoring extra value symtab block(s).\n"); + + for (i = 0, count = 0; i < block->record_count; ++i) + count += block->records[i]->code == VST_CODE_ENTRY; + + if (!(sm6->global_symbols = vkd3d_calloc(count, sizeof(*sm6->global_symbols)))) + { + ERR("Failed to allocate global symbols.\n"); + return VKD3D_ERROR_OUT_OF_MEMORY; + } + + for (i = 0; i < block->record_count; ++i) + { + record = block->records[i]; + + if (record->code != VST_CODE_ENTRY) + { + FIXME("Unhandled symtab code %u.\n", record->code); + continue; + } + if (record->operand_count < 2) + { + WARN("Missing operands for function symbol %zu.\n", sm6->global_symbol_count); + continue; + } + + symbol = &sm6->global_symbols[sm6->global_symbol_count]; + symbol->id = record->operands[0]; + if (!(symbol->name = dxil_record_to_string(record, 1))) + { + ERR("Failed to allocate symbol name.\n"); + return VKD3D_ERROR_OUT_OF_MEMORY; + } + ++sm6->global_symbol_count; + } + + qsort(sm6->global_symbols, sm6->global_symbol_count, sizeof(*sm6->global_symbols), global_symbol_compare); + for (i = 1; i < sm6->global_symbol_count; ++i) + { + if (sm6->global_symbols[i].id == sm6->global_symbols[i - 1].id) + { + WARN("Invalid duplicate symbol id %u.\n", sm6->global_symbols[i].id); + return VKD3D_ERROR_INVALID_SHADER; + } + } + + return VKD3D_OK; +} + static void sm6_type_table_cleanup(struct sm6_type *types, size_t count) { size_t i; @@ -1112,6 +1197,15 @@ static void sm6_type_table_cleanup(struct sm6_type *types, size_t count) vkd3d_free(types); } +static void sm6_symtab_cleanup(struct sm6_symbol *symbols, size_t count) +{ + size_t i; + + for (i = 0; i < count; ++i) + vkd3d_free((void *)symbols[i].name); + vkd3d_free(symbols); +} + static void sm6_parser_destroy(struct vkd3d_shader_parser *parser) { struct sm6_parser *sm6 = sm6_parser(parser); @@ -1120,6 +1214,7 @@ static void sm6_parser_destroy(struct vkd3d_shader_parser *parser) dxil_global_abbrevs_cleanup(sm6->abbrevs, sm6->abbrev_count); shader_instruction_array_destroy(&parser->instructions); sm6_type_table_cleanup(sm6->types, sm6->type_count); + sm6_symtab_cleanup(sm6->global_symbols, sm6->global_symbol_count); free_shader_desc(&parser->shader_desc); vkd3d_free(sm6); } @@ -1273,6 +1368,19 @@ static enum vkd3d_result sm6_parser_init(struct sm6_parser *sm6, const uint32_t return ret; } + if ((ret = sm6_parser_symtab_init(sm6)) < 0) + { + if (ret == VKD3D_ERROR_OUT_OF_MEMORY) + vkd3d_shader_error(message_context, &location, VKD3D_SHADER_ERROR_DXIL_OUT_OF_MEMORY, + "Out of memory parsing DXIL value symbol table."); + else if (ret == VKD3D_ERROR_INVALID_SHADER) + vkd3d_shader_error(message_context, &location, VKD3D_SHADER_ERROR_DXIL_INVALID_VALUE_SYMTAB, + "DXIL value symbol table is invalid."); + else + vkd3d_unreachable(); + return ret; + } + dxil_block_destroy(&sm6->root_block); return VKD3D_OK; diff --git a/libs/vkd3d-shader/vkd3d_shader_private.h b/libs/vkd3d-shader/vkd3d_shader_private.h index ade0bbd02..d30da3f16 100644 --- a/libs/vkd3d-shader/vkd3d_shader_private.h +++ b/libs/vkd3d-shader/vkd3d_shader_private.h @@ -156,6 +156,7 @@ enum vkd3d_shader_error VKD3D_SHADER_ERROR_DXIL_INVALID_CHUNK_SIZE = 8003, VKD3D_SHADER_ERROR_DXIL_INVALID_BITCODE = 8004, VKD3D_SHADER_ERROR_DXIL_INVALID_TYPE_TABLE = 8005, + VKD3D_SHADER_ERROR_DXIL_INVALID_VALUE_SYMTAB = 8006, VKD3D_SHADER_WARNING_DXIL_UNKNOWN_MAGIC_NUMBER = 8300, VKD3D_SHADER_WARNING_DXIL_UNKNOWN_SHADER_TYPE = 8301, -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/259
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- libs/vkd3d-shader/dxil.c | 58 ++++++++++++++++++++++++ libs/vkd3d-shader/vkd3d_shader_private.h | 1 + 2 files changed, 59 insertions(+) diff --git a/libs/vkd3d-shader/dxil.c b/libs/vkd3d-shader/dxil.c index dda6c2c75..8093fd6a7 100644 --- a/libs/vkd3d-shader/dxil.c +++ b/libs/vkd3d-shader/dxil.c @@ -70,6 +70,13 @@ enum bitcode_address_space ADDRESS_SPACE_GROUPSHARED, }; +enum bitcode_module_code +{ + MODULE_CODE_VERSION = 1, + MODULE_CODE_GLOBALVAR = 7, + MODULE_CODE_FUNCTION = 8, +}; + enum bitcode_type_code { TYPE_CODE_NUMENTRY = 1, @@ -1171,6 +1178,51 @@ static enum vkd3d_result sm6_parser_symtab_init(struct sm6_parser *sm6) return VKD3D_OK; } +static enum vkd3d_result sm6_parser_globals_init(struct sm6_parser *sm6) +{ + const struct dxil_block *block = &sm6->root_block; + const struct dxil_record *record; + uint64_t version; + size_t i; + + for (i = 0; i < block->record_count; ++i) + { + record = block->records[i]; + switch (record->code) + { + case MODULE_CODE_FUNCTION: + FIXME("Functions are not implemented yet.\n"); + break; + + case MODULE_CODE_GLOBALVAR: + FIXME("Global variables are not implemented yet.\n"); + break; + + case MODULE_CODE_VERSION: + if (!record->operand_count) + { + WARN("Missing version operand.\n"); + break; + } + if (record->operand_count > 1) + WARN("Ignoring %u extra operands.\n", record->operand_count - 1); + if ((version = record->operands[0]) != 1) + { + FIXME("Unsupported format version %#"PRIx64".\n", version); + vkd3d_shader_parser_error(&sm6->p, VKD3D_SHADER_ERROR_DXIL_UNSUPPORTED_BITCODE_FORMAT, + "Bitcode format version %#"PRIx64" is unsupported.", version); + return VKD3D_ERROR_INVALID_SHADER; + } + break; + + default: + break; + } + } + + return VKD3D_OK; +} + static void sm6_type_table_cleanup(struct sm6_type *types, size_t count) { size_t i; @@ -1381,6 +1433,12 @@ static enum vkd3d_result sm6_parser_init(struct sm6_parser *sm6, const uint32_t return ret; } + if ((ret = sm6_parser_globals_init(sm6)) < 0) + { + WARN("Failed to load global declarations.\n"); + return ret; + } + dxil_block_destroy(&sm6->root_block); return VKD3D_OK; diff --git a/libs/vkd3d-shader/vkd3d_shader_private.h b/libs/vkd3d-shader/vkd3d_shader_private.h index d30da3f16..0edcaaa36 100644 --- a/libs/vkd3d-shader/vkd3d_shader_private.h +++ b/libs/vkd3d-shader/vkd3d_shader_private.h @@ -157,6 +157,7 @@ enum vkd3d_shader_error VKD3D_SHADER_ERROR_DXIL_INVALID_BITCODE = 8004, VKD3D_SHADER_ERROR_DXIL_INVALID_TYPE_TABLE = 8005, VKD3D_SHADER_ERROR_DXIL_INVALID_VALUE_SYMTAB = 8006, + VKD3D_SHADER_ERROR_DXIL_UNSUPPORTED_BITCODE_FORMAT = 8007, VKD3D_SHADER_WARNING_DXIL_UNKNOWN_MAGIC_NUMBER = 8300, VKD3D_SHADER_WARNING_DXIL_UNKNOWN_SHADER_TYPE = 8301, -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/259
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- libs/vkd3d-shader/dxil.c | 234 ++++++++++++++++++++++- libs/vkd3d-shader/vkd3d_shader_private.h | 1 + 2 files changed, 234 insertions(+), 1 deletion(-) diff --git a/libs/vkd3d-shader/dxil.c b/libs/vkd3d-shader/dxil.c index 8093fd6a7..90c9fc939 100644 --- a/libs/vkd3d-shader/dxil.c +++ b/libs/vkd3d-shader/dxil.c @@ -155,6 +155,28 @@ struct sm6_type } u; }; +enum sm6_value_type +{ + VALUE_TYPE_FUNCTION, +}; + +struct sm6_function_data +{ + const char *name; + bool is_prototype; + unsigned int attribs_id; +}; + +struct sm6_value +{ + const struct sm6_type *type; + enum sm6_value_type value_type; + union + { + struct sm6_function_data function; + } u; +}; + struct dxil_record { unsigned int code; @@ -211,6 +233,10 @@ struct sm6_parser struct sm6_symbol *global_symbols; size_t global_symbol_count; + struct sm6_value *values; + size_t value_count; + size_t value_capacity; + struct vkd3d_shader_parser p; }; @@ -810,6 +836,15 @@ static enum vkd3d_result dxil_block_init(struct dxil_block *block, const struct return ret; } +static size_t dxil_block_compute_module_decl_count(const struct dxil_block *block) +{ + size_t i, count; + + for (i = 0, count = 0; i < block->record_count; ++i) + count += block->records[i]->code == MODULE_CODE_FUNCTION; + return count; +} + static void dxil_global_abbrevs_cleanup(struct dxil_global_abbrev **abbrevs, size_t count) { size_t i; @@ -1109,6 +1144,89 @@ static enum vkd3d_result sm6_parser_type_table_init(struct sm6_parser *sm6) return VKD3D_OK; } +static inline bool sm6_type_is_void(const struct sm6_type *type) +{ + return type->class == TYPE_CLASS_VOID; +} + +static inline bool sm6_type_is_numeric(const struct sm6_type *type) +{ + return type->class == TYPE_CLASS_INTEGER || type->class == TYPE_CLASS_FLOAT; +} + +static inline bool sm6_type_is_pointer(const struct sm6_type *type) +{ + return type->class == TYPE_CLASS_POINTER; +} + +static bool sm6_type_is_numeric_aggregate(const struct sm6_type *type) +{ + unsigned int i; + + switch (type->class) + { + case TYPE_CLASS_ARRAY: + case TYPE_CLASS_VECTOR: + return sm6_type_is_numeric(type->u.array.elem_type); + + case TYPE_CLASS_STRUCT: + /* Do not handle nested structs. Support can be added if they show up. */ + for (i = 0; i < type->u.struc->elem_count; ++i) + if (!sm6_type_is_numeric(type->u.struc->elem_types[i])) + return false; + return true; + + default: + return false; + } +} + +static inline bool sm6_type_is_struct(const struct sm6_type *type) +{ + return type->class == TYPE_CLASS_STRUCT; +} + +static inline bool sm6_type_is_function(const struct sm6_type *type) +{ + return type->class == TYPE_CLASS_FUNCTION; +} + +static inline bool sm6_type_is_handle(const struct sm6_type *type) +{ + return sm6_type_is_struct(type) && !strcmp(type->u.struc->name, "dx.types.Handle"); +} + +static const struct sm6_type *sm6_type_get_pointer_to_type(const struct sm6_type *type, + enum bitcode_address_space addr_space, struct sm6_parser *sm6) +{ + size_t i, start = type - sm6->types; + const struct sm6_type *pointer_type; + + /* DXC seems usually to place the pointer type immediately after its pointee. */ + for (i = (start + 1) % sm6->type_count; i != start; i = (i + 1) % sm6->type_count) + { + pointer_type = &sm6->types[i]; + if (sm6_type_is_pointer(pointer_type) && pointer_type->u.pointer.type == type + && pointer_type->u.pointer.addr_space == addr_space) + return pointer_type; + } + + return NULL; +} + +static const struct sm6_type *sm6_parser_get_type(const struct sm6_parser *sm6, const char *function, uint64_t type_id) +{ + if (type_id >= sm6->type_count) + { + vkd3d_dbg_printf(VKD3D_DBG_LEVEL_WARN, function, "Invalid type index %"PRIu64" at %zu.\n", + type_id, sm6->value_count); + return NULL; + } + return &sm6->types[type_id]; +} + +#define sm6_parser_get_type(sm6, id) sm6_parser_get_type(sm6, __FUNCTION__, id) + static int global_symbol_compare(const void *a, const void *b) { return vkd3d_u32_compare(((const struct sm6_symbol *)a)->id, ((const struct sm6_symbol *)b)->id); @@ -1178,6 +1296,105 @@ static enum vkd3d_result sm6_parser_symtab_init(struct sm6_parser *sm6) return VKD3D_OK; } +static const char *sm6_parser_get_global_symbol_name(const struct sm6_parser *sm6, size_t id) +{ + size_t i, start; + + /* id == array index is normally true */ + i = start = id % sm6->global_symbol_count; + do + { + if (sm6->global_symbols[i].id == id) + return sm6->global_symbols[i].name; + i = (i + 1) % sm6->global_symbol_count; + } while (i != start); + + return NULL; +} + +static inline bool sm6_value_is_dx_intrinsic_dcl(const struct sm6_value *fn) +{ + assert(fn->value_type == VALUE_TYPE_FUNCTION); + return fn->u.function.is_prototype && !strncmp(fn->u.function.name, "dx.op.", 6); +} + +static inline struct sm6_value *sm6_parser_get_current_value(const struct sm6_parser *sm6) +{ + assert(sm6->value_count < sm6->value_capacity); + return &sm6->values[sm6->value_count]; +} + +static void sm6_parser_compute_max_value_count(struct sm6_parser *sm6, + const struct dxil_block *block) +{ + sm6->value_capacity = dxil_block_compute_module_decl_count(block); +} + +static bool sm6_parser_declare_function(struct sm6_parser *sm6, const struct dxil_record *record) +{ + const struct sm6_type *ret_type; + struct sm6_value *fn; + unsigned int i, j; + + if (record->operand_count < 8) + { + WARN("Missing operands for function %zu.\n", sm6->value_count); + return false; + } + + fn = sm6_parser_get_current_value(sm6); + fn->value_type = VALUE_TYPE_FUNCTION; + if (!(fn->u.function.name = sm6_parser_get_global_symbol_name(sm6, sm6->value_count))) + { + WARN("Missing symbol name for function %zu.\n", sm6->value_count); + fn->u.function.name = ""; + } + + if (!(fn->type = sm6_parser_get_type(sm6, record->operands[0]))) + return false; + if (!sm6_type_is_function(fn->type)) + { + WARN("Type is not a function.\n"); + return false; + } + ret_type = fn->type->u.function->ret_type; + + if (!(fn->type = sm6_type_get_pointer_to_type(fn->type, ADDRESS_SPACE_DEFAULT, sm6))) + { + WARN("Failed to get pointer type for type %u.\n", fn->type->class); + return false; + } + + if (record->operands[1]) + WARN("Ignoring calling convention %#"PRIx64".\n", record->operands[1]); + + fn->u.function.is_prototype = !!record->operands[2]; + + if (record->operands[3]) + WARN("Ignoring linkage %#"PRIx64".\n", record->operands[3]); + + if (record->operands[4] > UINT_MAX) + WARN("Invalid attributes id %#"PRIx64".\n", record->operands[4]); + /* 1-based index. */ + if ((fn->u.function.attribs_id = record->operands[4])) + TRACE("Ignoring function attributes.\n"); + + for (i = 5, j = 0; i < record->operand_count; ++i) + j += !!record->operands[i]; + if (j) + WARN("Ignoring %u operands.\n", j); + + if (sm6_value_is_dx_intrinsic_dcl(fn) && !sm6_type_is_void(ret_type) && !sm6_type_is_numeric(ret_type) + && !sm6_type_is_numeric_aggregate(ret_type) && !sm6_type_is_handle(ret_type)) + { + WARN("Unexpected return type for dx intrinsic function '%s'.\n", fn->u.function.name); + } + + ++sm6->value_count; + + return true; +} + static enum vkd3d_result sm6_parser_globals_init(struct sm6_parser *sm6) { const struct dxil_block *block = &sm6->root_block; @@ -1191,7 +1408,12 @@ static enum vkd3d_result sm6_parser_globals_init(struct sm6_parser *sm6) switch (record->code) { case MODULE_CODE_FUNCTION: - FIXME("Functions are not implemented yet.\n"); + if (!sm6_parser_declare_function(sm6, record)) + { + vkd3d_shader_parser_error(&sm6->p, VKD3D_SHADER_ERROR_DXIL_INVALID_FUNCTION_DCL, + "A DXIL function declaration is invalid."); + return VKD3D_ERROR_INVALID_SHADER; + } break; case MODULE_CODE_GLOBALVAR: @@ -1267,6 +1489,7 @@ static void sm6_parser_destroy(struct vkd3d_shader_parser *parser) shader_instruction_array_destroy(&parser->instructions); sm6_type_table_cleanup(sm6->types, sm6->type_count); sm6_symtab_cleanup(sm6->global_symbols, sm6->global_symbol_count); + vkd3d_free(sm6->values); free_shader_desc(&parser->shader_desc); vkd3d_free(sm6); } @@ -1433,6 +1656,15 @@ static enum vkd3d_result sm6_parser_init(struct sm6_parser *sm6, const uint32_t return ret; } + sm6_parser_compute_max_value_count(sm6, &sm6->root_block); + if (!(sm6->values = vkd3d_calloc(sm6->value_capacity, sizeof(*sm6->values)))) + { + ERR("Failed to allocate value array.\n"); + vkd3d_shader_error(message_context, &location, VKD3D_SHADER_ERROR_DXIL_OUT_OF_MEMORY, + "Out of memory allocating DXIL value array."); + return VKD3D_ERROR_OUT_OF_MEMORY; + } + if ((ret = sm6_parser_globals_init(sm6)) < 0) { WARN("Failed to load global declarations.\n"); diff --git a/libs/vkd3d-shader/vkd3d_shader_private.h b/libs/vkd3d-shader/vkd3d_shader_private.h index 0edcaaa36..5824dfaf6 100644 --- a/libs/vkd3d-shader/vkd3d_shader_private.h +++ b/libs/vkd3d-shader/vkd3d_shader_private.h @@ -158,6 +158,7 @@ enum vkd3d_shader_error VKD3D_SHADER_ERROR_DXIL_INVALID_TYPE_TABLE = 8005, VKD3D_SHADER_ERROR_DXIL_INVALID_VALUE_SYMTAB = 8006, VKD3D_SHADER_ERROR_DXIL_UNSUPPORTED_BITCODE_FORMAT = 8007, + VKD3D_SHADER_ERROR_DXIL_INVALID_FUNCTION_DCL = 8008, VKD3D_SHADER_WARNING_DXIL_UNKNOWN_MAGIC_NUMBER = 8300, VKD3D_SHADER_WARNING_DXIL_UNKNOWN_SHADER_TYPE = 8301, -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/259
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- libs/vkd3d-shader/dxil.c | 331 ++++++++++++++++++++++- libs/vkd3d-shader/vkd3d_shader_private.h | 2 + 2 files changed, 329 insertions(+), 4 deletions(-) diff --git a/libs/vkd3d-shader/dxil.c b/libs/vkd3d-shader/dxil.c index 90c9fc939..32e12266b 100644 --- a/libs/vkd3d-shader/dxil.c +++ b/libs/vkd3d-shader/dxil.c @@ -77,6 +77,19 @@ enum bitcode_module_code MODULE_CODE_FUNCTION = 8, }; +enum bitcode_constant_code +{ + CST_CODE_SETTYPE = 1, + CST_CODE_NULL = 2, + CST_CODE_UNDEF = 3, + CST_CODE_INTEGER = 4, + CST_CODE_FLOAT = 6, + CST_CODE_STRING = 8, + CST_CODE_CE_GEP = 12, + CST_CODE_CE_INBOUNDS_GEP = 20, + CST_CODE_DATA = 22, +}; + enum bitcode_type_code { TYPE_CODE_NUMENTRY = 1, @@ -158,6 +171,7 @@ struct sm6_type enum sm6_value_type { VALUE_TYPE_FUNCTION, + VALUE_TYPE_REG, }; struct sm6_function_data @@ -174,6 +188,7 @@ struct sm6_value union { struct sm6_function_data function; + struct vkd3d_shader_register reg; } u; }; @@ -259,6 +274,12 @@ struct dxil_global_abbrev struct dxil_abbrev abbrev; }; +static size_t size_add_with_overflow_check(size_t a, size_t b) +{ + size_t i = a + b; + return (i < a) ? SIZE_MAX : i; +} + static struct sm6_parser *sm6_parser(struct vkd3d_shader_parser *parser) { return CONTAINING_RECORD(parser, struct sm6_parser, p); @@ -845,6 +866,15 @@ static size_t dxil_block_compute_module_decl_count(const struct dxil_block *bloc return count; } +static size_t dxil_block_compute_constants_count(const struct dxil_block *block) +{ + size_t i, count; + + for (i = 0, count = 0; i < block->record_count; ++i) + count += block->records[i]->code != CST_CODE_SETTYPE; + return count; +} + static void dxil_global_abbrevs_cleanup(struct dxil_global_abbrev **abbrevs, size_t count) { size_t i; @@ -1149,6 +1179,16 @@ static inline bool sm6_type_is_void(const struct sm6_type *type) return type->class == TYPE_CLASS_VOID; } +static inline bool sm6_type_is_integer(const struct sm6_type *type) +{ + return type->class == TYPE_CLASS_INTEGER; +} + +static inline bool sm6_type_is_floating_point(const struct sm6_type *type) +{ + return type->class == TYPE_CLASS_FLOAT; +} + static inline bool sm6_type_is_numeric(const struct sm6_type *type) { return type->class == TYPE_CLASS_INTEGER || type->class == TYPE_CLASS_FLOAT; @@ -1196,6 +1236,11 @@ static inline bool sm6_type_is_handle(const struct sm6_type *type) return sm6_type_is_struct(type) && !strcmp(type->u.struc->name, "dx.types.Handle"); } +static inline const struct sm6_type *sm6_type_get_element_type(const struct sm6_type *type) +{ + return (type->class == TYPE_CLASS_ARRAY || type->class == TYPE_CLASS_VECTOR) ? type->u.array.elem_type : type; +} + static const struct sm6_type *sm6_type_get_pointer_to_type(const struct sm6_type *type, enum bitcode_address_space addr_space, struct sm6_parser *sm6) { @@ -1324,10 +1369,70 @@ static inline struct sm6_value *sm6_parser_get_current_value(const struct sm6_pa return &sm6->values[sm6->value_count]; } -static void sm6_parser_compute_max_value_count(struct sm6_parser *sm6, - const struct dxil_block *block) +static enum vkd3d_data_type vkd3d_data_type_from_sm6_type(const struct sm6_type *type) { - sm6->value_capacity = dxil_block_compute_module_decl_count(block); + if (type->class == TYPE_CLASS_INTEGER) + { + switch (type->u.width) + { + case 8: + return VKD3D_DATA_UINT8; + case 32: + return VKD3D_DATA_UINT; + default: + FIXME("Unhandled width %u.\n", type->u.width); + return VKD3D_DATA_UINT; + } + } + else if (type->class == TYPE_CLASS_FLOAT) + { + switch (type->u.width) + { + case 32: + return VKD3D_DATA_FLOAT; + case 64: + return VKD3D_DATA_DOUBLE; + default: + FIXME("Unhandled width %u.\n", type->u.width); + return VKD3D_DATA_FLOAT; + } + } + + FIXME("Unhandled type %u.\n", type->class); + return VKD3D_DATA_UINT; +} + +/* Recurse through the block tree while maintaining a current value count. The current + * count is the sum of the global count plus all declarations within the current function. + * Store into value_capacity the highest count seen. */ +static size_t sm6_parser_compute_max_value_count(struct sm6_parser *sm6, + const struct dxil_block *block, size_t value_count) +{ + size_t i, old_value_count = value_count; + + if (block->id == MODULE_BLOCK) + value_count = size_add_with_overflow_check(value_count, dxil_block_compute_module_decl_count(block)); + + for (i = 0; i < block->child_block_count; ++i) + value_count = sm6_parser_compute_max_value_count(sm6, block->child_blocks[i], value_count); + + switch (block->id) + { + case CONSTANTS_BLOCK: + /* Function local constants are contained in a child block of the function block. */ + value_count = size_add_with_overflow_check(value_count, dxil_block_compute_constants_count(block)); + break; + case FUNCTION_BLOCK: + sm6->value_capacity = max(sm6->value_capacity, value_count); + /* The value count returns to its previous value after handling a function. */ + if (value_count < SIZE_MAX) + value_count = old_value_count; + break; + default: + break; + } + + return value_count; } static bool sm6_parser_declare_function(struct sm6_parser *sm6, const struct dxil_record *record) @@ -1395,6 +1500,170 @@ static bool sm6_parser_declare_function(struct sm6_parser *sm6, const struct dxi return true; } +static inline uint64_t decode_rotated_signed_value(uint64_t value) +{ + if (value != 1) + { + bool neg = value & 1; + value >>= 1; + return neg ? -value : value; + } + return value << 63; +} + +static inline float bitcast_uint64_to_float(uint64_t value) +{ + union + { + uint32_t uint32_value; + float float_value; + } u; + + u.uint32_value = value; + return u.float_value; +} + +static inline double bitcast_uint64_to_double(uint64_t value) +{ + union + { + uint64_t uint64_value; + double double_value; + } u; + + u.uint64_value = value; + return u.double_value; +} + +static enum vkd3d_result sm6_parser_constants_init(struct sm6_parser *sm6, const struct dxil_block *block) +{ + enum vkd3d_shader_register_type reg_type = VKD3DSPR_INVALID; + const struct sm6_type *type, *elem_type; + enum vkd3d_data_type reg_data_type; + const struct dxil_record *record; + struct sm6_value *dst; + size_t i, value_idx; + uint64_t value; + + for (i = 0, type = NULL; i < block->record_count; ++i) + { + record = block->records[i]; + value_idx = sm6->value_count; + + if (record->code == CST_CODE_SETTYPE) + { + if (!record->operand_count) + { + WARN("Missing type id for constant idx %zu.\n", value_idx); + return VKD3D_ERROR_INVALID_SHADER; + } + if (record->operand_count > 1) + WARN("Ignoring %u extra operands.\n", record->operand_count - 1); + + if (!(type = sm6_parser_get_type(sm6, record->operands[0]))) + return VKD3D_ERROR_INVALID_SHADER; + + elem_type = sm6_type_get_element_type(type); + if (sm6_type_is_numeric(elem_type)) + { + reg_data_type = vkd3d_data_type_from_sm6_type(elem_type); + reg_type = elem_type->u.width > 32 ? VKD3DSPR_IMMCONST64 : VKD3DSPR_IMMCONST; + } + else + { + reg_data_type = VKD3D_DATA_UNUSED; + reg_type = VKD3DSPR_INVALID; + } + + if (i == block->record_count - 1) + WARN("Unused SETTYPE record.\n"); + + continue; + } + + if (!type) + { + WARN("Constant record %zu has no type.\n", value_idx); + return VKD3D_ERROR_INVALID_SHADER; + } + + dst = sm6_parser_get_current_value(sm6); + dst->type = type; + dst->value_type = VALUE_TYPE_REG; + dst->u.reg.type = reg_type; + dst->u.reg.immconst_type = VKD3D_IMMCONST_SCALAR; + dst->u.reg.data_type = reg_data_type; + + switch (record->code) + { + case CST_CODE_NULL: + /* Register constant data is already zero-filled. */ + break; + + case CST_CODE_INTEGER: + if (!record->operand_count) + { + WARN("Missing integer value for constant idx %zu.\n", value_idx); + return VKD3D_ERROR_INVALID_SHADER; + } + if (record->operand_count > 1) + WARN("Ignoring %u extra operands.\n", record->operand_count - 1); + + if (!sm6_type_is_integer(type)) + { + WARN("Invalid integer of non-integer type %u at constant idx %zu.\n", type->class, value_idx); + return VKD3D_ERROR_INVALID_SHADER; + } + + value = decode_rotated_signed_value(record->operands[0]); + if (type->u.width <= 32) + dst->u.reg.u.immconst_uint[0] = value & ((1ull << type->u.width) - 1); + else + dst->u.reg.u.immconst_uint64[0] = value; + + break; + + case CST_CODE_FLOAT: + if (!record->operand_count) + { + WARN("Missing floating point value for constant idx %zu.\n", value_idx); + return VKD3D_ERROR_INVALID_SHADER; + } + if (record->operand_count > 1) + WARN("Ignoring %u extra operands.\n", record->operand_count - 1); + + if (!sm6_type_is_floating_point(type)) + { + WARN("Invalid float of non-fp type %u at constant idx %zu.\n", type->class, value_idx); + return VKD3D_ERROR_INVALID_SHADER; + } + + if (type->u.width == 16) + FIXME("Half float type is not supported yet.\n"); + else if (type->u.width == 32) + dst->u.reg.u.immconst_float[0] = bitcast_uint64_to_float(record->operands[0]); + else if (type->u.width == 64) + dst->u.reg.u.immconst_double[0] = bitcast_uint64_to_double(record->operands[0]); + else + vkd3d_unreachable(); + + break; + + case CST_CODE_DATA: + WARN("Unhandled constant array.\n"); + break; + + default: + FIXME("Unhandled constant code %u.\n", record->code); + break; + } + + ++sm6->value_count; + } + + return VKD3D_OK; +} + static enum vkd3d_result sm6_parser_globals_init(struct sm6_parser *sm6) { const struct dxil_block *block = &sm6->root_block; @@ -1445,6 +1714,41 @@ static enum vkd3d_result sm6_parser_globals_init(struct sm6_parser *sm6) return VKD3D_OK; } +static enum vkd3d_result sm6_parser_module_init(struct sm6_parser *sm6, const struct dxil_block *block, + unsigned int level) +{ + enum vkd3d_result ret; + size_t i; + + for (i = 0; i < block->child_block_count; ++i) + { + if ((ret = sm6_parser_module_init(sm6, block->child_blocks[i], level + 1)) < 0) + return ret; + } + + switch (block->id) + { + case CONSTANTS_BLOCK: + return sm6_parser_constants_init(sm6, block); + + case BLOCKINFO_BLOCK: + case MODULE_BLOCK: + case PARAMATTR_BLOCK: + case PARAMATTR_GROUP_BLOCK: + case VALUE_SYMTAB_BLOCK: + case METADATA_BLOCK: + case METADATA_ATTACHMENT_BLOCK: + case TYPE_BLOCK: + break; + + default: + FIXME("Unhandled block id %u.\n", block->id); + break; + } + + return VKD3D_OK; +} + static void sm6_type_table_cleanup(struct sm6_type *types, size_t count) { size_t i; @@ -1656,7 +1960,13 @@ static enum vkd3d_result sm6_parser_init(struct sm6_parser *sm6, const uint32_t return ret; } - sm6_parser_compute_max_value_count(sm6, &sm6->root_block); + if (sm6_parser_compute_max_value_count(sm6, &sm6->root_block, 0) == SIZE_MAX) + { + WARN("Value array count overflowed.\n"); + vkd3d_shader_error(message_context, &location, VKD3D_SHADER_ERROR_DXIL_INVALID_MODULE, + "Overflow occurred in the DXIL module value count."); + return VKD3D_ERROR_INVALID_SHADER; + } if (!(sm6->values = vkd3d_calloc(sm6->value_capacity, sizeof(*sm6->values)))) { ERR("Failed to allocate value array.\n"); @@ -1671,6 +1981,19 @@ static enum vkd3d_result sm6_parser_init(struct sm6_parser *sm6, const uint32_t return ret; } + if ((ret = sm6_parser_module_init(sm6, &sm6->root_block, 0)) < 0) + { + if (ret == VKD3D_ERROR_OUT_OF_MEMORY) + vkd3d_shader_error(message_context, &location, VKD3D_SHADER_ERROR_DXIL_OUT_OF_MEMORY, + "Out of memory parsing DXIL module."); + else if (ret == VKD3D_ERROR_INVALID_SHADER) + vkd3d_shader_error(message_context, &location, VKD3D_SHADER_ERROR_DXIL_INVALID_MODULE, + "DXIL module is invalid."); + else + vkd3d_unreachable(); + return ret; + } + dxil_block_destroy(&sm6->root_block); return VKD3D_OK; diff --git a/libs/vkd3d-shader/vkd3d_shader_private.h b/libs/vkd3d-shader/vkd3d_shader_private.h index 5824dfaf6..3e4d2a703 100644 --- a/libs/vkd3d-shader/vkd3d_shader_private.h +++ b/libs/vkd3d-shader/vkd3d_shader_private.h @@ -159,6 +159,7 @@ enum vkd3d_shader_error VKD3D_SHADER_ERROR_DXIL_INVALID_VALUE_SYMTAB = 8006, VKD3D_SHADER_ERROR_DXIL_UNSUPPORTED_BITCODE_FORMAT = 8007, VKD3D_SHADER_ERROR_DXIL_INVALID_FUNCTION_DCL = 8008, + VKD3D_SHADER_ERROR_DXIL_INVALID_MODULE = 8009, VKD3D_SHADER_WARNING_DXIL_UNKNOWN_MAGIC_NUMBER = 8300, VKD3D_SHADER_WARNING_DXIL_UNKNOWN_SHADER_TYPE = 8301, @@ -533,6 +534,7 @@ enum vkd3d_data_type VKD3D_DATA_DOUBLE, VKD3D_DATA_CONTINUED, VKD3D_DATA_UNUSED, + VKD3D_DATA_UINT8, }; enum vkd3d_immconst_type -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/259
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- libs/vkd3d-shader/dxil.c | 261 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 259 insertions(+), 2 deletions(-) diff --git a/libs/vkd3d-shader/dxil.c b/libs/vkd3d-shader/dxil.c index 32e12266b..e36195b01 100644 --- a/libs/vkd3d-shader/dxil.c +++ b/libs/vkd3d-shader/dxil.c @@ -90,6 +90,29 @@ enum bitcode_constant_code CST_CODE_DATA = 22, }; +enum bitcode_function_code +{ + FUNC_CODE_DECLAREBLOCKS = 1, + FUNC_CODE_INST_BINOP = 2, + FUNC_CODE_INST_CAST = 3, + FUNC_CODE_INST_RET = 10, + FUNC_CODE_INST_BR = 11, + FUNC_CODE_INST_SWITCH = 12, + FUNC_CODE_INST_PHI = 16, + FUNC_CODE_INST_ALLOCA = 19, + FUNC_CODE_INST_LOAD = 20, + FUNC_CODE_INST_EXTRACTVAL = 26, + FUNC_CODE_INST_CMP2 = 28, + FUNC_CODE_INST_VSELECT = 29, + FUNC_CODE_INST_CALL = 34, + FUNC_CODE_INST_ATOMICRMW = 38, + FUNC_CODE_INST_LOADATOMIC = 41, + FUNC_CODE_INST_GEP = 43, + FUNC_CODE_INST_STORE = 44, + FUNC_CODE_INST_STOREATOMIC = 45, + FUNC_CODE_INST_CMPXCHG = 46, +}; + enum bitcode_type_code { TYPE_CODE_NUMENTRY = 1, @@ -205,6 +228,21 @@ struct sm6_symbol const char *name; }; +struct sm6_block +{ + struct vkd3d_shader_instruction *instructions; + size_t instruction_capacity; + size_t instruction_count; +}; + +struct sm6_function +{ + const struct sm6_value *declaration; + + struct sm6_block *blocks[1]; + size_t block_count; +}; + struct dxil_block { const struct dxil_block *parent; @@ -248,6 +286,9 @@ struct sm6_parser struct sm6_symbol *global_symbols; size_t global_symbol_count; + struct sm6_function *functions; + size_t function_count; + struct sm6_value *values; size_t value_count; size_t value_capacity; @@ -857,6 +898,16 @@ static enum vkd3d_result dxil_block_init(struct dxil_block *block, const struct return ret; } +static size_t dxil_block_compute_function_count(const struct dxil_block *root) +{ + size_t i, count; + + for (i = 0, count = 0; i < root->child_block_count; ++i) + count += root->child_blocks[i]->id == FUNCTION_BLOCK; + + return count; +} + static size_t dxil_block_compute_module_decl_count(const struct dxil_block *block) { size_t i, count; @@ -1231,6 +1282,11 @@ static inline bool sm6_type_is_function(const struct sm6_type *type) return type->class == TYPE_CLASS_FUNCTION; } +static inline bool sm6_type_is_function_pointer(const struct sm6_type *type) +{ + return sm6_type_is_pointer(type) && sm6_type_is_function(type->u.pointer.type); +} + static inline bool sm6_type_is_handle(const struct sm6_type *type) { return sm6_type_is_struct(type) && !strcmp(type->u.struc->name, "dx.types.Handle"); @@ -1423,6 +1479,9 @@ static size_t sm6_parser_compute_max_value_count(struct sm6_parser *sm6, value_count = size_add_with_overflow_check(value_count, dxil_block_compute_constants_count(block)); break; case FUNCTION_BLOCK: + /* A function must start with a block count, which emits no value. This formula is likely to + * overestimate the value count somewhat, but this should be no problem. */ + value_count = size_add_with_overflow_check(value_count, max(block->record_count, 1u) - 1); sm6->value_capacity = max(sm6->value_capacity, value_count); /* The value count returns to its previous value after handling a function. */ if (value_count < SIZE_MAX) @@ -1435,6 +1494,21 @@ static size_t sm6_parser_compute_max_value_count(struct sm6_parser *sm6, return value_count; } +#define DXIL_RECORD_VALIDATE_OPERAND_MIN_COUNT(min_count) do {\ + if (record->operand_count < (min_count))\ + {\ + WARN("Invalid operand count %u.\n", record->operand_count);\ + return;\ + } } while (false) + +#define DXIL_RECORD_VALIDATE_OPERAND_MAX_COUNT(max_count) do {\ + if (record->operand_count > (max_count))\ + WARN("Ignoring %u extra operands.\n", record->operand_count - (max_count)); } while (false) + +#define DXIL_RECORD_VALIDATE_OPERAND_COUNT(min_count, max_count) do {\ + DXIL_RECORD_VALIDATE_OPERAND_MIN_COUNT(min_count);\ + DXIL_RECORD_VALIDATE_OPERAND_MAX_COUNT(max_count); } while (false) + static bool sm6_parser_declare_function(struct sm6_parser *sm6, const struct dxil_record *record) { const struct sm6_type *ret_type; @@ -1714,11 +1788,155 @@ static enum vkd3d_result sm6_parser_globals_init(struct sm6_parser *sm6) return VKD3D_OK; } +static const struct sm6_value *sm6_parser_next_function_definition(struct sm6_parser *sm6) +{ + size_t i, count = sm6->function_count; + + for (i = 0; i < sm6->value_count; ++i) + { + if (sm6_type_is_function_pointer(sm6->values[i].type) && !sm6->values[i].u.function.is_prototype && !count--) + break; + } + if (i == sm6->value_count) + return NULL; + + ++sm6->function_count; + return &sm6->values[i]; +} + +static struct sm6_block *sm6_block_create() +{ + struct sm6_block *block = vkd3d_calloc(1, sizeof(*block)); + return block; +} + +static void sm6_parser_emit_ret(struct sm6_parser *sm6, const struct dxil_record *record, + struct sm6_block *code_block, struct vkd3d_shader_instruction *ins) +{ + DXIL_RECORD_VALIDATE_OPERAND_COUNT(0, 1); + + if (record->operand_count) + FIXME("Non-void return is not implemented.\n"); + + ins->handler_idx = VKD3DSIH_NOP; +} + +static enum vkd3d_result sm6_parser_function_init(struct sm6_parser *sm6, const struct dxil_block *block, + struct sm6_function *function) +{ + struct vkd3d_shader_instruction *ins; + const struct dxil_record *record; + struct sm6_block *code_block; + struct sm6_value *dst; + size_t i, block_idx; + bool ret_found; + enum + { + RESULT_VALUE, + RESULT_TERMINATE, + } result_type; + + if (sm6->function_count) + { + FIXME("Multiple functions are not supported yet.\n"); + return VKD3D_ERROR_INVALID_SHADER; + } + if (!(function->declaration = sm6_parser_next_function_definition(sm6))) + { + WARN("Failed to find definition to match function body.\n"); + return VKD3D_ERROR_INVALID_SHADER; + } + + if (block->record_count < 2) + { + /* It should contain at least a block count and a RET instruction. */ + WARN("Invalid function block record count %zu.\n", block->record_count); + return VKD3D_ERROR_INVALID_SHADER; + } + if (block->records[0]->code != FUNC_CODE_DECLAREBLOCKS || !block->records[0]->operand_count + || block->records[0]->operands[0] > UINT_MAX) + { + WARN("Block count declaration not found or invalid.\n"); + return VKD3D_ERROR_INVALID_SHADER; + } + + if (!(function->block_count = block->records[0]->operands[0])) + { + WARN("Function contains no blocks.\n"); + return VKD3D_ERROR_INVALID_SHADER; + } + if (function->block_count > 1) + { + FIXME("Branched shaders are not supported yet.\n"); + return VKD3D_ERROR_INVALID_SHADER; + } + + if (!(function->blocks[0] = sm6_block_create())) + { + ERR("Failed to allocate code block.\n"); + return VKD3D_ERROR_OUT_OF_MEMORY; + } + code_block = function->blocks[0]; + + for (i = 1, block_idx = 0, ret_found = false; i < block->record_count; ++i) + { + /* block->record_count - 1 is the instruction count, but some instructions + * can emit >1 IR instruction, so extra may be used. */ + if (!vkd3d_array_reserve((void **)&code_block->instructions, &code_block->instruction_capacity, + max(code_block->instruction_count + 1, block->record_count), sizeof(*code_block->instructions))) + { + ERR("Failed to allocate instructions.\n"); + return VKD3D_ERROR_OUT_OF_MEMORY; + } + + ins = &code_block->instructions[code_block->instruction_count]; + ins->handler_idx = VKD3DSIH_INVALID; + + dst = sm6_parser_get_current_value(sm6); + dst->type = NULL; + dst->value_type = VALUE_TYPE_REG; + result_type = RESULT_VALUE; + + record = block->records[i]; + switch (record->code) + { + case FUNC_CODE_INST_RET: + sm6_parser_emit_ret(sm6, record, code_block, ins); + result_type = RESULT_TERMINATE; + ret_found = true; + break; + default: + FIXME("Unhandled dxil instruction %u.\n", record->code); + return VKD3D_ERROR_INVALID_SHADER; + } + + if (result_type == RESULT_TERMINATE) + { + ++block_idx; + code_block = (block_idx < function->block_count) ? function->blocks[block_idx] : NULL; + } + if (code_block) + code_block->instruction_count += ins->handler_idx != VKD3DSIH_NOP; + else + assert(ins->handler_idx == VKD3DSIH_NOP); + sm6->value_count += !!dst->type; + } + + if (!ret_found) + { + WARN("Function contains no RET instruction.\n"); + return VKD3D_ERROR_INVALID_SHADER; + } + + return VKD3D_OK; +} + static enum vkd3d_result sm6_parser_module_init(struct sm6_parser *sm6, const struct dxil_block *block, unsigned int level) { + size_t i, old_value_count = sm6->value_count; + struct sm6_function *function; enum vkd3d_result ret; - size_t i; for (i = 0; i < block->child_block_count; ++i) { @@ -1731,6 +1949,17 @@ static enum vkd3d_result sm6_parser_module_init(struct sm6_parser *sm6, const st case CONSTANTS_BLOCK: return sm6_parser_constants_init(sm6, block); + case FUNCTION_BLOCK: + function = &sm6->functions[sm6->function_count]; + if ((ret = sm6_parser_function_init(sm6, block, function)) < 0) + return ret; + /* The value index returns to its previous value after handling a function. It's usually nonzero + * at the start because of global constants/variables/function declarations. Function constants + * occur in a child block, so value_count is already saved before they are emitted. */ + memset(&sm6->values[old_value_count], 0, (sm6->value_count - old_value_count) * sizeof(*sm6->values)); + sm6->value_count = old_value_count; + break; + case BLOCKINFO_BLOCK: case MODULE_BLOCK: case PARAMATTR_BLOCK: @@ -1784,6 +2013,24 @@ static void sm6_symtab_cleanup(struct sm6_symbol *symbols, size_t count) vkd3d_free(symbols); } +static void sm6_block_destroy(struct sm6_block *block) +{ + vkd3d_free(block->instructions); + vkd3d_free(block); +} + +static void sm6_functions_cleanup(struct sm6_function *functions, size_t count) +{ + size_t i, j; + + for (i = 0; i < count; ++i) + { + for (j = 0; j < functions[i].block_count; ++j) + sm6_block_destroy(functions[i].blocks[j]); + } + vkd3d_free(functions); +} + static void sm6_parser_destroy(struct vkd3d_shader_parser *parser) { struct sm6_parser *sm6 = sm6_parser(parser); @@ -1793,6 +2040,7 @@ static void sm6_parser_destroy(struct vkd3d_shader_parser *parser) shader_instruction_array_destroy(&parser->instructions); sm6_type_table_cleanup(sm6->types, sm6->type_count); sm6_symtab_cleanup(sm6->global_symbols, sm6->global_symbol_count); + sm6_functions_cleanup(sm6->functions, sm6->function_count); vkd3d_free(sm6->values); free_shader_desc(&parser->shader_desc); vkd3d_free(sm6); @@ -1809,11 +2057,11 @@ static enum vkd3d_result sm6_parser_init(struct sm6_parser *sm6, const uint32_t const struct vkd3d_shader_location location = {.source_name = source_name}; uint32_t version_token, dxil_version, token_count, magic; unsigned int chunk_offset, chunk_size; + size_t count, length, function_count; enum bitcode_block_abbreviation abbr; struct vkd3d_shader_version version; struct dxil_block *block; enum vkd3d_result ret; - size_t count, length; count = byte_code_size / sizeof(*byte_code); if (count < 6) @@ -1960,6 +2208,15 @@ static enum vkd3d_result sm6_parser_init(struct sm6_parser *sm6, const uint32_t return ret; } + function_count = dxil_block_compute_function_count(&sm6->root_block); + if (!(sm6->functions = vkd3d_calloc(function_count, sizeof(*sm6->functions)))) + { + ERR("Failed to allocate function array.\n"); + vkd3d_shader_error(message_context, &location, VKD3D_SHADER_ERROR_DXIL_OUT_OF_MEMORY, + "Out of memory allocating DXIL function array."); + return VKD3D_ERROR_OUT_OF_MEMORY; + } + if (sm6_parser_compute_max_value_count(sm6, &sm6->root_block, 0) == SIZE_MAX) { WARN("Value array count overflowed.\n"); -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/259
On Fri Jul 7 13:27:41 2023 +0000, Conor McCarthy wrote:
The "plus 1" is to allow the alignment to be unspecified, and I have never encountered a nonzero value here. Ok, got it.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/259#note_38500
```diff +#define TYPE_RECORD_VALIDATE_OPERAND_MIN_COUNT(min_count) do {\ + if (record->operand_count < (min_count))\ + {\ + WARN("Invalid operand count %u for type code %u, id %zu.\n", record->operand_count, record->code, type_index);\ + return VKD3D_ERROR_INVALID_SHADER;\ + } } while (false) + +#define TYPE_RECORD_VALIDATE_OPERAND_MAX_COUNT(max_count) do {\ + if (record->operand_count > (max_count))\ + WARN("Ignoring %u extra operands for type code %u, id %zu.\n", record->operand_count - (max_count),\ + record->code, type_index); } while (false) + +#define TYPE_RECORD_VALIDATE_OPERAND_COUNT(min_count, max_count) do {\ + TYPE_RECORD_VALIDATE_OPERAND_MIN_COUNT(min_count);\ + TYPE_RECORD_VALIDATE_OPERAND_MAX_COUNT(max_count); } while (false) ```
Do we really need these to be macros? It seems like the only reason these aren't regular functions is the usage of WARN, and I'm inclined to say we should be using something built on top of vkd3d_shader_verror()/vkd3d_shader_vwarning() anyway, much like e.g. hlsl_error() and hlsl_warning(). Not just here, of course. I do want to resolve error reporting/handling sooner rather than later. While to some extent that's something that can be tweaked later case-by-case, it's also something that both impacts the basic structure of the parser, and something that's generally settled for the other vkd3d-shader parsers. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/259#note_38517
On Mon Jul 10 14:11:19 2023 +0000, Henri Verbeet wrote:
```diff +#define TYPE_RECORD_VALIDATE_OPERAND_MIN_COUNT(min_count) do {\ + if (record->operand_count < (min_count))\ + {\ + WARN("Invalid operand count %u for type code %u, id %zu.\n",
record->operand_count, record->code, type_index);\
+ return VKD3D_ERROR_INVALID_SHADER;\ + } } while (false) + +#define TYPE_RECORD_VALIDATE_OPERAND_MAX_COUNT(max_count) do {\ + if (record->operand_count > (max_count))\ + WARN("Ignoring %u extra operands for type code %u, id %zu.\n", record->operand_count - (max_count),\ + record->code, type_index); } while (false) + +#define TYPE_RECORD_VALIDATE_OPERAND_COUNT(min_count, max_count) do {\ + TYPE_RECORD_VALIDATE_OPERAND_MIN_COUNT(min_count);\ + TYPE_RECORD_VALIDATE_OPERAND_MAX_COUNT(max_count); } while (false) ``` Do we really need these to be macros? It seems like the only reason these aren't regular functions is the usage of WARN, and I'm inclined to say we should be using something built on top of vkd3d_shader_verror()/vkd3d_shader_vwarning() anyway, much like e.g. hlsl_error() and hlsl_warning(). Not just here, of course. I do want to resolve error reporting/handling sooner rather than later. While to some extent that's something that can be tweaked later case-by-case, it's also something that both impacts the basic structure of the parser, and something that's generally settled for the other vkd3d-shader parsers. I declared `sm6_parser_get_type()` with a `const char *function` parameter and use a `sm6_parser_get_type` macro to insert `__FUNCTION__` into each call. Is that an acceptable approach generally? Validating operand counts would be neater if done that way.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/259#note_38520
I declared `sm6_parser_get_type()` with a `const char *function` parameter and use a `sm6_parser_get_type` macro to insert `__FUNCTION__` into each call. Is that an acceptable approach generally? Validating operand counts would be neater if done that way.
Generally, sure; we do that kind of thing for the check_readback_data_*() helpers in the tests for example. It doesn't seem particularly appropriate here though; I think we care much more about the location/context of the offending code in the source shader than about the location of the code parsing it. The latter would generally be implied by the former anyway. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/259#note_38546
participants (4)
-
Conor McCarthy -
Conor McCarthy (＠cmccarthy)
-
Giovanni Mascellani (＠giomasce)
-
Henri Verbeet (＠hverbeet)