Catch crash when destructing 'Version' struct on exit

[kevinushey]

Description

I'm seeing a weird crash when my process exits after running Catch unit tests. The gdb stack trace:

#0  0x00007f8306345428 in __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:54
#1  0x00007f830634702a in __GI_abort () at abort.c:89
#2  0x00007f83063877ea in __libc_message (do_abort=do_abort@entry=2, fmt=fmt@entry=0x7f83064a02e0 "*** Error in `%s': %s: 0x%s ***\n")
    at ../sysdeps/posix/libc_fatal.c:175
#3  0x00007f830638fe0a in malloc_printerr (ar_ptr=<optimized out>, ptr=<optimized out>, str=0x7f830649d0b2 "free(): invalid pointer",
    action=3) at malloc.c:5004
#4  _int_free (av=<optimized out>, p=<optimized out>, have_lock=0) at malloc.c:3865
#5  0x00007f830639398c in __GI___libc_free (mem=<optimized out>) at malloc.c:2966
#6  0x00007f8306cc70b4 in std::basic_string<char, std::char_traits<char>, std::allocator<char> >::~basic_string() ()
   from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#7  0x000000000137eeac in Catch::Version::~Version (this=0x22a2a60 <Catch::libraryVersion>, __in_chrg=<optimized out>)
    at /home/kevin/rstudio/src/cpp/tests/cpp/tests/vendor/catch.hpp:5947
#8  0x00007f8306349ff8 in __run_exit_handlers (status=0, listp=0x7f83066d35f8 <__exit_funcs>, run_list_atexit=run_list_atexit@entry=true)
    at exit.c:82
#9  0x00007f830634a045 in __GI_exit (status=<optimized out>) at exit.c:104
#10 0x0000000001316133 in (anonymous namespace)::exitEarly (status=0) at /home/kevin/rstudio/src/cpp/session/SessionMain.cpp:336
...

The fact that the error is occurring when calling the ~Version() destructor is really throwing me off -- perhaps I'm somehow hitting a double free, or something similar? This isn't necessarily a bug in Catch itself but I'm curious if anyone has ideas as to how I can try to run this down further.

Steps to reproduce

I haven't been able to generate a minimally reproducible example yet 😞 . Any advice in further narrowing down what could be going on would be hugely appreciated.

Extra information

• Catch version: v1.8.2
• Operating System: Ubuntu 16.04 64bit
• Compiler+version: gcc 4.8.5

[kevinushey]

FWIW, some local testing seems to indicate that using an inline function rather than a static version struct works just fine. I'll try to clean that up and submit that as a PR (although entirely understand if we don't want to take it)

[lightmare]

#7 0x000000000137eeac in Catch::Version::~Version (this=0x22a2a60 Catch::libraryVersion, __in_chrg=) at /home/kevin/rstudio/src/cpp/tests/cpp/tests/vendor/catch.hpp:5947

This doesn't look right. In Catch v1.8.2 single_include, line 5947 is in TrackerContext class, nowhere near Version. Could be that you have object files compiled with different Catch versions linked together.

[kevinushey]

Sorry; I think that is what I saw when using an older version of Catch (although I definitely saw it with the current release; I copied the wrong stack trace initially).

Trying again with current Catch to be sure:

#7 0x0000000001384c16 in Catch::Version::~Version (this=0x22aae40 Catch::libraryVersion, __in_chrg=)
at /home/kevin/rstudio/src/cpp/tests/cpp/tests/vendor/catch.hpp:6796

which points where we would expect: https://github.com/philsquared/Catch/blob/4dc06bdb7034d36071caa2219f14c4b2ce52c80a/single_include/catch.hpp#L6796

[horenmar]

I am going to mark this as possible bug until I can take a look at why this is so -- quick look through how the libraryVersion instance is used doesn't yield any reason for this to happen.

[refi64]

Sure this isn't because of memory corruption inside of your own tests? Your "fix" could be working because of moving memory addresses. Have you tried running the tests with something like Clang's memory sanitizers or Valgrind?

[kevinushey]

After digging in more, I think @lightmare was right -- there are multiple versions of Catch in play.

Just to give this some context, I'm using Catch for unit tests in an executable, rsession. This executable runs an R interpreter (used for running programs written in the R language), and that R interpreter can (and does in this case) load other libraries that might use Catch for their own unit tests as well.

When the application exits, any libraries loaded by the R interpreter are also unloaded -- that triggers the Catch version destructors, thereby cleaning up the static libraryVersion struct. Then, when my application exits, we then attempt to clean up libraryVersion once again, and this double-destruct causes a crash in my case.

The first destruction on exit:

Thread 1 "rsession" hit Breakpoint 1, Catch::Version::~Version (this=0x22aae40 <Catch::libraryVersion>, __in_chrg=<optimized out>)
    at ../inst/include/testthat/vendor/catch.h:5489
5489    ../inst/include/testthat/vendor/catch.h: No such file or directory.

The second:

Thread 1 "rsession" hit Breakpoint 1, Catch::Version::~Version (this=0x22aae40 <Catch::libraryVersion>, __in_chrg=<optimized out>)
    at /home/kevin/rstudio/src/cpp/tests/cpp/tests/vendor/catch.hpp:6796
6796        struct Version {

Note that the two this pointers are the same, showing that we're in effect destructing the same struct twice.

I don't know if this can really be called a Catch bug since it's likely outside of the original envisioned use case, but moving away from having a static version struct would likely help out for cases where multiple versions of Catch are inadvertently mixed.

[horenmar]

@kevinushey Interesting. I don't think there is a reason not make the change, but it might have to wait until next minor.

[kevinushey]

Thanks!