[PATCH 0/4] MR198: vkd3d-shader: Add more TPF validations.
Introduces a `idx_count` field to `struct vkd3d_shader_register`. Another patch set is needed for further validations and to use this field where applicable. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/198
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- libs/vkd3d-shader/d3dbc.c | 3 ++ libs/vkd3d-shader/ir.c | 6 +--- libs/vkd3d-shader/tpf.c | 42 ++++++++++++++++++++++++ libs/vkd3d-shader/vkd3d_shader_private.h | 2 ++ 4 files changed, 48 insertions(+), 5 deletions(-) diff --git a/libs/vkd3d-shader/d3dbc.c b/libs/vkd3d-shader/d3dbc.c index adc7544c..a6f49b45 100644 --- a/libs/vkd3d-shader/d3dbc.c +++ b/libs/vkd3d-shader/d3dbc.c @@ -465,6 +465,7 @@ static void shader_sm1_parse_src_param(uint32_t param, const struct vkd3d_shader src->reg.idx[1].rel_addr = NULL; src->reg.idx[2].offset = ~0u; src->reg.idx[2].rel_addr = NULL; + src->reg.idx_count = 1; src->swizzle = swizzle_from_sm1((param & VKD3D_SM1_SWIZZLE_MASK) >> VKD3D_SM1_SWIZZLE_SHIFT); src->modifiers = (param & VKD3D_SM1_SRC_MODIFIER_MASK) >> VKD3D_SM1_SRC_MODIFIER_SHIFT; } @@ -483,6 +484,7 @@ static void shader_sm1_parse_dst_param(uint32_t param, const struct vkd3d_shader dst->reg.idx[1].rel_addr = NULL; dst->reg.idx[2].offset = ~0u; dst->reg.idx[2].rel_addr = NULL; + dst->reg.idx_count = 1; dst->write_mask = (param & VKD3D_SM1_WRITEMASK_MASK) >> VKD3D_SM1_WRITEMASK_SHIFT; dst->modifiers = (param & VKD3D_SM1_DST_MODIFIER_MASK) >> VKD3D_SM1_DST_MODIFIER_SHIFT; dst->shift = (param & VKD3D_SM1_DSTSHIFT_MASK) >> VKD3D_SM1_DSTSHIFT_SHIFT; @@ -664,6 +666,7 @@ static void shader_sm1_read_immconst(struct vkd3d_shader_sm1_parser *sm1, const src_param->reg.idx[1].rel_addr = NULL; src_param->reg.idx[2].offset = ~0u; src_param->reg.idx[2].rel_addr = NULL; + src_param->reg.idx_count = 0; src_param->reg.immconst_type = type; memcpy(src_param->reg.u.immconst_uint, *ptr, count * sizeof(uint32_t)); src_param->swizzle = VKD3D_SHADER_NO_SWIZZLE; diff --git a/libs/vkd3d-shader/ir.c b/libs/vkd3d-shader/ir.c index 11ba7563..d0dc2978 100644 --- a/libs/vkd3d-shader/ir.c +++ b/libs/vkd3d-shader/ir.c @@ -300,11 +300,7 @@ static bool shader_dst_param_normalise_outpointid(struct vkd3d_shader_dst_param if (normaliser_is_in_control_point_phase(normaliser) && reg->type == VKD3DSPR_OUTPUT) { - if (reg->idx[2].offset != ~0u) - { - FIXME("Cannot insert phase id.\n"); - return false; - } + assert(reg->idx[2].offset == ~0u); if (reg->idx[1].offset != ~0u) { WARN("Unexpected address at index 1.\n"); diff --git a/libs/vkd3d-shader/tpf.c b/libs/vkd3d-shader/tpf.c index 38d5c180..7a436c3a 100644 --- a/libs/vkd3d-shader/tpf.c +++ b/libs/vkd3d-shader/tpf.c @@ -1669,6 +1669,8 @@ static bool shader_sm4_read_param(struct vkd3d_shader_sm4_parser *priv, const ui return false; } + param->idx_count = order; + if (register_type == VKD3D_SM4_RT_IMMCONST || register_type == VKD3D_SM4_RT_IMMCONST64) { enum vkd3d_sm4_dimension dimension = (token & VKD3D_SM4_DIMENSION_MASK) >> VKD3D_SM4_DIMENSION_SHIFT; @@ -1710,6 +1712,7 @@ static bool shader_sm4_read_param(struct vkd3d_shader_sm4_parser *priv, const ui * other values up one slot. Normalize to SM5.1. */ param->idx[2] = param->idx[1]; param->idx[1] = param->idx[0]; + ++param->idx_count; } map_register(priv, param); @@ -1742,6 +1745,37 @@ static uint32_t swizzle_from_sm4(uint32_t s) return vkd3d_shader_create_swizzle(s & 0x3, (s >> 2) & 0x3, (s >> 4) & 0x3, (s >> 6) & 0x3); } +static bool shader_register_is_input_output(const struct vkd3d_shader_register *reg) +{ + switch (reg->type) + { + case VKD3DSPR_INPUT: + case VKD3DSPR_OUTPUT: + case VKD3DSPR_COLOROUT: + case VKD3DSPR_INCONTROLPOINT: + case VKD3DSPR_OUTCONTROLPOINT: + case VKD3DSPR_PATCHCONST: + return true; + + default: + return false; + } +} + +static bool shader_sm4_validate_input_output_register(struct vkd3d_shader_sm4_parser *priv, + const struct vkd3d_shader_register *reg) +{ + if (!reg->idx_count || reg->idx_count > 2) + { + /* No I/O registers use order 0 or > 2. The normaliser relies on this. TODO: strict validation. */ + vkd3d_shader_parser_error(&priv->p, VKD3D_SHADER_ERROR_TPF_INVALID_REGISTER_INDEX_COUNT, + "Invalid index count %u for register type %#x.\n", reg->idx_count, reg->type); + return false; + } + + return true; +} + static bool shader_sm4_read_src_param(struct vkd3d_shader_sm4_parser *priv, const uint32_t **ptr, const uint32_t *end, enum vkd3d_data_type data_type, struct vkd3d_shader_src_param *src_param) { @@ -1793,6 +1827,10 @@ static bool shader_sm4_read_src_param(struct vkd3d_shader_sm4_parser *priv, cons } } + if (shader_register_is_input_output(&src_param->reg) && !shader_sm4_validate_input_output_register(priv, + &src_param->reg)) + return false; + return true; } @@ -1830,6 +1868,10 @@ static bool shader_sm4_read_dst_param(struct vkd3d_shader_sm4_parser *priv, cons dst_param->modifiers = 0; dst_param->shift = 0; + if (shader_register_is_input_output(&dst_param->reg) && !shader_sm4_validate_input_output_register(priv, + &dst_param->reg)) + return false; + return true; } diff --git a/libs/vkd3d-shader/vkd3d_shader_private.h b/libs/vkd3d-shader/vkd3d_shader_private.h index c06cecb6..63ab7466 100644 --- a/libs/vkd3d-shader/vkd3d_shader_private.h +++ b/libs/vkd3d-shader/vkd3d_shader_private.h @@ -74,6 +74,7 @@ enum vkd3d_shader_error VKD3D_SHADER_ERROR_TPF_MISMATCHED_CF = 1000, VKD3D_SHADER_ERROR_TPF_INVALID_REGISTER_RANGE = 1001, VKD3D_SHADER_ERROR_TPF_OUT_OF_MEMORY = 1002, + VKD3D_SHADER_ERROR_TPF_INVALID_REGISTER_INDEX_COUNT = 1003, VKD3D_SHADER_ERROR_SPV_DESCRIPTOR_BINDING_NOT_FOUND = 2000, VKD3D_SHADER_ERROR_SPV_INVALID_REGISTER_TYPE = 2001, @@ -676,6 +677,7 @@ struct vkd3d_shader_register bool non_uniform; enum vkd3d_data_type data_type; struct vkd3d_shader_register_index idx[3]; + unsigned int idx_count; enum vkd3d_immconst_type immconst_type; union { -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/198
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- libs/vkd3d-shader/tpf.c | 28 ++++++++++++++++++++++++ libs/vkd3d-shader/vkd3d_shader_private.h | 1 + 2 files changed, 29 insertions(+) diff --git a/libs/vkd3d-shader/tpf.c b/libs/vkd3d-shader/tpf.c index 7a436c3a..12f032b6 100644 --- a/libs/vkd3d-shader/tpf.c +++ b/libs/vkd3d-shader/tpf.c @@ -2187,6 +2187,26 @@ static bool shader_sm4_init(struct vkd3d_shader_sm4_parser *sm4, const uint32_t return true; } +static bool shader_sm4_parser_validate_signature(struct vkd3d_shader_sm4_parser *sm4, + const struct vkd3d_shader_signature *signature, const char *name) +{ + unsigned int i, register_idx; + + for (i = 0; i < signature->element_count; ++i) + { + if ((register_idx = signature->elements[i].register_index) >= MAX_REG_OUTPUT && register_idx != ~0u) + { + WARN("%s signature element %u unhandled register index %u.\n", name, i, register_idx); + vkd3d_shader_parser_error(&sm4->p, VKD3D_SHADER_ERROR_TPF_TOO_MANY_REGISTERS, + "%s signature element %u register index %u exceeds maximum index of %u.\n", name, + i, register_idx, MAX_REG_OUTPUT - 1); + return false; + } + } + + return true; +} + int vkd3d_shader_sm4_parser_create(const struct vkd3d_shader_compile_info *compile_info, struct vkd3d_shader_message_context *message_context, struct vkd3d_shader_parser **parser) { @@ -2220,6 +2240,14 @@ int vkd3d_shader_sm4_parser_create(const struct vkd3d_shader_compile_info *compi return VKD3D_ERROR_INVALID_ARGUMENT; } + if (!shader_sm4_parser_validate_signature(sm4, &shader_desc->input_signature, "Input") + || !shader_sm4_parser_validate_signature(sm4, &shader_desc->output_signature, "Output") + || !shader_sm4_parser_validate_signature(sm4, &shader_desc->patch_constant_signature, "Patch constant")) + { + shader_sm4_destroy(&sm4->p); + return VKD3D_ERROR_INVALID_SHADER; + } + instructions = &sm4->p.instructions; while (sm4->ptr != sm4->end) { diff --git a/libs/vkd3d-shader/vkd3d_shader_private.h b/libs/vkd3d-shader/vkd3d_shader_private.h index 63ab7466..da7a43ce 100644 --- a/libs/vkd3d-shader/vkd3d_shader_private.h +++ b/libs/vkd3d-shader/vkd3d_shader_private.h @@ -75,6 +75,7 @@ enum vkd3d_shader_error VKD3D_SHADER_ERROR_TPF_INVALID_REGISTER_RANGE = 1001, VKD3D_SHADER_ERROR_TPF_OUT_OF_MEMORY = 1002, VKD3D_SHADER_ERROR_TPF_INVALID_REGISTER_INDEX_COUNT = 1003, + VKD3D_SHADER_ERROR_TPF_TOO_MANY_REGISTERS = 1004, VKD3D_SHADER_ERROR_SPV_DESCRIPTOR_BINDING_NOT_FOUND = 2000, VKD3D_SHADER_ERROR_SPV_INVALID_REGISTER_TYPE = 2001, -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/198
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- libs/vkd3d-shader/tpf.c | 8 +++++++- libs/vkd3d-shader/vkd3d_shader_private.h | 2 ++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/libs/vkd3d-shader/tpf.c b/libs/vkd3d-shader/tpf.c index 12f032b6..ea7f2082 100644 --- a/libs/vkd3d-shader/tpf.c +++ b/libs/vkd3d-shader/tpf.c @@ -2190,7 +2190,7 @@ static bool shader_sm4_init(struct vkd3d_shader_sm4_parser *sm4, const uint32_t static bool shader_sm4_parser_validate_signature(struct vkd3d_shader_sm4_parser *sm4, const struct vkd3d_shader_signature *signature, const char *name) { - unsigned int i, register_idx; + unsigned int i, register_idx, mask; for (i = 0; i < signature->element_count; ++i) { @@ -2202,6 +2202,12 @@ static bool shader_sm4_parser_validate_signature(struct vkd3d_shader_sm4_parser i, register_idx, MAX_REG_OUTPUT - 1); return false; } + if (!vkd3d_bitmask_is_contiguous(mask = signature->elements[i].mask)) + { + WARN("%s signature element %u mask %#x is not contiguous.\n", name, i, mask); + vkd3d_shader_parser_warning(&sm4->p, VKD3D_SHADER_WARNING_TPF_MASK_NOT_CONTIGUOUS, + "%s signature element %u mask %#x is not contiguous.\n", name, i, mask); + } } return true; diff --git a/libs/vkd3d-shader/vkd3d_shader_private.h b/libs/vkd3d-shader/vkd3d_shader_private.h index da7a43ce..dede7a2d 100644 --- a/libs/vkd3d-shader/vkd3d_shader_private.h +++ b/libs/vkd3d-shader/vkd3d_shader_private.h @@ -77,6 +77,8 @@ enum vkd3d_shader_error VKD3D_SHADER_ERROR_TPF_INVALID_REGISTER_INDEX_COUNT = 1003, VKD3D_SHADER_ERROR_TPF_TOO_MANY_REGISTERS = 1004, + VKD3D_SHADER_WARNING_TPF_MASK_NOT_CONTIGUOUS = 1300, + 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, -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/198
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- libs/vkd3d-shader/tpf.c | 76 +++++++++++++++++++++--- libs/vkd3d-shader/vkd3d_shader_private.h | 1 + 2 files changed, 69 insertions(+), 8 deletions(-) diff --git a/libs/vkd3d-shader/tpf.c b/libs/vkd3d-shader/tpf.c index ea7f2082..c0675b66 100644 --- a/libs/vkd3d-shader/tpf.c +++ b/libs/vkd3d-shader/tpf.c @@ -564,6 +564,11 @@ struct vkd3d_shader_sm4_parser unsigned int output_map[MAX_REG_OUTPUT]; + enum vkd3d_shader_opcode phase; + unsigned int input_register_masks[MAX_REG_OUTPUT]; + unsigned int output_register_masks[MAX_REG_OUTPUT]; + unsigned int patch_constant_register_masks[MAX_REG_OUTPUT]; + struct vkd3d_shader_parser p; }; @@ -1425,7 +1430,7 @@ static void map_register(const struct vkd3d_shader_sm4_parser *sm4, struct vkd3d if (reg_idx >= ARRAY_SIZE(sm4->output_map)) { - ERR("Invalid output index %u.\n", reg_idx); + /* Validated later */ break; } @@ -1762,9 +1767,20 @@ static bool shader_register_is_input_output(const struct vkd3d_shader_register * } } +static unsigned int mask_from_swizzle(unsigned int swizzle) +{ + return (1u << vkd3d_swizzle_get_component(swizzle, 0)) + | (1u << vkd3d_swizzle_get_component(swizzle, 1)) + | (1u << vkd3d_swizzle_get_component(swizzle, 2)) + | (1u << vkd3d_swizzle_get_component(swizzle, 3)); +} + static bool shader_sm4_validate_input_output_register(struct vkd3d_shader_sm4_parser *priv, - const struct vkd3d_shader_register *reg) + const struct vkd3d_shader_register *reg, unsigned int mask) { + const unsigned int *masks; + unsigned int register_idx; + if (!reg->idx_count || reg->idx_count > 2) { /* No I/O registers use order 0 or > 2. The normaliser relies on this. TODO: strict validation. */ @@ -1773,6 +1789,41 @@ static bool shader_sm4_validate_input_output_register(struct vkd3d_shader_sm4_pa return false; } + switch (reg->type) + { + case VKD3DSPR_INPUT: + case VKD3DSPR_INCONTROLPOINT: + masks = priv->input_register_masks; + break; + case VKD3DSPR_OUTPUT: + masks = (priv->phase == VKD3DSIH_HS_FORK_PHASE || priv->phase == VKD3DSIH_HS_JOIN_PHASE) + ? priv->patch_constant_register_masks + : priv->output_register_masks; + break; + case VKD3DSPR_COLOROUT: + case VKD3DSPR_OUTCONTROLPOINT: + masks = priv->output_register_masks; + break; + case VKD3DSPR_PATCHCONST: + masks = priv->patch_constant_register_masks; + break; + + default: + vkd3d_unreachable(); + } + + register_idx = reg->idx[reg->idx_count - 1].offset; + /* The signature element registers have already been checked against MAX_REG_OUTPUT. */ + if (register_idx >= MAX_REG_OUTPUT || (masks[register_idx] & mask) != mask) + { + WARN("Failed to find signature element for register type %#x, index %u and mask %#x.\n", + reg->type, register_idx, mask); + vkd3d_shader_parser_error(&priv->p, VKD3D_SHADER_ERROR_TPF_INVALID_IO_REGISTER, + "Could not find signature element matching register type %#x, index %u and mask %#x.\n", + reg->type, register_idx, mask); + return false; + } + return true; } @@ -1828,7 +1879,7 @@ static bool shader_sm4_read_src_param(struct vkd3d_shader_sm4_parser *priv, cons } if (shader_register_is_input_output(&src_param->reg) && !shader_sm4_validate_input_output_register(priv, - &src_param->reg)) + &src_param->reg, mask_from_swizzle(src_param->swizzle))) return false; return true; @@ -1869,7 +1920,7 @@ static bool shader_sm4_read_dst_param(struct vkd3d_shader_sm4_parser *priv, cons dst_param->shift = 0; if (shader_register_is_input_output(&dst_param->reg) && !shader_sm4_validate_input_output_register(priv, - &dst_param->reg)) + &dst_param->reg, dst_param->write_mask)) return false; return true; @@ -2009,6 +2060,9 @@ static void shader_sm4_read_instruction(struct vkd3d_shader_sm4_parser *sm4, str } ins->handler_idx = opcode_info->handler_idx; + if (ins->handler_idx == VKD3DSIH_HS_CONTROL_POINT_PHASE || ins->handler_idx == VKD3DSIH_HS_FORK_PHASE + || ins->handler_idx == VKD3DSIH_HS_JOIN_PHASE) + sm4->phase = ins->handler_idx; ins->flags = 0; ins->coissue = false; ins->raw = false; @@ -2188,7 +2242,7 @@ static bool shader_sm4_init(struct vkd3d_shader_sm4_parser *sm4, const uint32_t } static bool shader_sm4_parser_validate_signature(struct vkd3d_shader_sm4_parser *sm4, - const struct vkd3d_shader_signature *signature, const char *name) + const struct vkd3d_shader_signature *signature, unsigned int *masks, const char *name) { unsigned int i, register_idx, mask; @@ -2208,6 +2262,9 @@ static bool shader_sm4_parser_validate_signature(struct vkd3d_shader_sm4_parser vkd3d_shader_parser_warning(&sm4->p, VKD3D_SHADER_WARNING_TPF_MASK_NOT_CONTIGUOUS, "%s signature element %u mask %#x is not contiguous.\n", name, i, mask); } + + if (register_idx != ~0u) + masks[register_idx] |= mask; } return true; @@ -2246,9 +2303,12 @@ int vkd3d_shader_sm4_parser_create(const struct vkd3d_shader_compile_info *compi return VKD3D_ERROR_INVALID_ARGUMENT; } - if (!shader_sm4_parser_validate_signature(sm4, &shader_desc->input_signature, "Input") - || !shader_sm4_parser_validate_signature(sm4, &shader_desc->output_signature, "Output") - || !shader_sm4_parser_validate_signature(sm4, &shader_desc->patch_constant_signature, "Patch constant")) + if (!shader_sm4_parser_validate_signature(sm4, &shader_desc->input_signature, + sm4->input_register_masks, "Input") + || !shader_sm4_parser_validate_signature(sm4, &shader_desc->output_signature, + sm4->output_register_masks, "Output") + || !shader_sm4_parser_validate_signature(sm4, &shader_desc->patch_constant_signature, + sm4->patch_constant_register_masks, "Patch constant")) { shader_sm4_destroy(&sm4->p); return VKD3D_ERROR_INVALID_SHADER; diff --git a/libs/vkd3d-shader/vkd3d_shader_private.h b/libs/vkd3d-shader/vkd3d_shader_private.h index dede7a2d..dabd3c6b 100644 --- a/libs/vkd3d-shader/vkd3d_shader_private.h +++ b/libs/vkd3d-shader/vkd3d_shader_private.h @@ -76,6 +76,7 @@ enum vkd3d_shader_error VKD3D_SHADER_ERROR_TPF_OUT_OF_MEMORY = 1002, VKD3D_SHADER_ERROR_TPF_INVALID_REGISTER_INDEX_COUNT = 1003, VKD3D_SHADER_ERROR_TPF_TOO_MANY_REGISTERS = 1004, + VKD3D_SHADER_ERROR_TPF_INVALID_IO_REGISTER = 1005, VKD3D_SHADER_WARNING_TPF_MASK_NOT_CONTIGUOUS = 1300, -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/198
```diff diff --git a/libs/vkd3d-shader/ir.c b/libs/vkd3d-shader/ir.c index 11ba7563..d0dc2978 100644 --- a/libs/vkd3d-shader/ir.c +++ b/libs/vkd3d-shader/ir.c @@ -300,11 +300,7 @@ static bool shader_dst_param_normalise_outpointid(struct vkd3d_shader_dst_param
if (normaliser_is_in_control_point_phase(normaliser) && reg->type == VKD3DSPR_OUTPUT) { - if (reg->idx[2].offset != ~0u) - { - FIXME("Cannot insert phase id.\n"); - return false; - } + assert(reg->idx[2].offset == ~0u); if (reg->idx[1].offset != ~0u) { WARN("Unexpected address at index 1.\n"); ```
We could as well compare reg->idx_count against ARRAY_SIZE(reg->idx) here. We should also increase reg->idx_count here because we're adding an extra index. This series will need to be adjusted for the signature changes in !197. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/198#note_31868
participants (3)
-
Conor McCarthy -
Conor McCarthy (＠cmccarthy)
-
Henri Verbeet (＠hverbeet)