- if (!data || !data_size || flags || !blob) + if (!data || data_size < DXBC_HEADER_SIZE || flags || !blob)Ideally this would be a separate change with a test to validate its correctness (AFAICS it is correct but there isn't a test already for it).
Well, it more or less preserves existing behaviour. Previously this case would simply read beyond "data_size" and (typically) either hit the "data_size != total_size" condition in dxbc_parse() and return D3DERR_INVALIDCALL, or hit the "tag != TAG_DXBC" condition and return E_FAIL. None of that is particularly great, of course.
In vkd3d-shader's parse_dxbc(), we instead hit the "data_size < VKD3D_DXBC_HEADER_SIZE" condition, which returns VKD3D_ERROR_INVALID_ARGUMENT. That's the same error as returned for the "tag != TAG_DXBC" case. None of this seems particularly worth worrying about on first sight, but in the case of D3DStripShader() we happen to have a test that hits these paths. (I.e., the earlier "blob.c:364: Test failed: Got unexpected hr 0x80004005." failure.)
There are a couple of different ways we could address this of course, including:
- Explicitly check the tag in d3dcompiler_strip_shader(). - Explicitly check the size in d3dcompiler_strip_shader(). - Return a different error for these two cases in vkd3d-shader. - Remove the offending test.
These aren't mutually exclusive options, of course. I chose the second option because it seemed the easiest, particularly because there are already existing "data_size" checks in d3dcompiler_strip_shader() and d3dcompiler_get_blob_part(), but I could certainly be convinced to take a different approach.
The d3dcompiler_get_blob_part() change is just intended to keep the behaviour consistent with d3dcompiler_strip_shader(), which generally seems desirable, but in principle it could be dropped.
- /* some parts aren't full DXBCs, they contain only the data */ + /* some parts aren't full DXBCs, they contain only the data. */Inane nitpick: this change could be moved 2 patches back where nearby code is already being touched.
Actually, I didn't intend to touch that line at all; I've dropped it in v4.
Similar to the previous patch here. Also can get rid of the || !data_size part.
Right, not removing the "!data_size" check was an oversight; I've dropped it in v4. For the DXBC_HEADER_SIZE check, see the explanation above.