eranpeer / FakeIt

C++ mocking made easy. A simple yet very expressive, headers only library for c++ mocking.
MIT License
1.24k stars 170 forks source link

Mocking destructor + gcc -O2 flag #84

Open Matthiasvanderhallen opened 7 years ago

Matthiasvanderhallen commented 7 years ago

Mocking a destructor and using the -O2 compiler flag (or higher) with gcc leads to incorrect behaviour. It works perfectly fine when passing -O1.

Concretely, I've made an example which uses standalone fakeit at commit 2eb812b7ef7e72cb2e0ed2e1283b8bcc858fc555 but the erratic behaviour also occurs when using fakeit for catch.

#include "fakeit.hpp"

using namespace fakeit;
using namespace std;

class SearchContext {

public:
  SearchContext();
  virtual ~SearchContext() {};
  virtual int getValue();
};

SearchContext::SearchContext() {};

int SearchContext::getValue() {
  return 4;
};

int main(){
  cout << "Start of tests" << endl;

  Mock<SearchContext> scMock;
  When(Method(scMock,getValue)).AlwaysDo([](){return 4;});

  // As per https://github.com/eranpeer/FakeIt/issues/10 and https://github.com/eranpeer/FakeIt/wiki/Quickstart
  // Both should be equivalent, and indeed, both fail.
  // When(Dtor(scMock)).AlwaysDo([](){});
  Fake(Dtor(scMock));
  cout << "End of tests" << endl;
  return 0;
};

which I compile with g++ -std=c++14 -O2 example.cpp -o example; ./example. This leads to a SIGSEGV when running the binary on ubuntu, or to a linking error when running the same command on Mac OS X. I suspect the discrepancy between the two platforms is caused by the linker, which defaults to the GNU ld on ubuntu, and apple ld on mac.

Versions: Ubuntu: 15.04, gcc: 5.3.0, ld: GNU ld (GNU Binutils) 2.28 Mac: 10.12.3, gcc 6.1.0, ld: @(#)PROGRAM:ld PROJECT:ld64-274.2

I have attached the linking error (mac OS X) and the output of valgrind (Ubuntu):

ld.txt valgrind.txt

Matthiasvanderhallen commented 7 years ago

Some further testing on my part showed that the problem is linked to the -fdevirtualize flag that-O2 activates, i.e.

g++ -std=c++14 -O1 -fdevirtualize example.cpp -o example; ./example

fails with the same errors as in my original post, whereas

g++ -std=c++14 -O2 -fno-devirtualize example.cpp -o example; ./example

runs perfectly fine, although it specifies -O2 (and even with -O3).

miqelm commented 6 years ago

@eranpeer Do you think something about fixing it? Maybe someone else has idea? For me segmentation fault also appers, gdb output, fakeit version 2.0.2:

0x000000000048e7d2 in fakeit::MockImpl<VoicePIN::ICommandLineHelpPrinter>::MethodMockingContextBase<void>::addMethodInvocationHandler(fakeit::ActualInvocation<>::Matcher*, fakeit::MethodInvocationHandler<void>*) (this=0x7db460,
    matcher=0x8000004fb8c0, invocationHandler=0x2) at /home/mmaka/.conan/data/FakeIt/2.0.2/hinrikg/stable/package/d61fe2c485b154f9c6ac35b53ce4abf2c31fddc0/include/fakeit/MockImpl.hpp:111

When checking backtrace, method fakeit::MockImpl<VoicePIN::ICommandLineHelpPrinter>::MethodMockingContextBase<void>::addMethodInvocationHandler() is called thousands of times, which causes stack overflow due to a lot of recursion execution.

eranpeer commented 6 years ago

I'll be happy if someone will find a solution and submit a pull request.

On Nov 8, 2017 02:14, "miqelm" notifications@github.com wrote:

@eranpeer https://github.com/eranpeer Do you think something about fixing it? Maybe someone else has idea? For me segmentation fault also appers, gdb output:

0x000000000048e7d2 in fakeit::MockImpl::MethodMockingContextBase::addMethodInvocationHandler(fakeit::ActualInvocation<>::Matcher, fakeit::MethodInvocationHandler) (this=0x7db460, matcher=0x8000004fb8c0, invocationHandler=0x2) at /home/mmaka/.conan/data/FakeIt/2.0.2/hinrikg/stable/package/d61fe2c485b154f9c6ac35b53ce4abf2c31fddc0/include/fakeit/MockImpl.hpp:111

When checking backtrace, method fakeit::MockImpl<VoicePIN:: ICommandLineHelpPrinter>::MethodMockingContextBase ::addMethodInvocationHandler() is called thousands of times, which causes stack overflow due to a lot of recursion execution.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/eranpeer/FakeIt/issues/84#issuecomment-342772138, or mute the thread https://github.com/notifications/unsubscribe-auth/ACc8gljtqztwi59aeaySNiu4sR-fXdzcks5s0X8GgaJpZM4Mj62_ .

jackgerrits commented 6 years ago

I am hitting this issue too on gcc version 8.1.0, adding the -fno-devirtualize flag fixed the segfault but it would be good to run the tests with the same optimization as the shipping code.

eranpeer commented 6 years ago

Fakit hacks the virtual table, with higher optimization the compiler may choose to use some pointers is a way that fakeit can't predict. Currently the only way around it is to reduce the optimization level on tests. I know it is not optimal, but that's the flip side of the simplicity and usability of the framework.

On Tue, Aug 21, 2018, 13:56 Jack Gerrits notifications@github.com wrote:

I am hitting this issue too on gcc version 8.1.0, adding the -fno-devirtualize flag stopped the segfault but it would be good to run the tests with the same optimization as the shipping code.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/eranpeer/FakeIt/issues/84#issuecomment-414819000, or mute the thread https://github.com/notifications/unsubscribe-auth/ACc8gsMf6srJ31vD_0ZtF4lJECAKalMxks5uTHP4gaJpZM4Mj62_ .

mfontanini commented 6 years ago

If this is a known issue, please put a large disclaimer somewhere. I tried FakeIt for the first time inside some application and it took me a while to realize the crash was tied to -O2.

eranpeer commented 6 years ago

You're right. I will.

On Wed, Aug 22, 2018, 16:23 Matias Fontanini notifications@github.com wrote:

If this is a known issue, please put a large disclaimer somewhere. I tried FakeIt for the first time inside some application and it took me a while to realize the crash was tied to -O2.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/eranpeer/FakeIt/issues/84#issuecomment-415219324, or mute the thread https://github.com/notifications/unsubscribe-auth/ACc8grr_fBIT-ICQrDKXiFpjst2ACGGSks5uTefigaJpZM4Mj62_ .

eranpeer commented 6 years ago

I added a line to the limitations section.

On Wed, Aug 22, 2018 at 8:25 PM Eran Pe'er eranpe@gmail.com wrote:

You're right. I will.

On Wed, Aug 22, 2018, 16:23 Matias Fontanini notifications@github.com wrote:

If this is a known issue, please put a large disclaimer somewhere. I tried FakeIt for the first time inside some application and it took me a while to realize the crash was tied to -O2.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/eranpeer/FakeIt/issues/84#issuecomment-415219324, or mute the thread https://github.com/notifications/unsubscribe-auth/ACc8grr_fBIT-ICQrDKXiFpjst2ACGGSks5uTefigaJpZM4Mj62_ .

-- Eran 1-424-2504000

mfontanini commented 6 years ago

Thanks! Awesome library btw.

Warchant commented 4 years ago

It should be possible to add this prefix right after inclusion guard, to solve this problem:

#ifdef __GNUG__

// Fakeit does not work with GCC's devirtualization
// which is enabled with -O2 (the default) or higher.
#pragma GCC optimize("no-devirtualize")

#endif
JaredCarlsonImsar commented 1 year ago

If this is the only reason that the -O2 and -O3 flags aren't supported for gcc I think it should get expressed in the limitations. I know for my use case it is a limitation that I can work with given I need to compile in -O3.

FranckRJ commented 1 year ago

It's probably not the only one. It's hard to make a list, especially because it could be different for each versions, but I guess any compiler with optimization enabled could devirtualize some calls (because FakeIt never inherit any classes it mocks) which would break the library.