device_memory_report: guard extension device commands in vkGetInstanceProcAddr - #37
Conversation
…eProcAddr Split known device functions into core commands and extension commands. Return core device functions directly from vkGetInstanceProcAddr, but query the instance dispatch chain before handing out extension commands to ensure applications cannot falsely detect unsupported extensions.
Include the object type when cleaning up tracked resources on destruction so handle-to-resource tracking can distinguish between different object types (e.g. buffers vs images) that share the same handle value.
olehkuznetsov
left a comment
There was a problem hiding this comment.
Thanks for working on this! Hardening extension dispatch and cleaning up handle lifetimes are great improvements.
A couple of architectural observations on the two commits in this PR:
-
Commit 1 (
guard extension device commands in vkGetInstanceProcAddr):
The split between core and extension device commands is the right direction, but in the current implementation,devmemreport_known_device_extension_functionsreturnsnullptr, while the 4 intercepted KHR commands (vkBindBufferMemory2KHR,vkBindImageMemory2KHR,vkGetImageMemoryRequirements2KHR,vkGetBufferMemoryRequirements2KHR) were left indevmemreport_known_core_device_functions.
Because GIPA returns core device commands without querying downstream, the extension guard at lines 89–103 is never actually reached for any extension command—making the commit a functional no-op as-is. Moving the 4*KHRfunctions intodevmemreport_known_device_extension_functionsmakes the guard work as intended. (See inline comment for snippet).
(Additionally, indevice_memory_report_handwritten_functions.h,vkBindBufferMemory2KHRandvkBindImageMemory2KHRcurrently dereferencedevice_dispatch_table(device)->Bind*Memory2KHRwithout null-checking—unlikeGetImageMemoryRequirements2KHRwhich has a null guard. Adding null checks there will protect against crashes regardless of whether an application resolves these commands via GIPA or GDPA). -
Commit 2 (
pass VkObjectType to OnDestroyObject):
OnDestroyObjectacceptsVkObjectType object_type, but the parameter is unused—the internal tables (resources_andresource_to_memory_map_) remain keyed strictly onuint64_t, and creation hooks (OnCreateImage/OnCreateBuffer) do not store the type either.
On 64-bit platforms (Android / Linux), distinct live buffers and images having identical 64-bit handle values is not demonstrable in practice. Unless there is an active plan to convertresources_to composite keys (struct ResourceKey { VkObjectType, uint64_t }), we recommend dropping/reverting this commit to avoid churn across 16 call sites (production dispatch + unit tests).
Detailed inline comments with suggested fixes are below.
This reverts commit 43e6ddc.
… null guards Move vkBindBufferMemory2KHR, vkBindImageMemory2KHR, vkGetImageMemoryRequirements2KHR, and vkGetBufferMemoryRequirements2KHR from devmemreport_known_core_device_functions to devmemreport_known_device_extension_functions so vkGetInstanceProcAddr checks underlying driver support before returning interceptors. Additionally, add null checks in vkBindBufferMemory2KHR and vkBindImageMemory2KHR before dereferencing the device dispatch table.
This PR hardens command dispatch in the
VK_LAYER_GOOGLE_DeviceMemoryReportlayer by ensuring extension device commands queried viavkGetInstanceProcAddrare verified against the underlying dispatch chain before returning interceptors.Summary of changes:
vkCreateInstance,vkEnumerateInstanceExtensionProperties,vkEnumerateInstanceLayerProperties,vkGetInstanceProcAddr) are handled first without requiring a valid instance.instance == nullptrfor non-global functions.vkGetInstanceProcAddr.vkBindBufferMemory2KHR,vkBindImageMemory2KHR,vkGetImageMemoryRequirements2KHR,vkGetBufferMemoryRequirements2KHR) query the downstream dispatch table first. If unsupported by the underlying driver/layer chain,vkGetInstanceProcAddrreturnsnullptrto avoid applications falsely detecting unsupported extensions.vkBindBufferMemory2KHRandvkBindImageMemory2KHRto ensure dispatch table entry points are non-null when called.