This is an archive of the discontinued LLVM Phabricator instance.

Fix libcxx build with MinGW winpthreads
AcceptedPublic

Authored by SquallATF on Oct 23 2018, 6:32 PM.

Details

Reviewers
EricWF
Summary
  1. winpthreads build broken after D42214, should not build thread_win32.cpp when enable pthread witht MinGW.
  2. _LIBCPP_SAFE_STATIC incompatible with winpthreads, because winpthread PTHREAD_*_INITIALIZER use reinterpret_cast are not const.

Event Timeline

SquallATF created this revision.Oct 23 2018, 6:32 PM
mclow.lists added inline comments.
include/__config
1228 ↗(On Diff #170802)

I think you want to look at line #1155, and see if _LIBCPP_HAS_THREAD_API_WIN32 is the condition you want here.

EricWF requested changes to this revision.Oct 24 2018, 3:28 PM

_LIBCPP_SAFE_STATIC incompatible with winpthreads, because winpthread PTHREAD_*_INITIALIZER use reinterpret_cast are not const.

winpthread PTHREAD_*_INITIALIZER is incompatible with the POSIX specification for PTHREAD_*_INITIALIZER. If we need to work around that, then fine, but it causes mutex to become really dangerous.

this is a bug in winpthreads and they should fix it.

include/__config
1228 ↗(On Diff #170802)

This change is *not* OK.

You want to disable the application of this macro only in the specific case that's giving you problems, not generally.

This revision now requires changes to proceed.Oct 24 2018, 3:28 PM
  • I created a patch to MinGW winpthreads, remove _LIBCPP_SAFE_STATIC winpthread block
EricWF accepted this revision.Oct 26 2018, 4:55 PM

LGTM. Thanks for the patch.

This revision is now accepted and ready to land.Oct 26 2018, 4:55 PM

Can somebody land it before LLVM 8 is branched? Patch author does not have commit access.

Thanks in advance.

I'm sorry that nobody has replied to your request earlier. AFAICS the libcxx code has changed. Does it work now, or do we need to make thread_win32.cpp conditional to LIBCXX_HAS_WIN32_THREAD_API still?