New issue
Advanced search Search tips

Issue 749077 link

Starred by 5 users

Issue metadata

Status: Verified
Owner:
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug-Regression



Sign in to add a comment

glibc dependency creeped up to 2.18 in M61, breaking EL7 support

Reported by tv@duh.org, Jul 26 2017

Issue description

UserAgent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/59.0.3071.115 Safari/537.36

Steps to reproduce the problem:
yum install google-chrome-*-61.*

What is the expected behavior?
Installation and running

What went wrong?
---> Package google-chrome-unstable.x86_64 0:61.0.3159.5-1 will be installed
--> Processing Dependency: libc.so.6(GLIBC_2.18)(64bit) for package: google-chrome-unstable-61.0.3159.5-1.x86_64
--> Finished Dependency Resolution
--> Finding unneeded leftover dependencies
Found and removing 0 unneeded dependencies
Error: Package: google-chrome-unstable-61.0.3159.5-1.x86_64 (google-chrome)
           Requires: libc.so.6(GLIBC_2.18)(64bit)

===

Manually installing the bare RPM with --nodeps and looking for the symbols from glibc>2.17 shows that the ABI requirement is in all of:

./chrome: /lib64/libc.so.6: version `GLIBC_2.18' not found (required by ./chrome)
./libEGL.so: /lib64/libc.so.6: version `GLIBC_2.18' not found (required by ./libEGL.so)
./libGLESv2.so: /lib64/libc.so.6: version `GLIBC_2.18' not found (required by ./libGLESv2.so)
./libwidevinecdmadapter.so: /lib64/libc.so.6: version `GLIBC_2.18' not found (required by ./libwidevinecdmadapter.so)

Did this work before? Yes M60

Chrome version: 61.0.3159.5  Channel: n/a
OS Version: EL7
Flash Version: 

 Issue 580892  was a similarly reported issue where glibc-2.17 was promised as the ABI requirement level, to ensure that Chrome would work on some version of RHEL and its derivative distributions (CentOS, Oracle Linux, Scientific Linux, ...).

Looks like something is calling newer glibc functions as of M61, and breaking the previous promise along the way.

(Note that if this was for ostensible bug fixes in glibc, it's important to remember that long-lived distro versions such as EL make a point of backporting fixes to the stable glibc base version.)
 
Labels: Needs-Triage-M61
Cc: pbomm...@chromium.org thomasanderson@chromium.org
cc'ing Tom for further insights on the bug.
Cc: thakis@chromium.org thestig@chromium.org
Owner: thomasanderson@chromium.org
Status: Assigned (was: Unconfirmed)
I think r483644 bumped up the glibc requirement. The question is, how much work is it to get the requirement down to 2.17?

Also, a reminder regarding RHEL support from our last conversation... https://bugs.chromium.org/p/chromium/issues/detail?id=580892#c5
Yes, it was r483644.

It looks like we don't *actually* use any glibc 2.18 features as the symbol that's adding the dependency is called "GLIBC_2.18".  Just a matter of finding out why that is getting added..
I take that back, the symbol we need is __cxa_thread_atexit_impl@GLIBC_2.18.  Also we need one for 2.17: clock_gettime@GLIBC_2.17
Labels: -Needs-Triage-M61 ReleaseBlock-Stable M-61
Hm.. it looks like libc++ defines __cxa_thread_atexit_impl as a weak symbol (in case the user's glibc is too old, it falls back on its own impl), but when linking, it adds a 2.18 requirement anyway.

Comment 9 by tv@duh.org, Jul 28 2017

The linker is probably choosing the GLIBC_2.18 variant specifically because it's the non-weak symbol, and the installed version of glibc at link time contains it, so it takes precedence.

Offhand all the fixes I can think of are quite hacky (like de-weaking the symbol in libsupc++ via objcopy), but I'm sure there is a usable solution here and I'm too brain fried for the day to think of it.
Cc: p...@chromium.org
Status: Started (was: Assigned)
Discussed this yesterday with pcc@.  I think the only option is to use a different/older libc.so that doesn't have __cxa_thread_atexit_impl (pcc@ please correct me if any of this is wrong).

However, bundling an older libc in all of our sysroots would be a challenge, and I think it's better to directly modify libc.so so that it doesn't contain the symbol.  But it seems both objcopy and strip are of no use when trying to remove a dynamic symbol.  The only solution I found is "sed -i 's/__cxa_thread_atexit_impl/__dead_beef_dead_beef___/g' libc-2.19.so", and I think this is our best bet.

Comment 11 by p...@chromium.org, Jul 28 2017

It later occurred to me that another option would be to send a patch upstream to libcxxabi to make it use dlsym to look up the __cxa_thread_atexit_impl symbol.

Comment 12 by tv@duh.org, Jul 28 2017

Hm. Did some UTSL here. I don't see __cxa_thread_atexit_impl as a weak symbol in libsupc++ where it's used.

In gcc, __cxa_thread_atexit_impl() is provided by glibc to do thread-local storage tracking alongside dlopen(),[1] and if it's available at the time of building libstdc++ itself, it will *always* be used at runtime through __cxxabiv1::__cxa_thread_atexit() rather than fallback logic. It's compile-time differentiated.[2]

So there may be no safe way to elide this just with a linking hack. However, it's certainly possible to compile that source file, with _GLIBCXX_HAVE___CXA_THREAD_ATEXIT_IMPL intentionally undefined, to provide the fallback code unconditionally.

Where this might get sticky is with the GCC Runtime Library Exception[3] and the Widevine shared object, which isn't OSS. By my reading of the Exception text, this usage is still covered by the Exception as long as the single source file is shipped/shippable per GPL, as it's just gcc still using a piece of gcc's runtime. But IANAL and can't guarantee that.

sigh.

===

[1] https://sourceware.org/glibc/wiki/Destructor%20support%20for%20thread_local%20variables

[2] assuming an Ubuntu Trusty based build environment with its default gcc 4.8.x based compiler, https://github.com/gcc-mirror/gcc/blob/gcc-4_8_4-release/libstdc%2B%2B-v3/libsupc%2B%2B/atexit_thread.cc

[3] https://www.gnu.org/licenses/gcc-exception-3.1.en.html
c#11: libc++abi bug opened here: https://bugs.llvm.org/show_bug.cgi?id=33983
Not sure if it's going to go anywhere though

c#12: We're already compiling with _GLIBCXX_HAVE___CXA_THREAD_ATEXIT_IMPL undefined.

Comment 14 by tv@duh.org, Jul 28 2017

Ah. LLVM libc++abi, not GCC libsupc++. My misunderstanding, sorry about that.

However, looking at the source of libc++abi's version,[1] perhaps the same effect can be obtained by using llvm-ld's equivalent (does it have one?) of GNU ld's

  --undefined __cxa_thread_atexit_impl

which should force it to remain undefined and make the weak symbol reference come up NULL at runtime.

===

[1] https://llvm.org/viewvc/llvm-project/libcxxabi/trunk/src/cxa_thread_atexit.cpp?view=markup
    at "if (__cxa_thread_atexit_impl) {"

Comment 15 by tv@duh.org, Jul 28 2017

Never mind, ignore me. binutils always surprises me when it really shouldn't:

$ cat <<EOF >x.c
#include <stdio.h>
__attribute__((__weak__))
__cxa_thread_atexit_impl(void(*)(void*), void*, void*);
int main(void) {
    printf("%p\n", __cxa_thread_atexit_impl);
    return 0;
}
EOF

$ cc -Wl,-u,__cxa_thread_atexit_impl x.c

$ nm a.out |grep cxa
                 U __cxa_thread_atexit_impl@@GLIBC_2.18

(repeat 'sigh.' here.)
Project Member

Comment 16 by bugdroid1@chromium.org, Jul 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c2e32f4639a6c56eef7fc4f65160ac2896eca0f1

commit c2e32f4639a6c56eef7fc4f65160ac2896eca0f1
Author: Tom Anderson <thomasanderson@chromium.org>
Date: Fri Jul 28 23:34:24 2017

Relax glibc requirement to 2.17

This CL adds a hack that removes the symbol __cxa_thread_atexit_impl
from libc.so in the sysroots.  In addition, it also rolls the sysroots
and updates the expected deps to reflect the symbol no longer being
required.

BUG= 749077 
R=thestig@chromium.org

Change-Id: I7501d45b8ecff66e97287002bea9ee1e351348c5
Reviewed-on: https://chromium-review.googlesource.com/592279
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#490541}
[modify] https://crrev.com/c2e32f4639a6c56eef7fc4f65160ac2896eca0f1/build/linux/sysroot_scripts/packagelist.jessie.amd64
[modify] https://crrev.com/c2e32f4639a6c56eef7fc4f65160ac2896eca0f1/build/linux/sysroot_scripts/packagelist.jessie.arm
[modify] https://crrev.com/c2e32f4639a6c56eef7fc4f65160ac2896eca0f1/build/linux/sysroot_scripts/packagelist.jessie.arm64
[modify] https://crrev.com/c2e32f4639a6c56eef7fc4f65160ac2896eca0f1/build/linux/sysroot_scripts/packagelist.jessie.i386
[modify] https://crrev.com/c2e32f4639a6c56eef7fc4f65160ac2896eca0f1/build/linux/sysroot_scripts/packagelist.jessie.mipsel
[modify] https://crrev.com/c2e32f4639a6c56eef7fc4f65160ac2896eca0f1/build/linux/sysroot_scripts/sysroot-creator.sh
[modify] https://crrev.com/c2e32f4639a6c56eef7fc4f65160ac2896eca0f1/build/linux/sysroot_scripts/sysroots.json
[modify] https://crrev.com/c2e32f4639a6c56eef7fc4f65160ac2896eca0f1/chrome/installer/linux/debian/expected_deps_x64_jessie
[modify] https://crrev.com/c2e32f4639a6c56eef7fc4f65160ac2896eca0f1/chrome/installer/linux/rpm/expected_deps_x86_64

Labels: Merge-Request-61
Project Member

Comment 18 by sheriffbot@chromium.org, Jul 29 2017

Labels: -Merge-Request-61 Merge-Review-61 Hotlist-Merge-Review
This bug requires manual review: DEPS changes referenced in bugdroid comments.
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), ketakid @(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Before we approve merge to M61, could you please confirm cl listed at #16 is verified on ToT, having enough automation tests coverage and will be a safe merge to M61?
c#19: Yes, there were 2 official builds yesterday that were green that verified the deps change.
Labels: -Merge-Review-61 Merge-Approved-61
Approving merge to M61 branch 3163 based on comment #20. Please merge by 4:00 PM PT on Monday (07/31) so we can take it in for next week last Dev release. Thank you.
 thomasanderson@, please take a look at bug 750537 before merging change to M61. Bug 750537 probably not related but pls double check. Thank you.
The last CL broke gcc compilation in V8:
https://chromium-review.googlesource.com/c/592753/

Could you have a look how to resolve this?
Cc: pnangunoori@chromium.org
Labels: Needs-Feedback
Tried to install reported Chrome #61.0.3159.5 and latest #62.0.3172.0 and was able to install successfully using the command ‘sudo yum install google-chrome-*-61.*’ successfully on Fedora 24; ‘Sudo apt-get install google-chrome-*-61.*’ on Ubuntu 14.04. Attached are the screenshots of the installation for both reported and latest Chrome versions.

@tv -- Could you please verify if the issue is still replicated when the updated version is available. 

Please let us know if any other information is required or if we have missed anything.

Thanks in advance.
749077-61.0.3159.5.png
198 KB View Download
749077-62.0.3172.0.png
186 KB View Download

Comment 25 by tv@duh.org, Jul 31 2017

C23: I suspected this would happen. gcc has no link-time fallback built-in to lib{std,sup}c++ like LLVM does, which is the reason for all my babbling about these libraries above.

To build with gcc, it will be necessary to use libc as-is rather than twiddled to hide this symbol -- or else gcc itself will have to be built with support for this mechanism (_GLIBCXX_HAVE___CXA_THREAD_ATEXIT_IMPL) disabled, to enable its fallback logic.

Comment 26 by tv@duh.org, Jul 31 2017

C23: ...so an exception for this logic may need to be enabled, such that it only kicks in for LLVM. Since the issue is production Chrome (not external Chromium) builds, that would be a reasonable exception -- gcc Chromium builds are likely by distro maintainers who are building against that distro's glibc revision.

C24: Under Fedora, you'd need to be on F19 to see the problem, since it has glibc-2.17. (Or CentOS 7, which you could spin up under e.g. Docker).
c#23 On it
Labels: -Merge-Approved-61 Merge-Review-61
As per The last CL broke gcc compilation in V8:
https://chromium-review.googlesource.com/c/592753/. So removing "Merge-Approved-61" label and applying back "Merge-Review-61" label.

 thomasanderson@, please update the bug this looks good to merge. Thank you.



I think there was actually a way simpler fix that I overlooked.  The file that references __cxa_thread_atexit_impl (cxa_thread_atexit.cpp) only provides one symbol, __cxa_thread_atexit, which is not referenced anywhere in chrome.

I tested omitting compilation of that file and a release build just completed successfully.  I think this is actually the way to go..

Comment 30 by tv@duh.org, Jul 31 2017

Right, __cxa_thread_atexit is only invoked if you're using the C++11 thread_local storage qualifier, which is still pretty rare. It's intended to allow proper cleanup of thread_local scoped objects in loaded .so's.

(I was surprised at this, because I was pretty sure V8 and the rest didn't use the thread_local C++11 scope qualifier. Now it's making a little more sense, in that libc++ is being linked statically to the binaries to avoid a dependency on the target system, and this compilation unit crept in along the way.)

Comment 31 by p...@chromium.org, Jul 31 2017

I believe that the __cxa_thread_atexit function is not normally called explicitly. Instead, the compiler will insert a call to that function if it sees a thread-local global variable which requires the compiler to emit a destructor call.

Can we reasonably expect nothing in chrome (including its third-party dependencies) to ever declare such a variable? The style guide recommends against globals with initializers, but I guess that wouldn't necessarily apply to third-party code.

Comment 32 by tv@duh.org, Jul 31 2017

I have yet to encounter anything using C++11 thread_local on a global variable in the wild.

Certainly it will eventually creep into code here or there, but thread-local storage variables are themselves somewhat rare beasts in C-language-family code. The fact that auto-storage (stack) temporaries are most commonly used to keep track of a thread's state has a lot to do with this.

And if code used by Chrome at some point needs this functionality, it will be noticed pretty quickly when the build fails. :)
Project Member

Comment 33 by bugdroid1@chromium.org, Jul 31 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/buildtools/+/d511e4d53d6fc8037badbdf4b225c137e94b52fb

commit d511e4d53d6fc8037badbdf4b225c137e94b52fb
Author: Tom Anderson <thomasanderson@chromium.org>
Date: Mon Jul 31 22:04:57 2017

Omit compilation of cxa_thread_atexit.cpp on Linux

That file pulls in a symbol that's only present on glibc 2.18.  We
don't want to increase the glibc version on Linux, so compile without
it.  It only provided one symbol that was unused in chrome, anyway.

BUG= 749077 
R=thakis@chromium.org

Change-Id: I977509eefbff639e3b428af634355591f2058332

[modify] https://crrev.com/d511e4d53d6fc8037badbdf4b225c137e94b52fb/third_party/libc++abi/BUILD.gn

Project Member

Comment 34 by bugdroid1@chromium.org, Aug 1 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d12900b02e627d4d6af55c93b731e8ac6d5ee906

commit d12900b02e627d4d6af55c93b731e8ac6d5ee906
Author: Tom Anderson <thomasanderson@chromium.org>
Date: Tue Aug 01 07:01:53 2017

Roll buildtools to d511e4

This roll includes the following 6 revisions:
https://chromium.googlesource.com/chromium/buildtools/+log/5ad14542a6a74dd914f067b948c5d3e8d170396b..d511e4d53d6fc8037badbdf4b225c137e94b52fb

BUG= 749077 
TBR=thakis@chromium.org

Change-Id: Ib3ceda961fa83e93fed5877d02b3b4b39bcfd9c1
Reviewed-on: https://chromium-review.googlesource.com/594910
Reviewed-by: Thomas Anderson <thomasanderson@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#490894}
[modify] https://crrev.com/d12900b02e627d4d6af55c93b731e8ac6d5ee906/DEPS

Project Member

Comment 35 by bugdroid1@chromium.org, Aug 1 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e8df45bfd5386216b9b6ff178b26461902c7ae3a

commit e8df45bfd5386216b9b6ff178b26461902c7ae3a
Author: Tom Anderson <thomasanderson@chromium.org>
Date: Tue Aug 01 23:17:18 2017

Remove __cxa_thread_atexit_impl sysroot hack

This CL reverts the changes in sysroot-creator.sh and
rolls the sysroots forward to reflect the change.

BUG= 749077 
R=thestig@chromium.org

Change-Id: Ia9ba4427666dbf9268138f7ec7a2f77df70a172f
Reviewed-on: https://chromium-review.googlesource.com/596419
Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491148}
[modify] https://crrev.com/e8df45bfd5386216b9b6ff178b26461902c7ae3a/build/linux/sysroot_scripts/sysroot-creator.sh
[modify] https://crrev.com/e8df45bfd5386216b9b6ff178b26461902c7ae3a/build/linux/sysroot_scripts/sysroots.json

machenbach@ can you kick off another v8 roll that includes everything up to the cl in c#35?
Thanks, auto-roller made https://chromium-review.googlesource.com/c/597389 - I also included switching off cxx14 on our gcc bots as the gcc version seems too old. Made use of this CL https://chromium-review.googlesource.com/c/596870 for adding a switch to the respective bots.
I want to commend the Chrome team on fixing this issue so quickly and with a minimum amount of change.

The Red Hat glibc team have been watching this closely since the libc++ change entered the build and added the GLIBC_2.18 requirement.

The fix is exactly what I would have expected. The __cxa_thread_atexit_impl is not needed to the C++ thread_local destructor support (both gcc and clang have fallbacks) and should be removed from the libc.so.6 in your sysroot (either older glibc, or hiding the symbol as you did).

Thank you again, this allows us to continue deploying Chrome in EL7 (and RHEL 7).

If you ever had any questions for upstream glibc, please do not hesitate to reach out to me or others in the community.

Comment 39 by tv@duh.org, Aug 2 2017

The difference is that gcc has a compile-time fallback (builds with gcc will use whatever libstdc++ was built to use), but LLVM libc++abi has a link-time fallback.

The final fix above was to omit the thread_local support fragment from LLVM libc++ since it isn't in active use anyway.
Cc: gov...@chromium.org
govind@: The issue in c#28 is resolved
Thank you  thomasanderson@. Could you pls confirm now it will be fully safe to merge?
I think so.  The only thing we need to merge now is a buildtools roll
Labels: -Merge-Review-61 Merge-Approved-61
Approving merge to M61 branch 3163 based on comment #41 and #42.
Project Member

Comment 44 by bugdroid1@chromium.org, Aug 2 2017

Labels: -merge-approved-61 merge-merged-3163
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e158b0ebe8ba891c971dd2f3fbddfccd696af119

commit e158b0ebe8ba891c971dd2f3fbddfccd696af119
Author: Tom Anderson <thomasanderson@chromium.org>
Date: Wed Aug 02 18:54:23 2017

Relax glibc dependency to 2.17 on M61

This CL is a merge of "Roll buildtools to d511e4" [1], and a selective
merge of "Relax glibc requirement to 2.17" [2] (only the
expected_deps* files).

[1] https://chromium.googlesource.com/chromium/src.git/+/d12900b02e627d4d6af55c93b731e8ac6d5ee906
[2] https://chromium.googlesource.com/chromium/src.git/+/c2e32f4639a6c56eef7fc4f65160ac2896eca0f1

BUG= 749077 
TBR=thestig@chromium.org
NOPRESUBMIT=true
NOTRY=true
NOTREECHECKS=true

Change-Id: I8d2aa785b1db4011815c91c5e7d34d9ea89157b9
Reviewed-on: https://chromium-review.googlesource.com/598631
Reviewed-by: Thomas Anderson <thomasanderson@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#243}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/e158b0ebe8ba891c971dd2f3fbddfccd696af119/DEPS
[modify] https://crrev.com/e158b0ebe8ba891c971dd2f3fbddfccd696af119/chrome/installer/linux/debian/expected_deps_x64_jessie
[modify] https://crrev.com/e158b0ebe8ba891c971dd2f3fbddfccd696af119/chrome/installer/linux/rpm/expected_deps_x86_64

Project Member

Comment 45 by bugdroid1@chromium.org, Aug 2 2017

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/chrome/tools/buildspec/+/ccc9283ace8209ebc6d4787d01f21ebc86fbf56b

commit ccc9283ace8209ebc6d4787d01f21ebc86fbf56b
Author: Tom Anderson <thomasanderson@google.com>
Date: Wed Aug 02 19:28:10 2017

Status: Fixed (was: Started)
Still having this issue, yet status was set to fixed. Any update?

Comment 48 by tv@duh.org, Aug 7 2017

"Fixed" means a bug's fix was committed to source code control. It doesn't necessarily mean the fixed code has been built and released to the public yet (that is still pending).
Ah I see, thanks for the info and thank you to the devs for fixing so quickly!
Do you already know when this fix will be released?
Labels: -Needs-Feedback
For M62: 62.0.3174.0 and newer.
For M61: 61.0.3163.29 and newer.

As of this writing Chrome Dev Channel is 62.0.3178.0 and Chrome Beta Channel is 61.0.3163.31, so both channels should be fixed.

Comment 52 by tv@duh.org, Aug 9 2017

Installed and verified both channels to work as expected on EL7-based distros (tried basic browsing and Netflix playback):

google-chrome-beta-61.0.3163.31-1
google-chrome-unstable-62.0.3178.0-1
Status: Verified (was: Fixed)
Thanks for reporting the bug and verifying the fix.

Sign in to add a comment