This is an archive of the discontinued LLVM Phabricator instance.

[Support] Lazy load of dbghlp.dll on Windows
ClosedPublic

Authored by lenykholodov on Jun 25 2015, 10:07 AM.

Details

Summary

This patch changes linkage with dbghlp.dll for clang from static (at load time) to on demand (at the first use of required functions). Clang uses dbghlp.dll only in minor use-cases. First of all in case of crash and in case of plugin load. The dbghlp.dll library can be absent on system. In this case clang will fail to load. With lazy load of dbghlp.dll clang can work even if dbghlp.dll is not available.

Diff Detail

Repository
rL LLVM

Event Timeline

lenykholodov retitled this revision from to [Support] Lazy load of dbghlp.dll on Windows.
lenykholodov updated this object.
lenykholodov edited the test plan for this revision. (Show Details)
lenykholodov added reviewers: Bigcheese, rnk, zturner.
lenykholodov set the repository for this revision to rL LLVM.
lenykholodov added a subscriber: Unknown Object (MLST).
zturner edited edge metadata.Jun 25 2015, 6:37 PM

Why not just make it delay load dll instead of all the LoadLibrary stuff?

Why not just make it delay load dll instead of all the LoadLibrary stuff?

This was done for supporting mingw distributions without 64-bit versions of the DebugHlp APIs (please, see lib/Support/Windows/Signals.inc line 49 and below). Existing code base has already implemented dll loading part for some cases so I have prepared the patch as close to current implementation as possible.

I don't really know much about MinGW, and i also am not in a position to be
able to look at any code right now as I'm technically OOO for another 3
weeks. That said, if it is possible to make this work as a delayload dll,
it would be much preferred, even if it strays from some existing
implementation that does something different. With MSVC, this is as simple
as adding a linker flag, and you don't have to change even 1 line of code.
I'm not sure if MinGW toolchain supports this, but please check first. I'll
leave the final lgtm to someone else on this review.

rnk edited edge metadata.Jun 29 2015, 2:39 PM

Can you remove the configure and cmake test for libimagehlp along with this? Don't worry about regenerating autoconf, I can get Eric to do that.

I don't really know much about MinGW, and i also am not in a position to be
able to look at any code right now as I'm technically OOO for another 3
weeks. That said, if it is possible to make this work as a delayload dll,
it would be much preferred, even if it strays from some existing
implementation that does something different. With MSVC, this is as simple
as adding a linker flag, and you don't have to change even 1 line of code.
I'm not sure if MinGW toolchain supports this, but please check first. I'll
leave the final lgtm to someone else on this review.

I have checked MinGW possibilities and have not found easy way for delay loading. So at this moment I see following aproaches for this issue:

  1. Leave manual loading for MinGW in the code but use MSVC -delayload linker flag. The main drawback of this approach is two different ways for implementing lazy loading of imghelp.dll.
  2. Use dlltool (https://sourceware.org/binutils/docs/binutils/dlltool.html) for MinGW to generate imghelp.a file with lazy loading and simple -delayload linker flag for MSVC. This approach removes manual loading code from the patch, but adds configuration code for launching dlltool and most probably def file. Moreover, some of MinGW implementations don't provide --output-delaylib parameter (for example, 4.3 and lower). So building of LLVM may be impossible for this versions.
  3. Use manual loading for both compilers. This approach has evident benefit with same implementation for both compilers (msvc, mingw) and without using of extra tools.
lenykholodov edited edge metadata.

libimagehlp was removed from configure and cmake files.

The additional issue with automatic DLL delay loading is error handling. By default, application will fail in case of DLL absense, if there are no hooks or SEH handlers (https://msdn.microsoft.com/en-us/library/3aeywt27.aspx). So for automatic delay-loaded DLLs we have to write such hooks/handlers. Manual loading aproach doesn't have such drawback.

rnk added a comment.Jun 30 2015, 2:05 PM

I'm fine with the dynamic approach over the delayload approach. We use this DLL so infrequently that it's fine.

configure
8661

configure is a generated file. You want to change llvm/autoconf/configure.ac, and let Eric run autoconf to update this file.

  • configure changes were removed;
  • autoconf/configure.ac tesing of imghelp was removed.
rnk accepted this revision.Jul 1 2015, 10:44 AM
rnk edited edge metadata.

Thanks, lgtm!

This revision is now accepted and ready to land.Jul 1 2015, 10:44 AM
This revision was automatically updated to reflect the committed changes.