Fix ASLR Windows build configuration

Fix ASLR Windows build configuration

1) Actually turn off ASLR for Debug build.
   Despite the comments, this was not happening in reality
   because the /dynamicbase linker flag overrode the
   VCLinkerTool.RandomizedBaseAddress setting.

2) Make the ASLR setting configurable from outside
   (target usage is for particular Dr. Memory builds).

TEST=Built googleurl_unittests and ensured its ALSR setting matched that requested, both for default and for custom via new gyp var

BUG=109767
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=117533

[Sigurður Ásgeirsson, 2012-01-10 20:18:34]

lgtm - though it'd be better to be consistent in assigning and comparing either
numerically or as string.

http://codereview.chromium.org/9169020/diff/1/build/common.gypi
File build/common.gypi (right):

http://codereview.chromium.org/9169020/diff/1/build/common.gypi#newcode987
build/common.gypi:987: 'win_release_RandomizedBaseAddress%': '1',
You set this to a stringified-one '1' here, but the test is against a numeric
one.

http://codereview.chromium.org/9169020/diff/1/build/common.gypi#newcode1539
build/common.gypi:1539: ['win_debug_RandomizedBaseAddress==1', {
did you intend 'win_debug_RandomizedBaseAddress=="1"', or should the default
value assignment above perhaps be numeric?

[Derek Bruening, 2012-01-10 20:48:52]

BTW, siggi you checked in the original code to disable ASLR: did it work?  It
looks like /dynamicbase was added earlier so if ASLR was truly disabled by your
commit then I have no explanation for what the difference is between then and
now.

http://codereview.chromium.org/9169020/diff/1/build/common.gypi
File build/common.gypi (right):

http://codereview.chromium.org/9169020/diff/1/build/common.gypi#newcode987
build/common.gypi:987: 'win_release_RandomizedBaseAddress%': '1',
On 2012/01/10 20:18:34, Ruðrugis wrote:
> You set this to a stringified-one '1' here, but the test is against a numeric
> one.
Right, I will change this to numeric.

http://codereview.chromium.org/9169020/diff/1/build/common.gypi#newcode1539
build/common.gypi:1539: ['win_debug_RandomizedBaseAddress==1', {
On 2012/01/10 20:18:34, Ruðrugis wrote:
> did you intend 'win_debug_RandomizedBaseAddress=="1"', or should the default
> value assignment above perhaps be numeric?

Above should be numeric.

[M-A Ruel, 2012-01-10 21:02:00]

this can be checked in but I'm still sad about the current state of affairs.
lgtm

http://codereview.chromium.org/9169020/diff/1/build/common.gypi
File build/common.gypi (right):

http://codereview.chromium.org/9169020/diff/1/build/common.gypi#newcode987
build/common.gypi:987: 'win_release_RandomizedBaseAddress%': '1',
I must say I'm unhappy about that, that's why I filed http://crbug.com/109780.

Why not just create a new configuration for Dr Memory instead? All these
settings shouldn't be settings, I want to do the exact reverse. :/

[Sigurður Ásgeirsson, 2012-01-10 21:34:58]

On Tue, Jan 10, 2012 at 15:48, <bruening@chromium.org> wrote:

> BTW, siggi you checked in the original code to disable ASLR: did it work?
>  It
> looks like /dynamicbase was added earlier so if ASLR was truly disabled by
> your
> commit then I have no explanation for what the difference is between then
> and
> now.
>
> I remember testing to see that the bit was off after a rebuild, but I
don't think I did any testing outside that. It's possible that there was an
interaction with incremental linking or such.


>
>
>
http://codereview.chromium.**org/9169020/diff/1/build/**common.gypi<http://co...
> File build/common.gypi (right):
>
> http://codereview.chromium.**org/9169020/diff/1/build/**
>
common.gypi#newcode987<http://codereview.chromium.org/9169020/diff/1/build/common.gypi#newcode987>
> build/common.gypi:987: 'win_release_**RandomizedBaseAddress%': '1',
> On 2012/01/10 20:18:34, Ruðrugis wrote:
>
>> You set this to a stringified-one '1' here, but the test is against a
>>
> numeric
>
>> one.
>>
> Right, I will change this to numeric.
>
>
> http://codereview.chromium.**org/9169020/diff/1/build/**
>
common.gypi#newcode1539<http://codereview.chromium.org/9169020/diff/1/build/common.gypi#newcode1539>
> build/common.gypi:1539: ['win_debug_**RandomizedBaseAddress==1', {
> On 2012/01/10 20:18:34, Ruðrugis wrote:
>
>> did you intend 'win_debug_**RandomizedBaseAddress=="1"', or should the
>>
> default
>
>> value assignment above perhaps be numeric?
>>
>
> Above should be numeric.
>
>
http://codereview.chromium.**org/9169020/<http://codereview.chromium.org/9169...
>

[Derek Bruening, 2012-01-10 21:36:35]

On 2012/01/10 21:02:00, Marc-Antoine Ruel wrote:
> build/common.gypi:987: 'win_release_RandomizedBaseAddress%': '1',
> Why not just create a new configuration for Dr Memory instead? All these
> settings shouldn't be settings, I want to do the exact reverse. :/

I see what you're saying.  A new configuration could work.  Here I'm following
what's been done in the past, where
build/scripts/master/factory/chromium_factory.py sets some GYP vars in order to
tweak default-config builds for tools.  Let's discuss w/ those who set all that
up in the issue you filed, for future changes.

[commit-bot: I haz the power, 2012-01-11 01:14:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bruening@chromium.org/9169020/3

[commit-bot: I haz the power, 2012-01-11 02:17:31]

Try job failure for 9169020-3 (retry) on linux_rel for step "browser_tests".
It's a second try, previously, step "browser_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2012-01-11 15:05:05]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bruening@chromium.org/9169020/3

[commit-bot: I haz the power, 2012-01-11 15:43:27]

Try job failure for 9169020-3 (retry) on mac_rel for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2012-01-11 15:45:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bruening@chromium.org/9169020/3

[M-A Ruel, 2012-01-11 15:45:42]

On 2012/01/11 15:43:27, I haz the power (commit-bot) wrote:
> Try job failure for 9169020-3 (retry) on mac_rel for step "compile" (clobber
> build).
> It's a second try, previously, step "compile" failed.
>
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

*sigh*

[Derek Bruening, 2012-01-11 16:17:28]

On 2012/01/11 15:45:42, Marc-Antoine Ruel wrote:
> On 2012/01/11 15:43:27, I haz the power (commit-bot) wrote:
> > Try job failure for 9169020-3 (retry) on mac_rel for step "compile" (clobber
> > build).
> > It's a second try, previously, step "compile" failed.
> >
>
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...
> 
> *sigh*

At some point I'm going to directly check in...

[commit-bot: I haz the power, 2012-01-11 16:24:34]

Try job failure for 9169020-3 (retry) on mac_rel for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[M-A Ruel, 2012-01-11 16:28:40]

LINK : warning LNK4224: /OPT:NOWIN98 is no longer supported;  ignored
LINK : fatalerror LNK1295: '/FIXED' not compatible with '/DYNAMICBASE'
specification; link without '/FIXED'

Your change is invalid.

[commit-bot: I haz the power, 2012-01-12 20:08:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bruening@chromium.org/9169020/19003

[commit-bot: I haz the power, 2012-01-12 22:27:00]

Change committed as 117533