Skip to content
This repository has been archived by the owner on Jul 19, 2018. It is now read-only.

PFN_GetPhysicalDeviceProcAddr has C++ interface. #2330

Open
stroyan opened this issue Jan 9, 2018 · 2 comments
Open

PFN_GetPhysicalDeviceProcAddr has C++ interface. #2330

stroyan opened this issue Jan 9, 2018 · 2 comments
Assignees
Labels
Milestone

Comments

@stroyan
Copy link
Contributor

stroyan commented Jan 9, 2018

When PFN_GetPhysicalDeviceProcAddr was added to vk_icd.h and vk_layer.h in commit b5f087a it was done without an extern "C" declaration.
The pfnGetPhysicalDeviceProcAddr member in struct VkNegotiateLayerInterface and the vk_layerGetPhysicalDeviceProcAddr functions in validation layers are all using C++ interfaces.
It will be awkward to change to a C binding now. But it will be troubling to use a C++ binding forever.

$ nm ~/Downloads/VulkanSDK/1.0.46.0/x86_64/lib/libVkLayer_threading.so | grep vk_layerGetPhysicalDeviceProcAddr
000000000001f2b0 T _Z33vk_layerGetPhysicalDeviceProcAddrP12VkInstance_TPKc

@mark-lunarg mark-lunarg added the bug label Jan 9, 2018
@mark-lunarg mark-lunarg added this to the P2 milestone Jan 9, 2018
@lenny-lunarg
Copy link
Contributor

It looks like there's some weird stuff going on with these functions, in general. I think the missing extern "C" really belongs in the function declaration or definition. I wonder why it isn't a part of VK_LAYER_EXPORT already. vkGetInstanceProcAddr does not get mangled in the layers because it gets declared in vulkan.h, which is then included in threading.cpp. Without any changes to the repo, vkGetInstanceProcAddr does not get mangled:

$ nm libVkLayer_threading.so | grep vkGetInstanceProcAddr
0000000000033641 T vkGetInstanceProcAddr

But when I comment out the declaration in vulkan.h and rebuild, then the function does get mangled:

$ nm libVkLayer_threading.so | grep vkGetInstanceProcAddr
0000000000033651 T _Z21vkGetInstanceProcAddrP12VkInstance_TPKc

As a result, I don't think this has anything to do wih the declaration for PFN_vkGetInstanceProcAddr being wrapped by extern "C". In addition to this, without making any changes to the repo, we somehow wind up with both a mangled and un-mangled version of vkNegotiateLoaderLayerInterfaceVersion:

$ nm libVkLayer_threading.so | grep vkNegotiateLoaderLayerInterfaceVersion
000000000003368b T vkNegotiateLoaderLayerInterfaceVersion
00000000000a1f60 r _ZZ38vkNegotiateLoaderLayerInterfaceVersionE19__PRETTY_FUNCTION__

I have no explanation for why this happens.

The fact that vk_layerGetPhysicalDeviceProcAddr is getting mangled doesn't really matter, since it gets passed as a function pointer when the interface version is negotiated. That being said, we do need to make sure that any symbols that the loader searches for are not mangled (vkNegotiateLoaderLayerInterfaceVersion being the main one). It looks to me like the general solution would really be to add extern "C" to VK_LAYER_EXPORT when using a C++ compiler.

@stroyan
Copy link
Contributor Author

stroyan commented Jan 11, 2018

There are two different matters that the c++ name mangling brings into question.
Finding symbols by name is important. But the calling convention for C vs C++ may also matter.
Many implementations of C++ have a calling convention that is binary compatible with C for simple types.
But there is no guarantee of that. Both function typedefs and function declarations should be C for vulkan. I see many places where functions themselves have c++ declarations in the layers and mock icd.

The *PRETTY_FUNCTION symbols seem to be harmless. They are string constants that result from function name identifiers like PRETTY_FUNCTION
https://gcc.gnu.org/onlinedocs/gcc/Function-Names.html

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants