Page MenuHomePhabricator

[libcxx] [test] Don't use header defines for detecting linking against a DLL
ClosedPublic

Authored by mstorsjo on May 18 2022, 1:01 PM.

Details

Summary

In clang-cl/MSVC environments, linking against a DLL C++ standard
library requires having dllimport attributes in the headers; this
has been used for detecting whether the tests link against a DLL,
by looking at the libc++ specific define
_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS.

In mingw environments, thanks to slightly different code generation
and a couple linker tricks, it's possible to link against a DLL C++
standard library without dllimport attributes. Therefore, don't
rely on the libc++ specific header define for the detection.

Replace the detection with a runtime test.

There are two potential alternatives for a test (I included both
in the patch, with one of them commented out); one simple, libc++
specific which just checks if the libc++ DLL exists loaded in the
process, and a more complex one which inspects the PE image and
checks whether the address of std::cout exists within the address
range of the EXE (which can detect whether linking against a DLL
version of other C++ standard libraries than libc++).

Diff Detail

Event Timeline

mstorsjo created this revision.May 18 2022, 1:01 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 18 2022, 1:01 PM
Herald added a subscriber: arichardson. · View Herald Transcript
mstorsjo requested review of this revision.May 18 2022, 1:01 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 18 2022, 1:01 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
ldionne accepted this revision.Thu, Jun 2, 7:54 AM
ldionne added inline comments.
libcxx/utils/libcxx/test/features.py
236–248

Would this really work with other standard libraries? If so, I think this is the better alternative.

Whichever check you end up going for, I would add a short comment explaining how the check works, because it's not obvious for a non-Windows-savy person.

This revision is now accepted and ready to land.Thu, Jun 2, 7:54 AM
mstorsjo updated this revision to Diff 433821.Thu, Jun 2, 11:47 AM

Settle on the C++ library agnostic feature test, add comments explaining what the test actually does.