Skip to content
Snippets Groups Projects

vkd3d-shader/hlsl: Always work with the extern resource's component type.

Merged Francisco Casas requested to merge fcasas/vkd3d:fix_rasteriser_ordered into master

This fixes runtime errors reported by UBSan, such as this:

vkd3d/libs/vkd3d-shader/tpf.c:6075:87: runtime error: load of value 7, which is not a valid value for type '_Bool'

when trying to compile shaders that contain UAV arrays on 4.0 profiles.

Before this commit, tpf.c accesses the

hlsl_type->e.resource.rasteriser_ordered

field, but on 4.0 and 4.1 profiles these code paths can also be reached by UAV arrays which are HLSL_CLASS_ARRAY and this field is not supposed to be accessed.

By coincidence, the value of hlsl_type->e.array.elements_count is being read because these fields have the same offset in the hlsl_type.e union.

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • I'm not convinced this is the right solution. Take for example write_sm4_dcl_textures(): the code clearly assumes that the resource is a texture or a UAV in a few other places (e.g. by reading resource->data_type->e.resource or resource->data_type->sampler_dim). So either an array shouldn't reach that code, or the whole function should be ready to accept it.

    Also, I'd try to avoid duplicating information in struct extern_resource. It seems that resource->data_type already has all the information that code needs. Maybe we just need to check resource->data_type->class before reading from the union.

  • I'm not convinced this is the right solution. Take for example write_sm4_dcl_textures(): the code clearly assumes that the resource is a texture or a UAV in a few other places (e.g. by reading resource->data_type->e.resource or resource->data_type->sampler_dim). So either an array shouldn't reach that code, or the whole function should be ready to accept it.

    Yeah, we should probably just be storing the component type in extern_resource. I don't think there's any reason not to do that.

  • Elizabeth Figura approved this merge request

    approved this merge request

  • Elizabeth Figura unapproved this merge request

    unapproved this merge request

  • Francisco Casas added 30 commits

    added 30 commits

    • fdb577ab...51abec8c - 29 commits from branch wine:master
    • 419b32ae - vkd3d-shader/hlsl: Always work with the extern resource's component type.

    Compare with previous version

  • Author Developer

    Rebased and reimplemented the patch with the suggested strategy of simply storing (and using) the component type in the extern_resource.

  • Francisco Casas changed title from vkd3d-shader/hlsl: Use a extern_resource.rasteriser_ordered field. to vkd3d-shader/hlsl: Always work with the extern resource's component type.

    changed title from vkd3d-shader/hlsl: Use a extern_resource.rasteriser_ordered field. to vkd3d-shader/hlsl: Always work with the extern resource's component type.

  • This needs a rebase after the assert() changes.

  • Francisco Casas added 25 commits

    added 25 commits

    • 419b32ae...80b2a2eb - 24 commits from branch wine:master
    • 10ed8a68 - vkd3d-shader/hlsl: Always work with the extern resource's component type.

    Compare with previous version

  • Elizabeth Figura approved this merge request

    approved this merge request

  • Henri Verbeet approved this merge request

    approved this merge request

  • Henri Verbeet added 10 commits

    added 10 commits

    • 10ed8a68...9cf479d4 - 9 commits from branch wine:master
    • 412f91a1 - vkd3d-shader/hlsl: Always work with the extern resource's component type.

    Compare with previous version

Please register or sign in to reply
Loading