gabime / spdlog

Fast C++ logging library.
Other
23.88k stars 4.48k forks source link

Using Log.h class inside extern class #1505

Closed ranoke closed 4 years ago

ranoke commented 4 years ago

I added inside my Memory (read&write) class Logging(Log.h) and used extern Memory but it didnt work so i did some debugging and found this:

Exception thrown: read access violation.
**std::_Atomic_address_as<long const ,std::_Atomic_padded<int> const >**(...) returned 0x50. occurred

Log.h

#include <memory>
#include <spdlog/spdlog.h>
#include <spdlog/fmt/ostr.h>
class Log
{
private:
    static std::shared_ptr<spdlog::logger> s_ClientLogger;
public:
    static void Init();
    inline static std::shared_ptr<spdlog::logger>& GetClientLogger() { return s_ClientLogger; }
};
//log macros            
#define AC_TRACE(...)         ::Log::GetClientLogger()->trace(__VA_ARGS__)
#define AC_INFO(...)          ::Log::GetClientLogger()->info(__VA_ARGS__)
#define AC_WARN(...)          ::Log::GetClientLogger()->warn(__VA_ARGS__)
#define AC_ERROR(...)         ::Log::GetClientLogger()->error(__VA_ARGS__)
#define AC_FATAL(...)         ::Log::GetClientLogger()->fatal(__VA_ARGS__)

If needed i can provide more code. The reason i dont do it right now becouse its my first Issue on github and i dont know will it be usefull or not. Im not a great coder - just learning Sorry for bad english. Thanks

tt4g commented 4 years ago

s_ClientLogger is not initialized in your snippet.

tt4g commented 4 years ago

I recommend that do not use inline and static to export variables. Please try the following changes:

// Log.h
class Log
{
private:
-   static std::shared_ptr<spdlog::logger> s_ClientLogger;
public:
    static void Init();
-   inline static std::shared_ptr<spdlog::logger>& GetClientLogger() { return s_ClientLogger; }
+   static spdlog::logger* GetClientLogger();
};

// Log.cpp
static spdlog::logger* Log::GetClientLogger()
{
    static std::shared_ptr<spdlog::logger> s_ClientLogger = 
            std::make_shared<spdlog::logger>(/* initializing code here */);

    return s_ClientLogger .get();
}
ranoke commented 4 years ago

Thanks you! I found a logical bug in my code -> Log has to be initialized first what i did :) UPD: that was dumb by me LOL

zentrixg commented 4 years ago

I know I am late, sorry. Anyway, I would much apreciate if you would be so kind to help me, as I had no avail when trying to solve the exception.

So the code is from The Cherno's game engine series (https://www.youtube.com/watch?v=xnopUoZbMEk&t=1096s), and at around 31:55, he tries to log a hardwired event, only for proof of concept. The point is that everything up until that point works fine, but when the program hits the line that should print the event details, it throws an exception

Unhandled exception thrown: read access violation. std::_Atomic_address_as<long const ,std::_Atomic_padded const >(...) returned 0x50.

This is my code that I duct-taped together, so that it worked momentarily. It is all in the header

Log.h

#pragma once

#include <memory>

#include "Core.h"
#include "spdlog/spdlog.h"
#include "spdlog/sinks/stdout_color_sinks.h"
#include "spdlog/fmt/ostr.h"

namespace Hazel {

    class HAZEL_API Log
    {
    private:
        inline static std::shared_ptr<spdlog::logger> s_CoreLogger;
        inline static std::shared_ptr<spdlog::logger> s_ClientLogger;

    public:
        static void init();

        inline static std::shared_ptr<spdlog::logger>& GetCoreLogger() { return s_CoreLogger; }
        //inline static std::shared_ptr<spdlog::logger>& getClientLogger() { return s_ClientLogger; }
        static spdlog::logger* GetClientLogger() { return s_ClientLogger.get(); }

    };

    void Log::init() {
        spdlog::set_pattern("%^[%T] %n: %v%$");

        s_CoreLogger = spdlog::stdout_color_mt("HAZEL");
        s_CoreLogger->set_level(spdlog::level::trace);

        s_ClientLogger = spdlog::stdout_color_mt("APP");
        s_ClientLogger->set_level(spdlog::level::trace);
    };

}

// Core log macros
#define HZ_CORE_TRACE(...)    ::Hazel::Log::GetCoreLogger()->trace(__VA_ARGS__)
#define HZ_CORE_INFO(...)     ::Hazel::Log::GetCoreLogger()->info(__VA_ARGS__)
#define HZ_CORE_WARN(...)     ::Hazel::Log::GetCoreLogger()->warn(__VA_ARGS__)
#define HZ_CORE_ERROR(...)    ::Hazel::Log::GetCoreLogger()->error(__VA_ARGS__)
#define HZ_CORE_CRITICAL(...) ::Hazel::Log::GetCoreLogger()->critical(__VA_ARGS__)

// Client log macros
#define HZ_TRACE(...)         ::Hazel::Log::GetClientLogger()->trace(__VA_ARGS__)
#define HZ_INFO(...)          ::Hazel::Log::GetClientLogger()->info(__VA_ARGS__)
#define HZ_WARN(...)          ::Hazel::Log::GetClientLogger()->warn(__VA_ARGS__)
#define HZ_ERROR(...)         ::Hazel::Log::GetClientLogger()->error(__VA_ARGS__)
#define HZ_CRITICAL(...)      ::Hazel::Log::GetClientLogger()->critical(__VA_ARGS__)

To give a little detail about the project, the engine is made in a .dll, and the app is an exe. Sorry for the maybe, incorrect way of posting. Anyway, any answer will be much appreciated. Thanks alot!

Edit: Added context

ranoke commented 4 years ago

@zentrixg In the entry point of an Engine project u must have a Log::Init() to be called first.. Seems u have an error like me when u first calling Print function and after it initializing Log

zentrixg commented 4 years ago

EntryPoint.h

#pragma once

#ifdef HZ_PLATFORM_WINDOWS

extern Hazel::Application* Hazel::CreateApplication();

int main(int argc, char** argv) {

    Hazel::Log::init();
    HZ_CORE_WARN("Initialized Log");

    int x = 984;
    float y = 3.1415;

    HZ_INFO("X={0}", x);

    auto app = Hazel::CreateApplication();
    app->Run();
    delete app;
}
#endif

EntryPoint.cpp

#include "Application.h"

#include "Hazel/Events/ApplicationEvent.h"
#include "Hazel/Log.h"

namespace Hazel {

    Application::Application() {};
    Application::~Application() {};

    void Application::Run() {
        WindowResizeEvent e(1280, 720);
        HZ_TRACE(e.ToString());

        while (1);
    }

}
tt4g commented 4 years ago

@zentrixg If you're using std::shared_ptr, you shouldn't be getting references or pointers.

Compiler optimizations may cause unexpected behavior. It's best to make the following changes and see if they reproduce the bug.

    class HAZEL_API Log
    {
    private:
-       inline static std::shared_ptr<spdlog::logger> s_CoreLogger;
-       inline static std::shared_ptr<spdlog::logger> s_ClientLogger;
+       static std::shared_ptr<spdlog::logger> s_CoreLogger;
+       static std::shared_ptr<spdlog::logger> s_ClientLogger;

    public:
        static void init();

-       inline static std::shared_ptr<spdlog::logger>& GetCoreLogger() { return s_CoreLogger; }
-       //inline static std::shared_ptr<spdlog::logger>& getClientLogger() { return s_ClientLogger; }
-       static spdlog::logger* GetClientLogger() { return s_ClientLogger.get(); }
+       static std::shared_ptr<spdlog::logger> GetCoreLogger() { return s_CoreLogger; }
+       static std::shared_ptr<spdlog::logger> getClientLogger() { return s_ClientLogger; }

    };
zentrixg commented 4 years ago
namespace Hazel {

    class HAZEL_API Log
    {
    private:
        static std::shared_ptr<spdlog::logger> s_CoreLogger;
        static std::shared_ptr<spdlog::logger> s_ClientLogger;

    public:
        static void init();

        static std::shared_ptr<spdlog::logger>& GetCoreLogger() { return s_CoreLogger; }
        static spdlog::logger* GetClientLogger() { return s_ClientLogger.get(); }

    };

    void Log::init() {
        spdlog::set_pattern("%^[%T] %n: %v%$");

        s_CoreLogger = spdlog::stdout_color_mt("HAZEL");
        s_CoreLogger->set_level(spdlog::level::trace);

        s_ClientLogger = spdlog::stdout_color_mt("APP");
        s_ClientLogger->set_level(spdlog::level::trace);
    };

}

Now Getting lots of unresolved externals regarding the loggers. These unresolved symbols are said to be in

Application.obj (One of the DLL's files) SandboxApp.obj (DLL's client) SandboxApp.exe (Client Executable)

ranoke commented 4 years ago
#pragma once

#include <memory>

#include "Core.h"
#include "spdlog/spdlog.h"
#include "spdlog/sinks/stdout_color_sinks.h"
#include "spdlog/fmt/ostr.h"

namespace Hazel {

 class HAZEL_API Log
 {
 private:
 static std::shared_ptr<spdlog::logger> s_CoreLogger;
 static std::shared_ptr<spdlog::logger> s_ClientLogger;

 public:
  static void init();

  inline static std::shared_ptr<spdlog::logger>& GetCoreLogger() { return s_CoreLogger; }
  inline static std::shared_ptr<spdlog::logger>& GetClientLogger() { return s_ClientLogger; }

 };

 void Log::init() {
  spdlog::set_pattern("%^[%T] %n: %v%$");

  s_CoreLogger = spdlog::stdout_color_mt("HAZEL");
  s_CoreLogger->set_level(spdlog::level::trace);

  s_ClientLogger = spdlog::stdout_color_mt("APP");
  s_ClientLogger->set_level(spdlog::level::trace);
 };

}

// Core log macros
#define HZ_CORE_TRACE(...)    ::Hazel::Log::GetCoreLogger()->trace(__VA_ARGS__)
#define HZ_CORE_INFO(...)     ::Hazel::Log::GetCoreLogger()->info(__VA_ARGS__)
#define HZ_CORE_WARN(...)     ::Hazel::Log::GetCoreLogger()->warn(__VA_ARGS__)
#define HZ_CORE_ERROR(...)    ::Hazel::Log::GetCoreLogger()->error(__VA_ARGS__)
#define HZ_CORE_CRITICAL(...) ::Hazel::Log::GetCoreLogger()->critical(__VA_ARGS__)

// Client log macros
#define HZ_TRACE(...)         ::Hazel::Log::GetClientLogger()->trace(__VA_ARGS__)
#define HZ_INFO(...)          ::Hazel::Log::GetClientLogger()->info(__VA_ARGS__)
#define HZ_WARN(...)          ::Hazel::Log::GetClientLogger()->warn(__VA_ARGS__)
#define HZ_ERROR(...)         ::Hazel::Log::GetClientLogger()->error(__VA_ARGS__)
#define HZ_CRITICAL(...)      ::Hazel::Log::GetClientLogger()->critical(__VA_ARGS__)

Shoud fix the errors i guess. I'm sorry for the formatting, but the phone is not made for this

zentrixg commented 4 years ago

Nope, the errors are still there, sadly

Severity Code Description Project File Line Suppression State

Error LNK2001 unresolved external symbol "private: static class std::shared_ptr Hazel::Log::s_ClientLogger" (?s_ClientLogger@Log@Hazel@@0V?$shared_ptr@Vlogger@spdlog@@@std@@A) C:\Users\Alex\source\repos\Hazel\Sandbox\SandboxApp.obj 1

Error LNK2001 unresolved external symbol "private: static class std::shared_ptr Hazel::Log::s_CoreLogger" (?s_CoreLogger@Log@Hazel@@0V?$shared_ptr@Vlogger@spdlog@@@std@@A) C:\Users\Alex\source\repos\Hazel\Hazel\Application.obj 1

Error LNK2001 unresolved external symbol "private: static class std::shared_ptr Hazel::Log::s_ClientLogger" (?s_ClientLogger@Log@Hazel@@0V?$shared_ptr@Vlogger@spdlog@@@std@@A) C:\Users\Alex\source\repos\Hazel\Hazel\Application.obj 1

Error LNK1120 2 unresolved externals Hazel C:\Users\Alex\source\repos\Hazel\bin\Debug-windows-x86_64\Hazel\Hazel.dll 1

Error LNK2001 unresolved external symbol "private: static class std::shared_ptr Hazel::Log::s_CoreLogger" (?s_CoreLogger@Log@Hazel@@0V?$shared_ptr@Vlogger@spdlog@@@std@@A) C:\Users\Alex\source\repos\Hazel\Sandbox\SandboxApp.obj 1

Error LNK1120 2 unresolved externals Sandbox C:\Users\Alex\source\repos\Hazel\bin\Debug-windows-x86_64\Sandbox\Sandbox.exe 1

Sorry for the bad error formatting

But maybe the error is elsewhere? Because it's a linking error, so maybe the symbols were not exported properly? if so, any advice on fixing that? I am not really good with dll's, I started looking on the Hazel engine series so i could enforce my skills, but I had a pretty rough start, possibly because of the multitude of files. Anyway, thanks for all the help up untill now, and I would be glad if you would stick with me to fix this entire ugly thing

tt4g commented 4 years ago

@Zentrixg Are you using CMake in your project? If so, How do you link spdlog to your application?

spdlog uses macros in the header file to identify header-only builds and library builds. You have to build your application by defining the specified spdlog macro. If the macros are different, the function definitions do not match and this will result in a link error.

If the CMake spdlog target is linked with target_link_libraries() for your application, the macro definitions needed to build your application will be added.

Example: https://github.com/gabime/spdlog/blob/v1.6.0/example/CMakeLists.txt NOTE spdlog::spdlog is a static or shared library target and spdlog::spdlog_header_only is a header-only target.

P.S. When commenting on GitHub, error messages and source code are formatted by enclosing them in ```. Use it for ease of reading.

See: https://guides.github.com/features/mastering-markdown/

ranoke commented 4 years ago

I think this should help

// Log.h
#pragma once

#include <memory>

#include "Core.h"
#include "spdlog/spdlog.h"
#include "spdlog/sinks/stdout_color_sinks.h"
#include "spdlog/fmt/ostr.h"

namespace Hazel {

 class HAZEL_API Log
 {
 private:
 static std::shared_ptr<spdlog::logger> s_CoreLogger;
 static std::shared_ptr<spdlog::logger> s_ClientLogger;

 public:
  static void init();

  inline static std::shared_ptr<spdlog::logger>& GetCoreLogger() { return s_CoreLogger; }
  inline static std::shared_ptr<spdlog::logger>& GetClientLogger() { return s_ClientLogger; }

 };

// here ur macroses 
//Log.cpp
#include "Log.h" 
namespace Hazel { 
    // declare these as part of Log!      
      std::shared_ptr<spdlog::logger> Log::s_CoreLogger;
      std::shared_ptr<spdlog::logger> Log::s_ClientLogger; 
    void Log::init() {
    spdlog::set_pattern("%^[%T] %n: %v%$"); 
    } 
}
IHPsss commented 1 year ago

I get the same issue as you, and the cause of this lies on Core.h where some typos create some link problem as typos make os can not replace AZEL_API with __declspec(dllexport). Also do not forget to check typos in c++ preprocesssor !!