Add use_rtti gn arg to enable rtti globally in the build

Add use_rtti gn arg to enable rtti globally in the build

Some sanitizer configs already enabled rtti, move the logic to the arg
default value to make experimenting easier. Can be useful when dealing
with some asan-instrumented build startup issued that may go away when
rtti is enabled globally, but don't fall into the existing enable-rtti
cases.

One example scenario I had was with a shared_library built with both rtti
and exceptions (in a downstream project) causing immediate asan failure
on startup around std::exception typeinfo alignment. Rebuilding with
rtti enabled globally made things work, so it helps if that's possible
with just a gn arg.

Note that targets that need rtti to be not disabled regardless of global
settings (e.g. third party libraries like icu that need rtti to build)
can still remove the //build/config/compiler:no_rtti config and add
//build/config/compiler:rtti in its place; this is unchanged.

BUG=webrtc:6468
Review-Url: https://codereview.chromium.org/2607903002
Cr-Commit-Position: refs/heads/master@{#442227}
Committed: https://chromium.googlesource.com/chromium/src/+/d1a04b008747abf9231b676ad1b34e53bad43973

[tsniatowski, 2016-12-29 08:56:18]

Description was changed from

==========
Add use_rtti gn arg to enable rtti globally in the build

Some sanitizer configs already enabled rtti, move the logic to the arg
default value to make experimenting easier. Can be useful when dealing
with some asan-instrumented build startup issued that may go away when
rtti is enabled globally, but don't fall into the existing enable-rtti
cases.

One example scenario I had was with a shared_library built with both rtti
and exceptions (in a downstream project) causing immediate asan failure
on startup around std::exception typeinfo alignment. Rebuilding with
rtti enabled globally made things work, so it helps if that's possible
with just a gn arg.
==========

to

==========
Add use_rtti gn arg to enable rtti globally in the build

Some sanitizer configs already enabled rtti, move the logic to the arg
default value to make experimenting easier. Can be useful when dealing
with some asan-instrumented build startup issued that may go away when
rtti is enabled globally, but don't fall into the existing enable-rtti
cases.

One example scenario I had was with a shared_library built with both rtti
and exceptions (in a downstream project) causing immediate asan failure
on startup around std::exception typeinfo alignment. Rebuilding with
rtti enabled globally made things work, so it helps if that's possible
with just a gn arg.
==========

[tsniatowski, 2016-12-29 08:56:18]

tsniatowski@opera.com changed reviewers:
+ dpranke@chromium.org

[tsniatowski, 2016-12-29 08:57:09]

On 2016/12/29 08:56:18, tsniatowski wrote:
> mailto:tsniatowski@opera.com changed reviewers:
> + mailto:dpranke@chromium.org

PTAL

[Dirk Pranke, 2017-01-04 02:56:20]

Is this replaced by https://codereview.chromium.org/2603183002/ ?

[tsniatowski, 2017-01-04 07:01:31]

On 2017/01/04 02:56:20, Dirk Pranke wrote:
> Is this replaced by https://codereview.chromium.org/2603183002/ ?

This is the change I would prefer to see as I argue in the other review

[Dirk Pranke, 2017-01-09 06:33:39]

lgtm. Let's land this one instead of kjellander's CL.

[kjellander_chromium, 2017-01-09 07:10:56]

Description was changed from

==========
Add use_rtti gn arg to enable rtti globally in the build

Some sanitizer configs already enabled rtti, move the logic to the arg
default value to make experimenting easier. Can be useful when dealing
with some asan-instrumented build startup issued that may go away when
rtti is enabled globally, but don't fall into the existing enable-rtti
cases.

One example scenario I had was with a shared_library built with both rtti
and exceptions (in a downstream project) causing immediate asan failure
on startup around std::exception typeinfo alignment. Rebuilding with
rtti enabled globally made things work, so it helps if that's possible
with just a gn arg.
==========

to

==========
Add use_rtti gn arg to enable rtti globally in the build

Some sanitizer configs already enabled rtti, move the logic to the arg
default value to make experimenting easier. Can be useful when dealing
with some asan-instrumented build startup issued that may go away when
rtti is enabled globally, but don't fall into the existing enable-rtti
cases.

One example scenario I had was with a shared_library built with both rtti
and exceptions (in a downstream project) causing immediate asan failure
on startup around std::exception typeinfo alignment. Rebuilding with
rtti enabled globally made things work, so it helps if that's possible
with just a gn arg.

BUG=webrtc:6468
==========

[kjellander_chromium, 2017-01-09 07:11:02]

kjellander@chromium.org changed reviewers:
+ kjellander@chromium.org

[kjellander_chromium, 2017-01-09 07:11:02]

lgtm but it would be if you could clarify that the
//build/config/compiler:no_rtti config must be removed for targets that wish to
enable RTTI in your CL description.

I added a reference to BUG=webrtc:6468

[tsniatowski, 2017-01-09 08:16:04]

On 2017/01/09 07:11:02, kjellander_chromium wrote:
> lgtm but it would be if you could clarify that the
> //build/config/compiler:no_rtti config must be removed for targets that wish
to
> enable RTTI in your CL description.
> 
> I added a reference to BUG=webrtc:6468

In the CL description, or do you have a specific place in mind?

[tsniatowski, 2017-01-09 08:16:41]

On 2017/01/09 08:16:04, tsniatowski wrote:
> On 2017/01/09 07:11:02, kjellander_chromium wrote:
> > lgtm but it would be if you could clarify that the
> > //build/config/compiler:no_rtti config must be removed for targets that wish
> to
> > enable RTTI in your CL description.
> > 
> > I added a reference to BUG=webrtc:6468
> 
> In the CL description, or do you have a specific place in mind?

(note that this behavior is unchanged, targets that wanted to build with rtti
always on had to do this already)

[kjellander_chromium, 2017-01-09 08:31:07]

On 2017/01/09 08:16:41, tsniatowski wrote:
> On 2017/01/09 08:16:04, tsniatowski wrote:
> > On 2017/01/09 07:11:02, kjellander_chromium wrote:
> > > lgtm but it would be if you could clarify that the
> > > //build/config/compiler:no_rtti config must be removed for targets that
wish
> > to
> > > enable RTTI in your CL description.
> > > 
> > > I added a reference to BUG=webrtc:6468
> > 
> > In the CL description, or do you have a specific place in mind?
> 
> (note that this behavior is unchanged, targets that wanted to build with rtti
> always on had to do this already)

Well it's just unchanged for the targets that had use_cfi_diag || is_ubsan_vptr
|| is_ubsan_security
previously. Now it becomes possible for a wider audience to enable RTTI and for
those I though it
would be nice if it was clear in the CL description (the Git commit message)
what they need to do
so they don't have to do a lot of code searching to figure out how it works.

[tsniatowski, 2017-01-09 08:34:24]

On 2017/01/09 08:31:07, kjellander_chromium wrote:
> On 2017/01/09 08:16:41, tsniatowski wrote:
> > On 2017/01/09 08:16:04, tsniatowski wrote:
> > > On 2017/01/09 07:11:02, kjellander_chromium wrote:
> > > > lgtm but it would be if you could clarify that the
> > > > //build/config/compiler:no_rtti config must be removed for targets that
> wish
> > > to
> > > > enable RTTI in your CL description.
> > > > 
> > > > I added a reference to BUG=webrtc:6468
> > > 
> > > In the CL description, or do you have a specific place in mind?
> > 
> > (note that this behavior is unchanged, targets that wanted to build with
rtti
> > always on had to do this already)
> 
> Well it's just unchanged for the targets that had use_cfi_diag ||
is_ubsan_vptr
> || is_ubsan_security
> previously. Now it becomes possible for a wider audience to enable RTTI and
for
> those I though it
> would be nice if it was clear in the CL description (the Git commit message)
> what they need to do
> so they don't have to do a lot of code searching to figure out how it works.

No, things don't change for plain targets that have no idea what sanitizers are:
they could (and still can) reenable rtti -- see
https://cs.chromium.org/chromium/src/third_party/icu/BUILD.gn?q=icu/BU&sq=pac...
for example.

This CL only adds a global rtti switch, no changes on the per-target config
level. That said, I don't mind adding some more text in the commit message.

[tsniatowski, 2017-01-09 08:45:43]

Description was changed from

==========
Add use_rtti gn arg to enable rtti globally in the build

Some sanitizer configs already enabled rtti, move the logic to the arg
default value to make experimenting easier. Can be useful when dealing
with some asan-instrumented build startup issued that may go away when
rtti is enabled globally, but don't fall into the existing enable-rtti
cases.

One example scenario I had was with a shared_library built with both rtti
and exceptions (in a downstream project) causing immediate asan failure
on startup around std::exception typeinfo alignment. Rebuilding with
rtti enabled globally made things work, so it helps if that's possible
with just a gn arg.

BUG=webrtc:6468
==========

to

==========
Add use_rtti gn arg to enable rtti globally in the build

Some sanitizer configs already enabled rtti, move the logic to the arg
default value to make experimenting easier. Can be useful when dealing
with some asan-instrumented build startup issued that may go away when
rtti is enabled globally, but don't fall into the existing enable-rtti
cases.

One example scenario I had was with a shared_library built with both rtti
and exceptions (in a downstream project) causing immediate asan failure
on startup around std::exception typeinfo alignment. Rebuilding with
rtti enabled globally made things work, so it helps if that's possible
with just a gn arg.

Note that targets that need rtti to be not disabled regardless of global
settings (e.g. third party libraries like icu that need rtti to build)
can still remove the //build/config/compiler:no_rtti config and add
//build/config/compiler:rtti in its place; this is unchanged.

BUG=webrtc:6468
==========

[tsniatowski, 2017-01-09 08:48:28]

On 2017/01/09 08:34:24, tsniatowski wrote:
> On 2017/01/09 08:31:07, kjellander_chromium wrote:
> > On 2017/01/09 08:16:41, tsniatowski wrote:
> > > On 2017/01/09 08:16:04, tsniatowski wrote:
> > > > On 2017/01/09 07:11:02, kjellander_chromium wrote:
> > > > > lgtm but it would be if you could clarify that the
> > > > > //build/config/compiler:no_rtti config must be removed for targets
that
> > wish
> > > > to
> > > > > enable RTTI in your CL description.
> > > > > 
> > > > > I added a reference to BUG=webrtc:6468
> > > > 
> > > > In the CL description, or do you have a specific place in mind?
> > > 
> > > (note that this behavior is unchanged, targets that wanted to build with
> rtti
> > > always on had to do this already)
> > 
> > Well it's just unchanged for the targets that had use_cfi_diag ||
> is_ubsan_vptr
> > || is_ubsan_security
> > previously. Now it becomes possible for a wider audience to enable RTTI and
> for
> > those I though it
> > would be nice if it was clear in the CL description (the Git commit message)
> > what they need to do
> > so they don't have to do a lot of code searching to figure out how it works.
> 
> No, things don't change for plain targets that have no idea what sanitizers
are:
> they could (and still can) reenable rtti -- see
>
https://cs.chromium.org/chromium/src/third_party/icu/BUILD.gn?q=icu/BU&sq=pac...
> for example.
> 
> This CL only adds a global rtti switch, no changes on the per-target config
> level. That said, I don't mind adding some more text in the commit message.

@kjellander_chromium take a look now?

[kjellander_chromium, 2017-01-09 08:54:43]

On 2017/01/09 08:48:28, tsniatowski wrote:
> On 2017/01/09 08:34:24, tsniatowski wrote:
> > On 2017/01/09 08:31:07, kjellander_chromium wrote:
> > > On 2017/01/09 08:16:41, tsniatowski wrote:
> > > > On 2017/01/09 08:16:04, tsniatowski wrote:
> > > > > On 2017/01/09 07:11:02, kjellander_chromium wrote:
> > > > > > lgtm but it would be if you could clarify that the
> > > > > > //build/config/compiler:no_rtti config must be removed for targets
> that
> > > wish
> > > > > to
> > > > > > enable RTTI in your CL description.
> > > > > > 
> > > > > > I added a reference to BUG=webrtc:6468
> > > > > 
> > > > > In the CL description, or do you have a specific place in mind?
> > > > 
> > > > (note that this behavior is unchanged, targets that wanted to build with
> > rtti
> > > > always on had to do this already)
> > > 
> > > Well it's just unchanged for the targets that had use_cfi_diag ||
> > is_ubsan_vptr
> > > || is_ubsan_security
> > > previously. Now it becomes possible for a wider audience to enable RTTI
and
> > for
> > > those I though it
> > > would be nice if it was clear in the CL description (the Git commit
message)
> > > what they need to do
> > > so they don't have to do a lot of code searching to figure out how it
works.
> > 
> > No, things don't change for plain targets that have no idea what sanitizers
> are:
> > they could (and still can) reenable rtti -- see
> >
>
https://cs.chromium.org/chromium/src/third_party/icu/BUILD.gn?q=icu/BU&sq=pac...
> > for example.
> > 
> > This CL only adds a global rtti switch, no changes on the per-target config
> > level. That said, I don't mind adding some more text in the commit message.
> 
> @kjellander_chromium take a look now?

Thanks. Still lgtm

[tsniatowski, 2017-01-09 09:14:26]

The CQ bit was checked by tsniatowski@opera.com

[commit-bot: I haz the power, 2017-01-09 09:14:41]

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-01-09 10:31:18]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1483953266303240, "parent_rev":
"ef911f1fd846f9116ec49c42dded7af7a6b6c525", "commit_rev":
"d1a04b008747abf9231b676ad1b34e53bad43973"}

[commit-bot: I haz the power, 2017-01-09 10:31:45]

Description was changed from

==========
Add use_rtti gn arg to enable rtti globally in the build

Some sanitizer configs already enabled rtti, move the logic to the arg
default value to make experimenting easier. Can be useful when dealing
with some asan-instrumented build startup issued that may go away when
rtti is enabled globally, but don't fall into the existing enable-rtti
cases.

One example scenario I had was with a shared_library built with both rtti
and exceptions (in a downstream project) causing immediate asan failure
on startup around std::exception typeinfo alignment. Rebuilding with
rtti enabled globally made things work, so it helps if that's possible
with just a gn arg.

Note that targets that need rtti to be not disabled regardless of global
settings (e.g. third party libraries like icu that need rtti to build)
can still remove the //build/config/compiler:no_rtti config and add
//build/config/compiler:rtti in its place; this is unchanged.

BUG=webrtc:6468
==========

to

==========
Add use_rtti gn arg to enable rtti globally in the build

Some sanitizer configs already enabled rtti, move the logic to the arg
default value to make experimenting easier. Can be useful when dealing
with some asan-instrumented build startup issued that may go away when
rtti is enabled globally, but don't fall into the existing enable-rtti
cases.

One example scenario I had was with a shared_library built with both rtti
and exceptions (in a downstream project) causing immediate asan failure
on startup around std::exception typeinfo alignment. Rebuilding with
rtti enabled globally made things work, so it helps if that's possible
with just a gn arg.

Note that targets that need rtti to be not disabled regardless of global
settings (e.g. third party libraries like icu that need rtti to build)
can still remove the //build/config/compiler:no_rtti config and add
//build/config/compiler:rtti in its place; this is unchanged.

BUG=webrtc:6468

Review-Url: https://codereview.chromium.org/2607903002
Cr-Commit-Position: refs/heads/master@{#442227}
Committed:
https://chromium.googlesource.com/chromium/src/+/d1a04b008747abf9231b676ad1b3...
==========

[commit-bot: I haz the power, 2017-01-09 10:31:46]

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