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

SEGV when using spdlog inside dll #1562

Closed
L0ric0 opened this issue May 20, 2020 · 6 comments
Closed

SEGV when using spdlog inside dll #1562

L0ric0 opened this issue May 20, 2020 · 6 comments

Comments

@L0ric0
Copy link

L0ric0 commented May 20, 2020

i'm trying to use spdlog inside of an so/dll. i'm getting a segfault when trying to call the line logger = spdlog::stdout_color_mt(logger_name); from https://github.com/gabime/spdlog/wiki/How-to-use-spdlog-in-DLLs

her is the backtrace:

Thread 1 "monstr_el_ph_tt" received signal SIGSEGV, Segmentation fault.
0x00007ffff789228d in spdlog::details::registry::initialize_logger (
    this=this@entry=0x5555556c97c0 <spdlog::details::registry::instance()::s_instance>,                                                                       
    new_logger=std::shared_ptr<spdlog::logger> (use count 2, weak count 0) = {...}) at /usr/include/c++/8/bits/unique_ptr.h:342                               
342           get() const noexcept
(gdb) bt
#0  0x00007ffff789228d in spdlog::details::registry::initialize_logger (this=this@entry=0x5555556c97c0 <spdlog::details::registry::instance()::s_instance>,   
    new_logger=std::shared_ptr<spdlog::logger> (use count 2, weak count 0) = {...}) at /usr/include/c++/8/bits/unique_ptr.h:342                               
#1  0x00007ffff788e376 in spdlog::synchronous_factory::create<spdlog::sinks::ansicolor_stdout_sink<spdlog::details::console_mutex>, spdlog::color_mode&> (    
    logger_name="") at /usr/include/c++/8/ext/atomicity.h:96
#2  spdlog::stdout_color_mt<spdlog::synchronous_factory> (mode=spdlog::color_mode::automatic, logger_name="librethfeld_logger")                               
    at /usr/local/include/spdlog/1.6.0/spdlog/sinks/stdout_color_sinks-inl.h:18                                                                               
#3  librethfeld::setup_logger (sinks=std::vector of length 0, capacity 0, logger_name="librethfeld_logger") at src/logging.cpp:45                             
#4  0x0000555555629708 in configureLogging (ini_file="etc/templates/ttm/el_ph_ttm_laser.ini") at /usr/include/c++/8/ext/new_allocator.h:79                    
#5  0x00005555555aeb36 in main (argc=2, argv=0x7fffffffdb88) at /usr/include/c++/8/bits/basic_string.h:936
@tt4g
Copy link
Contributor

tt4g commented May 20, 2020

As far as the backtrace shows, in the spdlog::registory::initialize_logger() function, the
The reason for the segmentation fault is access to the dangling pointer managed by std::unique_ptr.
I assume that std::unique_ptr::get() returned a dangling pointer, but called a member function through that pointer.

This is a strange occurrence. spdlog::registroy::instance() is a static object and has only two std::unique_ptr member variables.

SPDLOG_INLINE registry &registry::instance()
{
static registry s_instance;
return s_instance;
}

std::unique_ptr<formatter> formatter_;
level::level_enum flush_level_ = level::off;
void (*err_handler_)(const std::string &msg);
std::shared_ptr<thread_pool> tp_;
std::unique_ptr<periodic_worker> periodic_flusher_;

This is a strange occurrence. Can you provide a snippet?

NOTE: I've had similar issue reports before. The cause of this bug is the initialization of spdlog with the DllMain() entry function.
If you are using spdlog with a special entry point such as the DllMain() function, it is possible that the DLL variable is not initialized.
If the DLL variable is not initialized, spdlog will cause undefined behavior.

@L0ric0
Copy link
Author

L0ric0 commented May 20, 2020

thank you for the fast answer.
all that is done to init the logger in the dll is to call
auto logger = setup_logger({}, "logger_name");

header:

#ifndef LIBRETHFELD__LOGGING_HPP_
#define LIBRETHFELD__LOGGING_HPP_

//librethfeld

//global
#include <memory>
#include "spdlog/fmt/ostr.h"
#include "spdlog/spdlog.h"
#include <vector>

namespace librethfeld {

    static std::shared_ptr<spdlog::logger> logger;

    std::shared_ptr<spdlog::logger> setup_logger(
            std::vector<spdlog::sink_ptr> sinks,
            std::string logger_name
    );

}
#endif //LIBRETHFELD__LOGGING_HPP_

cpp file:

//librethfeld
#include "librethfeld/logging.hpp"

//global
#include <memory>
#include "spdlog/async.h"
#include "spdlog/fmt/ostr.h"
#include "spdlog/sinks/rotating_file_sink.h"
#include "spdlog/sinks/stdout_color_sinks.h"
#include "spdlog/spdlog.h"
#include <vector>


std::shared_ptr<spdlog::logger> librethfeld::setup_logger(std::vector<spdlog::sink_ptr> sinks, std::string logger_name)
{
    std::shared_ptr<spdlog::logger> logger = spdlog::get(logger_name);

    if (not logger){
        if (sinks.size() > 0){
            logger = std::make_shared<spdlog::logger>(
                    logger_name,
                    std::begin(sinks),
                    std::end(sinks));
            spdlog::register_logger(logger);
        }else {
            /*
            auto console_sink = std::make_shared<spdlog::sinks::stdout_color_sink_mt>();
            console_sink->set_pattern("[%^%l%$] %v");
            console_sink->set_level(spdlog::level::info);
    
            auto file_sink = std::make_shared<spdlog::sinks::rotating_file_sink_mt>("var/log/librethfeld", 1048576 * 5, 10, true);
            file_sink->set_pattern("[%^%l%$] %v");
            file_sink->set_level(spdlog::level::trace);
    
            spdlog::sinks_init_list sink_list = {file_sink, console_sink };
    
            auto logger = std::make_shared<spdlog::async_logger>(logger_name, sink_list, spdlog::thread_pool(), spdlog::async_overflow_policy::block);
            logger->set_level(spdlog::level::trace);
            */
            logger = spdlog::stdout_color_mt(logger_name);
        }
    }

    librethfeld::logger = logger;
    return logger;
}

@tt4g
Copy link
Contributor

tt4g commented May 20, 2020

@L0ric0 Are you really declaring std::shared_ptr<spdlog::logger> in the namespace?
This usage is unsafe because static variables declared in namespaces are placed in their own memory in all #include .cpp files.

I took a hint from your snippet and made a small project to generate a shared library and application that links to the static spdlog.

https://github.com/tt4g/spdlog-issue-1562

Project source here

Library header:

#ifndef SPDLOG_ISSUE_1562_HPP
#define SPDLOG_ISSUE_1562_HPP

#include "spdlog_issue_1562_export.hpp"

#include "spdlog/spdlog.h"
#include "spdlog/fmt/ostr.h"

#include <memory>
#include <vector>

namespace spdlog_issue_1562
{

    static std::shared_ptr<spdlog::logger> logger;

    std::shared_ptr<spdlog::logger> SPDLOG_ISSUE_1562_API setup_logger(
            std::vector<spdlog::sink_ptr> sinks,
            std::string logger_name
    );

} // namespace spdlog_issue_1562

#endif // SPDLOG_ISSUE_1562_HPP

Library implementation:

#include "spdlog_issue_1562.hpp"

#include "spdlog/async.h"
#include "spdlog/sinks/rotating_file_sink.h"
#include "spdlog/sinks/stdout_color_sinks.h"

#include <iterator>
#include <string>

namespace spdlog_issue_1562
{

std::shared_ptr<spdlog::logger> setup_logger(
        std::vector<spdlog::sink_ptr> sinks,
        std::string logger_name)
{
    std::shared_ptr<spdlog::logger> logger = spdlog::get(logger_name);

    if (!logger) {
        if (sinks.size() > 0){
            logger = std::make_shared<spdlog::logger>(
                    logger_name,
                    std::begin(sinks),
                    std::end(sinks));
            spdlog::register_logger(logger);
        } else {
            logger = spdlog::stdout_color_mt(logger_name);
        }
    }

    spdlog_issue_1562::logger = logger;
    return logger;
}

} // namespace spdlog_issue_1562

main.cpp:

#include "spdlog_issue_1562/spdlog_issue_1562.hpp"

#include <iostream>

int main(int argc, char** argv)
{
    auto logger = spdlog_issue_1562::setup_logger({}, "main");

    if (logger) {
        logger->info("Hello, World!");
    } else {
        std::cerr << "invalid logger\n";
    }

    if (spdlog_issue_1562::logger) {
        spdlog_issue_1562::logger->info("global: Hello, World!");
    } else {
        std::cerr << "invalid spdlog_issue_1562::logger\n";
    }

    return 0;
}

When I build and run this project with MSVC, I see the following output:

$ spdlog_issue_1562
[2020-05-20 21:46:47.314] [main] [info] Hello, World!
invalid spdlog_issue_1562::logger

That is, when an application accesses the spdlog_issue_1562::logger declared in the library header, it is an invalid pointer.

Can you build this project too and see if you get the same results as me?
If you have the same result as I do, I would probably expect that your application is accessing an uninitialized pointer declared in the library.

@L0ric0
Copy link
Author

L0ric0 commented May 21, 2020

ok i'm getting the same output as you. does that mean i have to declare the logger var as extern to have it as library wide global variable?

@tt4g
Copy link
Contributor

tt4g commented May 21, 2020

ok i'm getting the same output as you. does that mean i have to declare the logger var as extern to have it as library wide global variable?

Right.
However, I prefer to have an access function than an extern variable.

As an implementation, the library user accesses the global logger through an access function.
In this case, the global logger static std::shared_library is declared in the library implementation file in an anonymous namespace.

Example: Library header: ```diff - static std::shared_ptr logger; + std::shared_ptr SPDLOG_ISSUE_1562_API global_logger();
std::shared_ptr<spdlog::logger> SPDLOG_ISSUE_1562_API setup_logger(
        std::vector<spdlog::sink_ptr> sinks,
        std::string logger_name
);

Library implementation:
```diff
namespace spdlog_issue_1562
{
+
+ namespace
+ {
+
+ struct global_logger_holder
+ {
+     static std::shared_ptr<spdlog::logger> logger;
+ };
+
+ } // namespace
+
+ std::shared_ptr<spdlog::logger> SPDLOG_ISSUE_1562_API global_logger()
+ {
+     return atomic_load(&global_logger_holder::logger);
+ }

std::shared_ptr<spdlog::logger> setup_logger(
        std::vector<spdlog::sink_ptr> sinks,
        std::string logger_name)
{
    std::shared_ptr<spdlog::logger> logger = spdlog::get(logger_name);

    if (!logger) {
        if (sinks.size() > 0){
            logger = std::make_shared<spdlog::logger>(
                    logger_name,
                    std::begin(sinks),
                    std::end(sinks));
            spdlog::register_logger(logger);
        } else {
            logger = spdlog::stdout_color_mt(logger_name);
        }
    }

-     spdlog_issue_1562::logger = logger;
+    std::atomic_store(&global_logger_holder::logger, logger);
    return logger;
}

NOTE: If you are accessing the global logger from multiple applications at the same time, the read/write operation to the global logger must be atomic.
Before C++20, atomic_store( std::shared_ptr<T>*, std::shared_ptr<T>) and atomic_load( const std::shared_ptr<T>*) were used for this purpose.
For C++20 and later, use std::atomic<std::shared_ptr<T>>.
See for more details: https://en.cppreference.com/w/cpp/memory/shared_ptr/atomic

@L0ric0
Copy link
Author

L0ric0 commented May 24, 2020

that is working like a charm. thanks for your quick and extensive help

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants
@L0ric0 @tt4g and others