Add DISABLE_CFI_PERF attribute on the methods with CFI checks disabled.

Add DISABLE_CFI_PERF attribute on the methods with CFI checks disabled.

While we have not observed CFI to slowdown real-world use cases,
there are a few blink_perf microbenchmarks, which are somewhat affected
by the change. This change removes CFI protection from the methods
which contribute the most to the slowdown.

Eventually, when proposed optimizations in Clang are implemented
(https://crbug.com/638056, https://crbug.com/638064), these attributes
would be possible to remove from all/most of the methods.

This CL does not in fact change the list of disabled methods, it
just converts them from CFI blacklist entries into attributes.
This should increase visibility, and also serve as a guard against refactoring.

BUG=641536
Committed: https://crrev.com/eb64c60f4789ab32bca921f63291ff8ccb35ab82
Cr-Commit-Position: refs/heads/master@{#415893}

[krasin1, 2016-09-01 04:08:50]

The CQ bit was checked by krasin@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-01 04:09:06]

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

[krasin1, 2016-09-01 04:26:34]

The CQ bit was unchecked by krasin@chromium.org

[krasin1, 2016-09-01 05:10:05]

krasin@chromium.org changed reviewers:
+ tkent@chromium.org

[tkent, 2016-09-01 05:19:14]

tkent@chromium.org changed reviewers:
+ eae@chromium.org
- tkent@chromium.org

[tkent, 2016-09-01 05:22:05]

Delegate to eae@, who is the layout team TL.

Question: Can we disable CFI for a whole class by something like:
   class CORE_EXPORT DISABLE_CFI LayoutBox { ...
?

[eae, 2016-09-01 05:26:20]

On 2016/09/01 05:22:05, tkent wrote:
> Delegate to eae@, who is the layout team TL.
> 
> Question: Can we disable CFI for a whole class by something like:
>    class CORE_EXPORT DISABLE_CFI LayoutBox { ...
> ?

Good question, I don't know.
If we could that's probably preferable.

[krasin1, 2016-09-01 05:50:04]

On 2016/09/01 05:26:20, eae wrote:
> On 2016/09/01 05:22:05, tkent wrote:
> > Delegate to eae@, who is the layout team TL.
> > 
> > Question: Can we disable CFI for a whole class by something like:
> >    class CORE_EXPORT DISABLE_CFI LayoutBox { ...
> > ?
> 
> Good question, I don't know.
> If we could that's probably preferable.

Disabling Control Flow Integrity on the whole class would create two big issues:

1. It would disable CFI on more methods than needed reducing the effectiveness
of the security protection.
2. It would be impossible to remove these attributes one by one when
corresponding optimizations in Clang are implemented and landed to Chrome. And
implementing enough optimizations to remove the attribute from the whole class
might take too much time.

I would like to note that LayoutBox is the class with the largest number methods
disabled. If you look at tools/cfi/blacklist.txt, you will see that it's often
less than 5 with a few classes with ~10 methods disabled. But nothing comes
close to LayoutBox with its 20 methods. It means that a class-level attribute is
even less useful for other types.

[krasin1, 2016-09-01 05:53:19]

Btw, these are similar CLs to other parts of Chrome:

src/cc: https://codereview.chromium.org/2257323002/
v8: https://codereview.chromium.org/2258003002/

[eae, 2016-09-01 06:05:02]

Thanks for the clarification.

LGTM

[krasin1, 2016-09-01 06:07:42]

The CQ bit was checked by krasin@chromium.org

[commit-bot: I haz the power, 2016-09-01 06:07:57]

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-09-01 06:11:22]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-09-01 06:13:22]

Description was changed from

==========
Add DISABLE_CFI_PERF attribute on the methods with CFI checks disabled.

While we have not observed CFI to slowdown real-world use cases,
there are a few blink_perf microbenchmarks, which are somewhat affected
by the change. This change removes CFI protection from the methods
which contribute the most to the slowdown.

Eventually, when proposed optimizations in Clang are implemented
(https://crbug.com/638056, https://crbug.com/638064), these attributes
would be possible to remove from all/most of the methods.

This CL does not in fact change the list of disabled methods, it
just converts them from CFI blacklist entries into attributes.
This should increase visibility, and also serve as a guard against refactoring.

BUG=641536
==========

to

==========
Add DISABLE_CFI_PERF attribute on the methods with CFI checks disabled.

While we have not observed CFI to slowdown real-world use cases,
there are a few blink_perf microbenchmarks, which are somewhat affected
by the change. This change removes CFI protection from the methods
which contribute the most to the slowdown.

Eventually, when proposed optimizations in Clang are implemented
(https://crbug.com/638056, https://crbug.com/638064), these attributes
would be possible to remove from all/most of the methods.

This CL does not in fact change the list of disabled methods, it
just converts them from CFI blacklist entries into attributes.
This should increase visibility, and also serve as a guard against refactoring.

BUG=641536

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

[commit-bot: I haz the power, 2016-09-01 06:13:23]

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