godotengine
Fix re-spirv null pointer crash on invalid SPIR-V parsing
Description
Fixes a crash in re-spirv when parsing invalid or unsupported SPIR-V constructs (e.g., mediump/lowp keywords).
This fix does not resolve common issues with precision qualifiers and etc., as their processing is not implemented in re-spirv.
Problem
The respv::Shader constructor was not checking the result of parse() method. When parse() failed due to invalid SPIR-V, the Shader object remained uninitialized, but was then passed to Optimizer::run(), causing a null pointer dereference crash.
Related Issues
Closes #113665 Related to #111452 (re-spirv integration) Similar to #113684 (closed by me)
Hi, thanks for the report.
Can you try using the version from this commit instead as well as updating the hash on the thirdparty/README.md to b3aaf5151bf64c73f7236486be57c80cf196476c if it works? It shouldn't be necessary to add a new method either, the empty() method seemed to be not implemented by mistake and it should be enough.
https://github.com/renderbag/re-spirv/commit/b3aaf5151bf64c73f7236486be57c80cf196476c
Hi, thanks for the report.
Can you try using the version from this commit instead as well as updating the hash on the thirdparty/README.md to
b3aaf5151bf64c73f7236486be57c80cf196476cif it works? It shouldn't be necessary to add a new method either, the empty() method seemed to be not implemented by mistake and it should be enough.
Okay, thank you for clarifying that. I will test your current implementation now.
Hi, thanks for the report.
Can you try using the version from this commit instead as well as updating the hash on the thirdparty/README.md to
b3aaf5151bf64c73f7236486be57c80cf196476cif it works? It shouldn't be necessary to add a new method either, the empty() method seemed to be not implemented by mistake and it should be enough.
Yes, it works. I can update the pr - add only the file from your commit and update the readme. Or should I keep the current implementation?
You should update that and keep the change to the driver that adds an if branch, but you should be able to just use the empty() method alone. I can do a review directly in the code afterwards if anything else needs to be addressed.
The changes to the third party folder must be reverted and updated with the files from the commit I linked before.
Alternatively if you want, I can just merge this PR into mine since mine hasn't been merged yet and it also updates re-spirv to a newer version. https://github.com/godotengine/godot/pull/113582
Alternatively if you want, I can just merge this PR into mine since mine hasn't been merged yet and it also updates re-spirv to a newer version. #113582
Yes, if it's not too much trouble.
Yes, if it's not too much trouble.
Oops, mine was merged. That said, I should be perfectly able to update your PR myself, so I'll do that.
I've updated your branch, let me know if it resolves your issue still and I'll approve it.
I've updated your branch, let me know if it resolves your issue still and I'll approve it.
Oh, thanks! Yes, it solves the problem completely.
