MacOS c++ 17 build failures due to missing "shared_mutex" in standard library
Categories
(Firefox Build System :: General, defect, P3)
Tracking
(Not tracked)
People
(Reporter: shravanrn, Assigned: froydnj)
References
()
Details
When compiling firefox c++17 flag for MacOS, the build fails for code that includes "shared_mutex" from c++17's standard library. See here. In the prior link, ignore the other builds, as they fail for different reasons.
It is not clear if other portions of the c++ 17's standard library or language features are missing in the MacOS build and would also cause compile failures.
|
||
Comment 1•5 years ago
|
||
Note the error is error: 'shared_mutex' is unavailable: introduced in macOS 10.12 which means it is supported, but only if you target macOS >= 10.12, and we still support back to 10.9. That implies those old versions of macOS don't have the system primitives for shared mutexes.
Nothing currently in mozilla-central is using shared_mutex, so this really isn't much of a Build System bug, but rather something with the code you're importing not supporting older versions of macOS.
|
Reporter | |
Comment 2•5 years ago
|
||
Note the error is
error: 'shared_mutex' is unavailable: introduced in macOS 10.12which means it is supported, but only if you target macOS >= 10.12, and we still support back to 10.9. That implies those old versions of macOS don't have the system primitives for shared mutexes.
Yup, that makes sense. Apologies, for the misleading description - I am somewhat new to the codebase, and I'm still learning some of the ins and outs.
I've opened this bug mostly to track a conversation with Nathan Froyd [:froydnj] on IRC. The summary of this is that Bug 1554268 pulls in a third party dependency that uses c++17, including shared_mutex. However, the build fails (which is expected given your explanation).
@froydnj : Would you know a more appropriate spot in the bugzilla hierarchy to file this issue?
|
|
||
Comment 3•5 years ago
|
||
The options are, afaict:
- don't use shared_mutex.
- drop support for macOS 10.9 to 10.11 included.
- use an implementaiton of shared_mutex that supports older versions of macOS, if that exists or is even possible at all.
|
|
||
Comment 4•5 years ago
|
||
Copy/paste what I wrote on IRC:
actually, digging deeper, it does seem it is about what comes as libc++ in the OS... we /could/ ship a copy, or statically link it, but it seems the header assume the OS library is always used.
the latter can be disabled by defining _LIBCPP_DISABLE_AVAILABILITY
|
Assignee | |
Comment 5•5 years ago
|
||
Can we just use shared_timed_mutex instead? For inexplicable reasons, shared_timed_mutex is available in C++14, and it looks like it can be used identically to shared_mutex; the timed part of it only matters for try_lock_for and friends, which we don't care about.
And seriously, Apple, what are you doing? pthread_rwlock_t is a thing that exists everywhere.
You can file rlbox bugs in Core::XPCOM, I guess?
|
|
Reporter | |
Comment 6•5 years ago
•
|
||
The thing I am worried about is that there may be other C++17 library features used by RLBox missing in MacOS 10.9...
Did a quick pass on the features of libc++ version that ships with MacOs 10.9...
- Looks like the last XCode to ship on this was 6.2 which used Apple LLVM version 6.0 (clang-600.0.57) (based on LLVM 3.5svn). See here for ref
When I checked the libc++ feature version map here, one example of another thing that might break is "std::is_invocable" (earlier called "is_callable"). According to the link, this was shipped with libc++ 3.9 which was first included in XCode 8 as per here, which in turn first showed up in OS X 10.11 per here.
I think at this point, I want to confirm if the course of action is indeed
- Fix all upstream uses of lib c++ 17 features newer than that of libc++ 3.5 (which shipped with MacOS 10.9)
Or possibly - One of the options listed by Mike Hommey [:glandium] above such as including a copy of libc++ in the distribution
If the resolution here should be the former, will move this bug out of the "C++17 migration" task hierarchy into the "rlbox task hierarchy"
|
|
||
Comment 7•5 years ago
|
||
It doesn't matter what the system libc++ supports for STL features that are implemented entirely through templates, and std::is_invocable is very likely one of them.
|
|
Reporter | |
Comment 8•5 years ago
|
||
Gotcha... Will try replacing shared_mutex upstream.
|
|
Reporter | |
Updated•5 years ago
|
|
|
Reporter | |
Comment 9•5 years ago
|
||
Hmm... Also seeing an error for shared_timed_mutex on try
error: 'shared_timed_mutex' is unavailable: introduced in macOS 10.12
|
|
Assignee | |
Comment 10•5 years ago
|
||
(In reply to Shravan Narayan from comment #9)
Hmm... Also seeing an error for shared_timed_mutex on try
error: 'shared_timed_mutex' is unavailable: introduced in macOS 10.12
I cannot understand what libc++ is trying to do here. __shared_mutex_base is built on std:: primitives, not OS primitives, and yet something in the standard library that doesn't depend on the OS is restricted based on the version of the OS? Unless libc++ thinks you're going to compile against the headers, leave the __shared_mutex_base and shared_timed_mutex symbols unresolved (and not link against libc++.a?), and then you're going to fall over when you go to run on some system < 10.12?
Anyway, I think the simplest solution is to see if glandium's suggestion in comment 4 works by applying the following patch:
diff --git a/build/moz.configure/toolchain.configure b/build/moz.configure/toolchain.configure
index 79ebd9d6..6e66fc0 100755
--- a/build/moz.configure/toolchain.configure
+++ b/build/moz.configure/toolchain.configure
@@ -1360,6 +1360,14 @@ set_define('_LIBCPP_ALWAYS_INLINE', libcxx_override_visibility)
set_define('_LIBCPP_ALWAYS_INLINE_EXCEPT_GCC49', libcxx_override_visibility)
+@depends(target):
+def libcxx_disable_availability(target):
+ if target.kernel == 'Darwin':
+ return ''
+
+
+set_define('_LIBCPP_DISABLE_AVAILABILITY', libcxx_disable_availability)
+
@depends(target, check_build_environment)
def visibility_flags(target, env):
if target.os != 'WINNT':
|
|
Assignee | |
Comment 11•5 years ago
|
||
Sorry, that should be:
diff --git a/build/moz.configure/toolchain.configure b/build/moz.configure/toolchain.configure
index 79ebd9d6..6e66fc0 100755
--- a/build/moz.configure/toolchain.configure
+++ b/build/moz.configure/toolchain.configure
@@ -1360,6 +1360,14 @@ set_define('_LIBCPP_ALWAYS_INLINE', libcxx_override_visibility)
set_define('_LIBCPP_ALWAYS_INLINE_EXCEPT_GCC49', libcxx_override_visibility)
+@depends(target)
+def libcxx_disable_availability(target):
+ if target.kernel == 'Darwin':
+ return ''
+
+
+set_define('_LIBCPP_DISABLE_AVAILABILITY', libcxx_disable_availability)
+
@depends(target, check_build_environment)
def visibility_flags(target, env):
if target.os != 'WINNT':
|
|
||
Comment 12•5 years ago
|
||
I cannot understand what libc++ is trying to do here. __shared_mutex_base is built on std:: primitives, not OS primitives, and yet something in the standard library that doesn't depend on the OS is restricted based on the version of the OS?
Here's the problem: libc++ headers assume you're going to be using the system libc++.dylib. Which is our case, actually, so that's fine. The sad part is that for some reason, the implementation for shared_mutex is in the lib part, not in the templates part, so it needs the system libc++.dylib support. But the way clang does it, afaict, this totally ignores the legitimate cases where you statically link libc++, or ship your own libc++.dylib. _LIBCPP_DISABLE_AVAILABILITY is barely a workaround.
|
|
Reporter | |
Comment 13•5 years ago
|
||
Hmm... this seems to failing with some unrelated errors listed below. Try link
[task 2019-08-15T20:35:51.925Z] 20:35:51 INFO - In file included from /builds/worker/workspace/build/src/obj-firefox/gfx/thebes/Unified_mm_gfx_thebes0.mm:2:
[task 2019-08-15T20:35:51.926Z] 20:35:51 INFO - In file included from /builds/worker/workspace/build/src/gfx/thebes/gfxMacPlatformFontList.mm:63:
[task 2019-08-15T20:35:51.926Z] 20:35:51 ERROR - /builds/worker/workspace/build/src/obj-firefox/dist/include/nsCocoaUtils.h:52:15: error: use of undeclared identifier 'typeof'; did you mean 'typeid'?
[task 2019-08-15T20:35:51.926Z] 20:35:51 INFO - mObject = NS_OBJC_TRY_EXPR_ABORT([anObject retain]);
[task 2019-08-15T20:35:51.926Z] 20:35:51 INFO - ^
[task 2019-08-15T20:35:51.926Z] 20:35:51 INFO - /builds/worker/workspace/build/src/obj-firefox/dist/include/nsObjCExceptions.h:172:7: note: expanded from macro 'NS_OBJC_TRY_EXPR_ABORT'
[task 2019-08-15T20:35:51.927Z] 20:35:51 INFO - typeof(_e) _tmp; \
[task 2019-08-15T20:35:51.927Z] 20:35:51 INFO - ^
[task 2019-08-15T20:35:51.927Z] 20:35:51 INFO - In file included from /builds/worker/workspace/build/src/obj-firefox/gfx/thebes/Unified_mm_gfx_thebes0.mm:2:
[task 2019-08-15T20:35:51.927Z] 20:35:51 INFO - In file included from /builds/worker/workspace/build/src/gfx/thebes/gfxMacPlatformFontList.mm:63:
[task 2019-08-15T20:35:51.927Z] 20:35:51 ERROR - /builds/worker/workspace/build/src/obj-firefox/dist/include/nsCocoaUtils.h:52:15: error: use of undeclared identifier '_tmp'
[task 2019-08-15T20:35:51.927Z] 20:35:51 INFO - /builds/worker/workspace/build/src/obj-firefox/dist/include/nsObjCExceptions.h:174:9: note: expanded from macro 'NS_OBJC_TRY_EXPR_ABORT'
[task 2019-08-15T20:35:51.927Z] 20:35:51 INFO - _tmp = (_e); \
[task 2019-08-15T20:35:51.927Z] 20:35:51 INFO - ^
[task 2019-08-15T20:35:51.927Z] 20:35:51 INFO - In file included from /builds/worker/workspace/build/src/obj-firefox/gfx/thebes/Unified_mm_gfx_thebes0.mm:2:
[task 2019-08-15T20:35:51.928Z] 20:35:51 INFO - In file included from /builds/worker/workspace/build/src/gfx/thebes/gfxMacPlatformFontList.mm:63:
[task 2019-08-15T20:35:51.928Z] 20:35:51 ERROR - /builds/worker/workspace/build/src/obj-firefox/dist/include/nsCocoaUtils.h:52:15: error: use of undeclared identifier '_tmp'
[task 2019-08-15T20:35:51.928Z] 20:35:51 INFO - /builds/worker/workspace/build/src/obj-firefox/dist/include/nsObjCExceptions.h:178:7: note: expanded from macro 'NS_OBJC_TRY_EXPR_ABORT'
[task 2019-08-15T20:35:51.928Z] 20:35:51 INFO - _tmp; \
[task 2019-08-15T20:35:51.928Z] 20:35:51 INFO - ^
[task 2019-08-15T20:35:51.928Z] 20:35:51 INFO - In file included from /builds/worker/workspace/build/src/obj-firefox/gfx/thebes/Unified_mm_gfx_thebes0.mm:2:
[task 2019-08-15T20:35:51.928Z] 20:35:51 INFO - In file included from /builds/worker/workspace/build/src/gfx/thebes/gfxMacPlatformFontList.mm:63:
[task 2019-08-15T20:35:51.928Z] 20:35:51 ERROR - /builds/worker/workspace/build/src/obj-firefox/dist/include/nsCocoaUtils.h:52:15: error: assigning to 'id' from incompatible type 'void'
[task 2019-08-15T20:35:51.928Z] 20:35:51 INFO - mObject = NS_OBJC_TRY_EXPR_ABORT([anObject retain]);
[task 2019-08-15T20:35:51.928Z] 20:35:51 INFO - ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[task 2019-08-15T20:35:51.929Z] 20:35:51 INFO - /builds/worker/workspace/build/src/obj-firefox/dist/include/nsObjCExceptions.h:171:5: note: expanded from macro 'NS_OBJC_TRY_EXPR_ABORT'
[task 2019-08-15T20:35:51.929Z] 20:35:51 INFO - ({ \
[task 2019-08-15T20:35:51.929Z] 20:35:51 INFO - ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[task 2019-08-15T20:35:51.929Z] 20:35:51 INFO - 4 errors generated.
|
|
Assignee | |
Comment 14•5 years ago
|
||
Sigh, this is dumb and I probably should have figured this out sooner. typeof is a GNU extension:
https://gcc.gnu.org/onlinedocs/gcc/Typeof.html
Our normal compiler flag is -std=gnu++14 for non-Windows platforms, which is C++14 with GNU extensions, i.e. things like typeof.
The try patch in question uses -std=c++17, which of course turns GNU extensions off:
https://hg.mozilla.org/try/rev/68e3b98df3680e7fa10137e01294336a92bc3ac4#l14.43
I think glandium mentioned at one point that we wanted to be using -std=gnu++17 and of course he was right.
Compiling the file in question with -std=gnu++17 turns the typeof extension back on and poof, the error is gone. Unfortunately, we hit the somewhat obvious link errors:
Undefined symbols for architecture x86_64:
"__ZNSt3__118shared_timed_mutex11lock_sharedEv", referenced from:
__ZN5rlbox18rlbox_noop_sandbox19callback_trampolineILj0EmJmjmEEET0_DpT1_ in Unified_cpp_gfx_thebes0.o
__ZN5rlbox18rlbox_noop_sandbox19callback_trampolineILj1EmJmjmEEET0_DpT1_ in Unified_cpp_gfx_thebes0.o
__ZN5rlbox18rlbox_noop_sandbox19callback_trampolineILj2EmJmjmEEET0_DpT1_ in Unified_cpp_gfx_thebes0.o
__ZN5rlbox18rlbox_noop_sandbox19callback_trampolineILj3EmJmjmEEET0_DpT1_ in Unified_cpp_gfx_thebes0.o
__ZN5rlbox18rlbox_noop_sandbox19callback_trampolineILj4EmJmjmEEET0_DpT1_ in Unified_cpp_gfx_thebes0.o
__ZN5rlbox18rlbox_noop_sandbox19callback_trampolineILj5EmJmjmEEET0_DpT1_ in Unified_cpp_gfx_thebes0.o
__ZN5rlbox18rlbox_noop_sandbox19callback_trampolineILj6EmJmjmEEET0_DpT1_ in Unified_cpp_gfx_thebes0.o
...
"__ZNSt3__118shared_timed_mutex6unlockEv", referenced from:
__ZN12gfxFontEntry9GetGrFaceEv in Unified_cpp_gfx_thebes0.o
__ZN5rlbox13rlbox_sandboxINS_18rlbox_noop_sandboxEE17register_callbackIRS2_NS_14tainted_opaqueIPKvS1_EEJS8_NS5_IjS1_EENS5_IPmS1_EEEEENS_16sandbox_callbackIPFNS_6detail27detail_rlbox_remove_wrapper9unwrapperIT0_E4typeEDpNSF_IT1_E4typeEES1_EEPFSG_T_DpSJ_E in Unified_cpp_gfx_thebes0.o
__ZN5rlbox13rlbox_sandboxINS_18rlbox_noop_sandboxEE17register_callbackIRS2_vJNS_14tainted_opaqueIPKvS1_EES8_EEENS_16sandbox_callbackIPFNS_6detail27detail_rlbox_remove_wrapper9unwrapperIT0_E4typeEDpNSC_IT1_E4typeEES1_EEPFSD_T_DpSG_E in Unified_cpp_gfx_thebes0.o
__ZN5rlbox13rlbox_sandboxINS_18rlbox_noop_sandboxEE15destroy_sandboxEv in Unified_cpp_gfx_thebes0.o
__ZN5rlbox18rlbox_noop_sandbox24impl_unregister_callbackImJmjmEEEvPv in Unified_cpp_gfx_thebes0.o
__ZN5rlbox18rlbox_noop_sandbox24impl_unregister_callbackIvJmmEEEvPv in Unified_cpp_gfx_thebes0.o
__ZN5rlbox13rlbox_sandboxINS_18rlbox_noop_sandboxEE17register_callbackIRS2_NS_14tainted_opaqueIfS1_EEJNS5_IPKvS1_EENS5_ItS1_EEEEENS_16sandbox_callbackIPFNS_6detail27detail_rlbox_remove_wrapper9unwrapperIT0_E4typeEDpNSE_IT1_E4typeEES1_EEPFSF_T_DpSI_E in Unified_cpp_gfx_thebes1.o
...
"__ZNSt3__118shared_timed_mutex4lockEv", referenced from:
__ZN12gfxFontEntry9GetGrFaceEv in Unified_cpp_gfx_thebes0.o
__ZN5rlbox13rlbox_sandboxINS_18rlbox_noop_sandboxEE17register_callbackIRS2_NS_14tainted_opaqueIPKvS1_EEJS8_NS5_IjS1_EENS5_IPmS1_EEEEENS_16sandbox_callbackIPFNS_6detail27detail_rlbox_remove_wrapper9unwrapperIT0_E4typeEDpNSF_IT1_E4typeEES1_EEPFSG_T_DpSJ_E in Unified_cpp_gfx_thebes0.o
__ZN5rlbox13rlbox_sandboxINS_18rlbox_noop_sandboxEE17register_callbackIRS2_vJNS_14tainted_opaqueIPKvS1_EES8_EEENS_16sandbox_callbackIPFNS_6detail27detail_rlbox_remove_wrapper9unwrapperIT0_E4typeEDpNSC_IT1_E4typeEES1_EEPFSD_T_DpSG_E in Unified_cpp_gfx_thebes0.o
__ZN5rlbox13rlbox_sandboxINS_18rlbox_noop_sandboxEE15destroy_sandboxEv in Unified_cpp_gfx_thebes0.o
__ZN5rlbox18rlbox_noop_sandbox24impl_unregister_callbackImJmjmEEEvPv in Unified_cpp_gfx_thebes0.o
__ZN5rlbox18rlbox_noop_sandbox24impl_unregister_callbackIvJmmEEEvPv in Unified_cpp_gfx_thebes0.o
__ZN5rlbox13rlbox_sandboxINS_18rlbox_noop_sandboxEE17register_callbackIRS2_NS_14tainted_opaqueIfS1_EEJNS5_IPKvS1_EENS5_ItS1_EEEEENS_16sandbox_callbackIPFNS_6detail27detail_rlbox_remove_wrapper9unwrapperIT0_E4typeEDpNSE_IT1_E4typeEES1_EEPFSF_T_DpSI_E in Unified_cpp_gfx_thebes1.o
...
"__ZNSt3__118shared_timed_mutexC1Ev", referenced from:
__ZN12gfxFontEntry9GetGrFaceEv in Unified_cpp_gfx_thebes0.o
___cxx_global_var_init.55 in Unified_cpp_gfx_thebes0.o
"__ZNSt3__118shared_timed_mutex13unlock_sharedEv", referenced from:
__ZN5rlbox18rlbox_noop_sandbox19callback_trampolineILj0EmJmjmEEET0_DpT1_ in Unified_cpp_gfx_thebes0.o
__ZN5rlbox18rlbox_noop_sandbox19callback_trampolineILj1EmJmjmEEET0_DpT1_ in Unified_cpp_gfx_thebes0.o
__ZN5rlbox18rlbox_noop_sandbox19callback_trampolineILj2EmJmjmEEET0_DpT1_ in Unified_cpp_gfx_thebes0.o
__ZN5rlbox18rlbox_noop_sandbox19callback_trampolineILj3EmJmjmEEET0_DpT1_ in Unified_cpp_gfx_thebes0.o
__ZN5rlbox18rlbox_noop_sandbox19callback_trampolineILj4EmJmjmEEET0_DpT1_ in Unified_cpp_gfx_thebes0.o
__ZN5rlbox18rlbox_noop_sandbox19callback_trampolineILj5EmJmjmEEET0_DpT1_ in Unified_cpp_gfx_thebes0.o
__ZN5rlbox18rlbox_noop_sandbox19callback_trampolineILj6EmJmjmEEET0_DpT1_ in Unified_cpp_gfx_thebes0.o
...
ld: symbol(s) not found for architecture x86_64
clang-8: error: linker command failed with exit code 1 (use -v to see invocation)
...which is just the linker telling us that it can't find those symbols in the libc++ that comes with the SDK. I guess we get to figure out how to properly link libc++ statically?
|
|
Assignee | |
Comment 15•5 years ago
|
||
We apparently also need to figure out how to build libc++ statically, as our current cross toolchains only build it as a shared library (and, hilariously enough, intermingle the mach-o .dylibs with the ELF .a files in the same directory).
|
|
Reporter | |
Comment 16•5 years ago
|
||
Our normal compiler flag is -std=gnu++14 for non-Windows platforms, which is C++14 with GNU extensions, i.e. things like typeof.
Oh interesting, I wonder if this is also the cause of Bug 1576054
We apparently also need to figure out how to build libc++ statically
Wow, that sounds like it is starting to get complicated... Let me throw a couple of alternatives on upstream options, in case adjusting the build gets too painful
- Modify the upstream library to have a compile flag that makes it use a regular lock. This requires a low amount of work(~1 day), and we can enable this flag only for affected Mac versions. Downside is that, when we enable wasm sandboxing on these Mac versions, the performance will be impacted.
- Modify the upstream library with extension points where the main application can provide primitives for shared lock. Downside is that this is a bit more painful to setup (~2 days) and maintain long term as the code will get more cluttered. We can then use this extension to point to shared lock implementation Firefox has in-tree.
Either of the above can be done, but I will let you weigh in on when you believe the overhead of fixing the Mac builds outweighs the effort of one of the approaches above.
|
|
Assignee | |
Comment 17•5 years ago
|
||
I have a slight preference for just using a regular lock for now (this is a per-sandbox lock, right, not a global for all sandboxes lock?), since we're probably not enabling this on Mac to start with.
Oh actually, we're still executing this code even without a sandbox, aren't we? So maybe we should go with option 2 and just use mozilla::RWLock.
A third option is to provide the necessary libc++ symbols ourselves. We already do this for some libstdc++ symbols, so it wouldn't be much of a stretch. And we could basically copy the code that the libc++ symbols require due to their permissive licensing...
|
|
Reporter | |
Comment 18•5 years ago
|
||
Yes the lock code is executed without the sandbox as well... Option 2 looks reasonable. Will investigate
|
|
Reporter | |
Comment 19•5 years ago
|
||
I have implemented option 2 upstream and tested it, so this no longer blocks RLBox... I think we can probably close this bug?
|
|
Assignee | |
Comment 20•5 years ago
|
||
Closing based on comment 19.
Description
•