Vulkan-hpp is reinterpret_casting non-standard-layout class to another class. Is this legal?

Question

So recently I have been working with Vulkan-Hpp (The official c++ bindings of Vulkan Api, Github Link).

Looking into the source, I have found that they create wrapper classes around native Vulkan structs (e.g. vk::InstanceCreateInfo wraps around VkInstanceCreateInfo). (Note: Wrapping around, not deriving from)

When calling native Vulkan API, the pointers to wrapper classes are reinterpret_cast ed into the native Vulkan structs. An example using vk::InstanceCreateInfo:

//definition of vk::InstanceCreateInfo
struct InstanceCreateInfo
{
    /*  member function omitted  */
private:
  StructureType sType = StructureType::eInstanceCreateInfo;
public:
  const void* pNext = nullptr;
  InstanceCreateFlags flags;
  const ApplicationInfo* pApplicationInfo;
  uint32_t enabledLayerCount;
  const char* const* ppEnabledLayerNames;
  uint32_t enabledExtensionCount;
  const char* const* ppEnabledExtensionNames;
};

//definition of VkInstanceCreateInfo
typedef struct VkInstanceCreateInfo {
    VkStructureType             sType;
    const void*                 pNext;
    VkInstanceCreateFlags       flags;
    const VkApplicationInfo*    pApplicationInfo;
    uint32_t                    enabledLayerCount;
    const char* const*          ppEnabledLayerNames;
    uint32_t                    enabledExtensionCount;
    const char* const*          ppEnabledExtensionNames;
} VkInstanceCreateInfo;

//And the usage where reinterpret_cast takes place
template<typename Dispatch>
VULKAN_HPP_INLINE ResultValueType<Instance>::type createInstance( const InstanceCreateInfo &createInfo, Optional<const AllocationCallbacks> allocator, Dispatch const &d )
{
  Instance instance;
  Result result = static_cast<Result>( d.vkCreateInstance( reinterpret_cast<const VkInstanceCreateInfo*>( &createInfo ), reinterpret_cast<const VkAllocationCallbacks*>( static_cast<const AllocationCallbacks*>( allocator ) ), reinterpret_cast<VkInstance*>( &instance ) ) );
  return createResultValue( result, instance, VULKAN_HPP_NAMESPACE_STRING"::createInstance" );
}

So my question is: vk::InstanceCreateInfo and VkInstanceCreateInfo are two different types. Moreover, VkInstanceCreateInfo is standard layout but vk::InstanceCreateInfo is not (since it has mixed access specifiers). Is reinterpret_casting between the pointer of those two types (as done by Vulkan-Hpp) legal? Does this violate the strict aliasing rule?

Note: you can assume that VkInstanceCreateFlags and vk::InstanceCreateFlags are interchangeable in this case (otherwise it would make my question recursive)

@L.F. can you elaborate how private member breaks the layout? — Marek R, Jul 11 '19 at 09:59
@MarekR I remember reading somewhere that this has something to do with standard layout, but I am not sure either .. — L. F., Jul 11 '19 at 10:05
@MarekR Actually this kind of `reinterpret_cast` are everywhere around the library (I've checked the macros, and verified that it is not a compiler specific feature). And about access specifiers, `All non-static data members have the same access control` in order for a class to be standard layout. — ph3rin, Jul 11 '19 at 10:09
@L.F. And I do remember that the standard only allow pointer aliasing between same `effective types` (which simply means `type` when the type is declared). So this means even those two types are both standard layout types and have exact same members, type punning is not allowed. Maybe the correct way is to use inheritance? Feel free to correct me if I am wrong. — ph3rin, Jul 11 '19 at 10:15
@KaenbyouRin Even if this code is technically valid, it should be redesigned anyway ... — L. F., Jul 11 '19 at 10:16
The [Type aliasing](https://en.cppreference.com/w/cpp/language/reinterpret_cast#Type_aliasing) rule is pretty clear here - this is undefined behavior. — nikitablack, Jul 11 '19 at 10:26

score 6 · Accepted Answer · answered Jul 11 '19 at 13:29

6

As far as the standard is concerned, yes, accessing a vk::InstanceCreateInfo object through a pointer to a VkInstanceCreateInfo object violates strict aliasing. And it would still violate strict aliasing even if both types were standard layout with equivalent layout. The standard doesn't allow you to just pretend that one class type is another, even if they have equivalent layouts.

So this code is relying on some implementation-specific behavior.

Is that wrong? Would doing an additional copy for every single Vulkan interface function call make you feel better about it, even if it would work without it? It's ultimately up to you how much UB you feel like tolerating.

answered Jul 11 '19 at 13:29

Nicol Bolas

449,505
63
781
982

They should make C++ light with less UB added but more sugar substitutes. – Hatted Rooster Jul 11 '19 at 13:34
1

I believe they can derive their wrapper classes from the native ones, since pointer casting from derived to base is 100% well defined behavior. – ph3rin Jul 11 '19 at 14:15
1

@KaenbyouRin: The whole point of making that field private is so that users cannot accidentally change it. Deriving from the native type would work against that purpose. – Nicol Bolas Jul 11 '19 at 14:20
@NicolBolas This defeats the purpose of course, but I believe automating the initialization of that field should be enough (you don't fill in the wrong value). Any sane implementation won't try to modify that value after initialization anyway. I can see another approach that we derive `protected`ly or `private`ly from the base, then we write access functions for everything except the `sType` field. This would make things really complicated though... – ph3rin Jul 11 '19 at 14:34
@NicolBolas To correct the mistake from the last comment: we should use `public` inheritance and do additional `using` access declaration to make the field private. – ph3rin Jul 12 '19 at 01:20

Vulkan-hpp is reinterpret_casting non-standard-layout class to another class. Is this legal?

1 Answers1