vkd3d-shader/dxil: Read the DXIL module.
Merge request reports
Activity
- Resolved by Giovanni Mascellani
- Resolved by Giovanni Mascellani
- Resolved by Giovanni Mascellani
- Resolved by Giovanni Mascellani
- Resolved by Giovanni Mascellani
- Resolved by Giovanni Mascellani
- Resolved by Giovanni Mascellani
- Resolved by Giovanni Mascellani
1305 for (i = 0; i < block->child_block_count; ++i) 1306 value_count = sm6_parser_compute_max_value_count(sm6, block->child_blocks[i], value_count); 1307 1308 switch (block->id) 1309 { 1310 case FUNCTION_BLOCK: 1311 sm6->value_capacity = max(sm6->value_capacity, value_count); 1312 /* The value count returns to its previous value after handling a function. */ 1313 value_count = old_value_count; 1314 break; 1315 default: 1316 break; 1317 } 1318 1319 return value_count; 1320 } - Comment on lines +1297 to +1320
I don't understand what this function is supposed to calculate. At least for the moment, unless I'm missing something, the number of values that get allocated should exactly be the number of
MODULE_CODE_FUNCTION
records in the root block: why can't you just compute that thing and need something more complicated?Relatedly, and even more if we need this complicated function to decide how many values we need to store, maybe
sm6_parser_get_current_value()
couldassert()
that we're not overflowing.
- Resolved by Giovanni Mascellani
added 6 commits
- 920b530b - vkd3d-shader/dxil: Read the type table.
- b239968c - vkd3d-shader/dxil: Read the value symbol table.
- c67d9e16 - vkd3d-shader/dxil: Validate the module format version.
- 08633ad9 - vkd3d-shader/dxil: Read global function declarations.
- 2501855f - vkd3d-shader/dxil: Read numeric constants.
- 58eb3d1c - vkd3d-shader/dxil: Read function bodies.
Toggle commit list1186 1187 for (i = 0; i < block->record_count; ++i) 1188 { 1189 record = block->records[i]; 1190 switch (record->code) 1191 { 1192 case MODULE_CODE_FUNCTION: 1193 FIXME("Functions are not implemented yet.\n"); 1194 break; 1195 1196 case MODULE_CODE_GLOBALVAR: 1197 FIXME("Global variables are not implemented yet.\n"); 1198 break; 1199 1200 case MODULE_CODE_VERSION: 1201 if (!record->operand_count) changed this line in version 5 of the diff
1532 enum vkd3d_shader_register_type reg_type = VKD3DSPR_INVALID; 1533 const struct sm6_type *type, *elem_type; 1534 enum vkd3d_data_type reg_data_type; 1535 const struct dxil_record *record; 1536 unsigned int i, value_idx; 1537 struct sm6_value *dst; 1538 uint64_t value; 1539 1540 for (i = 0, type = NULL; i < block->record_count; ++i) 1541 { 1542 record = block->records[i]; 1543 value_idx = sm6->value_count; 1544 1545 if (record->code == CST_CODE_SETTYPE) 1546 { 1547 if (!record->operand_count) changed this line in version 5 of the diff
It's not possible for a DXBC chuck to contain enough records to cause overflow in value_count. I would add a check anyway if it was reasonably simple, but it's a bit messy. EDIT: the compression likely makes it possible so I'll see what can be done.
Edited by Conor McCarthyadded 7 commits
- f4eda564 - vkd3d-shader/dxil: Use size_t where applicable.
- 8330d35e - vkd3d-shader/dxil: Read the type table.
- 21e50ce3 - vkd3d-shader/dxil: Read the value symbol table.
- a88e9f68 - vkd3d-shader/dxil: Validate the module format version.
- bc94c2ab - vkd3d-shader/dxil: Read global function declarations.
- dc70c57c - vkd3d-shader/dxil: Read numeric constants.
- f7a9e309 - vkd3d-shader/dxil: Read function bodies.
Toggle commit list+#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.