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

Spy can cause use of incorrect calling convention resulting in crash (MSVC x64) #131

Closed regular384 closed 1 year ago

regular384 commented 6 years ago

FakeIt’s handling of member function pointers when using Spy can generate code with mismatched calling conventions in my MSVC 2017 x64 environment. This typically results in access violations or corruption writing to addresses taken from registers that were not set as expected.

Given:

struct A { int x = 0; };

class ToSpy {
public:
    virtual ~ToSpy() = default;
    virtual A memfun() { return A(); }
};

A nonmemfun(ToSpy* instance) { return A(); }

I observe a call to the member function returning A to pass the instance address in RCX and the address at which to construct the return value in RDX. The member function will construct the A struct there and return this same address in RAX.

A different calling convention is used for the non-member function. RCX holds the instance argument and RDX is not set. The function returns the value of A within the EAX register.

When using Spy, the attempt to invoke the original member function in MockImpl.hpp L174:L175 will union_cast a void* to a non-member function pointer with an object instance pointer as its first argument. (It does specify __thiscall in the cast but I think this is just ignored for x64).

This causes use of the non-member function convention without setting RDX to the address of some allocated space. The member function is invoked and starts to write to whatever RDX happens to hold. (Any function parameters passed in registers may also be offset from expectation, and a value rather than address is expected to be returned in EAX/RAX)

Changing the FakeIt code to union_cast to a member function pointer instead does result in the correct calling convention being used on my system but isn’t the union_cast mechanism still undefined behaviour?

FranckRJ commented 1 year ago

Should be fixed by https://github.com/eranpeer/FakeIt/pull/312, it will be available in FakeIt 2.3.3 or 2.4.0 depending on which is the first to be released.