Disable the SyzyASan instrumentation of chrome_child.dll for the official builds.

Disable the SyzyASan instrumentation of chrome_child.dll for the official builds.

It allows us to run an experiment where only chrome.dll is instrumented.

TBR=chrisha@chromium.org, gab@chromium.org
CC=laforge@chromium.org
BUG=

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

[Sébastien Marchand, 2013-09-07 23:33:57]

PTAL (I'll add more comment in a subsequent CL, this is a P0 CL).

[commit-bot: I haz the power, 2013-09-07 23:34:13]

No LGTM from a valid reviewer yet. Only full committers are accepted.
Even if an LGTM may have been provided, it was from a non-committer or
a lowly provisional committer, _not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[chrisha, 2013-09-08 00:53:40]

Thanks for grabbing this, Seb.

Lgtm
On Sep 7, 2013 4:33 PM, <sebmarchand@chromium.org> wrote:

> Reviewers: chrisha, gab,
>
> Message:
> PTAL (I'll add more comment in a subsequent CL, this is a P0 CL).
>
> Description:
> Disable the SyzyASan instrumentation of chrome_child.dll for the official
> builds.
>
> It allows us to run an experiment where only chrome.dll is instrumented.
>
> TBR=chrisha@chromium.org, gab@chromium.org
> CC=laforge@chromium.org
> BUG=
>
> Please review this at
https://codereview.chromium.**org/23719017/<https://codereview.chromium.org/2...
>
> SVN Base:
https://chromium.googlesource.**com/chromium/src.git@master<https://chromium....
>
> Affected files (+30, -1 lines):
>   M chrome/chrome_syzygy.gyp
>
>
> Index: chrome/chrome_syzygy.gyp
> diff --git a/chrome/chrome_syzygy.gyp b/chrome/chrome_syzygy.gyp
> index d4994ac1813c8432e8564dc25f1853**31d4a39f21..**
> 0801d18e62b43d9c9101ae5abbaa46**98b74bffd6 100644
> --- a/chrome/chrome_syzygy.gyp
> +++ b/chrome/chrome_syzygy.gyp
> @@ -19,7 +19,8 @@
>        ],
>      }],
>      # Note, not else.
> -    ['OS=="win" and fastbuild==0 and chrome_multiple_dll==1', {
> +    ['OS=="win" and fastbuild==0 and chrome_multiple_dll==1 and '
> +        '(asan!=1 or buildtype!="Official")', {
>        'variables': {
>          'dll_name': 'chrome_child',
>        },
> @@ -33,6 +34,34 @@
>            ],
>          },
>        ],
> +    }, {
> +      'conditions': [
> +        ['OS=="win" and fastbuild==0 and chrome_multiple_dll==1 and '
> +            'asan==1 and buildtype=="Official"', {
> +          'targets': [
> +          {
> +            'target_name': 'chrome_child_dll_syzygy',
> +            'type': 'none',
> +            'inputs': [
> +              '<(PRODUCT_DIR)/chrome_child.**dll',
> +              '<(PRODUCT_DIR)/chrome_child.**dll.pdb',
> +            ],
> +            'outputs': [
> +              '<(PRODUCT_DIR)/syzygy/chrome_**child.dll',
> +              '<(PRODUCT_DIR)/syzygy/chrome_**child.dll.pdb',
> +            ],
> +            'copies': [
> +              {
> +                'destination': '<(PRODUCT_DIR)/syzygy',
> +                'files': [
> +                  '<(PRODUCT_DIR)/chrome_child.**dll',
> +                  '<(PRODUCT_DIR)/chrome_child.**dll.pdb',
> +                ],
> +              },
> +            ],
> +          }],
> +        }],
> +      ],
>      }],
>    ],
>  }
>
>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[laforge, 2013-09-08 02:24:52]

lgtm

[commit-bot: I haz the power, 2013-09-08 02:24:56]

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

[commit-bot: I haz the power, 2013-09-08 23:45:49]

Change committed as 221961

[gab, 2013-09-10 18:27:02]

Post-commit review.

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

https://codereview.chromium.org/23719017/diff/1/chrome/chrome_syzygy.gyp#newc...
chrome/chrome_syzygy.gyp:39: ['OS=="win" and fastbuild==0 and
chrome_multiple_dll==1 and '
Feels like this would be cleaner if:

1) The condition on line 22 was only 'OS=="win" and fastbuild==0 and
chrome_multiple_dll==1'.
2) It had an internal condition '(asan!=1 or buildtype!="Official"'
3) This was the else block of that condition.

https://codereview.chromium.org/23719017/diff/1/chrome/chrome_syzygy.gyp#newc...
chrome/chrome_syzygy.gyp:59: ],
Which doesn't the main chrome DLL need to do this copy?

[gab, 2013-09-10 18:27:31]

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

https://codereview.chromium.org/23719017/diff/1/chrome/chrome_syzygy.gyp#newc...
chrome/chrome_syzygy.gyp:59: ],
On 2013/09/10 18:27:02, gab wrote:
> Which doesn't the main chrome DLL need to do this copy?

s/Which/Why

[Sébastien Marchand, 2013-09-10 18:58:56]

Response to Gab's comment/questions.

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

https://codereview.chromium.org/23719017/diff/1/chrome/chrome_syzygy.gyp#newc...
chrome/chrome_syzygy.gyp:39: ['OS=="win" and fastbuild==0 and
chrome_multiple_dll==1 and '
On 2013/09/10 18:27:02, gab wrote:
> Feels like this would be cleaner if:
> 
> 1) The condition on line 22 was only 'OS=="win" and fastbuild==0 and
> chrome_multiple_dll==1'.
> 2) It had an internal condition '(asan!=1 or buildtype!="Official"'
> 3) This was the else block of that condition.

Yeah, Chris is working on another CL that'll change all of this. See my response
to your other comment for the details.

https://codereview.chromium.org/23719017/diff/1/chrome/chrome_syzygy.gyp#newc...
chrome/chrome_syzygy.gyp:59: ],
On 2013/09/10 18:27:32, gab wrote:
> On 2013/09/10 18:27:02, gab wrote:
> > Which doesn't the main chrome DLL need to do this copy?
> 
> s/Which/Why

The purpose of this CL was to be able to run an experiment where only chrome.dll
is SyzyASan-instrumented. This allow us to ship a Canary/Dev build where only
the browser is instrumented, so the user experience is not impacted too much.

We're having a pretty good coverage for the renderer processes with Clusterfuzz,
but not so much for the browser, it's why we're interested in running this kind
of experiment.

Chris is working on a cleaner CL. He want to export fine-grained control for
each library with 'asan_chrome_dll' and 'asan_chrome_child_dll' variables, with
default values for those variables based on the value of asan and buildtype.