Page MenuHomePhabricator

[LLDB] Add a missing specification of linking against dbghelp
ClosedPublic

Authored by mstorsjo on Sep 21 2019, 2:22 PM.

Details

Summary

The PECOFF object file plugin uses the dbghelp API, but doesn't specify that it has to be linked in anywhere.

Current MSVC based builds have probably succeeded, as other parts in LLDB have had a "#pragma comment(lib, "dbghelp.lib")", but there's currently no such pragma in the PECOFF plugin.

The "#pragma comment(lib, ...)" approach doesn't work in MinGW mode (unless the compiler is given the -fms-extensions option, and even then, it's only supported by clang/lld, not by GCC/binutils), thus add it to be linked via CMake. (The other parts of LLDB that use dbghelp are within _MSC_VER ifdefs.)

Diff Detail

Event Timeline

mstorsjo created this revision.Sep 21 2019, 2:22 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 21 2019, 2:22 PM

@hhb - This is the only change left for builds of lldb on mingw to succeed for me (for x86), but I have a whole bunch of more changes lined up that fix warnings and other issues I've found.

labath accepted this revision.Sep 23 2019, 2:57 AM
labath added a subscriber: labath.

LGTM. It might be nice to add a comment that this is for saving of minidump files (I assume that's the only reason). The reason for that is the object file plugins are generally expected to work on any host, and so linking os-specific libraries is a bit surprising. Given that lldb is slowly growing support for minidump files on non-windows platforms, it is a matter of time when we will create an os-independent minidump writer (a lot of the pieces are already there), and then this bit of code can be removed.

This revision is now accepted and ready to land.Sep 23 2019, 2:57 AM

Just as a drive by comment, there is code in DebugInfo/Symbolize/Symbolize.cpp that loads dbghelp via #pragma comment(lib... trick

There is also code to load dbghelp dynamically in lib\Support\Windows\Signal.inc including loading MiniDumpWriteDump

Just as a drive by comment, there is code in DebugInfo/Symbolize/Symbolize.cpp that loads dbghelp via #pragma comment(lib... trick

Yes, but as I mentioned in the patch description here, #pragma comment(lib) only works in MSVC mode, it isn't supported when targeting MinGW unless -fms-extensions is specified (and if building with GCC and/or linking with GNU ld, it's not supported at all).

There is also code to load dbghelp dynamically in lib\Support\Windows\Signal.inc including loading MiniDumpWriteDump

I guess that could be an option as well, for a slightly larger change instead.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptSep 23 2019, 5:03 AM