64-bit Visual Studio /MT labels memmove as "memcpy", resulting in failure as replace_memcpy (correctly) does not handle overlap

[avaneev]

The issue happens in the DrMemory 1.10.0-2 64-bit on Windows 7 Pro. Due to this issue most code simply crashes.

The problem is that the original C memmove was designed to cope with overlapping memory regions. But the replacement memmove does not seem to work the same way.

[derekbruening]

Enabling our app_suite for 64-bit, I do see the memmove test failing. However, it looks like the issue is that the compiler is calling memcpy instead of memmove when the source code asks for memmove.

[avaneev]

No such issue happens if exactly the same executable is run without Dr.Memory. Not a compiler issue for sure.

[derekbruening]

There is no problem with 64-bit replace_memmove on Linux or with 64-bit mingw on Windows, or with 32-bit anywhere. This is a problem specific to 64-bit Visual Studio.

Building this app:

#include <stdio.h>
#include <string.h>

// From DrMemory's app_suite/strmem_tests.cpp
int main()
{
    const char *result = "01234563456789abcdefg";
    const char input[128] = "0123456789abcdefg";  // strlen(input) = 17.
    char tmp[128];
    strcpy(tmp, input);
    // Overlapping copy forwards, should skip 1 byte before going to fastpath.
    if (tmp+7 != memmove(tmp+7, tmp+3, strlen(tmp) + 1 - 3)) {
        printf("memmove return value is incorrect\n");
        return 1;
    }
    if (strcmp(result, tmp) != 0) {
        printf("memmove bug: %s instead of %s\n", input, result);
        return 1;
    }
    printf("memmove is correct\n");
    return 0;
}

With 32-bit Visual Studio 2010 or 2013, it always calls memmove, even with /GF and other flags.

But building it with 64-bit VS 2010, 2012, or 2013, with simple flags such as "cl /Zi /EHsc /MT", and it calls what is labeled as memcpy instead of memmove. This is a static routine included in the executable, and it appears to actually be handling overlap, but it is misleadingly labeled in the debug info as "memcpy".

Note that the dumpbin output can be misleading, as with VS 2010 it appears to call memmove:

# compilerVS2010_64 && ninja_x64 && rm -f memmove.{pdb,ilk,obj,exe} && cl /Zi /MT /EHsc memmove.cpp && dumpbin /disasm memmove.exe | grep -A 70 ^main: | grep mem
Microsoft (R) C/C++ Optimizing Compiler Version 16.00.40219.01 for x64
  00000001400010BB: E8 E0 03 00 00     call        memmove

Yet windbg shows it as memcpy:

   12 00000001`3fe510bb e8e0030000      call    memmove!memcpy (00000001`3fe514a0)

In fact there is no memmove:

0:000> x memmove!mem*
00000001`3fe514a0 memmove!memcpy (void)
00000001`3fe60090 memmove!memcmp (void)
00000001`3fe56910 memmove!memset (void)

Seeing a routine named "memcpy", Dr. Memory naturally replaces it with "replace_memcpy", leading to failure:

# compilerVS2010_64 && rm -f memmove.{pdb,ilk,obj,exe} && cl /Zi /EHsc /MT memmove.cpp
Microsoft (R) C/C++ Optimizing Compiler Version 16.00.40219.01 for x64

# ~/drmemory/releases/DrMemory-Windows-1.10.0-2/bin64/drmemory -batch -- ./memmove.exe
memmove bug: 0123456789abcdefg instead of 01234563456789abcdefg
~~Dr.M~~ WARNING: application exited with abnormal code 0x1

Using /MD, the compiler calls out to a shared library, where it has to get the names right, and everything works:

# compilerVS2010_64 && rm -f memmove.{pdb,ilk,obj,exe} && cl /Zi /EHsc /MD memmove.cpp
Microsoft (R) C/C++ Optimizing Compiler Version 16.00.40219.01 for x64

# ~/drmemory/releases/DrMemory-Windows-1.10.0-2/bin64/drmemory -batch -- ./memmove.exe
memmove is correct

I would call this a compiler bug, though one that has no correctness consequences for running natively. It is quite annoying, as the only simple fix is to always use replace_memmove on 64-bit Windows for anything called "memcpy", which will be a performance hit.

Note that MacOS and Linux libc implementations have run into similar things in the past, where applications relied on undefined behavior by assuming memcpy handled overlap, and I believe that MacOS libc aliases memcpy to memmove. The difference, though, is that they have both memcpy and memmove labeled properly, they just have both pointing at the same address. We'll handle this today b/c we process memmove after memcpy and will replace our memcpy replacement with our memmove replacement.

[derekbruening]

One workaround is to build your app /MD.

[derekbruening]

There are native consequences: an app calling a memcpy that handles overlap incurs unnecessary performance overhead.

[avaneev]

You are probably right, but I'm using Intel C++ Compiler 14 which means this bug is not limited to VS.

[derekbruening]

Please provide the corresponding info for Intel C++ Compiler: is it the same underlying behavior -- i.e., naming the overlap version "memcpy" while not providing anything called "memmove", and only doing this for a static libc? Is this behavior limited to 64-bit compilation?

[avaneev]

Yes, this issue is limited to 64-bit compilation and static libc. Since Intel C++ Compiler is configured to use VS libraries (or Microsoft Platform SDK), the problem may be in this 64-bit library, not in the compilers.

[derekbruening]

Visual Studio is confirmed as fixed.

I filed https://connect.microsoft.com/VisualStudio/feedback/details/2506331 about the Visual Studio behavior, though it's a minor issue: we'll see what they say.

[egrimley]

I think Linus Torvalds was in favour of aliasing memcpy to memmove:

https://bugzilla.redhat.com/show_bug.cgi?id=638477#c101

https://lwn.net/Articles/414467/

It sounds as though Microsoft may have done something similar, but a bit mixed up. There may be virtually no overhead from combining memcpy and memmove into a single function. There is certainly plenty of confusion if they call the combined function "memcpy".

[derekbruening]

As mentioned above, MacOS libc already aliases memcpy to memmove, and for Linux it's sthg like memcpy@GLIBC_2.2.5 is aliased to memmove, but with the proper inclusion of both names these all work out fine.