OpenXR-SDK-Source icon indicating copy to clipboard operation
OpenXR-SDK-Source copied to clipboard

Validation layer catch(...) behavior is confusing

Open Ralith opened this issue 4 years ago • 4 comments

I recently encountered behavior in SteamVR where xrAttachSessionActionSets would sometimes throw C++ exceptions. While this may be a bug in SteamVR (throwing exceptions through a C ABI is obviously not well-defined), my investigation was slowed by the validation layers' silent, blanket conversion of all caught exceptions to XR_ERROR_VALIDATION_FAILURE, giving the appearance that specific application misbehavior had been detected, although no diagnostics were produced. This was further confounded by (I presume) C++ ABI details causing the exceptions to only be caught in release builds of the layers.

Ideally, the validation layer should only catch its own exceptions, by e.g. inheriting them from a private base class, and then catching that base class by reference.

Ralith avatar Jul 26 '21 15:07 Ralith

An issue (number 1593) has been filed to correspond to this issue in the internal Khronos GitLab (Khronos members only: KHR:openxr/openxr#1593 ), to facilitate working group processes.

This GitHub issue will continue to be the main site of discussion.

rpavlik-bot avatar Aug 02 '21 15:08 rpavlik-bot

The trouble with "catching its own exceptions only" is that STL classes are used in the loader, so we can't entirely change the thrown exceptions. (Indeed I don't think we throw any exceptions explicitly, since you can do a no-exceptions build if you have a no-exceptions c++ standard library build - the browsers do this) This is definitely a SteamVR bug - no exceptions should leak out a C API - but it's possible we might be able to handle this better in the loader. If nothing else we could maybe wrap the actual invocation of the function in another try/catch so we could pre-emptively catch any erroneously-leaked exceptions from the runtime and do something more useful. (Turn them into runtime failure, at least) That said, I'm not sure, given that exceptions in a C ABI is not well defined, if the compiler would even retain such a try/catch that would appear impossible...

Do you know if anyone ever added a conformance test for the bug you found in SteamVR? We can probably also add exception-catching to the conformance layer, though same caveat around "not appearing possible" to the compiler applies here.

rpavlik avatar Dec 15 '21 15:12 rpavlik

The trouble with "catching its own exceptions only" is that STL classes are used in the loader, so we can't entirely change the thrown exceptions.

I don't follow. Does the validation layer intend to catch exceptions thrown by the loader? That seems like an ABI violation itself. If not, what's the problem?

I agree that catching runtime misbehavior in the validation layer seems a bit odd, also in that that shouldn't be the validation layer's problem to begin with.

Do you know if anyone ever added a conformance test for the bug you found in SteamVR?

Not to my knowledge. Unfortunately it seems to be ~impossible for third party developers to actually reach Valve about such issues... at least the bug never seemed to make it into stable.

Ralith avatar Dec 15 '21 20:12 Ralith

Oh I was confused when replying, I was thinking the problem was the catch in the loader. I'm not sure if the validation layer uses any stl things that might throw, I'll have to look at it again. If not we can remove the catches. Kind of strange that they're all turned into validation failure, runtime failure seems more appropriate.

rpavlik avatar Dec 16 '21 15:12 rpavlik