Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Formatter can not format enum type and causes SIGSEGV #262

Closed
energygreek opened this issue Feb 8, 2022 · 8 comments
Closed

Formatter can not format enum type and causes SIGSEGV #262

energygreek opened this issue Feb 8, 2022 · 8 comments

Comments

@energygreek
Copy link

energygreek commented Feb 8, 2022

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
Copy link
Collaborator

FranckRJ commented May 3, 2022

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

@energygreek
Copy link
Author

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
Copy link
Author

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

@FranckRJ
Copy link
Collaborator

FranckRJ commented May 5, 2022

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
Copy link
Contributor

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
Copy link
Collaborator

FranckRJ commented May 5, 2022

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
Copy link
Contributor

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
Copy link
Collaborator

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

@FranckRJ FranckRJ closed this as not planned Won't fix, can't repro, duplicate, stale May 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants