-
-
Notifications
You must be signed in to change notification settings - Fork 42
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
get_entry_points: Use after free. #118
Comments
Yes I think you're right. IIRC this was changed slightly when support was added for the wasm backend. I guess we haven't hit this issue yet because most shaders only have a single entry point.
|
check! has the side effect of making functions that can never return with failure be prototyped as if they can. I could see a case for wanting the future proof the API, but I think that's ill advised. |
Right, but the issue is that exceptions can be thrown by almost anywhere we interact with SPIRV-Cross, and unwinding across the Rust/C++ boundary is undefined behavior (i.e. see Although I think we're going to move to using assertions instead of exceptions anyway, through use of the assertions flag in SPIRV-Cross (some other codebases which use spirv_cross require C++ exceptions be disabled). This isn't great for making errors recoverable, but we could probably later split this into checked/unchecked variants later (where the checked variants could do some simple checking like validating IDs and still return |
I'd PM instead of cluttering the issue, but github sigh. I don't understand C++. I managed to add support for get_active_buffer_ranges() without any consideration for exceptions, didn't even realize they were part of C++ until now. |
spirv_cross/spirv_cross/src/compiler.rs
Lines 265 to 267 in 2315669
The array is allocated as one whole, but each element of the array is freed individually. As the first element is freed first the other accesses are then working on un-allocated memory.
Also
void free(void*)
, by definition, cannot return anything and thus never fails, but you are attempting to handle cases where it does.The text was updated successfully, but these errors were encountered: