Re: [PATCH v2 0/5] MR626: vkd3d-utils: Constant buffer reflection without vkd3d-shader.

[Henri Verbeet (＠hverbeet)]

> +static HRESULT parse_rdef(struct d3d12_reflection *reflection, const struct vkd3d_shader_dxbc_section_desc *section)
> +{
> +    const struct rdef_header *header;
> +    HRESULT hr;
> +
> +    if (!(header = get_section_data(section, 0, sizeof(*header))))
> +        return E_INVALIDARG;
> +
> +    reflection->desc.ConstantBuffers = header->buffer_count;
> +
> +    if (header->buffer_count)
> +    {
> +        const struct rdef_buffer *rdef_buffers;
> +
> +        if (!(rdef_buffers = get_section_data(section, header->buffers_offset,
> +                header->buffer_count * sizeof(*rdef_buffers))))
> +            return E_INVALIDARG;
> +
> +        if (!(reflection->buffers = vkd3d_calloc(header->buffer_count, sizeof(*reflection->buffers))))
> +            return E_OUTOFMEMORY;
> +
> +        for (uint32_t i = 0; i < header->buffer_count; ++i)
> +        {
> +            if ((hr = d3d12_buffer_init(&reflection->buffers[i], &rdef_buffers[i], section)))
> +                return hr;
> +        }
> +    }
> +
> +    return S_OK;
> +}

The "header->buffer_count * sizeof(*rdef_buffers)" multiplication can overflow, but when it does, the vkd3d_calloc() call will fail by virtue of sizeof(*reflection->buffers) being the same size as or larger than sizeof(*rdef_buffers). I suppose that works, but it seems a bit unfortunate/fragile.

vkd3d-shader's require_space() doesn't have that issue. Perhaps we can do better than require_space(), but I'm not immediately convinced get_section_data() is it. It also seems somewhat undesirable to have to pass vkd3d_shader_dxbc_section_desc structures around, instead of e.g. vkd3d_shader_code structures.
> +    for (unsigned int i = 0; i < dxbc_desc.section_count; ++i)
> +    {
> +        const struct vkd3d_shader_dxbc_section_desc *section = &dxbc_desc.sections[i];
> +
> +        if (section->tag == TAG_RDEF && FAILED(hr = parse_rdef(reflection, section)))
> +        {
> +            vkd3d_shader_free_scan_signature_info(&reflection->signature_info);
> +            vkd3d_shader_free_dxbc(&dxbc_desc);
> +            return hr;
> +        }
> +    }

Is that the right behaviour for inputs with multiple RDEF sections?