Closed Bug 812218 Opened 7 years ago Closed 7 years ago

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


(Firefox Build System :: General, defect)

Not set


(Not tracked)



(Reporter: Waldo, Assigned: Waldo)




(1 file)

Attached patch PatchSplinter Review
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:
Attachment #682051 - Flags: review?(khuey)
Comment on attachment 682051 [details] [diff] [review]

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

We should be on the lookout for regressions from fiddling with the order here.
Attachment #682051 - Flags: review?(khuey) → review+
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.
Target Milestone: --- → mozilla20
This unfortunately broken Windows PGO builds:

   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.

Target Milestone: mozilla20 → ---
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.
Blocks: 818689
Regarding comment 4, if this didn't break PGO, can this go back in?
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.
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:

Maybe user error in the previous attempt?  I dunno.  Anyway, if this keeps panning out, definitely this is going back in posthaste.  :-)
Do note that Try won't do PGO builds for you unless you force it to.
Bleh.  Repushed with PGO on to try:

RyanVM tells me that's green on the right stuff to land, so pushed.  Fingers crossed!
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.