Add GN configs for -fsanitizer=null.

Add GN configs for -fsanitizer=null.

This allows to sanitize null pointer accesses and
could increase readability of the messages from UBSan Vptr bot
(once enabled there).

BUG=616268, 616870
Committed: https://crrev.com/2ec1834c98f0fd1caf86983f31d0d1a122e0ad1e
Cr-Commit-Position: refs/heads/master@{#397538}

[krasin, 2016-06-02 20:22:58]

Description was changed from

==========
Add GN configs for -fsanitizer=null.

This allows to sanitize null pointer accesses and
could increase readability of the messages from UBSan Vptr bot
(once enabled there).

BUG=616268
==========

to

==========
Add GN configs for -fsanitizer=null.

This allows to sanitize null pointer accesses and
could increase readability of the messages from UBSan Vptr bot
(once enabled there).

BUG=616268,616870
==========

[krasin, 2016-06-02 20:23:21]

krasin@google.com changed reviewers:
+ dpranke@chromium.org, thakis@chromium.org

[krasin, 2016-06-02 20:25:50]

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

[commit-bot: I haz the power, 2016-06-02 20:26:35]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2028383006/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2028383006/1

[Dirk Pranke, 2016-06-02 20:54:43]

Do we really need to keep adding new flags for every ubsan variant? Why don't we
combine more of them together?

[krasin, 2016-06-02 20:55:51]

On 2016/06/02 20:54:43, Dirk Pranke wrote:
> Do we really need to keep adding new flags for every ubsan variant? Why don't
we
> combine more of them together?

We might combine them in the future, but there's some cleanup required. See
https://crbug.com/616870 (#1 and #2)

[krasin, 2016-06-02 20:56:32]

And yes, I considered bundling them. In the longer term, it seems to be a better
option.

[Nico, 2016-06-02 20:57:48]

Is there a lot of value in having this toggle? What about 1. fix, 2. turn on by
default if ubsan is enabled? That way we don't grow more config options, and it
sounds like a) we can't turn this on as long as mesa isn't fixed b) we'll turn
it on immediately once we can. So I don't see the value in having a toggle for
this (?)

[krasin, 2016-06-02 21:03:03]

On 2016/06/02 20:57:48, Nico wrote:
> Is there a lot of value in having this toggle? What about 1. fix, 2. turn on
by
> default if ubsan is enabled? That way we don't grow more config options, and
it
> sounds like a) we can't turn this on as long as mesa isn't fixed b) we'll turn
> it on immediately once we can. So I don't see the value in having a toggle for
> this (?)

This toggle might help me to cleanup, as I will not have to apply a patch each
time I work on a yet another fix.
It will also provide me with a way to give a reproducer for issues like
https://crbug.com/616268

[krasin, 2016-06-02 21:04:26]

The second part is important for the cleanup, as some of the cases will require
code owners to fix / provide an advice.

[Nico, 2016-06-02 21:06:09]

https://codereview.chromium.org/2028383006/diff/1/build/config/sanitizers/BUI...
File build/config/sanitizers/BUILD.gn (right):

https://codereview.chromium.org/2028383006/diff/1/build/config/sanitizers/BUI...
build/config/sanitizers/BUILD.gn:128: ldflags += [ "-fsanitize=undefined" ]
but is having to set a toggle really easier than just adding -fsanitize=null to
this line locally while working on this?

[Dirk Pranke, 2016-06-02 21:06:34]

On 2016/06/02 21:04:26, krasin wrote:
> The second part is important for the cleanup, as some of the cases will
require
> code owners to fix / provide an advice.

Okay, I'm fine w/ adding this for now as long as we make real progress towards
consolidating soon.

lgtm.

[krasin, 2016-06-02 21:10:36]

On 2016/06/02 21:06:09, Nico wrote:
>
https://codereview.chromium.org/2028383006/diff/1/build/config/sanitizers/BUI...
> File build/config/sanitizers/BUILD.gn (right):
> 
>
https://codereview.chromium.org/2028383006/diff/1/build/config/sanitizers/BUI...
> build/config/sanitizers/BUILD.gn:128: ldflags += [ "-fsanitize=undefined" ]
> but is having to set a toggle really easier than just adding -fsanitize=null
to
> this line locally while working on this?

Requiring people to patch (even a single-line change) in order to reproduce an
issue is definitely harder.
Myself, I could probably live with that (i.e. it's harder even for me but to a
lesser degree)

[commit-bot: I haz the power, 2016-06-02 23:01:11]

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

[commit-bot: I haz the power, 2016-06-02 23:01:12]

Dry run: This issue passed the CQ dry run.

[krasin, 2016-06-02 23:01:40]

The CQ bit was checked by krasin@google.com

[commit-bot: I haz the power, 2016-06-02 23:03:02]

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

[commit-bot: I haz the power, 2016-06-02 23:11:01]

Description was changed from

==========
Add GN configs for -fsanitizer=null.

This allows to sanitize null pointer accesses and
could increase readability of the messages from UBSan Vptr bot
(once enabled there).

BUG=616268,616870
==========

to

==========
Add GN configs for -fsanitizer=null.

This allows to sanitize null pointer accesses and
could increase readability of the messages from UBSan Vptr bot
(once enabled there).

BUG=616268,616870
==========

[commit-bot: I haz the power, 2016-06-02 23:11:02]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-06-02 23:13:32]

Description was changed from

==========
Add GN configs for -fsanitizer=null.

This allows to sanitize null pointer accesses and
could increase readability of the messages from UBSan Vptr bot
(once enabled there).

BUG=616268,616870
==========

to

==========
Add GN configs for -fsanitizer=null.

This allows to sanitize null pointer accesses and
could increase readability of the messages from UBSan Vptr bot
(once enabled there).

BUG=616268,616870

Committed: https://crrev.com/2ec1834c98f0fd1caf86983f31d0d1a122e0ad1e
Cr-Commit-Position: refs/heads/master@{#397538}
==========

[commit-bot: I haz the power, 2016-06-02 23:13:33]

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