A simple testcase causes Segmentation Fault.

[recolic]

Describe the bug

A simple testcase causes Segmentation Fault.

Expected behavior
It should run without segmentation fault. The tests may pass or fail or segfault, but catch should not crash.

Reproduction steps

I wrote a simple testcase with latest catch.hpp:

// src/string.cc
#define CATCH_CONFIG_MAIN
#include "catch.hpp"

#include <rlib/string.hpp>

using namespace rlib::literals;

TEST_CASE("literal _format", "[_format]") {
    //REQUIRE("fuck {} at {} a.m."_format("hust", 8) == "fuck hust at 8 a.m.");
    //REQUIRE("test {}/{}"_rs .format("???", 1.234) == "test ???/1.234");
    //REQUIRE("\\{}, \\\\{}, {}."_format("you") == "{}, \\you, {}.");
    //REQUIRE("{{}}, {, }{"_format("test") == "{test}, {, }{");
}

And compile it with

g++ -I. -std=c++14 src/string.cc rlib/libr.a -o src/string_14 -g

Then run src/string_14, got fish: 'src/string_14' terminated by signal SIGSEGV (Address boundary error).

Platform information:

• OS: Linux 5.0.9-arch1-1-ARCH x86_64 GNU/Linux
• Compiler+version: GCC v8.2.1
• Catch version: v2.7.2

Additional context

GDB backtrace:

~/c/c/r/test ❯❯❯ gdb src/string_14
GNU gdb (GDB) 8.2.1
Copyright (C) 2018 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64-pc-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
    <http://www.gnu.org/software/gdb/documentation/>.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from src/string_14...done.
(gdb) run
Starting program: /home/recolic/code/cpp/rlib/test/src/string_14

Program received signal SIGSEGV, Segmentation fault.
Catch::(anonymous namespace)::PosixColourImpl::setColour (this=0x517790 <Catch::(anonymous namespace)::PosixColourImpl::instance()::s_instance>, _escapeCode=0x4aea7a "[0;31m")
    at ./catch.hpp:8253
8253                getCurrentContext().getConfig()->stream()
(gdb) bt
#0  Catch::(anonymous namespace)::PosixColourImpl::setColour (this=0x517790 <Catch::(anonymous namespace)::PosixColourImpl::instance()::s_instance>, _escapeCode=0x4aea7a "[0;31m")
    at ./catch.hpp:8253
#1  0x000000000040f4a3 in Catch::(anonymous namespace)::PosixColourImpl::use (this=0x517790 <Catch::(anonymous namespace)::PosixColourImpl::instance()::s_instance>,
    _colourCode=Catch::Colour::Red) at ./catch.hpp:8229
#2  0x000000000040f9f7 in Catch::Colour::use (_colourCode=Catch::Colour::Red) at ./catch.hpp:8315
#3  0x000000000040f90c in Catch::Colour::Colour (this=0x7fffffffd13f, _colourCode=Catch::Colour::Red) at ./catch.hpp:8300
#4  0x0000000000418a6e in Catch::Session::Session (this=0x7fffffffd2b0) at ./catch.hpp:10992
#5  0x000000000042cabc in main (argc=1, argv=0x7fffffffd508) at ./catch.hpp:14623

[recolic]

Note: If I change line 9 to

TEST_CASE("literal _format", "[format]") {

Then it's working well.

[horenmar]

Well, this is interesting.

The problem is that [_format] is a reserved tag, and Catch attempts to report the error. This is the output from a Windows executable:

Errors occurred during startup!
  Tag name: [_format] is not allowed.
  Tag names starting with non alphanumeric characters are reserved
  C:\ubuntu\temp\tests.cpp(3)

However, on Linux, reporting the error fails because it attempts to use console colours for the output, but doing so fails -- at a quick glance, at this point the colours objects are not yet initialized .

[DirkGehlich]

I am having a problem at some similar location at Colour::use(). The problem is that the impl is NULL. It only happens if we run the tests in virtual machines.
I thought about creating a PR to just check for NULL as a workaround. At least then it doesn't crash anymore, but only prints the result without colour, which is fine imo.

[JoeyGrajciar]

@DirkGehlich looking at the platformColourInstance code and it is actually returning pointer to static instance of particular colour implementation. Can you describe more those windows virtual machines you are running into this issue? I can imagine that this could happen when static variable fails to be created on stack.

@recolic In your case i suspect that the output stream is not yet created.

[DirkGehlich]

The VMs we are using are vagrant boxes which use VirtualBox.

VirtualBox: v5.2.26
OS: Windows Server 2016 Standard 64bit

Do you need more information? Can we do some tests within our boxes to help you identify the issue?

EDIT: Tested also in a VMware box (same OS). Problem is the same, so it looks like a general virtual machine issue.

[JoeyGrajciar]

e680c4b fixed the colour output in startup exception case.

@DirkGehlich thinking what would help. Do you have some crash dump? If you can provide some minimal repro code i can try to check what is happening. Will try to check with my Windows 10 virtual machine.
Crash dumps can be created also using procdump utility from Sysinternals. Register it as post mortem debugger and it will create dumps automatically when something crashes.

[DirkGehlich]

So I was debugging and trying to find out whats the issue, which is quite tough.
What I noticed is that it works in debug mode, but not release.
We are using own makefiles (nmake) and additionally compile our own main in a seperate obj file which we then link to each exectubale which contains test cases.

I noticed that the platform colour instance methods will not be called at all in release mode, but in debug.
It could be some optimizers, removing those methods, as they are not used in the obj itself, or it could be some of our own compiler/linker switches. I was not able to find out by now.
So its taking much longer than expected :(

[horenmar]

@DirkGehlich Removing static init sounds like either there is a serious UB in the code on our side (the code is small and simple enough that I think this is unlikely), or a serious problem in your compilation setup.

I've thought about it, and I am going to merge the PR that adds the defensive check, because practicality sometimes trumps purity, but I am not particularly happy about it.