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

Issue 870919 link

Starred by 3 users

Issue metadata

Status: Verified
Owner:
Last visit > 30 days ago
Closed: Sep 24
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Local chrome(ium) builds don't have an ELF build-id note

Project Member Reported by mosescu@chromium.org, Aug 3

Issue description

For platforms using ELF binary formats the local builds are missing the build-id, ex. 

Official build:
/opt/google/chrome/chrome: ELF 64-bit LSB pie executable x86-64, version 1 (SYSV), dynamically linked, interpreter /lib64/ld-linux-x86-64.so.2, for GNU/Linux 3.2.0, BuildID[sha1]=a23e061ee24f3e16c47063f59d5efa03428c2e29, stripped

Local build:
chromium/src/out/release/chrome: ELF 64-bit LSB pie executable x86-64, version 1 (SYSV), dynamically linked, interpreter /lib64/ld-linux-x86-64.so.2, for GNU/Linux 3.2.0, with debug_info, not stripped

The lack of build-id creates subtle problems and this is only going to get worse. In particular the minidumps created by Breakpad (and soon Crashpad) may not be handled correctly by the debuggers (ex. LLDB, which is the foundation for the next crash processor as well as the only cross-platform debugger for Chrome)
 
Components: Build
Cc: mark@chromium.org
Owner: mosescu@chromium.org
Status: Started (was: Untriaged)
Project Member

Comment 3 by bugdroid1@chromium.org, Aug 13

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

commit e6bbcbfa0efcec85bf62fc9e0fdf8a5edc76d327
Author: Leonard Mosescu <mosescu@chromium.org>
Date: Mon Aug 13 19:09:44 2018

Make sure that both official and local (unofficial) builds have ELF build-ids

The local (unofficial) builds use --build-id=uuid, which unlike sha1 build IDs
does not slow down the build.

Bug:  870919 
Change-Id: Ib55cbc86e11e8208fa8560d98b3288bbc6a149de
Reviewed-on: https://chromium-review.googlesource.com/1169903
Reviewed-by: Mark Mentovai <mark@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Leonard Mosescu <mosescu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582658}
[modify] https://crrev.com/e6bbcbfa0efcec85bf62fc9e0fdf8a5edc76d327/build/config/compiler/BUILD.gn

Status: Verified (was: Started)
Project Member

Comment 5 by bugdroid1@chromium.org, Aug 14

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

commit bf029c05f8977d9d3d0c57cb762b8d5a3cc4da20
Author: Ben Pastene <bpastene@chromium.org>
Date: Tue Aug 14 00:35:17 2018

Revert "Make sure that both official and local (unofficial) builds have ELF build-ids"

This reverts commit e6bbcbfa0efcec85bf62fc9e0fdf8a5edc76d327.

Reason for revert: Breaks build determinism on linux
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Deterministic%20Linux/15728
See also crbug.com/873869

Original change's description:
> Make sure that both official and local (unofficial) builds have ELF build-ids
> 
> The local (unofficial) builds use --build-id=uuid, which unlike sha1 build IDs
> does not slow down the build.
> 
> Bug:  870919 
> Change-Id: Ib55cbc86e11e8208fa8560d98b3288bbc6a149de
> Reviewed-on: https://chromium-review.googlesource.com/1169903
> Reviewed-by: Mark Mentovai <mark@chromium.org>
> Reviewed-by: Dirk Pranke <dpranke@chromium.org>
> Commit-Queue: Leonard Mosescu <mosescu@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#582658}

TBR=dpranke@chromium.org,mark@chromium.org,mosescu@chromium.org

Change-Id: I0add42d7b05d1ffcebea83d1df6d29d6139f1b67
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  870919 , 873869
Reviewed-on: https://chromium-review.googlesource.com/1173671
Reviewed-by: Ben Pastene <bpastene@chromium.org>
Commit-Queue: Ben Pastene <bpastene@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582769}
[modify] https://crrev.com/bf029c05f8977d9d3d0c57cb762b8d5a3cc4da20/build/config/compiler/BUILD.gn

Status: Assigned (was: Verified)
FWIW the current behavior is intentional for build perf reasons, see  issue 622775 . (And the faster alternative is nondeterministic as you discovered.)
Thanks. I don't see any build perf numbers in  crbug.com/622775 , do you
remember what was the magnitude of the slowdown?

Does the deterministic build checker have the ability to explicitly ignore
parts of the binary (ex. sections)? I'm not suggesting this is necessarily
the best approach, just curious.
Cc: dpranke@chromium.org thakis@chromium.org
> Does the deterministic build checker have the ability to explicitly ignore parts of the binary

No. Swarming caches based on hash of the whole executable, so that would defeat the purpose of that test.

Speedup was I think something like maybe 30%? Easy to check: Pass -Wl,--build-id in non-official builds locally as well and measure :-)
If the caches implement the same selective hashing this shouldn't be a
problem, right? That's one of the reason I'm asking if there's a reusable
filtering (or binary patching) utility - I take it that this is not the
case today.

Then, how do we handle Windows builds? Don't the PE binaries have
non-deterministic timestamps?
Windows builds use https://chromium-review.googlesource.com/c/1161104/ to get deterministic timestamps. (Relanding that is blocked on https://chromium-review.googlesource.com/c/chromium/src/+/1167913 landing.)

(Note that PE has another issue in the PDB aging that we haven't thought about much yet, see https://bugs.chromium.org/p/chromium/issues/detail?id=330260#c38 But since we control the linker, chances are we can come up with...something.)
I did some measurements:

1. On a modest workstation (z440 with the build done on a Samsung SSD 860 Pro).

2. I ran 3 warm link steps with --build-id=sha1 and w/o any build id (ran a cold link, discarded the timing then re-linked chrome. Cold link steps add quite a bit even on a SSD. ~50sec slower)

3. I used an optimized build of Chromium (chrome binary is ~5.3Gb unstripped)

Here are the numbers:

sha1: 92.594s 87.265s 93.141s
none: 85.762s 83.803s 89.160s 

If we pick the median values the SHA1 overhead is minor (less than 2 sec). Comparing the mean values shows ~5 overhead, which is close to what I expected (doing a SHA1 over 5Gb is not fast, but relatively minor to what the linker has to do). 

My suggestion is simply enable --build-id=sha1 on all builds. If we start doing incremental linking with LLD the relative overhead would be a bit more noticeable but ~5sec per build is worth having consistent build IDs on all the binaries.

dpranke@, thakis@, what do you think?



Status: WontFix (was: Assigned)
The motivation for this is "the lack of build-id creates subtle problems and this is only going to get worse". That doesn't sound like a great argument for adding 5s to builds. (And official builds do have this enabled.)

Ima WontFix this for now so we don't spend a ton of time talking about it. Reopen once there's some convincing case for it.
Status: Assigned (was: WontFix)
> The motivation for this is "the lack of build-id creates subtle problems and > this is only going to get worse". That doesn't sound like a great argument
> for adding 5s to builds. (And official builds do have this enabled.)

Fair enough, although the next part describe the immediate problem we have:

> In particular the minidumps created by Breakpad (and soon Crashpad) may not
> be handled correctly by the debuggers (ex. LLDB, which is the foundation for > the next crash processor as well as the only cross-platform debugger for
> Chrome)

We're investing in shaping LLDB as the primary debugger for Chrome and the lack of build-ids hurts not only the end users (developers) but also the ongoing engineering work.

Also, note that the numbers I mentioned above are intended to be the upper bound (slow workstation, warm linking). In practice it's unlikely to have developers doing repeated linking so the actual overhead is around the noise level for build times.

I hope we can reconsider this since the workarounds are not pretty (the current top workaround is to have LLDB force match symbols - a very sharp knife I'd rather not ask developers to use regularly)
> We're investing in shaping LLDB

Is there a bug for that? That's the first I hear of that, and my team does all the toolchain stuff for chrome.

gdb doesn't need a build id to work. If lldb does, we can change that upstream.
Debugging a local builds, using unstripped binaries works (with both gdb
and lldb) since there's no need to lookup external files for debug
information.

If the debug information is external then the build-id is key.  The
situation is even more complex when Crashpad/Breakpad are in the loop
(short version: if the modules don't have build-ids, then Breakpad would
try to fabricate one using a custom hashing - which is great for Breakpad
processor but not for debuggers in general which are not aware of the
details of Breakpad's hashing)
When are you ever in a situation where you are doing non-local non-official builds?
The gap is local & non-official builds (which is what we normally use for
trying Chrome/Crashpad & LLDB changes during development).
Can you reply to "Is there a bug for that? That's the first I hear of that, and my team does all the toolchain stuff for chrome."

It sounds the use case here is to make your work on local lldb easier? Why don't you just hack up chrome's build flags locally?
We just realized that we always pass

      ldflags += [ "-Wl,--build-id=sha1" ]

If we pass just -Wl,--build-id, lld will instead compute an xxhash, which is way faster to compute and should be below noise level.

So something else that should probably work is to use =sha1 in official builds and always, unconditionally use -Wl,--build-id else (pending measurement).

Maybe we don't even need sha1 in official builds and can always use -Wl,--build-id.

But passing -Wl,--build-id without =sha1 in unofficial could likely be an alternative to the gn arg that we discussed.
(The "always" in the first sentence is supposed to mean "in official builds")
Project Member

Comment 23 by bugdroid1@chromium.org, Sep 18

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

commit 6d0e5f09c15e7c5e80490922aa7b62b460401ffe
Author: Leonard Mosescu <mosescu@chromium.org>
Date: Tue Sep 18 20:23:11 2018

Add a new GN arg: force_local_build_id

By default only official builds have build-ids. Setting
force_local_build_id = true can be used to add build-ids to
local builds as well.

Bug:  870919 
Change-Id: I007ffbfb0d6005e980b4dea5a394471396ad3d92
Reviewed-on: https://chromium-review.googlesource.com/1231374
Commit-Queue: Leonard Mosescu <mosescu@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#592174}
[modify] https://crrev.com/6d0e5f09c15e7c5e80490922aa7b62b460401ffe/build/config/compiler/BUILD.gn

Status: Verified (was: Assigned)

Sign in to add a comment