Linux: Turn --icf=safe back on for the gold linker. (try 2)

Linux: Turn --icf=safe back on for the gold linker. (try 2)

Previous try: https://codereview.chromium.org/137993011
Reverted: https://codereview.chromium.org/139153005

BUG=76280
R=jln@chromium.org

Committed: https://chromium.googlesource.com/chromium/src/+/d99e73cfcd7925a9a80a3db13b9f98d5d01a0576

[Lei Zhang, 2014-09-22 19:11:35]

thestig@chromium.org changed reviewers:
+ jln@chromium.org

[Lei Zhang, 2014-09-22 19:11:35]

I somehow broke the tree in a horrible way last time I tried in January, but I
can't repro the problem and all the trybots are green. Shall we give it another
try? I'll land this at night so if it breaks, I can quickly revert without
affecting too many people.

[Chris Evans, 2014-09-22 19:20:36]

On 2014/09/22 19:11:35, Lei Zhang wrote:
> I somehow broke the tree in a horrible way last time I tried in January, but I
> can't repro the problem and all the trybots are green. Shall we give it
another
> try? I'll land this at night so if it breaks, I can quickly revert without
> affecting too many people.

sure, LGTM

[jln (very slow on Chromium), 2014-09-22 19:21:20]

On 2014/09/22 19:11:35, Lei Zhang wrote:
> I somehow broke the tree in a horrible way last time I tried in January, but I
> can't repro the problem and all the trybots are green. Shall we give it
another
> try? I'll land this at night so if it breaks, I can quickly revert without
> affecting too many people.

lgtm

+cc: mdempsky (who is a good reviewer for linker-level things)

Maybe it's worth linking a bug (makes it so much easier to follow land/revert
sequences)?

My understanding is that even in "safe" mode, ICF can make stack traces
confusing. Do you know how it will appear via minidump_stackwalk / our crash
tool? Do we get a link to all the possibilities for a given piece of code?

[mdempsky, 2014-09-22 19:49:32]

On 2014/09/22 19:21:20, jln (slow on IPC reviews) wrote:
> Maybe it's worth linking a bug (makes it so much easier to follow land/revert
> sequences)?

+1

> My understanding is that even in "safe" mode, ICF can make stack traces
> confusing. Do you know how it will appear via minidump_stackwalk / our crash
> tool? Do we get a link to all the possibilities for a given piece of code?

According to the ICF paper
(http://static.googleusercontent.com/media/research.google.com/en/us/pubs/arch...),
they add additional DWARF debugging data to disambiguate the merged functions.
So I suspect by default we'll just randomly get one of the functions in a stack
trace?

Reading the "Debugger/Profiler Changes" section at
http://wiki.dwarfstd.org/index.php?title=ICF, it seems like breakpad could
reasonably handle disambiguating direct calls to merged functions. But virtual
calls to merged functions might be trickier since they involve checking which
vtable the "this" argument was pointing to, which I suspect minidumps won't
record?

[Lei Zhang, 2014-09-22 21:54:47]

On 2014/09/22 19:49:32, mdempsky wrote:
> On 2014/09/22 19:21:20, jln (slow on IPC reviews) wrote:
> > Maybe it's worth linking a bug (makes it so much easier to follow
land/revert
> > sequences)?
> 
> +1

Apparently I own bug 76280.

> > My understanding is that even in "safe" mode, ICF can make stack traces
> > confusing. Do you know how it will appear via minidump_stackwalk / our crash
> > tool? Do we get a link to all the possibilities for a given piece of code?
> 
> According to the ICF paper
>
(http://static.googleusercontent.com/media/research.google.com/en/us/pubs/arch...),
> they add additional DWARF debugging data to disambiguate the merged functions.
> So I suspect by default we'll just randomly get one of the functions in a
stack
> trace?
> 
> Reading the "Debugger/Profiler Changes" section at
> http://wiki.dwarfstd.org/index.php?title=ICF, it seems like breakpad could
> reasonably handle disambiguating direct calls to merged functions. But virtual
> calls to merged functions might be trickier since they involve checking which
> vtable the "this" argument was pointing to, which I suspect minidumps won't
> record?

Windows devs have to deal with this. Sometimes stack traces don't make sense,
but someone with windbg-fu chimes in and goes Foo got folded into Bar.

FYI, my stripped near-ToT, non official build went from 101616040 to 100530600
bytes. Saving 1085440 which is about 1%.

[Lei Zhang, 2014-09-24 07:57:50]

The CQ bit was checked by thestig@chromium.org

[commit-bot: I haz the power, 2014-09-24 07:58:49]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/583363004/1

[Lei Zhang, 2014-09-24 08:02:27]

The CQ bit was unchecked by thestig@chromium.org

[commit-bot: I haz the power, 2014-09-24 08:09:15]

Patchset 1 (id:??) landed as
https://crrev.com/d99e73cfcd7925a9a80a3db13b9f98d5d01a0576
Cr-Commit-Position: refs/heads/master@{#296366}

[Lei Zhang, 2014-09-24 08:09:16]

Committed patchset #1 (id:1) manually as d99e73c (presubmit successful).