Closed Bug 525677 Opened 11 years ago Closed 11 years ago
e10s builds broken in ns
Frame Loader, conflict of <cmath> and <math .h> with jsnum .h
7.46 KB, text/plain
600 bytes, patch
|Details | Diff | Splinter Review|
2.25 KB, patch
|Details | Diff | Splinter Review|
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!)
FWIW I can reproduce this with a stock Ubuntu 8.04 build (gcc 4.2.4).
"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.
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).
11 years ago
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.
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.
Pushed http://hg.mozilla.org/projects/electrolysis/rev/961bfa23f570. Waiting for a green cycle before closing.
Umm ... the maemo build went green from eliminating RTTI ... interesting.
11 years ago
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.