Repair MSVC-CRT leakage detector and enable this detection with Appveyor-CI (#5105)

CMakeLists.txt

@@ -255,7 +255,7 @@ endif()
 if(FLATBUFFERS_BUILD_FLATC)
   add_executable(flatc ${FlatBuffers_Compiler_SRCS})
   target_compile_options(flatc PRIVATE "${FLATBUFFERS_PRIVATE_CXX_FLAGS}")
-  if(FLATBUFFERS_CODE_SANITIZE)
+  if(FLATBUFFERS_CODE_SANITIZE AND NOT WIN32)
     add_fsanitize_to_target(flatc ${FLATBUFFERS_CODE_SANITIZE})
   endif()
   if(NOT FLATBUFFERS_FLATC_EXECUTABLE)
@@ -313,12 +313,17 @@ if(FLATBUFFERS_BUILD_TESTS)
   compile_flatbuffers_schema_to_cpp(tests/monster_test.fbs)
   include_directories(${CMAKE_CURRENT_BINARY_DIR}/tests)
   add_executable(flattests ${FlatBuffers_Tests_SRCS})
-  if(FLATBUFFERS_CODE_SANITIZE)
-    add_fsanitize_to_target(flattests ${FLATBUFFERS_CODE_SANITIZE})
-  endif()
   set_property(TARGET flattests
     PROPERTY COMPILE_DEFINITIONS FLATBUFFERS_TRACK_VERIFIER_BUFFER_SIZE
     FLATBUFFERS_DEBUG_VERIFICATION_FAILURE=1)
+  if(FLATBUFFERS_CODE_SANITIZE)
+    if(WIN32)
+      target_compile_definitions(flattests PRIVATE FLATBUFFERS_MEMORY_LEAK_TRACKING)
+      message(STATUS "Sanitizer MSVC::_CrtDumpMemoryLeaks added to flattests")
+    else()
+      add_fsanitize_to_target(flattests ${FLATBUFFERS_CODE_SANITIZE})
+    endif()
+  endif()
 
   compile_flatbuffers_schema_to_cpp(samples/monster.fbs)
   include_directories(${CMAKE_CURRENT_BINARY_DIR}/samples)

appveyor.yml

@@ -31,7 +31,7 @@ configuration:
 
 before_build:
   - set MONSTER_EXTRA=%MONSTER_EXTRA%
-  - cmake -G"Visual Studio %CMAKE_VS_VERSION%"
+  - cmake -G"Visual Studio %CMAKE_VS_VERSION%" -DFLATBUFFERS_CODE_SANITIZE=1
   # This cuts down on a lot of noise generated by xamarin warnings.
   - del "C:\Program Files (x86)\MSBuild\14.0\Microsoft.Common.targets\ImportAfter\Xamarin.Common.targets"
 

include/flatbuffers/base.h

@@ -2,9 +2,18 @@
 #define FLATBUFFERS_BASE_H_
 
 // clang-format off
+
+// If activate should be declared and included first.
 #if defined(FLATBUFFERS_MEMORY_LEAK_TRACKING) && \
     defined(_MSC_VER) && defined(_DEBUG)
+  // The _CRTDBG_MAP_ALLOC inside <crtdbg.h> will replace
+  // calloc/free (etc) to its debug version using #define directives.
   #define _CRTDBG_MAP_ALLOC
+  #include <stdlib.h>
+  #include <crtdbg.h>
+  // Replace operator new by trace-enabled version.
+  #define DEBUG_NEW new(_NORMAL_BLOCK, __FILE__, __LINE__)
+  #define new DEBUG_NEW
 #endif
 
 #if !defined(FLATBUFFERS_ASSERT)
@@ -23,13 +32,6 @@
 #include <cstdlib>
 #include <cstring>
 
-#if defined(FLATBUFFERS_MEMORY_LEAK_TRACKING) && \
-    defined(_MSC_VER) && defined(_DEBUG)
-  #include <crtdbg.h>
-  #define DEBUG_NEW new(_NORMAL_BLOCK, __FILE__, __LINE__)
-  #define new DEBUG_NEW
-#endif
-
 #if defined(ARDUINO) && !defined(ARDUINOSTL_M_H)
   #include <utility.h>
 #else

tests/test.cpp

@@ -2463,13 +2463,6 @@ void CreateSharedStringTest() {
 
 int FlatBufferTests() {
   // clang-format off
-  #if defined(FLATBUFFERS_MEMORY_LEAK_TRACKING) && \
-      defined(_MSC_VER) && defined(_DEBUG)
-    _CrtSetDbgFlag(_CRTDBG_ALLOC_MEM_DF | _CRTDBG_LEAK_CHECK_DF
-      // For more thorough checking:
-      //| _CRTDBG_CHECK_ALWAYS_DF | _CRTDBG_DELAY_FREE_MEM_DF
-    );
-  #endif
 
   // Run our various test suites:
 
@@ -2563,9 +2556,8 @@ int main(int /*argc*/, const char * /*argv*/ []) {
 
   if (!testing_fails) {
     TEST_OUTPUT_LINE("ALL TESTS PASSED");
-    return 0;
   } else {
     TEST_OUTPUT_LINE("%d FAILED TESTS", testing_fails);
-    return 1;
   }
+  return CloseTestEngine();
 }

tests/test_assert.cpp

@@ -1,6 +1,7 @@
-#include <assert.h>
 #include "test_assert.h"
 
+#include <assert.h>
+
 #ifdef _MSC_VER
 #  include <crtdbg.h>
 #endif
@@ -27,13 +28,9 @@ void TestEqStr(const char *expval, const char *val, const char *exp,
   if (strcmp(expval, val) != 0) { TestFail(expval, val, exp, file, line); }
 }
 
-#ifdef _MSC_VER
-// Without this hook function the message box not suppressed.
-int msvc_no_dialog_box_on_assert(int rpt_type, char *msg, int *ret_val) {
-  (void)ret_val;
-  TEST_OUTPUT_LINE("TEST ASSERTED: %d: %s", rpt_type, msg);
-  return 1;
-}
+#if defined(FLATBUFFERS_MEMORY_LEAK_TRACKING) && defined(_MSC_VER) && \
+    defined(_DEBUG)
+#define FLATBUFFERS_MEMORY_LEAK_TRACKING_MSVC
 #endif
 
 void InitTestEngine(TestFailEventListener listener) {
@@ -46,14 +43,37 @@ void InitTestEngine(TestFailEventListener listener) {
   // clang-format off
 
   #ifdef _MSC_VER
-    // Suppress pop-up message box on assertion (MSVC2010, MSVC2012).
-    // This message box hangs CI-test on the hour until timeout expired.
-    // Default mode is file, file is stderr.
+    // Send all reports to STDOUT.
+    // CrtDebug reports to _CRT_WARN channel.
+    _CrtSetReportMode(_CRT_WARN, _CRTDBG_MODE_FILE | _CRTDBG_MODE_DEBUG);
+    _CrtSetReportFile(_CRT_WARN, _CRTDBG_FILE_STDOUT);
+    // The assert from <assert.h> reports to _CRT_ERROR channel
+    _CrtSetReportMode(_CRT_ERROR, _CRTDBG_MODE_FILE | _CRTDBG_MODE_DEBUG);
+    _CrtSetReportFile(_CRT_ERROR, _CRTDBG_FILE_STDOUT);
+    // Internal CRT assert channel?
     _CrtSetReportMode(_CRT_ASSERT, _CRTDBG_MODE_FILE | _CRTDBG_MODE_DEBUG);
-    _CrtSetReportFile(_CRT_ASSERT, _CRTDBG_FILE_STDERR);
-    _CrtSetReportHook(msvc_no_dialog_box_on_assert);
+    _CrtSetReportFile(_CRT_ASSERT, _CRTDBG_FILE_STDOUT);
+  #endif
+
+  #if defined(FLATBUFFERS_MEMORY_LEAK_TRACKING_MSVC)
+    // For more thorough checking:
+    // _CRTDBG_DELAY_FREE_MEM_DF | _CRTDBG_CHECK_ALWAYS_DF
+    auto flags = _CrtSetDbgFlag(_CRTDBG_REPORT_FLAG);
+    _CrtSetDbgFlag(flags | _CRTDBG_ALLOC_MEM_DF);
   #endif
   // clang-format on
 
   fail_listener_ = listener;
 }
+
+int CloseTestEngine(bool force_report) {
+  if (!testing_fails || force_report) {
+  #if defined(FLATBUFFERS_MEMORY_LEAK_TRACKING_MSVC)
+      auto flags = _CrtSetDbgFlag(_CRTDBG_REPORT_FLAG);
+      flags &= ~_CRTDBG_DELAY_FREE_MEM_DF;
+      flags |= _CRTDBG_LEAK_CHECK_DF;
+      _CrtSetDbgFlag(flags);
+  #endif
+  }
+  return (0 != testing_fails);
+}

tests/test_assert.h

@@ -1,6 +1,7 @@
 #ifndef TEST_ASSERT_H
 #define TEST_ASSERT_H
 
+#include "flatbuffers/base.h"
 #include "flatbuffers/util.h"
 
 // clang-format off
@@ -42,6 +43,12 @@ typedef bool (*TestFailEventListener)(const char *expval, const char *val,
 // listener - this function will be notified on each TestFail call.
 void InitTestEngine(TestFailEventListener listener = nullptr);
 
+// Release all test-engine resources.
+// Prints or schedule a debug report if all test passed.
+// Returns 0 if all tests passed or 1 otherwise.
+// Memory leak report: FLATBUFFERS_MEMORY_LEAK_TRACKING && _MSC_VER && _DEBUG.
+int CloseTestEngine(bool force_report = false);
+
 // Write captured state to a log and terminate test run.
 void TestFail(const char *expval, const char *val, const char *exp,
               const char *file, int line, const char *func = 0);