The crash was uncovered in code I'm writing for Fennec, which is a xulrunner app. Here's the top of the stack: #4 <signal handler called> #5 Bfree (v=0x7f394e6a7570) at /home/db48x/moz/mozilla-central/nsprpub/pr/src/misc/prdtoa.c:625 #6 0x00007f394e3fc89a in dtoa (d=-0, mode=0, ndigits=0, decpt=0x7fff573c9404, sign=0x7fff573c9400, rve=0x7fff573c93f8) at /home/db48x/moz/mozilla-central/js/src/dtoa.c:2713 #7 0x00007f394e3fd709 in JS_dtostr (buffer=0x7fff573c9480 "\210\226<W�\177", bufferSize=26, mode=DTOSTR_STANDARD, precision=0, d=-0) at /home/db48x/moz/mozilla-central/js/src/jsdtoa.cpp:162 #8 0x00007f394e42ed4a in js_NumberToCString (cx=0x115aa20, d=0, buf=0x7fff573c9480 "\210\226<W�\177", bufSize=26) at /home/db48x/moz/mozilla-central/js/src/jsnum.cpp:720 #9 0x00007f394e42ed69 in js_NumberToString (cx=0xcca1a0, d=11244299.51238361) at /home/db48x/moz/mozilla-central/js/src/jsnum.cpp:735 #10 0x00007f394e460330 in js_ValueToString (cx=0x115aa20, v=15516762) at /home/db48x/moz/mozilla-central/js/src/jsstr.cpp:2729 #11 0x00007f394e42250d in js_Interpret (cx=0x115aa20) at /home/db48x/moz/mozilla-central/js/src/jsinterp.cpp:3630 #12 0x00007f394e428a91 in js_Invoke (cx=0x115aa20, argc=1, vp=0x11cdfc0, flags=0) at /home/db48x/moz/mozilla-central/js/src/jsinterp.cpp:1320 The JS stack indicates that calling the following function causes the crash: function hideTablist() { tablist.left = -tablist.boxObject.width; } From the stack, I would say that tablist.boxobject.width is 0, since dtoa is being called with a -0. This is with a xulrunner compiled from this morning's mozilla-central, though I first saw the crash a few days ago. This is a 64-bit linux build.

Ugh! Somehow dtoa.c is ending up linking to nspr's Bfree, even though it has its own! Bleah. I'll check this out as soon as I can.

> dtoa is being called with a -0. A negative zero? Is this on a ones-complement CPU? Ones-complement CPUs have separate representations (bit patterns) for positive zero and negative zero. Twos-complement CPUs, such as Intel x86, have no separate representation for negative zero. On them, zero is zero. The last ones complement machine on which I worked was a CDC 6500, now sitting in a computer museum. If any CPUs made this millennium are ones-complement, I'd like to know what they are. I'd be interesting in knowing how gdb decided it was a negative zero on a twos-complement machine.

Nelson: -0 is defined by IEEE 754 binary and 754r decimal specs. It's negative 0, useful, e.g., with atan2(y, x) -- a JS demo: js> Math.atan2(0,0) 0 js> Math.atan2(0,-0) 3.141592653589793 js> Math.atan2(-0,-0) -3.141592653589793 js> Math.atan2(-0,0) 0 /be

Oh, that number is a float/double? (I don't remember what dtoa does, guess I could have looked it up.)

Perhaps the shared library containing dtoa needs to be linked with -B symbolic Is this really an NSPR bug?

db48x: what compiler is this being compiled with? Bfree is "static" in both js/src and NSPR, so having them mix symbols is highly unusual.

gcc version 4.3.0 20080428 (Red Hat 4.3.0-8) (GCC)

This patch of crowder's "fixes" it: diff -r 85c82edb887a js/src/jsdtoa.cpp --- a/js/src/jsdtoa.cpp Mon Aug 18 20:49:32 2008 -0700 +++ b/js/src/jsdtoa.cpp Thu Aug 21 11:08:05 2008 -0500 @@ -88,6 +88,11 @@ #define LOCK_DTOA() #define UNLOCK_DTOA() #endif + +#define Bfree __internal_Bfree +#define Balloc __internal_Balloc +#define freedtoa __internal_freedtoa + #include "dtoa.c" JS_FRIEND_API(JSBool)

Need an assignee. /be

Wasn't this resolved as a build-bug on db's end?

Assigned to crowder for now, since he has both a patch and a root cause, it seems.

Daniel: Still see this?

No reply in a month, closing INCO. Please reopen if you can provide more info.

I've seen this internally @nokia, it'd be nice if we could fix the build system(s) to make this harder to trigger. Debugging it was a huge waste of my time (I hope that the engineer watching me do the debugging learned something but...)

so, while trying to get wtc's static build working, i hit this and spent a bit more time on it. i'm tired of spending time on this. everything else in these files from dtoa is marked as static. and the way we use dtoa, the data we 'return' is not supposed to be freed with 'freedtoa' so it is truly static. the js version (here) is slightly different in that jsdtoa #include's dtoa. Whereas nspr's version seems to just have an inlined copy (see next patch).

this is the equivalent patch for nspr. again, PR_dtoa manages its own memory in a way which is independent of the memory managed by dtoa itself. freedtoa should be marked as static just like all the other private functions in this file.

Comment on attachment 421351 [details] [diff] [review] mark freedtoa as static r=wtc. Yes, if dtoa() is static, so should freedtoa().

http://hg.mozilla.org/mozilla-central/rev/7f72dea3d949 http://hg.mozilla.org/mozilla-central/rev/d64fd027e4ad I'll let ted resolve this with the commit message for nspr CVS (which is locked...)

timeless: NSPR and NSS changes should be pushed to mozilla-central using the procedure at https://developer.mozilla.org/en/Updating_NSPR_or_NSS_in_mozilla-central

brother. so you fix it.

Comment on attachment 421351 [details] [diff] [review] mark freedtoa as static I checked in this patch on the NSPR trunk (NSPR 4.8.4). Checking in prdtoa.c; /cvsroot/mozilla/nsprpub/pr/src/misc/prdtoa.c,v <-- prdtoa.c new revision: 4.14; previous revision: 4.13 done

Looks like I'm having this problem with Prism as well. I can patch my tree locally but was wondering if the fix will make it into 1.9.2.

Comment on attachment 421350 [details] [diff] [review] mark freedtoa as static wtc: how does one pull in a fix into 1.9.2.x?

timeless: we use the same procedure as mozilla-central (https://developer.mozilla.org/en/Updating_NSPR_or_NSS_in_mozilla-central) except that the NSPR CVS tag needs to be an RTM (final) release, or a Beta release if there is time to switch to an RTM tag subsequently. Ted or I can generate a NSPR_4_8_4_BETA1 or NSPR_4_8_4_RTM tag for you. Just curious: why is this bug suddenly becoming critical? Was there some new Mozilla code that manifests this bug?

I was wondering that myself; I think what's happened is that this bug has emerged as being one of the major issues with a recent release of Prism. It's probably also one of those things that's causes lots of poorly-understood crashes elsewhere.

roughly, every embedder hits this (my team, prism, etc.). my team hits it once every 6 months. it's an interesting learning experience and a total waste of my time. i personally hit this trying to build your static nspr-nss stuff, we could call your code 'new' i suppose :).

Not sure what you mean by "embedder" but Prism is a XULRunner app. Personally I'm not sure the fix in this bug gets at the root cause of the issue. How come Firefox on Linux doesn't have the same problem?

it's roughly picking some random set of build flags for some 'static' build which will trigger this. whatever build flags firefox on linux *normally* uses doesn't trigger it. but it's certainly possible to trigger it w/ firefox on linux.... i'd rather not spend time chasing "why does this happen", it happens because two modules have the same function name which should have been declared static, and that's what this bug fixes.

Ah ok. For some reason when I first checked I only saw the patch that renames the functions in JS using #defines. Declaring as static seems like a clean solution.

This is fixed in NSPR CVS.

Comment on attachment 421350 [details] [diff] [review] mark freedtoa as static Didn't make 1.9.2.2 - this can be nominated for 1.9.2.3 and it's worth fixing crashes, but can you tell me whether this will be fixed by taking the new NSPR tag in bug 533983? If so, no need to renom, but please mark this bug's dependency on that one.

Jonathan: you need to use the NSPR_4_8_4_RTM CVS tag instead. That tag has been tested in mozilla-central for several weeks now. NSPR_4_8_4_RTM contains all the changes in the tag in bug 533983.

Just wanted to check whether/where this was slated to land. I'm still getting this crash on Linux from a fresh 1.9.2 pull.

Matthew: You're right. I found that neither the JS nor the NSPR patch in this bug has been checked in to mozilla-1.9.2. Can someone set the appropriate approval/blocking flags to start the process? I'm not up to date on the mozilla-1.9.2 status. Note: mozilla-1.9.2 now has a third copy of freedtoa, in ipc/chromium/src/base/third_party/dmg_fp/dtoa.cc. It's defined inside a "dmg_fp" namespace, so it is OK.

We'll take the JS patch here on the stable branches, and push to upgrade NSPR in bug 575620

Comment on attachment 421350 [details] [diff] [review] mark freedtoa as static Approved for 1.9.2.8, a=dveditz for release-drivers

What are the STR for the original crash so QA can verify the fix?

I'm not sure there are particularly good steps. The easiest thing to do is to check to see if the function in question is exported, if it isn't exported, then this can't happen = verified.

Al: I suggest you verify this by code inspection. Using this MXR query: http://mxr.mozilla.org/mozilla1.9.2/ident?i=freedtoa&filter= verify that the three copies of freedtoa() are all defined as 'static'. Note: nsprpub/pr/src/misc/dtoa.c is not used, so ignore that file. The freedtoa() in ipc/chromium/src/base/third_party/dmg_fp/dtoa.cc is not static. I think that's fine. You can make it static as well to be safe.

Verified for 1.9.2 in source using mxr.