Use a GN arg for controlling linker map file generation.

Use a GN arg for controlling linker map file generation.

Map files aren't generated on Linux by default now that linker map file
generation is hooked into !use_lld (https://crbug.com/716209).

This CL adds a GN arg, enable_linker_map, that defaults to true for
Android and makes it easy to generate map files on Linux.

BUG=717550, 716209
Review-Url: https://codereview.chromium.org/2888623003
Cr-Commit-Position: refs/heads/master@{#472834}
Committed: https://chromium.googlesource.com/chromium/src/+/0f24feacbffe77af7d69f64e739c73d834fa19b4

[estevenson, 2017-05-17 14:00:36]

estevenson@chromium.org changed reviewers:
+ agrieve@chromium.org

[estevenson, 2017-05-17 14:00:37]

ptal!

In ps1 I removed all old instances of "enable_linker_map = true" but then the
build fails (on some nacl thing).

[agrieve, 2017-05-17 14:29:23]

Description was changed from

==========
Use a GN arg for controlling linker map file generation.

Map files aren't generated on Linux by default now that linker map file
generation is hooked into !use_lld (https://crbug.com/716209).

This CL adds a GN arg, enable_linker_map, that defaults to true for
Android and makes it easy to generate map files on Linux.

BUG=717550
==========

to

==========
Use a GN arg for controlling linker map file generation.

Map files aren't generated on Linux by default now that linker map file
generation is hooked into !use_lld (https://crbug.com/716209).

This CL adds a GN arg, enable_linker_map, that defaults to true for
Android and makes it easy to generate map files on Linux.

BUG=717550,716209
==========

[agrieve, 2017-05-17 14:33:11]

lgtm. Might make sense to update diagnose_bloat.py to add that gn arg in this
change as well though?

https://codereview.chromium.org/2888623003/diff/20001/build/toolchain/gcc_too...
File build/toolchain/gcc_toolchain.gni (left):

https://codereview.chromium.org/2888623003/diff/20001/build/toolchain/gcc_too...
build/toolchain/gcc_toolchain.gni:257: # Currently disabled on LLD because of a
bug (fixed upstream).
nit: can you preserve this comment by moving it to the declare_args() for
enable_linker_map

https://codereview.chromium.org/2888623003/diff/20001/build/toolchain/toolcha...
File build/toolchain/toolchain.gni (right):

https://codereview.chromium.org/2888623003/diff/20001/build/toolchain/toolcha...
build/toolchain/toolchain.gni:45: "Linker map files can only be generated when
is_official_build = true")
nit: can -> should

[estevenson, 2017-05-17 23:02:08]

Another option would be to forgo the GN arg altogether and just set
enable_linker_map like this:

enable_linker_map = defined(invoker.enable_linker_map) &&
invoker.enable_linker_map && (is_android || allow_posix_link_time_opt == false).

Any preference?

https://codereview.chromium.org/2888623003/diff/20001/build/toolchain/gcc_too...
File build/toolchain/gcc_toolchain.gni (left):

https://codereview.chromium.org/2888623003/diff/20001/build/toolchain/gcc_too...
build/toolchain/gcc_toolchain.gni:257: # Currently disabled on LLD because of a
bug (fixed upstream).
On 2017/05/17 14:33:11, agrieve wrote:
> nit: can you preserve this comment by moving it to the declare_args() for
> enable_linker_map

Done.

https://codereview.chromium.org/2888623003/diff/20001/build/toolchain/toolcha...
File build/toolchain/toolchain.gni (right):

https://codereview.chromium.org/2888623003/diff/20001/build/toolchain/toolcha...
build/toolchain/toolchain.gni:45: "Linker map files can only be generated when
is_official_build = true")
On 2017/05/17 14:33:11, agrieve wrote:
> nit: can -> should

Done.

[agrieve, 2017-05-17 23:19:25]

I think my preference is to keep them separate, since then we'll be able to test
with & without lto by setting flags.

[estevenson, 2017-05-18 14:50:08]

The CQ bit was checked by estevenson@chromium.org

[estevenson, 2017-05-18 14:50:08]

The patchset sent to the CQ was uploaded after l-g-t-m from agrieve@chromium.org
Link to the patchset: https://codereview.chromium.org/2888623003/#ps40001
(title: "agrieve comments")

[commit-bot: I haz the power, 2017-05-18 14:50:32]

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-18 16:17:51]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1495119008250330,
"parent_rev": "40c27d78b91fc87eb549e263640e66d8a3f4ba67", "commit_rev":
"0f24feacbffe77af7d69f64e739c73d834fa19b4"}

[commit-bot: I haz the power, 2017-05-18 16:18:04]

Description was changed from

==========
Use a GN arg for controlling linker map file generation.

Map files aren't generated on Linux by default now that linker map file
generation is hooked into !use_lld (https://crbug.com/716209).

This CL adds a GN arg, enable_linker_map, that defaults to true for
Android and makes it easy to generate map files on Linux.

BUG=717550,716209
==========

to

==========
Use a GN arg for controlling linker map file generation.

Map files aren't generated on Linux by default now that linker map file
generation is hooked into !use_lld (https://crbug.com/716209).

This CL adds a GN arg, enable_linker_map, that defaults to true for
Android and makes it easy to generate map files on Linux.

BUG=717550,716209

Review-Url: https://codereview.chromium.org/2888623003
Cr-Commit-Position: refs/heads/master@{#472834}
Committed:
https://chromium.googlesource.com/chromium/src/+/0f24feacbffe77af7d69f64e739c...
==========

[commit-bot: I haz the power, 2017-05-18 16:18:05]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/0f24feacbffe77af7d69f64e739c...