4

We have a C++ library. We are providing a custom assert and abandoning Posix NDEBUG and assert (back story below).

The assert looks like so under Windows:

#  define CRYPTOPP_ASSERT(exp) {                                  \
    if (!(exp)) {                                                 \
      std::ostringstream oss;                                     \
      oss << "Assertion failed: " << (char*)(__FILE__) << "("     \
          << (int)(__LINE__) << "): " << (char*)(__FUNCTION__)    \
          << std::endl;                                           \
      std::cerr << oss.str();                                     \
      DebugBreak();                                               \
    }                                                             \
  }

The problem we are having is, we have to include <windows.h>, and it brings in a lot of extra cruft, even with WIN32_LEAN_AND_MEAN defined. Some of the extra cruft, like min and max, breaks C++ compiles. In fact, testing our changes broke us.

We looked through <windows.h> searching for a "debug includes only" type define but we could not find it. We also tried adding extern void WINAPI DebugBreak(void); according to Microsoft's docs on DebugBreak, but it resulted in compile errors due to redefined symbols.

Adding NO_MIN_MAX (I think that's the macro) is not an option because we are changing defines in user programs when the define cross-pollinates out of our header and into user code. Related, see Limiting Scope of #include Directives and friends.

Using #pragma push_macro and #pragma pop_macro is not an option because we support Microsoft compilers back to VC++ 6.0. The earliest the pragmas are available is VS2003.

Because of the breaks, we do not want to include <windows.h>. How can I get a declaration for DebugBreak without including Windows.h?

Thanks in advance.


Here's a reduced case:

// cl.exe /c assert.cpp

#include <algorithm>

// #include <windows.h>
// #ifndef WINAPI
// # define WINAPI __stdcall
// #endif
// extern void WINAPI DebugBreak(void);

#define MY_ASSERT(exp) { \
   if (!(exp)) {         \
     DebugBreak();       \
   }                     \
}

void dummy(int x1, int x2)
{
    MY_ASSERT(x1 == std::min(x1, x2));
}

Here's the error when simulating a user program with our extern declaration. This test occurred on Windows 8.1 x64 with VS2012 and VS2013 installed. We also used a developer command prompt.

Microsoft (R) C/C++ Optimizing Compiler Version 18.00.21005.1 for x86
Copyright (C) Microsoft Corporation.  All rights reserved.

assert.cpp
assert.cpp(11) : warning C4273: 'DebugBreak' : inconsistent dll linkage
        C:\Program Files (x86)\Windows Kits\8.1\include\um\debugapi.h(70) : see
previous definition of 'DebugBreak'
assert.cpp(21) : error C2589: '(' : illegal token on right side of '::'
assert.cpp(21) : error C2059: syntax error : '::'
assert.cpp(21) : error C2143: syntax error : missing ';' before '{'

When we inspect <debugapi.h> we see:

WINBASEAPI
VOID
WINAPI
DebugBreak(
    VOID
    );

WINBASEAPI expands into additional macros. I don't think we are going to be able to get them all right across all platforms.


We have a cross-platform C++ security library and it recently caught CVE-2016-7420. It was classified as an information disclosure due the potential data loss if an assert fired. The loss occurs when the sensitive data is egressed to the file system (core dumps and crash reports); and egressed to third parties (Apple via CrashReporter, Ubuntu via Apport, Microsoft via Windows Error Reporting, developers, etc).

The asserts never fired in production/release for us because our Makefile and our Visual Studio solution well configured the library. In production/release builds, the asserts are removed and a C++ throw() handles the error condition. The asserts are present in debug/developer configurations so the code will debug itself, and relieve the programmer from the task.

After we analyzed it, we realized documenting "release/production builds must use -DNDEBUG" is an incomplete remediation. Folks won't read the docs; if RTFM was going to work, then it would have happened by now. In addition, CMake won't define it, Autotools won't define it, Eclipse won't define it, etc. We are effectively at the same point we were before the CVE. All we did was move the blame around without reducing the risk.

Community
  • 1
  • 1
jww
  • 97,681
  • 90
  • 411
  • 885
  • I'm sorry to ask for it but do you have a [mcve] ? If you have, I'd look into it. I had this problem with `` defining TRUE and FALSE and breaking stuff when I just wanted the structs to measure accurate time and I sorted it out. – Jean-François Fabre Sep 17 '16 at 20:17
  • Steps of code debugging: 1) read it, 2) log it, 3) step through it, 4) put a trap 5) have a trap for the customer (in a release build). –  Sep 17 '16 at 20:26
  • @Jean-FrançoisFabre - Fair enough. Done. – jww Sep 17 '16 at 20:41
  • *"Some of the extra cruft, like `min` and `max`, breaks C++ compiles."* - That's why there's `#define NOMINMAX`. The Windows header files can be tamed. I'm not suggesting that you should go this route, but it's very likely possible if you have to. – IInspectable Sep 17 '16 at 21:06
  • @IInspectable - Please post your code that uses `NOMINMAX` such that it does not cross-pollinate out of our headers and into user headers. I'll be happy to test it. – jww Sep 17 '16 at 23:37
  • Why would you want to put the `NOMINMAX` macro in your **header** files? It belongs in your build settings, either the source code of the compilation units, or (probably more appropriately) into the build scripts to pass it along on the command line. – IInspectable Sep 18 '16 at 00:16
  • @IInspectable - we are a library. We neither add it or remove it from anywhere. We also don't control what a user does. Our code is expected to "just work" with any user configuration thrown at it. As the CVE shows, users will even do the wrong things, like shipping a release product in a debug configuration. That's why we are re-engineering this area. – jww Sep 18 '16 at 01:08

3 Answers3

4

You can use the intrinsic, it works without includes:

__debugbreak();
Daniel
  • 30,896
  • 18
  • 85
  • 139
2

Just add a new source code file which contains nothing but:

#include <windows.h>

void MyDebugBreak(void)
{
   DebugBreak();
}

Export as necessary, and call MyDebugBreak() instead of DebugBreak() in your macro.

You can either include the file only in Windows builds, or add #if blocks as appropriate.

Harry Johnston
  • 35,639
  • 6
  • 68
  • 158
  • Thanks @Harry. I think that would work ensure no cross-pollination. I'm going to try @Dani's solution since its a header-only solution, and [Microsoft states under requirements](https://msdn.microsoft.com/en-us/library/f408b4et.aspx): *"x86, ARM, x64"*. The requirement for `` is not quite right; it broke VS2002 and VS2003. [Microsoft also states](https://msdn.microsoft.com/en-us/library/ea9yy3ey.aspx) these benefits: *"You can call the DebugBreak Win32 function or the __debugbreak ... Because DebugBreak is a call to a system function, system debug symbols must be installed..."* – jww Sep 18 '16 at 01:56
  • @jww: my only concern with Dani's solution is that it is specific to Visual Studio. If that's not a problem in your context, then yes, it sounds like the best option. – Harry Johnston Sep 18 '16 at 02:00
  • Good point. Testing will reveal them. MinGW is my only concern at this point, but I don't have a test rig for it despite sincere efforts to set one up. Borland is another concern, but I only know of one user who uses it. I tried to get a gratis license from Embarcadero for testing but they refused. As a consequence, their users must suffer until someone alerts us and submits a patch. – jww Sep 18 '16 at 02:11
1

Declaring DebugBreak seems to work ok using Visual Studio 6 and 2015 provided you declare it with __stdcall and extern "C". As VC++ 6 doesn't seem to include std::min in the algorithm header I've modified your example a bit but if the first argument is greater than the second it raises the assertion when built using cl -nologo -W3 -O2 -Zi -NDEBUG -o assert.exe assert.cpp -link -subsystem:console -debug

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

extern "C" extern void __stdcall DebugBreak(void );

#define MY_ASSERT(exp) { \
   if (!(exp)) {         \
     DebugBreak();       \
   }                     \
}

void dummy(int x1, int x2)
{
    MY_ASSERT(x1 < x2);
}

int main(int argc, char *argv[])
{
    if (argc != 3) {
        fprintf(stderr, "usage: assert integer integer\n");
        exit(1);
    }
    int a = strtol(argv[1], NULL, 0);
    int b = strtol(argv[2], NULL, 0);
    dummy(a, b);
    return 0;
}
patthoyts
  • 32,320
  • 3
  • 62
  • 93
  • Thanks @patthoyts. Unfortunately, the `extern "C"` results in the same compile error due to different linkages. I also added the Microsoft signature for `DebugBreak` from ``. It appears they adorn the symbol with more than they document. – jww Sep 18 '16 at 01:17