Update GN files w/ changes needed to make sanitizers work.

Update GN files w/ changes needed to make sanitizers work.

This is a set of build changes that are needed to make the
sanitizers (and sanitizer bots) work correctly. This change
should have no functional effect on the bots, since none of
the sanitizer bots are using GN yet; a follow-on CL will flip
the bots to GN.

R=brettw@chromium.org, inferno@chromium.org, thakis@chromium.org
BUG=605732, 573288
Committed: https://crrev.com/9713aa43a050e81c74689be9ac11294632f44f8f
Cr-Commit-Position: refs/heads/master@{#397952}

[Dirk Pranke, 2016-06-02 00:15:22]

Patchset #1 (id:1) has been deleted

[Dirk Pranke, 2016-06-02 00:16:34]

Please take a look.

I'm splitting this out separately from the CLs that will actually flip the bots
to GN, to make things a bit easier to land and revert as needed.

[Dirk Pranke, 2016-06-02 00:27:55]

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

[commit-bot: I haz the power, 2016-06-02 00:29:41]

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

[commit-bot: I haz the power, 2016-06-02 00:38:12]

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

[commit-bot: I haz the power, 2016-06-02 00:38:12]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_clang_dbg_recipe on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_compile_dbg on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  cast_shell_android on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  linux_android_rel_ng on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[inferno, 2016-06-02 14:35:30]

lgtm

[Nico, 2016-06-02 18:16:41]

Nice, thanks!

https://codereview.chromium.org/2031903002/diff/40001/build/config/sanitizers...
File build/config/sanitizers/BUILD.gn (right):

https://codereview.chromium.org/2031903002/diff/40001/build/config/sanitizers...
build/config/sanitizers/BUILD.gn:52: data += [
"//third_party/llvm-build/Release+Asserts/lib/libstdc++.so.6" ]
Can you add a comment here that says that this is needed by llvm-symbolizer?
(While libc++ is used for all of "our" binaries)

https://codereview.chromium.org/2031903002/diff/40001/build/config/sanitizers...
build/config/sanitizers/BUILD.gn:177: cflags += [
What's the motivation for this block? I can't find the corresponding bit in gyp.

(I can find
https://code.google.com/p/chromium/codesearch#chromium/src/build/common.gypi&...
for tsan which is probably still missing here?)

https://codereview.chromium.org/2031903002/diff/40001/build/config/sanitizers...
build/config/sanitizers/BUILD.gn:178: "-O1",
Wouldn't the gn way for this be to pick a different default optimization level
in the compiler config if symbolized is set, instead of adding -O1 here?
Depending on config ordering, this -O1 might be before or after the -O flag from
the compiler config. Since the last flag wins, it's not clear if this will have
an effect.

https://codereview.chromium.org/2031903002/diff/40001/build/config/sanitizers...
build/config/sanitizers/BUILD.gn:179: "-fno-line-functions",
This is supposed to be -fno-inline-functions I assume

[Dirk Pranke, 2016-06-02 18:47:08]

https://codereview.chromium.org/2031903002/diff/40001/build/config/sanitizers...
File build/config/sanitizers/BUILD.gn (right):

https://codereview.chromium.org/2031903002/diff/40001/build/config/sanitizers...
build/config/sanitizers/BUILD.gn:52: data += [
"//third_party/llvm-build/Release+Asserts/lib/libstdc++.so.6" ]
On 2016/06/02 18:16:41, Nico wrote:
> Can you add a comment here that says that this is needed by llvm-symbolizer?
> (While libc++ is used for all of "our" binaries)

Sure.

https://codereview.chromium.org/2031903002/diff/40001/build/config/sanitizers...
build/config/sanitizers/BUILD.gn:177: cflags += [
On 2016/06/02 18:16:41, Nico wrote:
> What's the motivation for this block? I can't find the corresponding bit in
gyp.
> 
> (I can find
>
https://code.google.com/p/chromium/codesearch#chromium/src/build/common.gypi&...
> for tsan which is probably still missing here?)

These flags get passed directly in the 'release_extra_cflags' value of
GYP_DEFINES on the "symbolized" bots on chromium.lkgr, e.g.:

https://build.chromium.org/p/chromium.lkgr/builders/ASAN%20Release%20%28symbo...

I think the release_extra_cflags escape valve is a misfeature, which is why I
added the 'symbolized' config to MB and the GN files.

I'll look at the gyp block and see what might need to be merged in.

https://codereview.chromium.org/2031903002/diff/40001/build/config/sanitizers...
build/config/sanitizers/BUILD.gn:178: "-O1",
On 2016/06/02 18:16:41, Nico wrote:
> Wouldn't the gn way for this be to pick a different default optimization level
> in the compiler config if symbolized is set, instead of adding -O1 here?
> Depending on config ordering, this -O1 might be before or after the -O flag
from
> the compiler config. Since the last flag wins, it's not clear if this will
have
> an effect.

Hm, probably. Good catch. I'll look at reworking this.

https://codereview.chromium.org/2031903002/diff/40001/build/config/sanitizers...
build/config/sanitizers/BUILD.gn:179: "-fno-line-functions",
On 2016/06/02 18:16:41, Nico wrote:
> This is supposed to be -fno-inline-functions I assume

Yup, that's a typo. Another good catch.

[Dirk Pranke, 2016-06-03 04:14:28]

Description was changed from

==========
Update GN files w/ changes needed to make sanitizers work.

This is a set of build changes that are needed to make the
sanitizers (and sanitizer bots) work correctly. This change
should have no functional effect on the bots, since none of
the sanitizer bots are using GN yet; a follow-on CL will flip
the bots to GN.

R=brettw@chromium.org, inferno@chromium.org, thakis@chromium.org
BUG=605732, 573288
==========

to

==========
Update GN files w/ changes needed to make sanitizers work.

This is a set of build changes that are needed to make the
sanitizers (and sanitizer bots) work correctly. This change
should have no functional effect on the bots, since none of
the sanitizer bots are using GN yet; a follow-on CL will flip
the bots to GN.

R=brettw@chromium.org, inferno@chromium.org, thakis@chromium.org
BUG=605732, 573288
==========

[Dirk Pranke, 2016-06-03 04:14:28]

dpranke@chromium.org changed reviewers:
+ krasin@chromium.org, mbarbella@chromium.org

[Dirk Pranke, 2016-06-03 04:16:27]

Okay, after conversations w/ krasin@ and mbarbella@, we decided that the
"(symbolized)" configs (and bots) were not that useful and we'll shut them down
rather than migrate them to GN.

This simplifies the patch quite a bit. 

I've also added more comments.

Please take another look?

[Dirk Pranke, 2016-06-03 04:17:29]

https://codereview.chromium.org/2031903002/diff/60001/build/common.gypi
File build/common.gypi (left):

https://codereview.chromium.org/2031903002/diff/60001/build/common.gypi#oldco...
build/common.gypi:2293: ['build_for_tool=="memcheck" or build_for_tool=="tsan"',
{
The best we can tell, "memcheck" is now gone (no more valgrind bots) and it's
unclear whether we ever used build_for_tool="tsan", but we don't now, so this
whole block was dead code.

[krasin, 2016-06-03 04:43:51]

krasin@google.com changed reviewers:
+ krasin@google.com

[krasin, 2016-06-03 04:43:52]

LGTM

https://codereview.chromium.org/2031903002/diff/60001/build/common.gypi
File build/common.gypi (left):

https://codereview.chromium.org/2031903002/diff/60001/build/common.gypi#oldco...
build/common.gypi:2293: ['build_for_tool=="memcheck" or build_for_tool=="tsan"',
{
On 2016/06/03 04:17:29, Dirk Pranke wrote:
> The best we can tell, "memcheck" is now gone (no more valgrind bots) and it's
> unclear whether we ever used build_for_tool="tsan", but we don't now, so this
> whole block was dead code.

I confirm that (spoken with eugenis@)

[brettw, 2016-06-03 22:56:29]

lgtm

[brettw, 2016-06-03 22:58:07]

Don't add the default config and instead add it to shared_library_config, etc.

[Dirk Pranke, 2016-06-05 01:41:17]

Patchset #4 (id:80001) has been deleted

[Dirk Pranke, 2016-06-05 01:41:31]

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

[commit-bot: I haz the power, 2016-06-05 01:41:44]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2031903002/100001

[commit-bot: I haz the power, 2016-06-05 02:53:14]

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

[commit-bot: I haz the power, 2016-06-05 02:53:15]

Dry run: This issue passed the CQ dry run.

[Dirk Pranke, 2016-06-05 22:24:33]

The CQ bit was checked by dpranke@chromium.org

[Dirk Pranke, 2016-06-05 22:24:34]

The patchset sent to the CQ was uploaded after l-g-t-m from
inferno@chromium.org, krasin@google.com, brettw@chromium.org
Link to the patchset: https://codereview.chromium.org/2031903002/#ps100001
(title: "move buildconfig changes to build/config")

[commit-bot: I haz the power, 2016-06-05 22:24:46]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2031903002/100001

[commit-bot: I haz the power, 2016-06-05 22:28:24]

Description was changed from

==========
Update GN files w/ changes needed to make sanitizers work.

This is a set of build changes that are needed to make the
sanitizers (and sanitizer bots) work correctly. This change
should have no functional effect on the bots, since none of
the sanitizer bots are using GN yet; a follow-on CL will flip
the bots to GN.

R=brettw@chromium.org, inferno@chromium.org, thakis@chromium.org
BUG=605732, 573288
==========

to

==========
Update GN files w/ changes needed to make sanitizers work.

This is a set of build changes that are needed to make the
sanitizers (and sanitizer bots) work correctly. This change
should have no functional effect on the bots, since none of
the sanitizer bots are using GN yet; a follow-on CL will flip
the bots to GN.

R=brettw@chromium.org, inferno@chromium.org, thakis@chromium.org
BUG=605732, 573288
==========

[commit-bot: I haz the power, 2016-06-05 22:28:26]

Committed patchset #4 (id:100001)

[commit-bot: I haz the power, 2016-06-05 22:30:09]

Description was changed from

==========
Update GN files w/ changes needed to make sanitizers work.

This is a set of build changes that are needed to make the
sanitizers (and sanitizer bots) work correctly. This change
should have no functional effect on the bots, since none of
the sanitizer bots are using GN yet; a follow-on CL will flip
the bots to GN.

R=brettw@chromium.org, inferno@chromium.org, thakis@chromium.org
BUG=605732, 573288
==========

to

==========
Update GN files w/ changes needed to make sanitizers work.

This is a set of build changes that are needed to make the
sanitizers (and sanitizer bots) work correctly. This change
should have no functional effect on the bots, since none of
the sanitizer bots are using GN yet; a follow-on CL will flip
the bots to GN.

R=brettw@chromium.org, inferno@chromium.org, thakis@chromium.org
BUG=605732, 573288

Committed: https://crrev.com/9713aa43a050e81c74689be9ac11294632f44f8f
Cr-Commit-Position: refs/heads/master@{#397952}
==========

[commit-bot: I haz the power, 2016-06-05 22:30:10]

Patchset 4 (id:??) landed as
https://crrev.com/9713aa43a050e81c74689be9ac11294632f44f8f
Cr-Commit-Position: refs/heads/master@{#397952}