<functional>: Passing an object with an explicit alignment that owns a std::function by value leads to invalid free on x86

[StephanTLavavej]

Repros with VS 2019 16.7 Preview 3.1. Specific to x86.

C:\Temp>type meow.cpp
#include <functional>
#include <stdio.h>

struct alignas(8) Overaligned
{
    std::function<void()> call;
};

static void SomeFunction(Overaligned over)
{
}

int main()
{
    printf("%zu\n", alignof(std::function<bool()>));
    printf("%zu\n", alignof(Overaligned));

    Overaligned ov;
    ov.call = [] {};
    SomeFunction(ov);
}

C:\Temp>ren meow.cpp totally_unique_name.cpp

C:\Temp>cl /EHsc /nologo /W4 /Od /MD totally_unique_name.cpp
totally_unique_name.cpp

C:\Temp>totally_unique_name
8
8
***CRASH***

I currently observe that the executable will crash for the first 3 times that I try to run it, then (without recompilation!) it will stop displaying the crash dialog. Renaming the source file to something unique and recompiling will repro the crash again. (Maybe Windows is detecting frequently-crashing executables and suppressing the Windows Error Reporting crash dialog?)

Different from GH-690.

Originally reported as DevCom-367683, also tracked by Microsoft-internal VSO-720596.

[AlexGuteniev]

Compiler bug. Non-STL repro:

#include <stdio.h>

struct SelfReferencingType
{
    SelfReferencingType() 
    {
        ptr = this;
    }

    SelfReferencingType(const SelfReferencingType&) 
    { 
        printf("copy\n");
        ptr = this;
    }

    SelfReferencingType(SelfReferencingType&&) 
    { 
        printf("move\n");
        ptr = this; 
    }

    SelfReferencingType& operator=(const SelfReferencingType&)
    {
        printf("copy asg\n");
    }

    SelfReferencingType& operator=(SelfReferencingType&&) 
    {
        printf("move asg\n");
    }


    void Check() 
    {
        if (this != ptr)
            printf("fail\n");
    }

    SelfReferencingType* ptr;

    char pad[24]; // Not really essential for repro, but to see memcpy directly
};

struct alignas(8) Overaligned
{
    SelfReferencingType srt;
};

static void SomeFunction(Overaligned over)
{
    over.srt.Check();
}

int main()
{
    Overaligned ov;
    ov.srt.Check();
    SomeFunction(ov);
}

The problematic is memcpy inside SomeFunction that tries to align and does not use copy/move constructor

00BC22CC  mov         eax,dword ptr [ebx+8]  
00BC22CF  push        eax  
00BC22D0  lea         ecx,[over]  
00BC22D6  push        ecx  
00BC22D7  call        _memcpy (0BC13A2h)  
00BC22DC  add         esp,0Ch  
00BC22DF  lea         edx,[over]  
00BC22E5  mov         dword ptr [ebx+8],edx  
00BC22E8  mov         ecx,offset _274A5FCE_kuku@cpp (0BCC003h)  
00BC22ED  call        @__CheckForDebuggerJustMyCode@4 (0BC121Ch)  
    over.srt.Check();
00BC22F2  mov         ecx,dword ptr [ebx+8]  
00BC22F5  call        SelfReferencingType::Check (0BC12E9h)  

[AlexGuteniev]

Crash handling depends on various factors. Crash is definitely seen in debugger.
For command-line repro, you can just add printf("the execution does not reach here because of the crash"); at the end.

Also, build with /MDd (or /MTd) to trigger checks, otherwise the crash is indeed optional.

[AlexGuteniev]

It may make sense to avoid self-reference in std::function though.
It will not only work around this issue, but also will make function memcpy-movable by user, thus usable with CArray or CAtlArray with default traits.

[StephanTLavavej]

Thanks for the STL-free repro, I'll send this to the compiler backend team! Skipping the copy constructor is definitely bad.

Note that the self-referencing is inherent to the Small Functor Optimization which is critically important for performance.

[MikeGitb]

Note that the self-referencing is inherent to the Small Functor Optimization which is critically important for performance.

Are you sure? I thought Sean Parent had shown in his lightning talk "Polymorphic Task Template in Ten" (Meeting C++ 2017) how this works without the extra pointer: https://www.youtube.com/watch?v=2KGkcGtGVM4. I don't remember if there were any problems with his implementation that would prevent the same implementation strategy in std::function.

[StephanTLavavej]

Hadn't seen that talk - if you can write up a summary of the technique, a vNext performance issue would be great! (I suppose it could just link to the video if summarizing it is difficult.)

[MikeGitb]

For startes, you can find the slides here: https://sean-parent.stlab.cc/papers-and-presentations. I can see, if I can make a proper issue out of this on the weekend.

[AlexGuteniev]

I've tried to explain very briefly in #969