New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 700120 link

Starred by 11 users

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug

Blocking:
issue 79722
issue 703706



Sign in to add a comment

Component builds do not run on Ubuntu 16.04 or 16.10

Project Member Reported by thomasanderson@chromium.org, Mar 9 2017

Issue description

Chrome Version: ToT
OS: Ubuntu 16.04 or 16.10

What steps will reproduce the problem?
(1) Add is_component_build = true to args.gn
(2) Build
(3) Run chrome

What is the expected result?
Chrome runs without error

What happens instead?
Segmentation fault

 
Ok, it turns out this issue is due to multiple loading of libprotobuf-lite.so

Chrome has a direct dependency on a custom libprotobuf_lite in third_party, but
Xenial and Yakkety add a dependency on the system libprotobuf-lite via Mir (which
gets loaded from Gtk).

Our third_party protobuf is not compatible with the upstream protobuf.  Combine
this with the fact that Xenial uses version 2 while Yakkety uses version 3, and
it's basically impossible to make our third_party protobuf cooperate with the
system one.  I think the solution here is to always statically link protobuf on
Linux.
Project Member

Comment 2 by bugdroid1@chromium.org, Mar 10 2017

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

commit 4af1264005620a483a209f4344c3782f62eee69a
Author: thomasanderson <thomasanderson@google.com>
Date: Fri Mar 10 01:01:59 2017

Statically link libprotobuf_lite on Linux

Chrome has a direct dependency on a custom libprotobuf_lite in
third_party, but Xenial and Yakkety add a dependency on the system
libprotobuf-lite via Mir (which gets loaded from Gtk).

Our third_party protobuf is not compatible with the upstream protobuf.
Combine this with the fact that Xenial uses version 2 while Yakkety
uses version 3, and it's basically impossible to make our third_party
protobuf cooperate with the system one.  The solution is to always
statically link protobuf on Linux.

BUG= 79722 , 700120 
R=pkasting@chromium.org
TBR=bengr@chromium.org
CC=dpranke@chromium.org,rsimha@chromium.org

Review-Url: https://codereview.chromium.org/2746493002
Cr-Commit-Position: refs/heads/master@{#455936}

[modify] https://crrev.com/4af1264005620a483a209f4344c3782f62eee69a/net/BUILD.gn
[modify] https://crrev.com/4af1264005620a483a209f4344c3782f62eee69a/third_party/protobuf/BUILD.gn

Status: Fixed (was: Started)
Note this may trigger a different kind of weird behavior in components builds. If components A and B both link to protobuf_lite, static library builds will have one copy of protobuf_lite and shared library builds will have a private copy in each of A and B. This mostly matters if protobuf_lite has any global state.

Hopefully fine and I don't have a good alternative, but I figured I'd mention it to save someone some debugging if it comes up.

(We ran into this early on in the BoringSSL transition. We thought we could just use static libraries for all modes. Ultimately we needed to make a component. Hilariously, we now risk this problem because BoringSSL has symbol conflicts with OpenSSL. Everything is wonderful. :-D)
Project Member

Comment 5 by bugdroid1@chromium.org, Mar 10 2017

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

commit 9e80d868029c25fdb129f85d0f8f75e8f6c013e1
Author: kinuko <kinuko@chromium.org>
Date: Fri Mar 10 07:15:41 2017

Revert of Statically link libprotobuf_lite on Linux (patchset #2 id:20001 of https://codereview.chromium.org/2746493002/ )

Reason for revert:
Not sure why, but it looks this starts to cause CloudPolicyManagerTest failures.

(Confirmed that locally reverting this CL fixed them)

https://build.chromium.org/p/chromium.linux/builders/Linux%20Tests%20%28dbg%29%281%29%2832%29/builds/39213

https://build.chromium.org/p/chromium.linux/builders/Linux%20Tests%20%28dbg%29%281%29%2832%29/builds/39214

CloudPolicyManagerTest.Register
CloudPolicyManagerTest.RegisterWithRetry
failed to process gtest output JSON
CloudPolicyProtoTest.VerifyProtobufEquivalence

Original issue's description:
> Statically link libprotobuf_lite on Linux
>
> Chrome has a direct dependency on a custom libprotobuf_lite in
> third_party, but Xenial and Yakkety add a dependency on the system
> libprotobuf-lite via Mir (which gets loaded from Gtk).
>
> Our third_party protobuf is not compatible with the upstream protobuf.
> Combine this with the fact that Xenial uses version 2 while Yakkety
> uses version 3, and it's basically impossible to make our third_party
> protobuf cooperate with the system one.  The solution is to always
> statically link protobuf on Linux.
>
> BUG= 79722 , 700120 
> R=pkasting@chromium.org
> TBR=bengr@chromium.org
> CC=dpranke@chromium.org,rsimha@chromium.org
>
> Review-Url: https://codereview.chromium.org/2746493002
> Cr-Commit-Position: refs/heads/master@{#455936}
> Committed: https://chromium.googlesource.com/chromium/src/+/4af1264005620a483a209f4344c3782f62eee69a

TBR=pkasting@chromium.org,bengr@chromium.org,thomasanderson@google.com
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 79722 , 700120 

Review-Url: https://codereview.chromium.org/2739203004
Cr-Commit-Position: refs/heads/master@{#456013}

[modify] https://crrev.com/9e80d868029c25fdb129f85d0f8f75e8f6c013e1/net/BUILD.gn
[modify] https://crrev.com/9e80d868029c25fdb129f85d0f8f75e8f6c013e1/third_party/protobuf/BUILD.gn

Oh. Well, then. Comment #4 may be the cause.

Perhaps wrap all of protobuf_lite in an inline namespace? Being C++, there are some options available here not available to BoringSSL.
Status: Assigned (was: Fixed)
Cc: geoffl...@chromium.org kbr@chromium.org zmo@chromium.org ynovikov@chromium.org thomasanderson@chromium.org
 Issue 700104  has been merged into this issue.

Comment 9 by hta@chromium.org, Mar 13 2017

A temporary workaround is to build with a non-component build. Disadvantages of this are well known.

component chrom on Ubuntu 16.04 segfault with following log
> ./out/Release/chrome
[libprotobuf FATAL ../../third_party/protobuf/src/google/protobuf/stubs/common.cc:78] This program was compiled against version 2.6.1 of the Protocol Buffer runtime library, which is not compatible with the installed version (3.0.0).  Contact the program author for an update.  If you compiled the program yourself, make sure that your headers are from the same version of Protocol Buffers as your link-time library.  (Version verification failed in "/build/mir-pkdHET/mir-0.21.0+16.04.20160330/obj-x86_64-linux-gnu/src/protobuf/mir_protobuf.pb.cc".)
Aborted (core dumped)

#2 resolved this issue but reverted. I'm looking forwared to re-land (asap)
Can we revert "Switch to the Gtk3 theme" if there is a problem solving this issue?
I'd like to get https://build.chromium.org/p/chromium.gpu.fyi/builders/Linux%20Debug%20%28Intel%20HD%20530%29 running tests again.
#11 Yes, I think that would be for best.  I'll proceed with the revert now and reland the GTK3 switch after I land this: https://codereview.chromium.org/2756543002/
Blocking: 79722
Project Member

Comment 14 by bugdroid1@chromium.org, Mar 16 2017

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

commit fd5f9e0d7330c6cd8bbb2301acd77305aa63981c
Author: thomasanderson <thomasanderson@chromium.org>
Date: Thu Mar 16 19:23:34 2017

Revert of Linux UI: Switch to the Gtk3 theme (patchset #3 id:180001 of https://codereview.chromium.org/2670623002/ )

Reason for revert:
Reverting to fix component builds on systems with Mir.
Will reland (probably later today) once https://codereview.chromium.org/2756543002/ lands

Original issue's description:
> Linux UI: Switch to the Gtk3 theme
>
> Sets "use_gtk3 = true" by default.
>
> BUG= 79722 
> TBR=dpranke@chromium.org
> CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_msan_rel_ng;master.tryserver.chromium.linux:linux_chromium_dbg_32_ng
>
> > > Review-Url: https://codereview.chromium.org/2670623002
> > > Cr-Commit-Position: refs/heads/master@{#452118}
> > > Committed: https://chromium.googlesource.com/chromium/src/+/872a494bba52e597388aec738d9d681183a3d47b
>
> > Review-Url: https://codereview.chromium.org/2670623002
> > Cr-Commit-Position: refs/heads/master@{#454491}
> > Committed: https://chromium.googlesource.com/chromium/src/+/89be63a39b5014096115d3ec60d13fde39e73283
>
> Review-Url: https://codereview.chromium.org/2670623002
> Cr-Commit-Position: refs/heads/master@{#455596}
> Committed: https://chromium.googlesource.com/chromium/src/+/072522801041708fba94498a1844cb89f24d4590

TBR=erg@chromium.org,mmoss@chromium.org,thomasanderson@google.com
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 79722 , 700120 

Review-Url: https://codereview.chromium.org/2754913002
Cr-Commit-Position: refs/heads/master@{#457516}

[modify] https://crrev.com/fd5f9e0d7330c6cd8bbb2301acd77305aa63981c/build/config/linux/gtk/gtk.gni
[modify] https://crrev.com/fd5f9e0d7330c6cd8bbb2301acd77305aa63981c/chrome/installer/linux/debian/expected_deps_x64_jessie
[modify] https://crrev.com/fd5f9e0d7330c6cd8bbb2301acd77305aa63981c/chrome/installer/linux/debian/expected_deps_x64_wheezy
[modify] https://crrev.com/fd5f9e0d7330c6cd8bbb2301acd77305aa63981c/chrome/installer/linux/rpm/expected_deps_x86_64

Cc: thestig@chromium.org xyzzyz@chromium.org
 Issue 702629  has been merged into this issue.
Project Member

Comment 16 by bugdroid1@chromium.org, Mar 20 2017

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

commit 77bec4dfd9e172a99543b9bfe9daf2580721bfdd
Author: thomasanderson <thomasanderson@google.com>
Date: Mon Mar 20 07:09:25 2017

Statically link libprotobuf_lite on Linux component builds

Chrome has a direct dependency on a custom libprotobuf_lite in
third_party, but Xenial and Yakkety add a dependency on the system
libprotobuf-lite via Mir (which gets loaded from Gtk).

Our third_party protobuf is not compatible with the upstream protobuf.
Combine this with the fact that Xenial uses version 2 while Yakkety
uses version 3, and it's basically impossible to make our third_party
protobuf cooperate with the system one.  The solution is to always
statically link protobuf on Linux.

This alone, however, is not enough to fix the issue on component
builds.  If components A and B both have private copies of
libprotobuf_lite, they have their own sets of globals.  A problematic
global is "std::string* google::protobuf::internal::empty_string_".
Protobuf does pointer comparison against this string to determine if
strings are empty.  This is problematic when data is passed from
component A to B, each of which have an empty_string_ at a different
address.

This CL also extracts these types of globals into their own shared
library so they can be shared among Chromium components. It also
adds a 'cr_' prefix to them so they cannot conflict with the system
protobuf.

This change only affects desktop Linux component builds.  Release
binaries and debug binaries on other platforms should be unaffected.

Finally, this CL is a hack, and it should be reverted when a
longer-term solution is implemented.

BUG= 79722 , 700120 
R=pkasting@chromium.org
TBR=bengr@chromium.org
CC=dpranke@chromium.org,rsimha@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_dbg_ng;master.tryserver.chromium.mac:mac_chromium_dbg_ng;master.tryserver.chromium.win:win_chromium_dbg_ng

patch from issue 2746493002 at patchset 260001 (http://crrev.com/2746493002#ps260001)

Review-Url: https://codereview.chromium.org/2756543002
Cr-Commit-Position: refs/heads/master@{#458016}

[modify] https://crrev.com/77bec4dfd9e172a99543b9bfe9daf2580721bfdd/net/BUILD.gn
[modify] https://crrev.com/77bec4dfd9e172a99543b9bfe9daf2580721bfdd/third_party/protobuf/BUILD.gn
[modify] https://crrev.com/77bec4dfd9e172a99543b9bfe9daf2580721bfdd/third_party/protobuf/README.chromium
[add] https://crrev.com/77bec4dfd9e172a99543b9bfe9daf2580721bfdd/third_party/protobuf/patches/0012-extract-globals.patch
[modify] https://crrev.com/77bec4dfd9e172a99543b9bfe9daf2580721bfdd/third_party/protobuf/src/google/protobuf/arena.cc
[modify] https://crrev.com/77bec4dfd9e172a99543b9bfe9daf2580721bfdd/third_party/protobuf/src/google/protobuf/arena.h
[modify] https://crrev.com/77bec4dfd9e172a99543b9bfe9daf2580721bfdd/third_party/protobuf/src/google/protobuf/extension_set.cc
[modify] https://crrev.com/77bec4dfd9e172a99543b9bfe9daf2580721bfdd/third_party/protobuf/src/google/protobuf/extension_set.h
[modify] https://crrev.com/77bec4dfd9e172a99543b9bfe9daf2580721bfdd/third_party/protobuf/src/google/protobuf/generated_message_util.cc
[modify] https://crrev.com/77bec4dfd9e172a99543b9bfe9daf2580721bfdd/third_party/protobuf/src/google/protobuf/generated_message_util.h
[add] https://crrev.com/77bec4dfd9e172a99543b9bfe9daf2580721bfdd/third_party/protobuf/src/google/protobuf/globals.cc
[modify] https://crrev.com/77bec4dfd9e172a99543b9bfe9daf2580721bfdd/third_party/protobuf/src/google/protobuf/io/coded_stream.cc
[modify] https://crrev.com/77bec4dfd9e172a99543b9bfe9daf2580721bfdd/third_party/protobuf/src/google/protobuf/io/coded_stream.h
[modify] https://crrev.com/77bec4dfd9e172a99543b9bfe9daf2580721bfdd/third_party/protobuf/src/google/protobuf/stubs/atomicops_internals_x86_gcc.cc
[modify] https://crrev.com/77bec4dfd9e172a99543b9bfe9daf2580721bfdd/third_party/protobuf/src/google/protobuf/stubs/atomicops_internals_x86_gcc.h
[modify] https://crrev.com/77bec4dfd9e172a99543b9bfe9daf2580721bfdd/third_party/protobuf/src/google/protobuf/stubs/common.cc
[modify] https://crrev.com/77bec4dfd9e172a99543b9bfe9daf2580721bfdd/third_party/protobuf/src/google/protobuf/stubs/structurally_valid.cc
[modify] https://crrev.com/77bec4dfd9e172a99543b9bfe9daf2580721bfdd/third_party/protobuf/src/google/protobuf/stubs/strutil.cc

Status: Fixed (was: Assigned)

Comment 18 by kbr@chromium.org, Mar 21 2017

Blockedon: 703706

Comment 19 by kbr@chromium.org, Mar 21 2017

Blockedon: -703706

Comment 20 by kbr@chromium.org, Mar 21 2017

Blocking: 703706
Project Member

Comment 21 by bugdroid1@chromium.org, Mar 22 2017

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

commit 396d7f89575a83e9d8b23a609acf60bb383fbaf1
Author: eseckler <eseckler@chromium.org>
Date: Wed Mar 22 12:59:54 2017

[protobuf] Fix potential double definition build errors.

https://codereview.chromium.org/2756543002 introduced a protobuf_globals
target, whose sources include stubs/atomicops_internals_x86_gcc.cc.

Since this source file is also included in the protobuf_lite target,
some builds may complain about double definitions.

BUG= 700120 

Review-Url: https://codereview.chromium.org/2762193002
Cr-Commit-Position: refs/heads/master@{#458725}

[modify] https://crrev.com/396d7f89575a83e9d8b23a609acf60bb383fbaf1/third_party/protobuf/BUILD.gn

Sign in to add a comment