[PATCH vkd3d 1/2] vkd3d-shader: Introduce shader_read_error() for reporting via vkd3d_shader_verror().
Signed-off-by: Conor McCarthy [email protected] --- libs/vkd3d-shader/dxbc.c | 20 +++++++++++++++++++- libs/vkd3d-shader/vkd3d_shader_main.c | 2 +- libs/vkd3d-shader/vkd3d_shader_private.h | 6 +++++- 3 files changed, 25 insertions(+), 3 deletions(-)
diff --git a/libs/vkd3d-shader/dxbc.c b/libs/vkd3d-shader/dxbc.c index ce4cecff..e3a8351e 100644 --- a/libs/vkd3d-shader/dxbc.c +++ b/libs/vkd3d-shader/dxbc.c @@ -489,6 +489,8 @@ struct vkd3d_sm4_data struct list src_free; struct list src; struct vkd3d_shader_immediate_constant_buffer icb; + + struct vkd3d_shader_message_context *message_context; };
struct vkd3d_sm4_opcode_info @@ -561,6 +563,16 @@ static bool shader_is_sm_5_1(const struct vkd3d_sm4_data *priv) return version->major >= 5 && version->minor >= 1; }
+static void VKD3D_PRINTF_FUNC(3, 4) shader_read_error(struct vkd3d_sm4_data *priv, + enum vkd3d_shader_error error, const char *format, ...) +{ + va_list args; + + va_start(args, format); + vkd3d_shader_verror(priv->message_context, NULL, error, format, args); + va_end(args); +} + static bool shader_sm4_read_src_param(struct vkd3d_sm4_data *priv, const DWORD **ptr, const DWORD *end, enum vkd3d_data_type data_type, struct vkd3d_shader_src_param *src_param); static bool shader_sm4_read_dst_param(struct vkd3d_sm4_data *priv, const DWORD **ptr, const DWORD *end, @@ -629,7 +641,11 @@ static void shader_sm4_read_descriptor_register_range(struct vkd3d_sm4_data *pri range->first = reg->idx[shader_is_sm_5_1(priv) ? 1 : 0].offset; range->last = reg->idx[shader_is_sm_5_1(priv) ? 2 : 0].offset; if (range->last < range->first) + { FIXME("Invalid register range [%u:%u].\n", range->first, range->last); + shader_read_error(priv, VKD3D_SHADER_ERROR_SPV_INVALID_REGISTER_RANGE, + "Last register %u must not be less than first register %u in range.\n", range->last, range->first); + } }
static void shader_sm4_read_dcl_resource(struct vkd3d_shader_instruction *ins, @@ -1343,7 +1359,7 @@ static enum vkd3d_data_type map_data_type(char t) }
void *shader_sm4_init(const DWORD *byte_code, size_t byte_code_size, - const struct vkd3d_shader_signature *output_signature) + const struct vkd3d_shader_signature *output_signature, struct vkd3d_shader_message_context *message_context) { DWORD version_token, token_count; struct vkd3d_sm4_data *priv; @@ -1424,6 +1440,8 @@ void *shader_sm4_init(const DWORD *byte_code, size_t byte_code_size, list_init(&priv->src_free); list_init(&priv->src);
+ priv->message_context = message_context; + return priv; }
diff --git a/libs/vkd3d-shader/vkd3d_shader_main.c b/libs/vkd3d-shader/vkd3d_shader_main.c index 40d55b70..54654f3f 100644 --- a/libs/vkd3d-shader/vkd3d_shader_main.c +++ b/libs/vkd3d-shader/vkd3d_shader_main.c @@ -344,7 +344,7 @@ static int vkd3d_shader_parser_init(struct vkd3d_shader_parser *parser, }
if (!(parser->data = shader_sm4_init(shader_desc->byte_code, - shader_desc->byte_code_size, &shader_desc->output_signature))) + shader_desc->byte_code_size, &shader_desc->output_signature, message_context))) { WARN("Failed to initialize shader parser.\n"); free_shader_desc(shader_desc); diff --git a/libs/vkd3d-shader/vkd3d_shader_private.h b/libs/vkd3d-shader/vkd3d_shader_private.h index c61b3773..5e107a6b 100644 --- a/libs/vkd3d-shader/vkd3d_shader_private.h +++ b/libs/vkd3d-shader/vkd3d_shader_private.h @@ -73,6 +73,7 @@ enum vkd3d_shader_error VKD3D_SHADER_ERROR_SPV_DESCRIPTOR_BINDING_NOT_FOUND = 2000, VKD3D_SHADER_ERROR_SPV_INVALID_REGISTER_TYPE = 2001, VKD3D_SHADER_ERROR_SPV_INVALID_DESCRIPTOR_BINDING = 2002, + VKD3D_SHADER_ERROR_SPV_INVALID_REGISTER_RANGE = 2003,
VKD3D_SHADER_ERROR_RS_OUT_OF_MEMORY = 3000, VKD3D_SHADER_ERROR_RS_INVALID_VERSION = 3001, @@ -861,8 +862,11 @@ void vkd3d_shader_trace(void *data) DECLSPEC_HIDDEN;
const char *shader_get_type_prefix(enum vkd3d_shader_type type) DECLSPEC_HIDDEN;
+struct vkd3d_shader_message_context; + void *shader_sm4_init(const DWORD *byte_code, size_t byte_code_size, - const struct vkd3d_shader_signature *output_signature) DECLSPEC_HIDDEN; + const struct vkd3d_shader_signature *output_signature, + struct vkd3d_shader_message_context *message_context) DECLSPEC_HIDDEN; void shader_sm4_free(void *data) DECLSPEC_HIDDEN; void shader_sm4_read_header(void *data, const DWORD **ptr, struct vkd3d_shader_version *shader_version) DECLSPEC_HIDDEN;
Signed-off-by: Conor McCarthy [email protected] --- libs/vkd3d-shader/dxbc.c | 17 +++++++++++------ libs/vkd3d-shader/trace.c | 15 +++++++++++---- libs/vkd3d-shader/vkd3d_shader_main.c | 10 ++++------ libs/vkd3d-shader/vkd3d_shader_private.h | 2 +- 4 files changed, 27 insertions(+), 17 deletions(-)
diff --git a/libs/vkd3d-shader/dxbc.c b/libs/vkd3d-shader/dxbc.c index e3a8351e..8aea7525 100644 --- a/libs/vkd3d-shader/dxbc.c +++ b/libs/vkd3d-shader/dxbc.c @@ -491,6 +491,7 @@ struct vkd3d_sm4_data struct vkd3d_shader_immediate_constant_buffer icb;
struct vkd3d_shader_message_context *message_context; + bool valid; };
struct vkd3d_sm4_opcode_info @@ -571,6 +572,8 @@ static void VKD3D_PRINTF_FUNC(3, 4) shader_read_error(struct vkd3d_sm4_data *pri va_start(args, format); vkd3d_shader_verror(priv->message_context, NULL, error, format, args); va_end(args); + + priv->valid = false; }
static bool shader_sm4_read_src_param(struct vkd3d_sm4_data *priv, const DWORD **ptr, const DWORD *end, @@ -1441,6 +1444,7 @@ void *shader_sm4_init(const DWORD *byte_code, size_t byte_code_size, list_init(&priv->src);
priv->message_context = message_context; + priv->valid = true;
return priv; } @@ -1878,7 +1882,7 @@ static void shader_sm4_read_instruction_modifier(DWORD modifier, struct vkd3d_sh } }
-void shader_sm4_read_instruction(void *data, const DWORD **ptr, struct vkd3d_shader_instruction *ins) +bool shader_sm4_read_instruction(void *data, const DWORD **ptr, struct vkd3d_shader_instruction *ins) { const struct vkd3d_sm4_opcode_info *opcode_info; DWORD opcode_token, opcode, previous_token; @@ -1922,7 +1926,7 @@ void shader_sm4_read_instruction(void *data, const DWORD **ptr, struct vkd3d_sha FIXME("Unrecognized opcode %#x, opcode_token 0x%08x.\n", opcode, opcode_token); ins->handler_idx = VKD3DSIH_INVALID; *ptr += len; - return; + return false; }
ins->handler_idx = opcode_info->handler_idx; @@ -1949,6 +1953,7 @@ void shader_sm4_read_instruction(void *data, const DWORD **ptr, struct vkd3d_sha if (opcode_info->read_opcode_func) { opcode_info->read_opcode_func(ins, opcode, opcode_token, p, len, priv); + return priv->valid; } else { @@ -1973,7 +1978,7 @@ void shader_sm4_read_instruction(void *data, const DWORD **ptr, struct vkd3d_sha &priv->dst_param[i]))) { ins->handler_idx = VKD3DSIH_INVALID; - return; + return false; } priv->dst_param[i].modifiers |= instruction_dst_modifier; } @@ -1984,17 +1989,17 @@ void shader_sm4_read_instruction(void *data, const DWORD **ptr, struct vkd3d_sha &priv->src_param[i]))) { ins->handler_idx = VKD3DSIH_INVALID; - return; + return false; } } }
- return; + return true;
fail: *ptr = priv->end; ins->handler_idx = VKD3DSIH_INVALID; - return; + return false; }
bool shader_sm4_is_end(void *data, const DWORD **ptr) diff --git a/libs/vkd3d-shader/trace.c b/libs/vkd3d-shader/trace.c index 2d16714b..5a766f65 100644 --- a/libs/vkd3d-shader/trace.c +++ b/libs/vkd3d-shader/trace.c @@ -1751,11 +1751,18 @@ enum vkd3d_result vkd3d_dxbc_binary_to_text(void *data, { struct vkd3d_shader_instruction ins;
- shader_sm4_read_instruction(data, &ptr, &ins); - if (ins.handler_idx == VKD3DSIH_INVALID) + if (!shader_sm4_read_instruction(data, &ptr, &ins)) { - WARN("Skipping unrecognized instruction.\n"); - vkd3d_string_buffer_printf(buffer, "<unrecognized instruction>\n"); + if (ins.handler_idx == VKD3DSIH_INVALID) + { + WARN("Skipping unrecognized instruction.\n"); + vkd3d_string_buffer_printf(buffer, "<unrecognized instruction>\n"); + } + else + { + WARN("Skipping invalid instruction.\n"); + vkd3d_string_buffer_printf(buffer, "<invalid instruction>\n"); + } result = VKD3D_ERROR; continue; } diff --git a/libs/vkd3d-shader/vkd3d_shader_main.c b/libs/vkd3d-shader/vkd3d_shader_main.c index 54654f3f..3e5bab35 100644 --- a/libs/vkd3d-shader/vkd3d_shader_main.c +++ b/libs/vkd3d-shader/vkd3d_shader_main.c @@ -919,9 +919,8 @@ static int scan_dxbc(const struct vkd3d_shader_compile_info *compile_info,
while (!shader_sm4_is_end(parser.data, &parser.ptr)) { - shader_sm4_read_instruction(parser.data, &parser.ptr, &instruction); - - if (instruction.handler_idx == VKD3DSIH_INVALID) + if (!shader_sm4_read_instruction(parser.data, &parser.ptr, &instruction) + || instruction.handler_idx == VKD3DSIH_INVALID) { WARN("Encountered unrecognized or invalid instruction.\n"); if (scan_descriptor_info) @@ -1015,9 +1014,8 @@ static int compile_dxbc_tpf(const struct vkd3d_shader_compile_info *compile_info
while (!shader_sm4_is_end(parser.data, &parser.ptr)) { - shader_sm4_read_instruction(parser.data, &parser.ptr, &instruction); - - if (instruction.handler_idx == VKD3DSIH_INVALID) + if (!shader_sm4_read_instruction(parser.data, &parser.ptr, &instruction) + || instruction.handler_idx == VKD3DSIH_INVALID) { WARN("Encountered unrecognized or invalid instruction.\n"); ret = VKD3D_ERROR_INVALID_SHADER; diff --git a/libs/vkd3d-shader/vkd3d_shader_private.h b/libs/vkd3d-shader/vkd3d_shader_private.h index 5e107a6b..dec2d656 100644 --- a/libs/vkd3d-shader/vkd3d_shader_private.h +++ b/libs/vkd3d-shader/vkd3d_shader_private.h @@ -870,7 +870,7 @@ void *shader_sm4_init(const DWORD *byte_code, size_t byte_code_size, void shader_sm4_free(void *data) DECLSPEC_HIDDEN; void shader_sm4_read_header(void *data, const DWORD **ptr, struct vkd3d_shader_version *shader_version) DECLSPEC_HIDDEN; -void shader_sm4_read_instruction(void *data, const DWORD **ptr, +bool shader_sm4_read_instruction(void *data, const DWORD **ptr, struct vkd3d_shader_instruction *ins) DECLSPEC_HIDDEN; bool shader_sm4_is_end(void *data, const DWORD **ptr) DECLSPEC_HIDDEN;
On Wed, 30 Jun 2021 at 05:11, Conor McCarthy [email protected] wrote:
@@ -571,6 +572,8 @@ static void VKD3D_PRINTF_FUNC(3, 4) shader_read_error(struct vkd3d_sm4_data *pri va_start(args, format); vkd3d_shader_verror(priv->message_context, NULL, error, format, args); va_end(args);
- priv->valid = false;
}
[...]
@@ -1949,6 +1953,7 @@ void shader_sm4_read_instruction(void *data, const DWORD **ptr, struct vkd3d_sha if (opcode_info->read_opcode_func) { opcode_info->read_opcode_func(ins, opcode, opcode_token, p, len, priv);
return priv->valid;
I think that's a bit too coarse. In particular, if an error is not fatal (like for example the invalid register range one), it's often useful to continue parsing and also report subsequent errors and warnings.
On Wed, 30 Jun 2021 at 05:11, Conor McCarthy [email protected] wrote:
+static void VKD3D_PRINTF_FUNC(3, 4) shader_read_error(struct vkd3d_sm4_data *priv,
enum vkd3d_shader_error error, const char *format, ...)+{
- va_list args;
- va_start(args, format);
- vkd3d_shader_verror(priv->message_context, NULL, error, format, args);
- va_end(args);
+}
That should probably be called shader_sm4_error(). We'd also want to keep track of the source location, although that doesn't need to be in this patch.
static bool shader_sm4_read_src_param(struct vkd3d_sm4_data *priv, const DWORD **ptr, const DWORD *end, enum vkd3d_data_type data_type, struct vkd3d_shader_src_param *src_param); static bool shader_sm4_read_dst_param(struct vkd3d_sm4_data *priv, const DWORD **ptr, const DWORD *end, @@ -629,7 +641,11 @@ static void shader_sm4_read_descriptor_register_range(struct vkd3d_sm4_data *pri range->first = reg->idx[shader_is_sm_5_1(priv) ? 1 : 0].offset; range->last = reg->idx[shader_is_sm_5_1(priv) ? 2 : 0].offset; if (range->last < range->first)
- { FIXME("Invalid register range [%u:%u].\n", range->first, range->last);
shader_read_error(priv, VKD3D_SHADER_ERROR_SPV_INVALID_REGISTER_RANGE,"Last register %u must not be less than first register %u in range.\n", range->last, range->first);- }
}
VKD3D_SHADER_ERROR_SPV_ errors are for errors specific to SPIR-V generation. This should be a VKD3D_SHADER_ERROR_TPF_ error.
-
Conor McCarthy -
Henri Verbeet