I was asked to help debug a problem where a pointer passed to a function was received slightly offset from the original value. The pointer is passed as an opaque parameter and is supposed to be spit out the other end of a pipeline. But somehow, the value that came out the other end was slightly different.

For expository purposes, let’s say that the underlying function we’re trying to call is this one.

void SubmitWork(HWORKER worker, int commandCode, void* param);

You call Submit­Work with a worker, a command code, and a raw pointer to some data that depends on the command code.

The developer sent me some debugging output that showed that the function that handled the command code received a different pointer from what was passed. One possibility is that there was a bug in the library that resulted in it unintentionally modifying the final pointer parameter, but this seemed unlikely, since the value should be opaque to the library. I was able to get on a video call with the developer to watch them step through the code, and that’s where I noticed something interesting.

The developer’s code didn’t actually call Submit­Work, but rather used a wrapper that in turn called Submit­Work:

class Worker
{
public:
    void SubmitWork(int commandCode, void* param)
    {
        ::SubmitWork(m_worker, commandCode, param);
    }

    template<typename T>
    void SubmitWork(int commandCode, T& param)
        requires std::is_standard_layout_v<T>
    {
        // Delegate to the void* version.
        SubmitWork(m_worker, std::addressof(param));
    }
private:
    HWORKER m_worker;
};

The wrapper is trying to make the Submit­Work() function a little easier to use by letting you pass a standard layout type as the second parameter, and it will pass it by address. The idea is that in addition to passing a raw pointer, you can also pass a structure like this:

struct SetSizeInfo
{
    uint64_t itemId;
    uint64_t size;
};
SetSizeInfo info;
info.itemId = 31415;
info.size = 64;
worker.SubmitWork(CMD_SETSIZE, info);

and it turns into

::SubmitWork(m_hWorker, CMD_SETSIZE, &info);

with an extra check that info is a standard layout type (can be memcpy‘d).

But it turns out that this helper isn’t so helpful.

Suppose you had someone who called the method the old-fashioned pointer way:

char* name = allocate_name();
worker.SubmitWork(CMD_SETNAME, name);

The compiler sees that both of the overloads of the Submit­Work method are in play.

// Using conversion from char*& to void*
worker.SubmitWork(int, void*);

// Treating the char* as a standard layout type.
worker.SubmitWork<char*>(int, char*&);

The second overload is in play because pointers are standard layout types.

And the second overload wins because char*& is a better match for char*& than void*!

This means that instead of passing the pointer through to ::Submit­Work(), the templated helper function passes the address of the pointer to ::Submit­Work().

And that’s why the pointer arrives incorrectly when the work is processed: The pointer that was passed to ::Submit­Work was correctly passed through to the processor. The problem is that the wrong pointer was passed to ::Submit­Work in the first place!

One way to fix this is to make pointers ineligible for the template function.

    template<typename T>
    void SubmitWork(int commandCode, T& param)
        requires std::is_standard_layout_v<T>
        && (!std::is_pointer_v<T>)
    {
        // Delegate to the void* version.
        SubmitWork(m_worker, std::addressof(param));
    }

However, this approach still has a hole:

std::string name("hello");
worker.SubmitWork(CMD_SETNAME, &name);

Here, I am passing a pointer to a non-standard-layout type, and since the template function is not eligible for pointers, the only remaining overload is the one that takes a void*, which std::string* happily converts to.

The helpers were trying to make sure that only pointers to standard layout types are passed to ::Submit­Work, but I was able to sneak a std::string* into ::Submit­Work because the template function that did the enforcement removed itself from consideration rather than accepting the parameter and then complaining about it.

In C++, if the compiler is unable to satisfy the constraints of a function, the function is simply ignored and not considered for overload resolution. You sort of knew that: Suppose you have two overloads of the do_something() function:

void do_something(char* p);
void do_something(int v);

When you write do_something(42), the first overload is ignored because 42 cannot be converted to char*, and the second overload is used because 42 can indeed be converted to int. You did not expect to get an error from the char* overload.

But in our case with Submit­Work(), if you call it with a pointer to a non-standard-layout type, we don’t want the template version to be ignored. We want it to trigger an error! But it doesn’t, because it is a failed substitution, and as we all know, Substitution Failure Is Not An Error (SFINAE). What we want to do is to allow the substitution to succeed, but then generate an error in the function body if the type is not standard layout.

    // Basic version, no extra checking.
    void SubmitWork(int commandCode, void* param)
    {
        ::SubmitWork(m_worker, commandCode, param);
    }

    // This version matches all non-void pointers.     
    template<typename T>                               
    void SubmitWork(int commandCode, T* param)         
    {                                                  
        static_assert(std::is_standard_layout_v<T>,    
            "T must be a standard layout type");       
        // Delegate to the void* version.              
        SubmitWork(commandCode,                        
            static_cast<void*>(param));                
    }                                                  
                                                       
    // This overload matches all references.           
    template<typename T>                               
    void SubmitWork(int commandCode, T& param)         
    {                                                  
        // Delegate to the T* version.                 
        SubmitWork(commandCode, std::addressof(param));
    }                                                  

In my opinion, taking a reference just adds to the confusion. I took the solution a step further and got rid of the reference overload. Everybody just calls with the pointer they want to submit.

    // Basic version, no extra checking.
    void SubmitWork(int commandCode, void* param)
    {
        ::SubmitWork(m_worker, commandCode, param);
    }

    // This version matches all non-void pointers.     
    template<typename T>                               
    void SubmitWork(int commandCode, T* param)         
    {                                                  
        static_assert(std::is_standard_layout_v<T>,    
            "T must be a standard layout type");       
        // Delegate to the void* version.              
        SubmitWork(commandCode,                        
            static_cast<void*>(param));                
    }                                                  

    // This overload matches all references.
    // template<typename T>
    // void SubmitWork(int commandCode, T& param)
    // {
    //     // Delegate to the T* version.
    //     SubmitWork(commandCode, std::addressof(param));
    // }

This avoids confusion over whether passing a pointer means “I’m calling it with the pointer I want”, or whether it means “I’m calling it with a standard layout type that happens to be a pointer!” (so please pass a pointer to it).

In fact, you can go even further and get rid of the overload!

    // Basic version, no extra checking.
    // void SubmitWork(int commandCode, void* param)
    // {
    //     ::SubmitWork(m_worker, commandCode, param);
    // }

    template<typename T>
    void SubmitWork(int commandCode, T* param)
    {
        static_assert(std::is_same_v<T, void> ||   
                      std::is_standard_layout_v<T>,
            "T must be a standard layout type");
        ::SubmitWork(m_worker, commandCode, param));
    }

Now there is no confusion over which overload you are calling because there is no longer an overload.