Closed Bug 525677 Opened 10 years ago Closed 10 years ago

e10s builds broken in nsFrameLoader, conflict of <cmath> and <math.h> with jsnum.h

Categories

(Firefox Build System :: General, defect)

x86
Windows NT
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: benjamin, Assigned: fred23)

References

Details

Attachments

(3 files, 3 obsolete files)

The e10s builds are broken in scratchbox because `signbit` isn't defined. `signbit` isn't defined because <cmath> is included, which does #undef signbit after declaring it in the std:: namespace.

I'm not sure why this affects scratchbox and not the standard Linux builds: the cmath undef/redeclare bits are wrapped in:

#if _GLIBCXX_USE_C99_MATH
#if !_GLIBCXX_USE_C99_FP_MACROS_DYNAMIC

(Regression from bug 505847, which is .asyncDrawXULElement and we're not going to back that out!)
Assignee: benjamin → bugzillaFred
FWIW I can reproduce this with a stock Ubuntu 8.04 build (gcc 4.2.4).
Attached patch build fix (obsolete) — Splinter Review
"Head in the sand" kind of patch.  I don't care why it works, it just does.
Attachment #410006 - Attachment is obsolete: true
Ugh. We have to have the chromium headers on top (before we include NSPR) in order to fix the Windows build bustage with mismatched #defines for uint16

Maybe we can expcitly include <math.h> before that, though?
The last patch I posted doesn't quite fix all the bustage, working on v2.  I'm just posting them for reference, not r?, as this is blocking another bug I'm working on.
Attached patch hacks v2 (obsolete) — Splinter Review
Sorry to hack and run, hope this is useful.
Attachment #410011 - Attachment is obsolete: true
 environment:
  CC=/tools/gcc/bin/gcc
  CVS_RSH=ssh
  CXX=/tools/gcc/bin/g++

checking for c++... c++
checking whether the C++ compiler (c++  -Wl,-rpath-link,/usr/lib:/lib) works... yes
checking whether the C++ compiler (c++  -Wl,-rpath-link,/usr/lib:/lib) is a cross-compiler... no
checking whether we are using GNU C++... yes

c++ -o nsFrameLoader.o -c -fvisibility=hidden -DMOZILLA_INTERNAL_API -D_IMPL_NS_COM -DEXPORT_XPT_API -DEXPORT_XPTC_API -D_IMPL_NS_GFX -D_IMPL_NS_WIDGET -DIMPL_XREAPI -DIMPL_NS_NET -DIMPL_THEBES  -DZLIB_INTERNAL -DOSTYPE=\"Linux2.6.18-53.1.19\" -DOSARCH=Linux -DEXCLUDE_SKIA_DEPENDENCIES -DCHROMIUM_MOZILLA_BUILD  -DOS_LINUX=1 -DOS_POSIX=1  -D_IMPL_NS_LAYOUT -I/home/cltbld/build/maemo-electrolysis/electrolysis/ipc/chromium/src -I/home/cltbld/build/maemo-electrolysis/electrolysis/ipc/glue -I../../../ipc/ipdl/_ipdlheaders  -I/home/cltbld/build/maemo-electrolysis/electrolysis/content/base/src -I. -I../../../dist/include -I../../../dist/include/nsprpub  -I/home/cltbld/build/maemo-electrolysis/electrolysis/objdir/xulrunner/dist/include/nspr -I/home/cltbld/build/maemo-electrolysis/electrolysis/objdir/xulrunner/dist/include/nss      -I/home/cltbld/build/maemo-electrolysis/electrolysis/content/base/src/../../events/src -I/home/cltbld/build/maemo-electrolysis/electrolysis/content/base/src/../../xml/content/src -I/home/cltbld/build/maemo-electrolysis/electrolysis/content/base/src/../../../layout/xul/base/src -I/home/cltbld/build/maemo-electrolysis/electrolysis/content/base/src/../../xul/content/src -I/home/cltbld/build/maemo-electrolysis/electrolysis/content/base/src/../../html/content/src -I/home/cltbld/build/maemo-electrolysis/electrolysis/content/base/src/../../base/src -I/home/cltbld/build/maemo-electrolysis/electrolysis/content/base/src/../../xbl/src -I/home/cltbld/build/maemo-electrolysis/electrolysis/content/base/src/../../../layout/generic -I/home/cltbld/build/maemo-electrolysis/electrolysis/content/base/src/../../../layout/style -I/home/cltbld/build/maemo-electrolysis/electrolysis/content/base/src/../../../dom/base -I/home/cltbld/build/maemo-electrolysis/electrolysis/content/base/src/../../xml/document/src -I/home/cltbld/build/maemo-electrolysis/electrolysis/xpcom/io -I/home/cltbld/build/maemo-electrolysis/electrolysis/dom/ipc   -fPIC -frtti -fno-exceptions -Wall -Wpointer-arith -Woverloaded-virtual -Wsynth -Wno-ctor-dtor-privacy -Wno-non-virtual-dtor -Wcast-align -Wno-invalid-offsetof -Wno-long-long -fno-strict-aliasing -pthread -pipe -DNDEBUG -DTRIMMED -I/usr/include/gtk-2.0 -I/usr/lib/gtk-2.0/include -I/usr/include/atk-1.0 -I/usr/include/cairo -I/usr/include/pango-1.0 -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/include/freetype2 -I/usr/include/libpng12 -I/usr/include/gtk-unix-print-2.0   -Os -freorder-blocks -fno-reorder-functions -finline-limit=50   -DMOZILLA_CLIENT -include ../../../mozilla-config.h -Wp,-MD,.deps/nsFrameLoader.pp /home/cltbld/build/maemo-electrolysis/electrolysis/content/base/src/nsFrameLoader.cpp

I'm willing to bet folding money that these machines aren't building with gcc 4.3.  The errors here are exactly what I'm seeing on Ubuntu 8.04 with gcc 4.2.  And because of the __cxxabiv1::__class_type_info visibility bug, building against 4.2 is futile anyway.
This is scratchbox, we don't have a lot of control over that toolchain right now, so we have to live with whatever we're given.
(In reply to comment #6)
> Sorry to hack and run, hope this is useful.

while this may fix things up under ubuntu, it doesn't under scratchbox. See, I'd come up with something very close to what you just posted for nsFrameLoader.cpp, but it's really nsCanvasRenderingContext2D.cpp that's giving me a hard time (see the attachment).

I have a fix that seems to make all platforms happy, but it's very ugly (I'll post in next comment).
Initial comment from Benjamin states the problem very clearly : <cmath> gets includes by tr1/functional (by chromium) and kills our definition of signbit needed by jsnum.h.

So I played around in jsnum.h, trying to make it use the right version of signbit... either std::signbit (when cmath was kicking in) or the former signbit when not.

Initially, I tried to condition that with "#if _GLIBCXX_USE_C99_MATH", but for some reason, it didn't work. the very stupid hack that seems to make all platforms happy for now is using "#ifndef signbit" (see attachment).

I'll sleep on that for now, and we'll see tomorrow.
Blocks: 523094
Attached patch v2, for realzSplinter Review
Gak!  Oops, I posted the same patch twice.  Not sure it's still relevant, but attached is the version I meant to post.
Attachment #410026 - Attachment is obsolete: true
(In reply to comment #11)

> Not sure it's still relevant, but attached is the version I meant to post.

You bet it is !  Good catch cjones, your patch makes Scratchbox happy as well !
This hack looks clean to me, I guess you could just push that if Benjamin doesn't see other problems.
Umm ... the maemo build went green from eliminating RTTI ... interesting.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.