eranpeer / FakeIt

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

Formatter can not format enum type and causes SIGSEGV #262

Closed energygreek closed 2 years ago

energygreek commented 2 years ago

Formater can not format enum type argument and cause SIGSEGV when Exactly not match

`a

0x00007f8ee45d9ec6 in std::basic_ostream<char, std::char_traits<char> >& std::operator<< <char, std::char_traits<char>, std::allocator<char> >(std::basic_ostream<char, std::char_traits<char> >&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) () from /usr/lib/libstdc++.so.6
(gdb) bt
#0  0x00007f8ee45d9ec6 in std::basic_ostream<char, std::char_traits<char> >& std::operator<< <char, std::char_traits<char>, std::allocator<char> >(std::basic_ostream<char, std::char_traits<char> >&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) () from /usr/lib/libstdc++.so.6
#1  0x000055da09ab2de9 in fakeit::Formatter<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, void>::format (val=...)
    at /root/ha-engine/third/FakeIt/single_header/catch/fakeit.hpp:298
#2  0x000055da09ab2bf6 in fakeit::TuplePrinter<std::tuple<hasync::EventType, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&> const&, 2ul>::print (
    strm=..., t=...) at /root/ha-engine/third/FakeIt/single_header/catch/fakeit.hpp:315
#3  0x000055da09ab2b2d in fakeit::print<hasync::EventType, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&> (strm=..., t=...)
    at /root/ha-engine/third/FakeIt/single_header/catch/fakeit.hpp:335

code

  enum EventType {
    File = 0,
    Dir,
  };
  virtual void notify_add_event(EventType event_type,
                                const std::string& filepath) = 0;
  Fake(Method(appmanager_mock, notify_add_event));
  Verify(Method(appmanager_mock, notify_add_event)).Exactly(3);

os: g++ (Alpine 9.3.0) 9.3.0

FranckRJ commented 2 years ago

Could you provide an example when it happen on godbolt ? Because i can't reproduce it : https://godbolt.org/z/37nMsTPcv

energygreek commented 2 years ago

https://godbolt.org/z/adhsfd6nh, this happens in my env when call mocked method in other thread, but godbolt can not reproduce this problem, here's more details

FakeIt

commit c0551414b1ec30f7c19c4d6ee07cd9eb93013904 (HEAD, tag: 2.0.5)

Catch

commit c3c82f539c1d31df6b7f828efe4f4ba9eb650bf7

i run it in alpine docker

bash-5.0# cat /etc/os-release 
NAME="Alpine Linux"
ID=alpine
VERSION_ID=3.12.0
PRETTY_NAME="Alpine Linux v3.12"
HOME_URL="https://alpinelinux.org/"
BUG_REPORT_URL="https://bugs.alpinelinux.org/"
bash-5.0# g++ --version
g++ (Alpine 9.3.0) 9.3.0
Copyright (C) 2019 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

src file

#include <catch2/catch.hpp>
#include <fakeit.hpp>

#include <string>
#include <thread>

using fakeit::Verify;
using fakeit::Fake;
using fakeit::Mock;

namespace test {

enum class EventType : uint8_t {
  File = 0,
  Dir,
};

struct IAppManager {
  // inotify calls when new event
  virtual void notify_add_event(EventType event_type,
                                const std::string& filepath) = 0;
  virtual void notify_rename_event(EventType event_type,
                                   const std::string& filepath,
                                   const std::string& filepath_new) = 0;

  virtual ~IAppManager() = default;
};

TEST_CASE("Test Inotify") {
  Mock<IAppManager> appmanager_mock;
  Fake(Dtor(appmanager_mock));
  Fake(Method(appmanager_mock, notify_add_event));
  Fake(Method(appmanager_mock, notify_rename_event));

  auto& app_manager = appmanager_mock.get();
  SECTION("test add") {
    std::thread t1([&app_manager](){
      app_manager.notify_add_event(EventType::File, "asd");
      app_manager.notify_add_event(EventType::File, "asd");
      app_manager.notify_add_event(EventType::File, "asd");
    });
    t1.join();
    Verify(Method(appmanager_mock, notify_add_event)).Exactly(2);
  }
}

}  // namespace test

CMakeLists.txt

add_executable(test_ha_sync
    main.cpp
    app/TestInotify.cpp
    )

target_link_libraries(test_ha_sync
    FakeIt::Catch2
    )

linked libraries

Dynamic section at offset 0x190690 contains 28 entries:
  Tag        Type                         Name/Value
 0x0000000000000001 (NEEDED)             Shared library: [libstdc++.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [libgcc_s.so.1]
 0x0000000000000001 (NEEDED)             Shared library: [libc.musl-x86_64.so.1]
 0x000000000000000f (RPATH)              Library rpath: [$ORIGIN/../lib:$ORIGIN/../lib64]
 0x000000000000000c (INIT)               0x1f000
 0x000000000000000d (FINI)               0xff170
 0x0000000000000019 (INIT_ARRAY)         0x188890
 0x000000000000001b (INIT_ARRAYSZ)       16 (bytes)
 0x000000006ffffef5 (GNU_HASH)           0x290
 0x0000000000000005 (STRTAB)             0x2120
 0x0000000000000006 (SYMTAB)             0x458
 0x000000000000000a (STRSZ)              13661 (bytes)
 0x000000000000000b (SYMENT)             24 (bytes)
 0x0000000000000015 (DEBUG)              0x0
 0x0000000000000003 (PLTGOT)             0x191890
 0x0000000000000002 (PLTRELSZ)           5328 (bytes)
 0x0000000000000014 (PLTREL)             RELA
 0x0000000000000017 (JMPREL)             0x1d0f8
 0x0000000000000007 (RELA)               0x5908
 0x0000000000000008 (RELASZ)             96240 (bytes)
 0x0000000000000009 (RELAENT)            24 (bytes)
 0x0000000000000018 (BIND_NOW)           
 0x000000006ffffffb (FLAGS_1)            Flags: NOW PIE
 0x000000006ffffffe (VERNEED)            0x58e8
 0x000000006fffffff (VERNEEDNUM)         1
 0x000000006ffffff0 (VERSYM)             0x567e
 0x000000006ffffff9 (RELACOUNT)          3428
 0x0000000000000000 (NULL)               0x0
energygreek commented 2 years ago

i have tried in suse container and native machine, this only happens in alpine docker container, maybe it's a issue of musl?

FranckRJ commented 2 years ago

I reproduced the issue. It's not because of the enum, but because of the string reference. It keep a dangling reference to the string parameter (it's deleted right after the function call).

Keeping dangling reference of parameters is a know issue of FakeIt for the argument matching in the Verify method, but I never really thought about it for the formatter.

Disabling formatting for all references may be easy and would fix the bug, but it'll remove some debugging information in case the reference wasn't a dangling reference. I wonder what's the best thing to do here.

malcolmdavey commented 2 years ago

Been meaning to develop an optional solution to this (when I get around to it). If we can provide something like a parameter trait, or something else that works, which can control which types are copied and which ones the reference/pointer is stored, then that would solve it.

Whether is one by default for things like strings or not, is another question ...

FranckRJ commented 2 years ago

I've looked at how gmock handle it, and they simply don't have any "Verify" (checking parameters after function call) mechanism. Instead they use an "Except" (setting up requirements before calling functions mock and checking these requirements at call-time) mechanism.

I think it's a better way to do it, but it isn't a small feature to add to FakeIt.

malcolmdavey commented 2 years ago

We were currently using Googlemock, though being more complete, was a bit more clunky, not as modern C++ish, and also you need to manually create the mock classes. so wanted something cleaner and nicer, so are starting to use fakeit.

And yes - the verifying after is a specific feature of fakeit, just that this dangling reference, especially for things like string which would be a very common scenario for us.

FranckRJ commented 2 years ago

I'll centralize the discussion about this known bug in a new issue: #274