Turn off -Wl,-z,defs when building with CFI diagnostics.

Turn off -Wl,-z,defs and -fno-rtti when building with CFI diagnostics.

The CFI diagnostic mode requires a runtime library, so we need to disable
-Wl,-z,defs in order to allow DSOs to link with undefined references to the
runtime library, as we do for the other sanitizers which require runtime
libraries.

The diagnostic mode can also issue better diagnostics if RTTI is enabled,
so enable RTTI if the diagnostic mode is enabled.

BUG=512614
R=thakis@chromium.org

Committed: https://crrev.com/7f19422b7c2f1ecb2541a510321c9ae9ed1886ca
Cr-Commit-Position: refs/heads/master@{#342955}

[pcc1, 2015-07-30 23:57:19]

On 2015/07/22 15:34:45, pcc1 wrote:

Ping. (Sorry.)

[pcc1, 2015-08-10 18:27:18]

On 2015/07/30 23:57:19, pcc1 wrote:
> On 2015/07/22 15:34:45, pcc1 wrote:
> 
> Ping. (Sorry.)

Ping.

[Nico, 2015-08-10 19:13:09]

https://codereview.chromium.org/1243373003/diff/20001/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/1243373003/diff/20001/build/common.gypi#newco...
build/common.gypi:3642: ['(OS=="linux" or OS=="android") and asan==0 and msan==0
and tsan==0 and ubsan==0 and ubsan_vptr==0 and cfi_diag==0', {
This looks good.

https://codereview.chromium.org/1243373003/diff/20001/build/common.gypi#newco...
build/common.gypi:6112: ],
That's a pretty big behavioral difference. Is this really needed? How much
better do diagnostics get with this?

[pcc1, 2015-08-10 19:30:04]

https://codereview.chromium.org/1243373003/diff/20001/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/1243373003/diff/20001/build/common.gypi#newco...
build/common.gypi:6112: ],
On 2015/08/10 19:13:09, Nico (hiding) wrote:
> That's a pretty big behavioral difference. Is this really needed? How much
> better do diagnostics get with this?

This is needed to see the dynamic types of objects that fail the CFI check.
(Note that we already do something similar for ubsan==1 and ubsan_vptr==1, for
largely the same reason.)

[Nico, 2015-08-11 22:09:14]

https://codereview.chromium.org/1243373003/diff/20001/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/1243373003/diff/20001/build/common.gypi#newco...
build/common.gypi:6112: ],
On 2015/08/10 19:30:04, pcc1 wrote:
> On 2015/08/10 19:13:09, Nico (hiding) wrote:
> > That's a pretty big behavioral difference. Is this really needed? How much
> > better do diagnostics get with this?
> 
> This is needed to see the dynamic types of objects that fail the CFI check.
> (Note that we already do something similar for ubsan==1 and ubsan_vptr==1, for
> largely the same reason.)

But CFI is supposed to possibly be shipped to users while ubsan isn't, right? Or
is cfi_diag the not-shipping config? (In which case the names are similar enough
that it'd be easy to accidentally ship the wrong thing)

[pcc1, 2015-08-11 22:37:57]

https://codereview.chromium.org/1243373003/diff/20001/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/1243373003/diff/20001/build/common.gypi#newco...
build/common.gypi:6112: ],
On 2015/08/11 22:09:13, Nico (hiding) wrote:
> On 2015/08/10 19:30:04, pcc1 wrote:
> > On 2015/08/10 19:13:09, Nico (hiding) wrote:
> > > That's a pretty big behavioral difference. Is this really needed? How much
> > > better do diagnostics get with this?
> > 
> > This is needed to see the dynamic types of objects that fail the CFI check.
> > (Note that we already do something similar for ubsan==1 and ubsan_vptr==1,
for
> > largely the same reason.)
> 
> But CFI is supposed to possibly be shipped to users while ubsan isn't, right?
Or
> is cfi_diag the not-shipping config? (In which case the names are similar
enough
> that it'd be easy to accidentally ship the wrong thing)

cfi_diag==1 is the non-shipping config and is not the default. There are other
things we can do to make sure that we ship the right config, such as adding an
automated test that navigates the browser to chrome://badcastcrash which
intentionally causes a crash under the intended shipping config, cfi_vptr=1
cfi_diag=0.

[Nico, 2015-08-11 22:40:28]

lgtm

[pcc1, 2015-08-11 22:41:10]

The CQ bit was checked by pcc@chromium.org

[commit-bot: I haz the power, 2015-08-11 22:41:46]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1243373003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1243373003/20001

[Nico, 2015-08-11 22:43:42]

Oh, is this needed in some gn file too?

[pcc1, 2015-08-11 22:44:36]

On 2015/08/11 22:43:42, Nico (hiding) wrote:
> Oh, is this needed in some gn file too?

Not yet. (I should really start porting the cfi config to gn.)

[commit-bot: I haz the power, 2015-08-12 01:27:28]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-08-12 01:28:19]

Patchset 2 (id:??) landed as
https://crrev.com/7f19422b7c2f1ecb2541a510321c9ae9ed1886ca
Cr-Commit-Position: refs/heads/master@{#342955}