Copy asan_rtl.* and agent_logger.exe to the syzygy directory for the ASan builds.

Copy asan_rtl.* and agent_logger.exe to the syzygy directory for the ASan builds.

This is my second attempt to do this, the first time https://codereview.chromium.org/23460023/ has been reverted (https://codereview.chromium.org/23858006) because the new target 'copy_syzyasan_binaries' was not build, so the SyzyASan binaries were missing from the syzygy dir...

Currently the syzygy_optimize script was taking care of copying asan_rtl.[dll|pdb] to the syzygy directory, this was causing 2 warnings when building a SyzyASan splitted build with ninja:

- ninja: warning: multiple rules generate syzygy\asan_rtl.dll. builds involving this target will not be correct; continuing anyway
- ninja: warning: multiple rules generate syzygy\asan_rtl.dll.pdb. builds involving this target will not be correct; continuing anyway

The multiple rules come from the fact that both syzygy\chrome.dll and syzygy\chrome_child.dll were trying to generate those artifacts.

I've also added agent_logger.exe to the syzygy directory, with this it'll be easier for the person grabing a SyzyASan build from the LKGR builder to use it.

I've also removed all the "--agent-dll" logic from syzygy_instrument.py as it was only copying the asan binaries to the syzygy directory, it's now done directly in the 'copy_syzyasan_binaries' gyp target.

R=chrisha@chromium.org
BUG=

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=229085

[Sébastien Marchand, 2013-10-04 14:22:55]

PTAL.

[chrisha, 2013-10-04 15:39:30]

What's the difference between this CL and the last one? They both appear to use
the same approach. Why will it work this time? Did you ever track down *why* it
didn't work last time?

[Sébastien Marchand, 2013-10-04 19:04:26]

Yeah, it's almost the same code (the approach is the same). Roger, could you
take a look at this ? Last time I've tried to roll this the
copy_syzyasan_binaries target hasn't been generated... (this target takes care
of copying the binaries), but the target chrome_dll_syzygy is generated, and
have a direct dependency on copy_syzyasan_binaries ....

[Roger McFarlane (Chromium), 2013-10-04 20:57:30]

This looks like it should work to me. You've tried it with a local buildbot?

https://codereview.chromium.org/25890006/diff/1/chrome/chrome_syzygy.gyp
File chrome/chrome_syzygy.gyp (right):

https://codereview.chromium.org/25890006/diff/1/chrome/chrome_syzygy.gyp#newc...
chrome/chrome_syzygy.gyp:66: ['OS=="win" and asan==1', {
this section should probably be first, so the dependencies can reference it
directly.

https://codereview.chromium.org/25890006/diff/1/chrome/chrome_syzygy.gypi
File chrome/chrome_syzygy.gypi (right):

https://codereview.chromium.org/25890006/diff/1/chrome/chrome_syzygy.gypi#new...
chrome/chrome_syzygy.gypi:65:
'<(DEPTH)/chrome/chrome_syzygy.gyp:copy_syzyasan_binaries',
this gypi is included just before this target gets defined. with a small reorder
you should be able to reference the target directly.

[Sébastien Marchand, 2013-10-07 18:57:49]

I've tried this locally, it works fine with ninja, but not with MSVC, here's the
commands run on the builder:
1) c:\Program Files (x86)\Microsoft Visual Studio 10.0\Common7\IDE\devenv.com"
D:\src\chrome\src\build\all.sln /Build Release /Project chrome_official_builder
2) c:\Program Files (x86)\Microsoft Visual Studio 10.0\Common7\IDE\devenv.com"
D:\src\chrome\src\build\all.sln /Build Release /Project All_syzygy

Adding a dependency to copy_syzyasan_binaries in all.gyp fix the issue but I'm
not sure that this is the right way to do this...

https://codereview.chromium.org/25890006/diff/1/chrome/chrome_syzygy.gyp
File chrome/chrome_syzygy.gyp (right):

https://codereview.chromium.org/25890006/diff/1/chrome/chrome_syzygy.gyp#newc...
chrome/chrome_syzygy.gyp:66: ['OS=="win" and asan==1', {
On 2013/10/04 20:57:30, Roger McFarlane (Chromium) wrote:
> this section should probably be first, so the dependencies can reference it
> directly.

Done.

https://codereview.chromium.org/25890006/diff/1/chrome/chrome_syzygy.gypi
File chrome/chrome_syzygy.gypi (right):

https://codereview.chromium.org/25890006/diff/1/chrome/chrome_syzygy.gypi#new...
chrome/chrome_syzygy.gypi:65:
'<(DEPTH)/chrome/chrome_syzygy.gyp:copy_syzyasan_binaries',
On 2013/10/04 20:57:30, Roger McFarlane (Chromium) wrote:
> this gypi is included just before this target gets defined. with a small
reorder
> you should be able to reference the target directly.

Done, however I'm not sure that it was the issue, as this was working fine
locally...

[Sébastien Marchand, 2013-10-09 15:45:42]

PTAL, this is the best way I've found to make sure that the
copy_syzyasan_binaries target is build (when using msvs)

[Roger McFarlane (Chromium), 2013-10-09 15:47:21]

lgtm

[chrisha, 2013-10-09 15:48:26]

Are we sure that's the best way? Have we tried moving the dependency down the
chain, one step at a time to see where it breaks? It seems awfully flaky to
me...

(ie: Move it into mini_installer_syzygy.gyp, and see if that works. If it does,
move into the next dependency, etc.)

[Sébastien Marchand, 2013-10-15 20:57:20]

So it look like the problem that gyp doesn't like to have a dependency in an
'actions' section. I've moved the dependency to copy_syzyasan_binaries out of it
and the dependencies in all.sln look ok (I'll do a local build to confirm this).

PTAnL

[Sébastien Marchand, 2013-10-16 16:11:19]

ping ?

[chrisha, 2013-10-16 16:24:05]

lgtm

[Sébastien Marchand, 2013-10-16 19:23:37]

Thanks, committing.

[commit-bot: I haz the power, 2013-10-16 19:26:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sebmarchand@chromium.org/25890006/22001

[commit-bot: I haz the power, 2013-10-16 20:39:28]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sebmarchand@chromium.org/25890006/22001

[commit-bot: I haz the power, 2013-10-16 21:22:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sebmarchand@chromium.org/25890006/22001

[commit-bot: I haz the power, 2013-10-17 01:31:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sebmarchand@chromium.org/25890006/22001

[commit-bot: I haz the power, 2013-10-17 08:59:37]

Change committed as 229085