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

XrPath is excessively UB-prone #20

Closed
Ralith opened this issue Jul 1, 2019 · 4 comments

Comments

@Ralith
Copy link

commented Jul 1, 2019

The spec states:

An XrPath that is received from one XrInstance may not be used with another. Such an invalid use may be detected and result in an error being returned, or it may result in undefined behavior.

and

These XrPath values are only valid within a single XrInstance, and must not be shared between instances. Applications must instead use the string representation of a path in their code and configuration, and obtain the correct corresponding XrPath at runtime in each XrInstance

As the author openxrs, I'm trying to develop memory-safe Rust bindings for OpenXR. This means that every API call that isn't explicitly an unsafe escape hatch must not be capable of invoking undefined behavior. The above restrictions make this very difficult, as either anything that operates on an XrPath is potentially unsafe, representing a significant portion of the API. This makes it much harder to audit downstream code for safety violations. My only alternative is to represent XrPaths with a wrapper that tracks the associated instance, but that would undermine their value as compact lightweight tokens that can be trivially copied and stored.

Most implementations will likely implement XrPath as indexes into an internal table that can be bounds-checked, and path type-checking is already required, so guaranteeing defined behavior here seems reasonable.

CC @rpavlik

@Ralith

This comment has been minimized.

Copy link
Author

commented Jul 1, 2019

Note that this is not an issue for object handles, because it's much more natural for those to be exposed as larger, non-copyable types.

@rpavlik

This comment has been minimized.

Copy link
Contributor

commented Jul 2, 2019

Thanks for filing this! A corresponding issue (number 1149) has been raised within the working group.

I'm personally interested in resolving this apparent weakness, since XrPath atoms are intended to be more lightweight and usable than the path strings they represent, so I would consider it a design flaw if they aren't usable like this in practice.

@rpavlik

This comment has been minimized.

Copy link
Contributor

commented Jul 30, 2019

We intended to solve this with 1.0.0. If there are still places where the language is unclear, please let us know here. (Leaving issue open per chat)

@rpavlik

This comment has been minimized.

Copy link
Contributor

commented Jul 30, 2019

closed in favor of #22

@rpavlik rpavlik closed this Jul 30, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.