Add support for modules loaded at runtime

[bkloster-sma]

I noticed that cpptrace could not resolve symbols from DLLs that were loaded with LoadLibrary after already generating at least one stacktrace. According to the Microsoft documentation for Symbol Handler Initialization, one should call SymLoadModuleEx for this case.

I've implemented a new function cpptrace::experimental::load_symbols_for_file(const std::string& name) that does this.

[jeremy-rifkin]

Thanks so much for taking the time to contribute to cpptrace and put together a thoughtful implementation! I've left some comments below

[bkloster-sma]

Thanks for the comments!

If we make load_symbols_for_file a configuration-dependant function (i.e. it is not even declared when CPPTRACE_GET_SYMBOLS_WITH_DBGHELP is not defined), I wonder if we should change its signature and name to load_symbols_for_module(HANDLE hModule). After all, if a library user calls LoadLibrary, they already have the handle, so there would be no need for load_symbols_for_X to retrieve that handle again with GetModuleHandle.

[jeremy-rifkin]

I think that's reasonable, it would simplify implementation a lot. In the header what we'd want is the load_symbols_for_module function declared only for _WIN32 and then it can be defined to do something for dbghelp or a no-op if for some reason a different back-end is configured on windows

[jeremy-rifkin]

Revert to load_symbols_for_file to avoid including Windows.h

I think it should be possible to do load_symbols_for_module without including windows.h (I appreciate your thought about this #include). HMODULE is just a HANDLE which is just a void* so I think CPPTRACE_EXPORT void load_symbols_for_module(void* hModule); is good

[bkloster-sma]

Ha! I had the same idea after perusing this page. But the doc is wrong. A static assertion static_assert(std::is_same_v<HMODULE, void*>) fails, and when I trace back the definition of HMODULE in my headers, I find this in minwindef.h:

DECLARE_HANDLE(HINSTANCE);
typedef HINSTANCE HMODULE;      /* HMODULEs can be used in place of HINSTANCEs */

And DECLARE_HANDLE(HINSTANCE) expands to this:

struct HINSTANCE__ {
    int unused;
}; typedef struct HINSTANCE__* HINSTANCE

Now, if you really want to, we could still use the load_symbols_for_module(void*) signature, because HMODULE implicitly converts to void*. However, I see two arguments against it:

1. Pretty much all pointers implicitly convert to void*, not just HMODULE. So an inattentive user could pass in a const char* to the filename, causing undefined behaviour.
2. After writing the load_symbols_for_module(HMODULE) implementation, I realized that we need the filename anyway. So the implementation doesn't actually get simpler, we just exchange GetModuleHandle for GetModuleFileName.

[jeremy-rifkin]

Thanks, I see. That's unfortunate that the doc is wrong and foils the nicer solution here. It's also unfortunate that the filename is still needed, with that I'd say it definitely makes sense to use the string API instead of trying to make a handle API work

[jeremy-rifkin]

I think this looks good to me, I took the liberty of pushing a few minor changes and changing the base to dev. I might move this out of experimental:: before the next release, I think it's sensible enough.