Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[spirv] Fix null pointer dereference #3861

Merged
merged 1 commit into from Jul 9, 2021

Conversation

sudonatalie
Copy link
Collaborator

@sudonatalie sudonatalie commented Jul 6, 2021

Fix a null pointer dereference which can occur when an unimplemented
intrinsic is used. The repro shader from issue #2955 (added as a unit
test) now fails with a useful message rather than a silent crash.

@sudonatalie sudonatalie requested a review from jaebaek July 6, 2021 14:54
@sudonatalie
Copy link
Collaborator Author

@jaebaek This should fix the crash, but of course will not generate valid SPIR-V. Is this new intrinsic one that has been discussed before or that warrants tracking separately for the SPIR-V backend?

@AppVeyorBot
Copy link

@jaebaek
Copy link
Collaborator

jaebaek commented Jul 7, 2021

We have not implemented GetAttributeAtVertex support for the SPIR-V backend. It is relatively new feature. I found some commits about it, but I cannot find the spec about it. The DXIL code gen uses @dx.op.attributeAtVertex.f32() intrinsic but I have no idea we have a SPIR-V instruction matches it.

@pow2clk could you please point out the document about GetAttributeAtVertex?

@ghost
Copy link

ghost commented Jul 7, 2021

CLA assistant check
All CLA requirements met.

Fix a null pointer dereference which can occur when an unimplemented
intrinsic is used. The repro shader from issue microsoft#2955 (added as a unit
test) now fails with a useful message rather than a silent crash.
@AppVeyorBot
Copy link

@AppVeyorBot
Copy link

@pow2clk
Copy link
Member

pow2clk commented Jul 7, 2021

Hi @jaebaek!

GetAttributeAtPVertex is part of the barycentrics optional feature added in shader model 6.1: https://github.com/microsoft/DirectXShaderCompiler/wiki/SV_Barycentrics#per-vertex-attributes

Congrats @sudonatalie on your first PR to DXC! 😄

@jaebaek
Copy link
Collaborator

jaebaek commented Jul 8, 2021

Thanks @pow2clk !

@sudonatalie sudonatalie merged commit bf29b05 into microsoft:master Jul 9, 2021
@sudonatalie sudonatalie deleted the issue-2955 branch July 9, 2021 21:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants