Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(908)

Issue 2952963002: Add patched eu-strip in build/linux/bin (Closed)

Created:
3 years, 6 months ago by Tom Anderson
Modified:
3 years, 6 months ago
Reviewers:
Lei Zhang, Dirk Pranke, Nico
CC:
chromium-reviews, grt+watch_chromium.org, pennymac+watch_chromium.org, Michael Moss, wfh+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Add patched eu-strip in build/linux/bin The eu-strip that ships with Ubuntu Trusty segfaults when stripping a chrome binary that statically links libc++. The issue was fixed upstream at some point because the crash does not occur on Ubuntu Xenial. This CL bundles a patched eu-strip for x86_64 (and moves the x86 one that used to be in Google Cloud Storage into the chromium repo). The issue was due to a double-free in libelf.so. The patched version simply removes the offending call to free and statically links libelf into eu-strip. This probably results in a memory leak, but this is ok for now since the utility is a very short-lived process. BUG=593874 R=dpranke@chromium.org CC=thakis@chromium.org,thestig@chromium.org

Patch Set 1 #

Patch Set 2 : move to third_party #

Total comments: 2

Patch Set 3 : update with upstream elfutils link #

Unified diffs Side-by-side diffs Delta from patch set Stats (+29 lines, -17 lines) Patch
M DEPS View 1 1 chunk +0 lines, -13 lines 0 comments Download
M build/.gitignore View 1 chunk +0 lines, -1 line 0 comments Download
D build/linux/bin/eu-strip.sha1 View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/installer/linux/BUILD.gn View 1 1 chunk +2 lines, -2 lines 0 comments Download
A third_party/eu-strip/OWNERS View 1 1 chunk +3 lines, -0 lines 0 comments Download
A third_party/eu-strip/README.chromium View 1 2 1 chunk +24 lines, -0 lines 0 comments Download
A third_party/eu-strip/bin/x86/eu-strip View 1 Binary file 0 comments Download
A third_party/eu-strip/bin/x86_64/eu-strip View 1 Binary file 0 comments Download

Messages

Total messages: 25 (12 generated)
Tom Anderson
dpranke ptal
3 years, 6 months ago (2017-06-22 03:04:08 UTC) #1
Lei Zhang
Is the 64-bit eu-strip binary from Ubuntu Xenial or did we build it ourselves? If ...
3 years, 6 months ago (2017-06-22 03:14:17 UTC) #7
Tom Anderson
On 2017/06/22 03:14:17, Lei Zhang wrote: > Is the 64-bit eu-strip binary from Ubuntu Xenial ...
3 years, 6 months ago (2017-06-22 03:19:05 UTC) #8
Lei Zhang
On 2017/06/22 03:19:05, Tom Anderson wrote: > On 2017/06/22 03:14:17, Lei Zhang wrote: > > ...
3 years, 6 months ago (2017-06-22 03:24:28 UTC) #9
Dirk Pranke
Sorry for the back-and-forth, but can you create a directory under //third_party for this and ...
3 years, 6 months ago (2017-06-22 16:43:36 UTC) #13
Tom Anderson
On 2017/06/22 16:43:36, Dirk Pranke wrote: > Sorry for the back-and-forth, but can you create ...
3 years, 6 months ago (2017-06-22 18:21:25 UTC) #16
Lei Zhang
lgtm https://codereview.chromium.org/2952963002/diff/40001/third_party/eu-strip/README.chromium File third_party/eu-strip/README.chromium (right): https://codereview.chromium.org/2952963002/diff/40001/third_party/eu-strip/README.chromium#newcode2 third_party/eu-strip/README.chromium:2: URL: https://packages.ubuntu.com/source/trusty/elfutils Does elfutils have an upstream URL? ...
3 years, 6 months ago (2017-06-22 18:25:37 UTC) #17
Tom Anderson
Anyone know why the executable bit is not getting set? Presubmit keeps failing because of ...
3 years, 6 months ago (2017-06-22 18:27:32 UTC) #18
Tom Anderson
https://codereview.chromium.org/2952963002/diff/40001/third_party/eu-strip/README.chromium File third_party/eu-strip/README.chromium (right): https://codereview.chromium.org/2952963002/diff/40001/third_party/eu-strip/README.chromium#newcode2 third_party/eu-strip/README.chromium:2: URL: https://packages.ubuntu.com/source/trusty/elfutils On 2017/06/22 18:25:37, Lei Zhang wrote: > ...
3 years, 6 months ago (2017-06-22 18:32:48 UTC) #19
Nico
On 2017/06/22 18:27:32, Tom Anderson wrote: > Anyone know why the executable bit is not ...
3 years, 6 months ago (2017-06-22 18:54:02 UTC) #21
Nico
Also also, why do we need a 32-bit binary? Isn't the 64-bit one enough?
3 years, 6 months ago (2017-06-22 18:54:24 UTC) #23
Tom Anderson
On 2017/06/22 18:54:02, Nico wrote: > I think rietveld doesn't support file metadata changes in ...
3 years, 6 months ago (2017-06-22 19:05:33 UTC) #24
Nico
3 years, 6 months ago (2017-06-22 19:09:29 UTC) #25
Message was sent while issue was closed.
On Thu, Jun 22, 2017 at 3:05 PM, <thomasanderson@chromium.org> wrote:

> On 2017/06/22 18:54:02, Nico wrote:
> > I think rietveld doesn't support file metadata changes in CLs. You can
> try
> using
> > gerrit. Back in the day we used to `git cl land` changes like this
> directly.
>
> gerrit seems to work! Please send your reviews to
> https://chromium-review.googlesource.com/c/544798/
> (closing this CL)
>
> > How large are the binaries? Usually we check in .sha1 files for binaries
> and
> add
> > a hook to pull the actual binary, because then the git size is smaller,
> and we
> > can skip the download on e.g. win/mac where this binary is useless.
>
> They're a total of 148K. dpranke@ said these files should be in the repo
> and
> I'd agree with him. (the reasoning was they're small and don't change
> often)
>
> On 2017/06/22 18:54:24, Nico wrote:
> > Also also, why do we need a 32-bit binary? Isn't the 64-bit one enough?
>
> In case someone wants to build on an x86 host. (not sure if this can still
> happen though)
>

I don't think we support that. All our other binaries (ninja, clang, gn,
...) are 64-bit only.


>
> https://codereview.chromium.org/2952963002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

Powered by Google App Engine
This is Rietveld 408576698