Skip to content
Snippets Groups Projects

vkd3d-shader: Add more TPF validations.

Merged Conor McCarthy requested to merge cmccarthy/vkd3d:validate_tpf into master
2 unresolved threads
2 files
+ 32
0
Compare changes
  • Side-by-side
  • Inline
Files
2
+ 298
4
@@ -558,12 +558,35 @@ enum vkd3d_sm4_shader_data_type
VKD3D_SM4_SHADER_DATA_MESSAGE = 0x4,
};
struct sm4_index_range
{
unsigned int index;
unsigned int count;
unsigned int mask;
};
struct sm4_index_range_array
{
unsigned int count;
struct sm4_index_range ranges[MAX_REG_OUTPUT * 2];
};
struct vkd3d_shader_sm4_parser
{
const uint32_t *start, *end, *ptr;
unsigned int output_map[MAX_REG_OUTPUT];
enum vkd3d_shader_opcode phase;
bool has_control_point_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 sm4_index_range_array input_index_ranges;
struct sm4_index_range_array output_index_ranges;
struct sm4_index_range_array patch_constant_index_ranges;
struct vkd3d_shader_parser p;
};
@@ -720,7 +743,7 @@ static void shader_sm4_set_descriptor_register_range(struct vkd3d_shader_sm4_par
{
FIXME("Invalid register range [%u:%u].\n", range->first, range->last);
vkd3d_shader_parser_error(&sm4->p, VKD3D_SHADER_ERROR_TPF_INVALID_REGISTER_RANGE,
"Last register %u must not be less than first register %u in range.\n", range->last, range->first);
"Last register %u must not be less than first register %u in range.", range->last, range->first);
}
}
@@ -818,12 +841,109 @@ static void shader_sm4_read_dcl_sampler(struct vkd3d_shader_instruction *ins, ui
shader_sm4_read_register_space(priv, &tokens, end, &ins->declaration.sampler.range.space);
}
static bool sm4_parser_is_in_fork_or_join_phase(const struct vkd3d_shader_sm4_parser *sm4)
{
return sm4->phase == VKD3DSIH_HS_FORK_PHASE || sm4->phase == VKD3DSIH_HS_JOIN_PHASE;
}
static void shader_sm4_read_dcl_index_range(struct vkd3d_shader_instruction *ins, uint32_t opcode,
uint32_t opcode_token, const uint32_t *tokens, unsigned int token_count, struct vkd3d_shader_sm4_parser *priv)
{
struct vkd3d_shader_index_range *index_range = &ins->declaration.index_range;
unsigned int i, register_idx, register_count, write_mask;
enum vkd3d_shader_register_type type;
struct sm4_index_range_array *ranges;
unsigned int *io_masks;
shader_sm4_read_dst_param(priv, &tokens, &tokens[token_count], VKD3D_DATA_OPAQUE,
&ins->declaration.index_range.dst);
ins->declaration.index_range.register_count = *tokens;
&index_range->dst);
index_range->register_count = *tokens;
register_idx = index_range->dst.reg.idx[index_range->dst.reg.idx_count - 1].offset;
register_count = index_range->register_count;
write_mask = index_range->dst.write_mask;
if (vkd3d_write_mask_component_count(write_mask) != 1)
{
WARN("Unhandled write mask %#x.\n", write_mask);
vkd3d_shader_parser_warning(&priv->p, VKD3D_SHADER_WARNING_TPF_UNHANDLED_INDEX_RANGE_MASK,
"Index range mask %#x is not scalar.", write_mask);
}
switch ((type = index_range->dst.reg.type))
{
case VKD3DSPR_INPUT:
case VKD3DSPR_INCONTROLPOINT:
io_masks = priv->input_register_masks;
ranges = &priv->input_index_ranges;
break;
case VKD3DSPR_OUTPUT:
if (sm4_parser_is_in_fork_or_join_phase(priv))
{
io_masks = priv->patch_constant_register_masks;
ranges = &priv->patch_constant_index_ranges;
}
else
{
io_masks = priv->output_register_masks;
ranges = &priv->output_index_ranges;
}
break;
case VKD3DSPR_COLOROUT:
case VKD3DSPR_OUTCONTROLPOINT:
io_masks = priv->output_register_masks;
ranges = &priv->output_index_ranges;
break;
case VKD3DSPR_PATCHCONST:
io_masks = priv->patch_constant_register_masks;
ranges = &priv->patch_constant_index_ranges;
break;
default:
WARN("Unhandled register type %#x.\n", type);
vkd3d_shader_parser_error(&priv->p, VKD3D_SHADER_ERROR_TPF_INVALID_INDEX_RANGE_DCL,
"Invalid register type %#x for index range base %u, count %u, mask %#x.",
type, register_idx, register_count, write_mask);
return;
}
for (i = 0; i < ranges->count; ++i)
{
struct sm4_index_range r = ranges->ranges[i];
if (!(r.mask & write_mask))
continue;
/* Ranges with the same base but different lengths are not an issue. */
if (register_idx == r.index)
continue;
if ((r.index <= register_idx && register_idx - r.index < r.count)
|| (register_idx < r.index && r.index - register_idx < register_count))
{
WARN("Detected index range collision for base %u, count %u, mask %#x.\n",
register_idx, register_count, write_mask);
vkd3d_shader_parser_error(&priv->p, VKD3D_SHADER_ERROR_TPF_INVALID_INDEX_RANGE_DCL,
"Register index range base %u, count %u, mask %#x collides with a previous declaration.",
register_idx, register_count, write_mask);
return;
}
}
ranges->ranges[ranges->count].index = register_idx;
ranges->ranges[ranges->count].count = register_count;
ranges->ranges[ranges->count++].mask = write_mask;
for (i = 0; i < register_count; ++i)
{
if ((io_masks[register_idx + i] & write_mask) != write_mask)
{
WARN("No matching declaration for index range base %u, count %u, mask %#x.\n",
register_idx, register_count, write_mask);
vkd3d_shader_parser_error(&priv->p, VKD3D_SHADER_ERROR_TPF_INVALID_INDEX_RANGE_DCL,
"Input/output registers matching index range base %u, count %u, mask %#x were not declared.",
register_idx, register_count, write_mask);
return;
}
}
}
static void shader_sm4_read_dcl_output_topology(struct vkd3d_shader_instruction *ins, uint32_t opcode,
@@ -1425,7 +1545,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;
}
@@ -1669,6 +1789,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 +1832,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;
    • I don't really know what I'm talking about here, but after reading that my first though is: "Shouldn't we check that idx_count is not already three? And shouldn't we zero idx[0]?".

Please register or sign in to reply
}
map_register(priv, param);
@@ -1742,6 +1865,91 @@ 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 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 register_is_control_point_input(const struct vkd3d_shader_register *reg,
const struct vkd3d_shader_sm4_parser *priv)
{
return reg->type == VKD3DSPR_INCONTROLPOINT || reg->type == VKD3DSPR_OUTCONTROLPOINT
|| (reg->type == VKD3DSPR_INPUT && (priv->phase == VKD3DSIH_HS_CONTROL_POINT_PHASE
|| priv->p.shader_version.type == VKD3D_SHADER_TYPE_GEOMETRY));
}
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, unsigned int mask)
{
unsigned int idx_count = 1 + register_is_control_point_input(reg, priv);
const unsigned int *masks;
unsigned int register_idx;
if (reg->idx_count != idx_count)
{
vkd3d_shader_parser_error(&priv->p, VKD3D_SHADER_ERROR_TPF_INVALID_REGISTER_INDEX_COUNT,
"Invalid index count %u for register type %#x; expected count %u.",
reg->idx_count, reg->type, idx_count);
return false;
}
switch (reg->type)
{
case VKD3DSPR_INPUT:
case VKD3DSPR_INCONTROLPOINT:
masks = priv->input_register_masks;
break;
case VKD3DSPR_OUTPUT:
masks = sm4_parser_is_in_fork_or_join_phase(priv) ? 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.",
reg->type, register_idx, mask);
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 +2001,10 @@ static bool shader_sm4_read_src_param(struct vkd3d_shader_sm4_parser *priv, cons
}
}
if (register_is_input_output(&src_param->reg) && !shader_sm4_validate_input_output_register(priv,
&src_param->reg, mask_from_swizzle(src_param->swizzle)))
return false;
return true;
}
@@ -1830,6 +2042,10 @@ static bool shader_sm4_read_dst_param(struct vkd3d_shader_sm4_parser *priv, cons
dst_param->modifiers = 0;
dst_param->shift = 0;
if (register_is_input_output(&dst_param->reg) && !shader_sm4_validate_input_output_register(priv,
&dst_param->reg, dst_param->write_mask))
return false;
return true;
}
@@ -1967,6 +2183,10 @@ 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;
sm4->has_control_point_phase |= ins->handler_idx == VKD3DSIH_HS_CONTROL_POINT_PHASE;
ins->flags = 0;
ins->coissue = false;
ins->raw = false;
@@ -2145,6 +2365,67 @@ 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 shader_signature *signature, unsigned int *masks, const char *name)
{
unsigned int i, register_idx, register_count, mask;
for (i = 0; i < signature->element_count; ++i)
{
register_idx = signature->elements[i].register_index;
register_count = signature->elements[i].register_count;
if (register_idx != ~0u && (register_idx >= MAX_REG_OUTPUT || MAX_REG_OUTPUT - register_idx < register_count))
{
WARN("%s signature element %u unhandled register index %u, count %u.\n",
name, i, register_idx, register_count);
vkd3d_shader_parser_error(&sm4->p, VKD3D_SHADER_ERROR_TPF_TOO_MANY_REGISTERS,
"%s signature element %u register index %u, count %u exceeds maximum index of %u.", name,
i, register_idx, register_count, 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.", name, i, mask);
}
if (register_idx != ~0u)
masks[register_idx] |= mask;
}
return true;
}
static int index_range_compare(const void *a, const void *b)
{
return memcmp(a, b, sizeof(struct sm4_index_range));
}
static void shader_sm4_validate_default_phase_index_ranges(struct vkd3d_shader_sm4_parser *sm4)
{
if (!sm4->input_index_ranges.count || !sm4->output_index_ranges.count)
return;
if (sm4->input_index_ranges.count == sm4->output_index_ranges.count)
{
qsort(sm4->input_index_ranges.ranges, sm4->input_index_ranges.count, sizeof(sm4->input_index_ranges.ranges[0]),
index_range_compare);
qsort(sm4->output_index_ranges.ranges, sm4->output_index_ranges.count, sizeof(sm4->output_index_ranges.ranges[0]),
index_range_compare);
if (!memcmp(sm4->input_index_ranges.ranges, sm4->output_index_ranges.ranges,
sm4->input_index_ranges.count * sizeof(sm4->input_index_ranges.ranges[0])))
    • Comment on lines +2417 to +2418

      I think that's correct, but quite fragile: as soon as, for any reason, sm4_index_range_array is modified in a way that causes it to have some padding, the compare might break.

      No need to change for me, let's just keep this in mind.

Please register or sign in to reply
return;
}
/* This is very unlikely to occur and would complicate the default control point phase implementation. */
WARN("Default phase index ranges are not identical.\n");
vkd3d_shader_parser_error(&sm4->p, VKD3D_SHADER_ERROR_TPF_INVALID_INDEX_RANGE_DCL,
"Default control point phase input and output index range declarations are not identical.");
return;
}
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)
{
@@ -2178,6 +2459,17 @@ 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,
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;
}
instructions = &sm4->p.instructions;
while (sm4->ptr != sm4->end)
{
@@ -2199,6 +2491,8 @@ int vkd3d_shader_sm4_parser_create(const struct vkd3d_shader_compile_info *compi
}
++instructions->count;
}
if (sm4->p.shader_version.type == VKD3D_SHADER_TYPE_HULL && !sm4->has_control_point_phase && !sm4->p.failed)
shader_sm4_validate_default_phase_index_ranges(sm4);
*parser = &sm4->p;
Loading