get_entry_points: Use after free. about spirv_cross HOT 4 CLOSED

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! just catches all exceptions, so it should be fine if sc_internal_free_pointer will never actually throw anything.

from spirv_cross.

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.

from spirv_cross.

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 catch_unwind). So even if check! isn't needed for free here, some other call in the same function body might require it anyway, so the Result in the API is still necessary.

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 Result).

from spirv_cross.

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.

from spirv_cross.