Purify support:...

Purify support:
* Add third_party/purify/pure_api.c to the base\base_lib.scons sources.
* Support specification of CHROME_BUILD_TYPE (and CHROMIUM_BUILD
  while we're here) either on the command line or as an
  external environment variable.
* Fix syntax error in release_impl_purify.scons (previously unused).
* Add a central $CHROMIUM_CC_OPT_FLAGS variable (included in $CCFLAGS)
  that can be used to set explicitly the optimization level without
  having to worry about optimization flags being pulled in from other
  settings files.

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

[Erik does not do reviews, 2008-11-21 23:48:43]

LGTM

http://codereview.chromium.org/11368/diff/5/205
File build/internal/release_impl_purify.scons (right):

http://codereview.chromium.org/11368/diff/5/205#newcode21
Line 21: CCFLAGS = ['/O2'],
This model seems error-prone.  Maybe there should be an "optimization level"
flag somewhere and then common code that fills in CCFLAGS.

http://codereview.chromium.org/11368/diff/5/205#newcode30
Line 30: '/OPT:ICF',       # VCLinkerTool.EnableCOMDATFolding="1"
It might be worth indicating that in this case "1" means disabled.

[tony, 2008-11-21 23:57:36]

LGTM

[sgk, 2008-11-22 00:07:15]

http://codereview.chromium.org/11368/diff/5/205
File build/internal/release_impl_purify.scons (right):

http://codereview.chromium.org/11368/diff/5/205#newcode21
Line 21: CCFLAGS = ['/O2'],
On 2008/11/21 23:48:43, Erik Kay wrote:
> This model seems error-prone.  Maybe there should be an "optimization level"
> flag somewhere and then common code that fills in CCFLAGS.

Good point.  FilterOut() actually won't error if /O2 doesn't exist, but it would
be cleaner to handle it centrally.  I'll resubmit a central flag.

http://codereview.chromium.org/11368/diff/5/205#newcode30
Line 30: '/OPT:ICF',       # VCLinkerTool.EnableCOMDATFolding="1"
On 2008/11/21 23:48:43, Erik Kay wrote:
> It might be worth indicating that in this case "1" means disabled.

Hmm, glad you mentioned this.  I may have gotten this wrong, but I'm not sure if
"1" does mean disabled.  The VS documentation for optFoldingTypeEnumeration
suggests:

    0   optFolding
    1   optFoldingDefault
    2   optNoFolding

(At least, if I'm reading it right, which isn't guaranteed.)

So to make sure I'm clear:  we want no folding (that is, /OPT:NOICF) when
building with Purify, yes?

[Erik does not do reviews, 2008-11-22 00:38:16]

http://codereview.chromium.org/11368/diff/5/205
File build/internal/release_impl_purify.scons (right):

http://codereview.chromium.org/11368/diff/5/205#newcode30
Line 30: '/OPT:ICF',       # VCLinkerTool.EnableCOMDATFolding="1"
On 2008/11/22 00:07:15, sgk wrote:
> On 2008/11/21 23:48:43, Erik Kay wrote:
> > It might be worth indicating that in this case "1" means disabled.
> 
> Hmm, glad you mentioned this.  I may have gotten this wrong, but I'm not sure
if
> "1" does mean disabled.  The VS documentation for optFoldingTypeEnumeration
> suggests:
> 
>     0   optFolding
>     1   optFoldingDefault
>     2   optNoFolding
> 
> (At least, if I'm reading it right, which isn't guaranteed.)
> 
> So to make sure I'm clear:  we want no folding (that is, /OPT:NOICF) when
> building with Purify, yes?

That's right, we want no folding.  Sorry, I didn't bother to look up the number.
I was just going by what I knew it was supposed to mean. ;-)  COMDAT folding
allows methods with identical code to be shared.  Normally a good thing, but
when we're trying to generate accurate (reproducible) backtraces, it's not so
good.

[sgk, 2008-11-22 00:41:40]

New upload with new $CHROMIUM_CC_OPT_FLAGS for central setting of the
optimization level and explicit /OPT:NOICF.

[Erik does not do reviews, 2008-11-22 00:45:38]

http://codereview.chromium.org/11368/diff/14/215
File build/internal/release_impl_purify.scons (right):

http://codereview.chromium.org/11368/diff/14/215#newcode31
Line 31: '/OPT:NOICF',     # VCLinkerTool.EnableCOMDATFolding="2"
do we need to replace the /OPT:ICF that came in from release.defaults?