Closed Bug 1389422 Opened 3 years ago Closed 3 years ago

[gcc6] check_stdcxx | We do not want these libstdc++ symbol versions to be used: 2 symbols@GLIBCXX_3.4.22

Categories

(Firefox Build System :: General, enhancement)

enhancement
Not set

Tracking

(firefox57 fixed)

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: glandium, Assigned: glandium)

References

Details

Attachments

(1 file)

INFO -  TEST-UNEXPECTED-FAIL | check_stdcxx | We do not want these libstdc++ symbol versions to be used:
INFO -    _ZNSt6thread15_M_start_threadESt10unique_ptrINS_6_StateESt14default_deleteIS1_EEPFvvE@GLIBCXX_3.4.22
INFO -    _ZNSt6thread6_StateD2Ev@GLIBCXX_3.4.22
INFO -  /home/worker/workspace/build/src/config/rules.mk:717: recipe for target 'libxul.so' failed

They both come from the use of std::thread in https://dxr.mozilla.org/mozilla-central/rev/5322c03f4c8587fe526172d3f87160031faa6d75/media/libcubeb/src/cubeb_log.cpp#76
Blocks: gcc6
Comment on attachment 8896178 [details]
Bug 1389422 - Avoid @GLIBCXX_3.4.22 symbols from the use of std::thread when building with GCC 6.

https://reviewboard.mozilla.org/r/167448/#review172754

This is fine in principle, but some questions below.  I am particularly curious about how `thread::_M_start_thread(shared_ptr<_Impl_base>, void(*)())` works.

::: build/unix/stdc++compat/stdc++compat.cpp:121
(Diff revision 1)
> +   * There is an intermediate ABI at version 3.4.21, with
> +   * thread::_M_start_thread(shared_ptr<_Impl_base>, void(*)()).

This is convenient!

I am missing something here, though; this function is at 3.4.21, which the documentation above states that is GCC 5.0...but this symbol *is* present in my GCC 4.9 `libstdc++.so`.  How does that work?

And then how do we get that 3.4.21 symbol in our binaries in the first place, since we're checking for compat with 3.4.16?  I can't see that we're linking stdc++ statically...

::: build/unix/stdc++compat/stdc++compat.cpp:126
(Diff revision 1)
> +   * There is an intermediate ABI at version 3.4.21, with
> +   * thread::_M_start_thread(shared_ptr<_Impl_base>, void(*)()).
> +   * The void(*)() parameter is only there to keep a reference to pthread_create
> +   * on the caller side, and is unused in the implementation
> +   * We're creating an entry point for the new ABI, and make that call the old ABI,
> +   * so we need to a _Impl_base-derived class wrapping that _State. */

Nit: "...we need to have a `_Impl_base`-derived" or "...we need to create a `_Impl_base`-derived" or something similar?

::: build/unix/stdc++compat/stdc++compat.cpp:134
(Diff revision 1)
> +
> +    StateWrapper(unique_ptr<thread::_State> aState)
> +    : mState(std::move(aState))
> +    { }
> +
> +    void _M_run()

Maybe tag this `override`, to be clear that we're providing a virtual function here?

::: build/unix/stdc++compat/stdc++compat.cpp:144
(Diff revision 1)
> +
> +  __attribute__((weak))
> +  void thread::_M_start_thread(unique_ptr<_State> aState, void (*)())
> +  {
> +    auto impl = std::make_shared<StateWrapper>(std::move(aState));
> +    _M_start_thread(impl);

`_M_start_thread(std::move(impl))` for micro-efficiency?
Attachment #8896178 - Flags: review?(nfroyd) → review+
Comment on attachment 8896178 [details]
Bug 1389422 - Avoid @GLIBCXX_3.4.22 symbols from the use of std::thread when building with GCC 6.

https://reviewboard.mozilla.org/r/167448/#review172754

> This is convenient!
> 
> I am missing something here, though; this function is at 3.4.21, which the documentation above states that is GCC 5.0...but this symbol *is* present in my GCC 4.9 `libstdc++.so`.  How does that work?
> 
> And then how do we get that 3.4.21 symbol in our binaries in the first place, since we're checking for compat with 3.4.16?  I can't see that we're linking stdc++ statically...

Oh, I guess we're currently saying our minimum supported GCC version is 4.9?  We could delete some of the code in this file, then?
(In reply to Nathan Froyd [:froydnj] from comment #2)
> ::: build/unix/stdc++compat/stdc++compat.cpp:121
> (Diff revision 1)
> > +   * There is an intermediate ABI at version 3.4.21, with
> > +   * thread::_M_start_thread(shared_ptr<_Impl_base>, void(*)()).
> 
> This is convenient!
> 
> I am missing something here, though; this function is at 3.4.21, which the
> documentation above states that is GCC 5.0...but this symbol *is* present in
> my GCC 4.9 `libstdc++.so`.  How does that work?

The symbol in gcc 4.9 libstdc++.so is for thread::_M_start_thread(std::shared_ptr<std::thread::_Impl_base>) (without the extra void (*)() argument.

> And then how do we get that 3.4.21 symbol in our binaries in the first
> place, since we're checking for compat with 3.4.16?  I can't see that we're
> linking stdc++ statically...

We get a 3.4.2*2* symbol *reference* in our binaries by means of using GCC 6.
 
(In reply to Nathan Froyd [:froydnj] from comment #3)
> Oh, I guess we're currently saying our minimum supported GCC version is 4.9?
> We could delete some of the code in this file, then?

Our minimum supported libstdc++ version is 4.6.1. That is, we're maintaining binary compatibility with a version that is older than the minimum version of GCC we support. Thus we need hacks for versions above 3.4.16.
Ah, I see, I was missing that there were two overloads with shared_ptr<> as their first argument.  OK, this makes sense.  So we have to implement the 3.4.21 symbol, and we'll use the non-void (*)() overload (which is 3.4.11) to do that.
We have to implement the 3.4.22 symbol. But I'm also going to add the 3.4.21 one too, there's no reason not to. It might help make things clearer too.
Pushed by mh@glandium.org:
https://hg.mozilla.org/integration/autoland/rev/f92327a6d1f7
Avoid @GLIBCXX_3.4.22 symbols from the use of std::thread when building with GCC 6. r=froydnj
https://hg.mozilla.org/mozilla-central/rev/f92327a6d1f7
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Product: Core → Firefox Build System
Depends on: 1464084
You need to log in before you can comment on or make changes to this bug.