Disable CFI on a few methods.

Disable CFI on a few methods.

While they have not been observed to slow down real-world use cases,
some blink_layout microbenchmarks feel better with these methods
disabled. In order to be concervative at the launch time, lift
the CFI defense for these methods.

8/10 of these methods will become much faster when an optimization
proposed in https://crbug.com/638056 -- we only need to load vptr
once (before the loop) and have a single CFI check instead of
a check per iteration.

BUG=638056, 634139
Committed: https://crrev.com/e3ad26fee659e29604efb762dd05347df6506fa9
Cr-Commit-Position: refs/heads/master@{#38759}

[krasin1, 2016-08-18 23:30:58]

krasin@chromium.org changed reviewers:
+ bmeurer@chromium.org, krasin@chromium.org

[krasin1, 2016-08-18 23:30:59]

Hi Benedikt!

please, review this small CL that adds attributes to disable Control Flow
Integrity (CFI) on 10 methods for perf reasons. While I have not seen them to be
a problem while browser the web, a few blink_layout microbenchmarks seem to be
unhappy about it. The proposal is to blacklist these methods temporarily (that
will remove the protection offered by CFI from them) and them reenable them
after the optimization is implemented in Clang: https://crbug.com/638056

An alternative is to add these methods to Chromium-side blacklist
(src/tools/cfi/blacklist), but that will be more fragile as a method would
escape from the list if renamed.

[Benedikt Meurer, 2016-08-19 06:01:10]

lgtm

[krasin, 2016-08-19 06:03:22]

The CQ bit was checked by krasin@google.com

[commit-bot: I haz the power, 2016-08-19 06:03:26]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[krasin, 2016-08-19 06:03:33]

On 2016/08/19 06:01:10, Benedikt Meurer wrote:
> lgtm

Thanks!

[commit-bot: I haz the power, 2016-08-19 06:06:35]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-19 06:06:36]

Try jobs failed on following builders:
  v8_presubmit on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/22086)

[krasin, 2016-08-19 06:13:25]

krasin@google.com changed reviewers:
+ verwaest@chromium.org

[krasin, 2016-08-19 06:13:25]

Hi Toon,

may I have you LGTM for src/snapshot/serializer-common.cc, please? Benedikt has
reviewed the CL overall, but it seems I need an explicit approval from
src/snapshot owners.

Thanks!

[krasin, 2016-08-19 08:44:47]

The CQ bit was checked by krasin@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-08-19 08:44:49]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-19 08:46:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-19 08:46:20]

Dry run: This issue passed the CQ dry run.

[krasin, 2016-08-19 08:48:02]

The CQ bit was checked by krasin@google.com

[commit-bot: I haz the power, 2016-08-19 08:48:11]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-19 08:50:59]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-19 08:51:00]

Try jobs failed on following builders:
  v8_presubmit on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/22103)

[Toon Verwaest, 2016-08-19 12:46:56]

rubberstamp lgtm

[krasin, 2016-08-19 16:27:14]

The CQ bit was checked by krasin@google.com

[commit-bot: I haz the power, 2016-08-19 16:29:56]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-19 16:31:40]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-08-19 16:31:53]

Description was changed from

==========
Disable CFI on a few methods.

While they have not been observed to slow down real-world use cases,
some blink_layout microbenchmarks feel better with these methods
disabled. In order to be concervative at the launch time, lift
the CFI defense for these methods.

8/10 of these methods will become much faster when an optimization
proposed in https://crbug.com/638056 -- we only need to load vptr
once (before the loop) and have a single CFI check instead of
a check per iteration.

BUG=638056,634139
==========

to

==========
Disable CFI on a few methods.

While they have not been observed to slow down real-world use cases,
some blink_layout microbenchmarks feel better with these methods
disabled. In order to be concervative at the launch time, lift
the CFI defense for these methods.

8/10 of these methods will become much faster when an optimization
proposed in https://crbug.com/638056 -- we only need to load vptr
once (before the loop) and have a single CFI check instead of
a check per iteration.

BUG=638056,634139

Committed: https://crrev.com/e3ad26fee659e29604efb762dd05347df6506fa9
Cr-Commit-Position: refs/heads/master@{#38759}
==========

[commit-bot: I haz the power, 2016-08-19 16:31:54]

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