memory leaks detected by MSVCRT debugging facilities

[BeErikk]

Extra information

Environment: Windows 10
Compiler: Microsoft (R) Compiler Version 19.11.25547 for x64 (VS2017 15.4.4)
Compiler options: -sdl -guard:cf -Zc:inline -Zc:rvalueCast -Zc:referenceBinding -Zc:strictStrings -std:c++latest
Version of Catch: Current master git source

Description

I've just started with Catch and I created a test project following code as described in the Catch2/docs/tutorial.md. I added the #define CATCH_CONFIG_RUNNER and implemented my own wmain, basically a copy of the default. I also added the _CrtDumpMemoryLeaks() diagnostics in the end of wmain. I get the following output:

Detected memory leaks!
Dumping objects ->
{223} normal block at 0x0000013DA7B8EC30, 16 bytes long.
 Data: < ZQ             > B8 5A 51 F6 F7 7F 00 00 00 00 00 00 00 00 00 00 
Object dump complete.

Is it possible this could be a false indication of memory allocated by Catch waiting to be released at program exit? If so, is it by design?

Steps to reproduce

Compile the source file
catch_tutorial.cpp.txt

[BeErikk]

Just an additional note:
By adding the following and set a breakpoint at _CrtDumpMemoryLeaks(), I think you can be sure to have left wmain. _CrtDumpMemoryLeaks() still report a memory leak.

class silly
{
public:
    silly() {}
    virtual ~silly() 
    {
        _CrtDumpMemoryLeaks();  // set a debug breakpoint here
    }
};
silly me;

[horenmar]

There should be no leaks in Catch, and under Linux we run CI with Valgrind. We also support the crt debug heap, just define CATCH_CONFIG_WINDOWS_CRTDBG in the implementation file.

Anyway, I tried to reproduce this and I cannot:

C:\Users\Xarn\source>more a.cpp
#define CATCH_CONFIG_RUNNER
#include "catch.hpp"

extern "C" int __cdecl
wmain(_In_ int argc,_In_reads_(argc) _Pre_z_ wchar_t* argv[],_In_z_ wchar_t* /*envp*/[]) {
        int ret = 0;
        ret = Catch::Session().run(argc, argv);
        _CrtDumpMemoryLeaks();
        return ret;
}


unsigned int Factorial(unsigned int number) {
        return number > 1 ? Factorial(number-1)*number : 1;
}

TEST_CASE( "Factorials are computed", "[factorial]" ) {
        REQUIRE( Factorial(0) == 1 );
        REQUIRE( Factorial(1) == 1 );
        REQUIRE( Factorial(2) == 2 );
        REQUIRE( Factorial(3) == 6 );
        REQUIRE( Factorial(10) == 3628800 );
}

C:\Users\Xarn\source>cl a.cpp /EHsc -sdl -guard:cf -Zc:inline -Zc:rvalueCast -Zc:referenceBinding -Zc:strictStrings -std:c++latest /D_UNICODE /DUNICODE /W4
Microsoft (R) C/C++ Optimizing Compiler Version 19.11.25547 for x86
Copyright (C) Microsoft Corporation.  All rights reserved.

a.cpp
Microsoft (R) Incremental Linker Version 14.11.25547.0
Copyright (C) Microsoft Corporation.  All rights reserved.

/out:a.exe
/guard:cf
a.obj


C:\Users\Xarn\source>a.exe
===============================================================================
All tests passed (5 assertions in 1 test case)

[BeErikk]

crtdbg routines are debug build diagnostics. You will need a debug build.
see MSDN: crtdumpmemoryleaks
Furthermore, I see the crtdumpmemoryleaks logs in the Visual Studio output logging window during debugging. I think you'll need a debug session to see these log messages. I'm sure there are settings for CI and possibly even manual commandline tools. You can for example drag the executable you built from the commandline into VS and start a debug session.

Anyway, to confirm I also tested building from the commandline:

set INCLUDE=%INCLUDE%;G:\libraries\unittests\Catch\single_include
cl.exe -D_DEBUG -EHsc -MDd catch_tutorial.cpp

Then drag into VS for debug session

Updated source file
catch_tutorial2.cpp.txt

edit:
Also confirmed with v141_clang_c2 (MS supplied clang with Visual Studio)

[philsquared]

I'm fairly sure I've fixed this now (but not been able to test on Windows).

I recently added another singleton (I know, I know!) and forget to add it to the clean-up call.
The idea is that all singletons are cleaned up before any leak detection runs, so should not muddy the waters.

[BeErikk]

No, sorry I still get a dump from both end of main and the silly instance destructor

[philsquared]

I should have mentioned, btw, that my fix is not (yet) present in the single include - so you'd need to regenerate that (generateSingleInclude.py in the scripts folder).

[philsquared]

BTW you shouldn't need to explicitly call _CrtDumpMemoryLeaks() as Catch does it for you if it detects Windows.
And your original example won't work, as is, because you're calling _CrtDumpMemoryLeaks() before the destructor of Catch::Session, which is what performs all the clean-up.

[philsquared]

Sorry, correcting myself, Catch only does leak detection if you define CATCH_CONFIG_WINDOWS_CRTDBG

[philsquared]

(see: https://github.com/catchorg/Catch2/blob/master/docs/configuration.md#other-toggles)

[BeErikk]

Calling _CrtDumpMemoryLeaks() is just routine and was actually my initial question, if Catch generate a false indication due to late memory release. My "silly me" test was a try to confirm that.
I'm now trying the _CRTDBG_MAP_ALLOC macro to see where the memory is coming from (see for example Why doesn't leak detection give me line numbers?)
but I get errors catch.hpp(9321): error C2061: syntax error: identifier 'nothrow'

Well, I'll try to generate a new header and see if it went away

edit:
I also tried

int ret = 0;
{
    ret = Catch::Session().run(argc, argv);
}
_CrtDumpMemoryLeaks();

[BeErikk]

Updated singleheader and still get memory dumps

[philsquared]

🤔

[BeErikk]

To be sure, please generate a confirmed single header and push it to git.
BTW how should I write a custom main without the single header, using the Catch header tree?

[philsquared]

there's no difference with how you declare a custom main in the full project vs the single header, but note the full project now includes cpp files.

[BeErikk]

I get error C2039: 'Session': is not a member of 'Catch'
hm I think I need at least to add
#include "catch_session.h"
and then probably supply a static Catch lib?

[philsquared]

you should just need to do the usual, #define CATCH_CONFIG_RUNNER before #includeing "catch.hpp" from the include folder.
But you will also need to add all the cpp files (which, non-intuitively - this is transitional) are in include/internal.
You could build those as a static lib, if you like - or just include them all in your project.

[BeErikk]

Right, that's quite a list if I understand it correctly

@echo // FOR /f "tokens=*" %i in ('@dir /b "*.cpp"') DO @echo #include ^<%i^> >  catch_impl.h
FOR /f "tokens=*" %i in ('@dir /b "*.cpp"') DO @echo #include ^<%i^> >> catch_impl.h

catch_impl.h.txt

[philsquared]

Yes, at a glance that looks about right.

[BeErikk]

I tried to locate the line where the reported memory is allocated according to
Finding Memory Leaks Using the CRT Library
tldr ie

#define _CRTDBG_MAP_ALLOC
#include <stdlib.h>
#include <crtdbg.h>
#ifdef _DEBUG
#ifndef DBG_NEW
#define DBG_NEW new (_NORMAL_BLOCK,__FILE__,__LINE__)
#define new DBG_NEW
#endif
#endif

as mentioned before, this will clash against
internal/catch_test_registry.cpp(15): return new(std::nothrow) TestInvokerAsFunction( testAsFunction );
and if replaced with a simple new, compile will will fail the expression (at several places)

alignas(alignof(T)) char storage[sizeof(T)];
T* nullableValue = new( storage ) T( _value );

Not sure how to proceed

[philsquared]

I've never been very satisfied with the DBG_NEW stuff.
I've always resorted to setting a breakpoint on the allocation:
https://docs.microsoft.com/en-gb/visualstudio/debugger/finding-memory-leaks-using-the-crt-library#setting-breakpoints-on-a-memory-allocation-number

[BeErikk]

I changed to MTd (see MSDN comments), I get

Detected memory leaks!
Dumping objects ->
{106} normal block at 0x000001FC6B5FD690, 16 bytes long.
 Data: <  {             > F8 F8 7B FF F6 7F 00 00 00 00 00 00 00 00 00 00 
Object dump complete.

So setting a breakpoint at allocation number 106 => no effect for me

[philsquared]

Probably it's too early (usually happens if it's from a static constructor that gets called before main).
What I used to do in that case is break on a higher allocation number (say 1000) - doesn't matter what - and that should break you on the line in the MS code that triggers it. You can then stop, set a line breakpoint there, and that should give you a chance to set the allocation breakpoint.

[philsquared]

(sorry for vagueness, it's a couple of years since I've had to do that)

[BeErikk]

Still think it would be good to get the DBG_NEW macro to work
Could you figure out a compatible way?

[nyamatongwe]

This occurs for me with Catch 2.01 single header and Visual C++ 2017.6.4. Plugging the allocation number into _crtBreakAlloc breaks on line 9546 of catch.hpp:
const std::string unprintableString = "{?}";

Its not really a problem here - I'll just switch to later dumping of leaks after global tear-down by using
#define CATCH_CONFIG_WINDOWS_CRTDBG

@BeErikk : there is a technique described on stackoverflow for setting the allocation breakpoint early so it can catch allocations made before main.
https://stackoverflow.com/questions/3118741/memory-leak-unable-to-break-on-a-memory-allocation-number

[JoeyGrajciar]

I guess this should be already fixed by #1411

[horenmar]

@JoeyGrajciar It both is, and isn't.

1411 moves the final release point forward a bit, but you will still get false positives if you try to check allocations before ~LeakDetector runs. I am going to close this anyway, as fundamentally that FP potential will always be there.