812218 - User-specified CFLAGS/CXXFLAGS should be applied at the end of the commandline, not in the middle

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Jeff Walden [:Waldo]]

Attachment: Patch

b2g ends up using CXXFLAGS to include a header, which eventually drags in <stdint.h>.  In bug 730805 I'm trying to define the various __STDC_*_MACROS for all Gecko code, done in mozilla-config.h.  That include is done on the command line -- but it currently is *after* CXXFLAGS.  So when <stdint.h> is included, __STDC_*_MACROS isn't yet defined, so the features hidden behind those ifdefs aren't enabled, which breaks stuff on b2g.  (And only b2g, for what it's worth.)

The fundamental problem is that CXXFLAGS and CFLAGS similarly are being applied in the middle of the commandline -- after some defaults have been specified but before others.  They should instead be applied at the end, so that no matter the semantics CXXFLAGS/CFLAGS specify, they can't prevent defaults from applying correctly.  (The user could change those default semantics, sure, but by that point it's the user's fault if he breaks stuff.)

Try run:

https://tbpl.mozilla.org/?tree=Try&rev=7a439c4efd18

Comment 1

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 682051 [details] [diff] [review]
Patch

Review of attachment 682051 [details] [diff] [review]:
-----------------------------------------------------------------

We should be on the lookout for regressions from fiddling with the order here.

Comment 2

[Jeff Walden [:Waldo]]

Definitely.  Bug 730805 is ready to land along with this, but prudence does suggest holding off on landing that just yet, until anything lurking issues from this get smoked out.  So I landed this alone for now, and maybe in a week or so I'll go back to that bug and land it.

https://hg.mozilla.org/integration/mozilla-inbound/rev/9f8721399b14

Comment 3

[Ed Morley [:emorley]]

This unfortunately broken Windows PGO builds:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&jobname=WINNT%205.2%20mozilla-inbound%20pgo-build&fromchange=0c3ec1f82732&tochange=b44293987106

eg:
{
   Creating library xul.lib and object xul.exp
LINK : warning LNK4098: defaultlib 'LIBCMT' conflicts with use of other libs; use /NODEFAULTLIB:library
Generating code
e:\builds\moz2_slave\m-in-w32-pgo\build\layout\style\nscssparser.cpp(1071) : warning C4748: /GS can not protect parameters and local variables from local buffer overrun because optimizations are disabled in function
Finished generating code
PGOCVT : fatal error PG0001: An unexpected internal error was detected in source file 'f:\dd\vctools\compiler\utc\src\tools\pogo\cvtpgd\cvtpgd.cpp', line 800.
PGOCVT : fatal error PG0001: An unexpected internal error was detected in source file 'f:\dd\vctools\compiler\utc\src\tools\pogo\cvtpgd\cvtpgd.cpp', line 858.
}

Backout:
https://hg.mozilla.org/integration/mozilla-inbound/rev/38f5af022bf7

Comment 4

[Ed Morley [:emorley]]

Just to add to what philor said in #developers, the error strings in comment 3 are apparently present in all PGO builds (I've filed bug 813989 for this); so the causes of the failure here is due to the crash later on in the build that philor mentioned.

Comment 5

[alanjstr]

Regarding comment 4, if this didn't break PGO, can this go back in?

Comment 6

[Jeff Walden [:Waldo]]

I thought the claim was that it *did* break PGO, and so couldn't go back in.

The semi-recent Windows PGO fiddling motivated me to retest this and see if it'd work, a few months ago.  It didn't.  Random other stuff broke, and I didn't have time to investigate the failures.  :-\  So this is in a state of limbo until I can find said time.

Comment 7

[Jeff Walden [:Waldo]]

So, on a lark I repushed this to try, and it's significantly greener (and apparent-green-promising) thus far than I remember it being the last time I tried it:

https://tbpl.mozilla.org/?tree=Try&rev=74b327094a5b

Maybe user error in the previous attempt?  I dunno.  Anyway, if this keeps panning out, definitely this is going back in posthaste.  :-)

Comment 8

[(not currently active) Ted Mielczarek]

Do note that Try won't do PGO builds for you unless you force it to.

Comment 9

[Jeff Walden [:Waldo]]

Bleh.  Repushed with PGO on to try:

https://tbpl.mozilla.org/?tree=Try&rev=1c2a29db9d88

RyanVM tells me that's green on the right stuff to land, so pushed.  Fingers crossed!

https://hg.mozilla.org/integration/mozilla-inbound/rev/00e7c35894e2

Comment 10

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/00e7c35894e2