Page MenuHomePhabricator

[runtimes] Default LIB*_HERMETIC_STATIC_LIBRARY to ON on Windows
ClosedPublic

Authored by mstorsjo on May 16 2022, 11:45 AM.

Details

Reviewers
ldionne
phosek
Group Reviewers
Restricted Project
Restricted Project
Restricted Project
Commits
rGbedf657d0f4c: [runtimes] Default LIB*_HERMETIC_STATIC_LIBRARY to ON on Windows
Summary

(In the case of libunwind, the cmake option is called
LIBUNWIND_HIDE_SYMBOLS, but it has the same effect as
LIBCXX_HERMETIC_STATIC_LIBRARY and
LIBCXXABI_HERMETIC_STATIC_LIBRARY.)

Previously, the same issue was dealt with by setting a project wide
define (_LIBUNWIND_HIDE_SYMBOLS,
_LIBCXXABI_DISABLE_VISIBILITY_ANNOTATIONS and
_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS) if only building a static
library. If building both static and shared at the same time, this
wasn't set, and the static library would contain dllexport directives.

The LIB*_HERMETIC_STATIC_LIBRARY and LIBUNWIND_HIDE_SYMBOLS cmake
options only apply the defines to the static library in the build,
even if building both static and shared at the same time.

(This could only be done use after the object libraries were
enabled, as a shared libcxx needs libcxxabi object files built
with dllexports included.)

This allows removing inelegant code for deciding how to build the
libcxxabi static library and a TODO comment that suggested that
users should need to start setting an option, which they shouldn't
need to. Finally, this gets rid of two XFAILs in tests.

Diff Detail

Event Timeline

mstorsjo created this revision.May 16 2022, 11:45 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptMay 16 2022, 11:45 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
Herald added a subscriber: mgorny. · View Herald Transcript
mstorsjo requested review of this revision.May 16 2022, 11:45 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptMay 16 2022, 11:45 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
Herald added a reviewer: Restricted Project. · View Herald Transcript
ldionne accepted this revision.May 18 2022, 8:04 AM
This revision is now accepted and ready to land.May 18 2022, 8:04 AM
phosek accepted this revision.May 18 2022, 9:34 AM

LGTM

We may also want to consider renaming LIBCXXABI_HERMETIC_STATIC_LIBRARY and LIBCXX_HERMETIC_STATIC_LIBRARY to LIBCXXABI_HIDE_SYMBOLS and LIBCXX_HIDE_SYMBOLS respectively for consistency.