3

I apologize for this rather localized question, but I was hoping to get someone else's view on it to make sure I'm not doing something obviously wrong.

I believe I've run into a bug in either the Visual C++ Runtime library, or somewhere in Microsoft's implementation of std::stringstream. The issue only manifests itself with a combination of when:

  1. imbue() is called to change the locale on a stringstream, and
  2. A custom global operator new is used which returns a pointer offset from the base address returned by malloc() used to allocate the block.

I've been able to reproduce this with the following minimal test case:

#include <sstream>

static void *localMalloc(size_t bytes)
{
    unsigned char *ptr = static_cast<unsigned char *>( malloc(bytes + 64) );
    ptr += 64;
    printf("malloc of %d bytes: %ph\n", bytes, ptr);
    return ptr;
}

void *operator new(size_t bytes) { return localMalloc(bytes); }
void *operator new[](size_t bytes) { return localMalloc(bytes); }
void operator delete(void *ptr) throw() { /* do nothing */ }
void operator delete[](void *ptr) throw() { /* do nothing */ }

struct DecimalSeparator : std::numpunct<char>
{
    char do_decimal_point() const
    {
        return '.';
    }
};

int main()
{
    std::stringstream ss;
    ss.imbue(std::locale(std::locale(), new DecimalSeparator));
    ss << 5;  // <-- is_block_type_valid(header->_block_use) assertion failure here
    return 0;
}

If either:

  1. The ptr += 64; line in localMalloc() or
  2. The ss.imbue() call in main()

are commented out, the code works as expected, and the assertion does not happen.

I have attempted to step into the code using the debugger as much as I possibly can, but I am currently unable to pinpoint the location at where the code fails in the STL as Visual Studio dumps me into raw disassembly mode after stepping out of basic_stringbuf::overflow() making debugging nearly impossible. As far as I can tell, I haven't seen any invalid memory writes outside of the memory being allocated, so I'm not completely sure where the CRT is checking the heap or why it thinks the pointer is invalid.

Note that operator delete is intentionally ignoring the free in this test case for brevity. It makes no difference if free() is properly called on the memory block or not.

So far, tests on the following compilers and platforms have resulted in:

  1. VS2015 Update 2: fails
  2. VS2015 Update 3: fails
  3. VS2015 Update 3 + KB3165756: fails
  4. clang-703.0.31 on OSX: OK

Does anyone see anything odd here that I missed?

falken
  • 884
  • 9
  • 21
  • 2
    Side Note:The name `_localMalloc` is reserved for the compiler: http://stackoverflow.com/questions/228783/what-are-the-rules-about-using-an-underscore-in-a-c-identifier –  Jul 27 '16 at 10:33
  • @DieterLücking Good point, forgot about that. I've removed the underscore and retested again to make sure it wasn't an issue. – falken Jul 27 '16 at 10:45
  • Run a release version instead of a debug version. When you run a debug version, your code's behavior is probably undefined due to the debug runtime having its own version of `delete` being called, which does the check and sees that your pointer is not valid. – PaulMcKenzie Jul 27 '16 at 14:19
  • @PaulMcKenzie I expected something similar myself, but it crashes in Release mode as well: `Unhandled exception at ... (ucrtbase.dll) in test.exe: Fatal program exit requested.` So something in the CRT is calling `abort()`, perhaps? – falken Jul 28 '16 at 02:43
  • Doesn't [crash here](http://rextester.com/GSX54431) with the online VC++ compiler. I also took your code and compiled a release version using VS 2015 -- no crash. – PaulMcKenzie Jul 28 '16 at 02:47
  • @PaulMcKenzie Ok, now that's interesting. _MSC_VER on the online compiler you linked to shows 1900, so it's VS2015 as well. What Update are you using there? – falken Jul 28 '16 at 02:54
  • I'm using update 2. Don't know what the online compiler is using. But both do not crash. I think the behavior of your debug build is "expected", since the program is undefined if you have competing versions of `operator delete` defined in two separate modules (yours and the debug runtime). – PaulMcKenzie Jul 28 '16 at 02:56
  • Please see [Global replacements](http://en.cppreference.com/w/cpp/memory/new/operator_delete) here...`The behavior is undefined if more than one replacement is provided in the program `. Your debug version, according to that link, is in undefined behavior territory. – PaulMcKenzie Jul 28 '16 at 03:01
  • Here is an [updated VC++ test](http://rextester.com/NZR88334) with the version. It shows that the replacement global delete is being called. – PaulMcKenzie Jul 28 '16 at 03:09
  • @PaulMcKenzie Sorry, forget about the crash in Release mode: I'd forgotten I modified the code yesterday to try allocating the `DecimalSeparator` on the stack instead of the heap to see if it made any difference. Going back to the same snippet I posted here does not crash in Release mode. – falken Jul 28 '16 at 03:41

1 Answers1

1

This is more than likely not a bug, but instead your version of delete is not being called, and instead the Visual Studio's debug runtime library's version of the global delete is called. Having two or more versions of the global delete operator in the same program is undefined behavior.

From this reference (Global replacements), the behavior is stated to be undefined when this occurs.

From the C++ ISO standard:

3.7.4 Dynamic storage duration [basic.stc.dynamic]
//...

§2 The library provides default definitions for the global allocation and deallocation functions. Some global allocation and deallocation functions are replaceable (18.6.1). A C++ program shall provide at most one definition of a replaceable allocation or deallocation function.


Running with Visual Studio 2015 using the release version Visual Studio runtime library does not produce this error, and the replacement global delete does in fact get called.

Visual Studio 2015 online compiler results.

PaulMcKenzie
  • 34,698
  • 4
  • 24
  • 45
  • Yeah, it looks like this is what's happening. If I change the code generation options to statically link the CRT instead of using a DLL, the assert no longer happens. So basically it's UB to replace the global `operator delete` in any program that links to the CRT dynamically? How do all other applications handle this? – falken Jul 28 '16 at 03:39
  • I guess the other applications are aware of this and don't use debug runtime libraries. – PaulMcKenzie Jul 28 '16 at 03:47
  • Ah ok, so it's only the *debug* CRT that replaces `operator delete`? In that case I can use static linking with Debug configurations, but still use dynamic linking with Release configurations? – falken Jul 28 '16 at 03:51
  • 1
    I believe it should be ok (unless someone else chimes in), since the runtime library to use is a compile setting (not a linker setting) in Visual C++. If there are errors, then they would show up as linker errors. However, it is undefined behavior if indeed you have two or more global `delete` functions in the same program. You may have just been lucky that the statically linked debug library didn't have the same error. – PaulMcKenzie Jul 28 '16 at 03:56