Refactor test framework to detect invalid handles and support multiple instances#1922
Merged
charles-lunarg merged 8 commits intoJun 7, 2026
Conversation
Removed unused headers, added static to internal globals (fixed errors in ASAN on macOS), fixed compiler warnings.
The handles given out by the test layer are not actually valid Vulkan dispatchable handles.
Prior to this, the mock ICD did not track which instance enumerated which physical devices, which is invalid according to the spec. Not only that, it meant that if the loader didn't wrap the physical device handle, multiple instances would share the same physical device dispatchable handle and overwrite the dispatch handle. This is accomplished by splitting the 'info', extensions, properties, limits, etc, from the 'state', the dispatchable handles enumerated per instance. To improve performance, maps of the instance, physical device, and device handles with structs containing details of the objects. These details also include child objects and pointers to the 'state' (currently, only PhysicalDevice is separate from the TestICD). There is non-trivial logic keeping the vector of PhysicalDevice's and the created state structs up to date with eachother, as order of the PhysicalDevice vector is the order their associated state objects are returned in vkEnumeratePhysicalDevices.
With the refactor of the previous commit, the PhysicalDevice struct is no longer move-only, which removes the need for calling .finish().
Make sure each and every call to the driver only gives handles valid to the driver. Also turns ERROR_INITIALIZATION_FAILED return codes into hard aborts, to make sure the issue is detected.
|
CI Vulkan-Loader build queued with queue ID 763052. |
|
CI Vulkan-Loader build queued with queue ID 763069. |
|
CI Vulkan-Loader build # 3515 running. |
|
CI Vulkan-Loader build # 3515 failed. |
Add an internal function to the test_icd which validates that the passed in physical device is one from the driver.
070def6 to
7ebb4c4
Compare
|
CI Vulkan-Loader build queued with queue ID 763137. |
|
CI Vulkan-Loader build # 3516 running. |
|
CI Vulkan-Loader build queued with queue ID 763167. |
|
CI Vulkan-Loader build # 3517 running. |
|
CI Vulkan-Loader build # 3517 passed. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The test framework was originally written with some rather limiting assumptions, such as:
These all proved to be a problem with the attempted rewrite to not wrap physical device handles. While that rewrite is not finalized, breaking out these changes makes sure that if the rewrite fails, then these useful changes aren't lost.