Add MessageLoopForUI to the ubsan vptr blacklist.

Add MessageLoopForUI to the ubsan vptr blacklist.

MessageLoopForUI has the same layout as it base class. This
is enforced by a static_assert in
base/message_loop/message_loop.h:

// Do not add any member variables to MessageLoopForUI!  This is important b/c
// MessageLoopForUI is often allocated via MessageLoop(TYPE_UI).  Any extra
// data that you need should be stored on the MessageLoop's pump_ instance.
static_assert(sizeof(MessageLoop) == sizeof(MessageLoopForUI),
              "MessageLoopForUI should not have extra member variables");

BUG=725091
Review-Url: https://codereview.chromium.org/2902733002
Cr-Commit-Position: refs/heads/master@{#474276}
Committed: https://chromium.googlesource.com/chromium/src/+/7dfa3e2c27b8254e156adf9a15f6d0ae261bf602

[fdoray, 2017-05-23 13:32:36]

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

[commit-bot: I haz the power, 2017-05-23 13:32:48]

Dry run: CQ is trying da patch.

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

[fdoray, 2017-05-23 13:33:01]

fdoray@chromium.org changed reviewers:
+ thakis@chromium.org

[fdoray, 2017-05-23 13:33:02]

PTAL

[commit-bot: I haz the power, 2017-05-23 14:04:06]

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

[commit-bot: I haz the power, 2017-05-23 14:04:07]

Dry run: This issue passed the CQ dry run.

[Nico, 2017-05-23 14:29:23]

thakis@chromium.org changed reviewers:
+ pcc@chromium.org

[Nico, 2017-05-23 14:29:24]

Wow, sneaky. pcc, is CFI fine with things like this?

(Doing this is undefined behavior, so tools might be surprised by it...)

[pcc1, 2017-05-23 17:52:01]

On 2017/05/23 14:29:24, Nico wrote:
> Wow, sneaky. pcc, is CFI fine with things like this?
> 
> (Doing this is undefined behavior, so tools might be surprised by it...)

There's a hack in CFI that considers a class to be the same as its base if its
layout is the same and it does not override virtual functions. So I think CFI
would be fine with this. (I'm a little unhappy about this hack, and would like
to disable it in Chrome one day, but priorities...)

[fdoray, 2017-05-24 13:33:01]

On 2017/05/23 17:52:01, pcc1 wrote:
> On 2017/05/23 14:29:24, Nico wrote:
> > Wow, sneaky. pcc, is CFI fine with things like this?
> > 
> > (Doing this is undefined behavior, so tools might be surprised by it...)
> 
> There's a hack in CFI that considers a class to be the same as its base if its
> layout is the same and it does not override virtual functions. So I think CFI
> would be fine with this. (I'm a little unhappy about this hack, and would like
> to disable it in Chrome one day, but priorities...)

Since the assumption that MessageLoop(TYPE_UI) can be casted to MessageLoopForUI
has been there forever
https://chromium.googlesource.com/chromium/src/+/4d9bdfafcd1393385860bc9fe947...
and there is already a ubsan exception for MessageLoopForIO, can we land this CL
to make the ubsan bot green again? FYI, my team is working on deprecating
MessageLoop altogether, so this exception won't stay forever.

[Nico, 2017-05-24 13:39:11]

lgtm

[fdoray, 2017-05-24 13:44:53]

The CQ bit was checked by fdoray@chromium.org

[commit-bot: I haz the power, 2017-05-24 13:45:16]

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, 2017-05-24 13:59:25]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1495633493577390, "parent_rev":
"8a4f5257e60bc9011b788efcaae7605f6351a58e", "commit_rev":
"7dfa3e2c27b8254e156adf9a15f6d0ae261bf602"}

[commit-bot: I haz the power, 2017-05-24 13:59:35]

Description was changed from

==========
Add MessageLoopForUI to the ubsan vptr blacklist.

MessageLoopForUI has the same layout as it base class. This
is enforced by a static_assert in
base/message_loop/message_loop.h:

// Do not add any member variables to MessageLoopForUI!  This is important b/c
// MessageLoopForUI is often allocated via MessageLoop(TYPE_UI).  Any extra
// data that you need should be stored on the MessageLoop's pump_ instance.
static_assert(sizeof(MessageLoop) == sizeof(MessageLoopForUI),
              "MessageLoopForUI should not have extra member variables");

BUG=725091
==========

to

==========
Add MessageLoopForUI to the ubsan vptr blacklist.

MessageLoopForUI has the same layout as it base class. This
is enforced by a static_assert in
base/message_loop/message_loop.h:

// Do not add any member variables to MessageLoopForUI!  This is important b/c
// MessageLoopForUI is often allocated via MessageLoop(TYPE_UI).  Any extra
// data that you need should be stored on the MessageLoop's pump_ instance.
static_assert(sizeof(MessageLoop) == sizeof(MessageLoopForUI),
              "MessageLoopForUI should not have extra member variables");

BUG=725091

Review-Url: https://codereview.chromium.org/2902733002
Cr-Commit-Position: refs/heads/master@{#474276}
Committed:
https://chromium.googlesource.com/chromium/src/+/7dfa3e2c27b8254e156adf9a15f6...
==========

[commit-bot: I haz the power, 2017-05-24 13:59:36]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/7dfa3e2c27b8254e156adf9a15f6...