Closed
Bug 282994
Opened 19 years ago
Closed 4 months ago
Crash in debug build if I press submit button. Problem is with PR_dtoa function on ARM without FP unit.
Categories
(NSPR :: NSPR, defect)
Tracking
(Not tracked)
RESOLVED
WONTFIX
People
(Reporter: tmarn, Unassigned)
References
()
Details
Attachments
(6 files)
180.71 KB,
text/plain
|
Details | |
180.71 KB,
application/x-gzip
|
Details | |
1.15 KB,
text/plain
|
Details | |
1.00 KB,
patch
|
Details | Diff | Splinter Review | |
955 bytes,
patch
|
Details | Diff | Splinter Review | |
5.16 KB,
patch
|
wtc
:
review-
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.5) Gecko/20050110 Firefox/1.0 (Debian package 1.0+dfsg.1-2) Build Identifier: CVS Source : Stable Mozilla 1.7.5 - Released December 17, 2004 Problem is with a PR_dtoa function in mozilla/nsprpub/pr/src/misc/prdtoa.c Converting from double to string produce memory leak. Minimo crash will with Assertion failure: lock != NULL, at ptsynch.c:206 PR_dtoa is quite broken on ARM platform (PXA255-400Mhz version without FP unit), Minimo (from mozilla source 1.7.5) is compiled with softfloat compiler (gcc version 3.3.3/glibc 2.3.2, from www.kegel.org). The reason is because ARM platform (little endian) use (mixed endian) when manipulating with float numbers. Maybe the prdtoa.c functions are not compliant with that arhitecture. I also found the same problem in http://lists.debian.org/debian-arm/2003/10/msg00028.html Reproducible: Always Steps to Reproduce: 1. build debug version of Minimo. (release version will skip assert checking and "submit" will not be executed and not action forward to server :( ). 2. ./Minimo www.google.com 3. Enter something "test" into search textbox 4. Press "Google search" 5. Nothing happend after submit in release version. Crash in debug version: Assertion failure: lock != NULL, at ptsynch.c:206 Actual Results: Because pow5mult called from PR_dtoa is extremly slow you will got an error after 1 minute: Assertion failure: lock != NULL, at ptsynch.c:206 Expected Results: Function should convert double value to string, returns and than submit the entered text forward (I guess). Look into prdtoa.c: function static Bigint *mult(CONST Bigint *a, CONST Bigint *b) which is called from pow5mult generates list with numbers of multiply of 5. Set breakpoint into line 678 of mult function and print return c variable , The table which generates here must have numbers ( 5 25 125 625 3125 15626 ... } gdb: print *c Next nubmer which is generated is 78125, which is OK ( 5 * 15625) OK $2 = {next = 0xffffffff, k = 1, maxwds = 2, sign = 0, wds = 1, x = {78125}} Next nubmer is 390625, OK too. ( 5 * 78125) $4 = {next = 0x0, k = 1, maxwds = 2, sign = 0, wds = 1, x = {390625}} Than instead of 1953125 value of x is {2264035265}, IS THIS WRONG ???? $5 = {next = 0xffffffff, k = 1, maxwds = 2, sign = 0, wds = 2, x = {2264035265}} gdb: bt #0 mult (a=0x1f88c0, b=0x1f88c0) at prdtoa.c:678 #1 0x404933a4 in pow5mult (b=0x1f8900, k=77644143) at prdtoa.c:739 #2 0x40497294 in $a () at prdtoa.c:2341 Before the assertion happens in ptsynch.c:206 call stack is: (gdb) bt #0 PR_Unlock (lock=0x0) at ptsynch.c:206 #1 0x404926e4 in Balloc (k=16) at prdtoa.c:418 #2 0x40492ea0 in mult (a=0x424ab008, b=0x424ab008) at prdtoa.c:620 #3 0x404933a4 in pow5mult (b=0x424cc008, k=2369) at prdtoa.c:739 #4 0x40497294 in $a () at prdtoa.c:2341 Also, maybe something is wrong (Kmax) in Balloc function which generates freelist[k] table with pointers on allocated memory where converted numbers will be stored. (guess)
Comment 1•19 years ago
|
||
Tom, could you find out the CVS revision of prdtoa.c? You can find that out with the following commands: cd mozilla/nsprpub/pr/src/misc cvs status prdtoa.c In the current version of prdtoa.c, we have the following code that tries to handle ARM: #if defined(__arm) || defined(__arm__) || defined(__arm26__) \ || defined(__arm32__) #define IEEE_ARM #elif defined(IS_LITTLE_ENDIAN) #define IEEE_8087 #else #define IEEE_MC68k #endif This code was added by Darin Fisher to fix bug 209814. Does your prdtoa.c have this code? Could you find out if the above code causes IEEE_ARM to be defined in your environment?
(In reply to comment #1) > Tom, could you find out the CVS revision of > prdtoa.c? You can find that out with the > following commands: > > cd mozilla/nsprpub/pr/src/misc > cvs status prdtoa.c > > In the current version of prdtoa.c, we have > the following code that tries to handle ARM: > > #if defined(__arm) || defined(__arm__) || defined(__arm26__) \ > || defined(__arm32__) > #define IEEE_ARM > #elif defined(IS_LITTLE_ENDIAN) > #define IEEE_8087 > #else > #define IEEE_MC68k > #endif > > This code was added by Darin Fisher to fix bug 209814. > Does your prdtoa.c have this code? Could you find > out if the above code causes IEEE_ARM to be defined > in your environment? > Hi, CVS Revision status of prdtoa.c that you ask for is: File: prdtoa.c Status: Up-to-date Working revision: 3.7.4.4 Repository revision: 3.7.4.4 /cvsroot/mozilla/nsprpub/pr/src/misc/prdtoa.c,v Sticky Tag: MOZILLA_1_7_5_RELEASE (revision: 3.7.4.4) Sticky Date: (none) Sticky Options: (none) #define's are there but nothing helps, i also try with the newest 1.8.X (11.feb.2005), but problem still remains. File: prdtoa.c Status: Up-to-date Working revision: 3.7.4.7 Repository revision: 3.7.4.7 /cvsroot/mozilla/nsprpub/pr/src/misc/prdtoa.c,v Sticky Tag: NSPRPUB_PRE_4_2_CLIENT_BRANCH (branch: 3.7.4) Sticky Date: (none) Sticky Options: (none) Just ask, I will check anything to solve this problem.
Comment 3•19 years ago
|
||
Tom: thanks for the cvs revision info. Your file (rev. 3.7.4.4, MOZILLA_1_7_5_RELEASE) has Darin's fix. Could you confirm that prdtoa.c defines IEEE_ARM? (which means one of the following macros: __arm, __arm__, __arm26__, __arm32__ is defined) If so, this means Darin's fix does not address your problem. In that case I'll need to have Doug Turner take on this bug.
Comment 4•19 years ago
|
||
Tom: Specifically, please perform the following experiment: Edit prdtoa.c and add the following line #error "IEEE_ARM is defined" after the line #define IEEE_ARM and then go to the nsprpub/pr/src/misc directory in your build tree and do a "make". See if the compilation succeeds or fails.
alternatively, could you attach the file generated by: make prdtoa.i (make from the directory that has prdtoa.o)
(In reply to comment #4) > Tom: > > Specifically, please perform the following experiment: > > Edit prdtoa.c and add the following line > > #error "IEEE_ARM is defined" > Yes it fails after make, I confirming that IEEE_ARM define is use. prdtoa.c:138:2: #error IEEE_ARM make: *** [prdtoa.o] Error 1
This is the code (see attachment) for reproduce the same error. Just wait 30 seconds and you will get the same assertion as in the mozilla minimo browser. Assertion failure: lock != NULL, at ptsynch.c:206 Aborted copy code into mozilla/nsprpub/pr/tests/testdtoa.c and change Makefile (see testdtoa.c for details).
Reporter | ||
Comment 10•19 years ago
|
||
I found the location where PR_dtoa crashes. It is in a whatnspr.c file set_whatnspr function. Instead of returning from PR_dtoa with -1 value function dies. Patch skips fault version detection of nspr library and forces to setup correct nspr version with r = 0. See attached patch. tom
Reporter | ||
Comment 11•19 years ago
|
||
Replacing dtoa with sprintf in nsStringObsolite.cpp:Modified_cnvtf(char *buf, int bufsz, int prcsn, double fval) method. Note that this is just a workaround until dtoa is not fixed. tom
Comment 12•19 years ago
|
||
I am not seeing this problem on either Linux ARM or on WinCE ARM.
Reporter | ||
Comment 13•19 years ago
|
||
(In reply to comment #12) > I am not seeing this problem on either Linux ARM or on WinCE ARM. The problem is with Linux ARM without FPU (tested on PXA255) and Minimo compiled with softfloat compiler (for faster code) and with disabled kernel FastFPU emulation of course. I did workaround hacks, i can send you patches which work without crashing on my platform. The problem was with functions that call PR_dtoa.
Reporter | ||
Comment 14•19 years ago
|
||
PR_dtoa workaround (with debug printf's, just to see fixed locations.)
Updated•19 years ago
|
Assignee: dougt → nobody
Updated•19 years ago
|
Assignee: nobody → dougt
Component: General → NSPR
Product: Minimo → NSPR
Version: unspecified → 3.0
Updated•19 years ago
|
Attachment #190515 -
Flags: review?(wtchang)
Comment 15•19 years ago
|
||
Comment on attachment 190515 [details] [diff] [review] dtoa and jsdtoa workaround for ARM linux Tom, thanks for the patch. I will attach a better patch for mozilla/security/nss/lib/base/whatnspr.c. In short, the code that's causing problem for ARM in that file is obsolete, so the better fix is to simply delete that code. Your changes for the other files in this patch have two problems: - You use the C++ comment delimiter //, which is not supported by all C compilers yet. - You need to code these changes with ifdefs so that we continue to use the original code on other platforms.
Attachment #190515 -
Flags: review?(wtchang) → review-
Comment 16•19 years ago
|
||
Tom, I decided to open bug 317052 to deal with mozilla/security/nss/lib/base/whatnspr.c, so the patch for that file will be in that bug report.
Depends on: 317052
Reporter | ||
Comment 17•19 years ago
|
||
(In reply to comment #16) > Tom, I decided to open bug 317052 to deal with > mozilla/security/nss/lib/base/whatnspr.c, so the > patch for that file will be in that bug report. > Ok, I agree. I sent previous patches just for demonstration purpose where the bugs occured, and how to overcome the problems. I didn't pay attention on coding syntax. The patches were sent because of Doug Turner comment #12.
Comment 18•18 years ago
|
||
(In reply to comment #14) > Created an attachment (id=190515) [edit] > dtoa and jsdtoa workaround for ARM linux > > PR_dtoa workaround (with debug printf's, just to see fixed locations.) Tom, I have some question about the patch of hack4 in jsnum.c + double num = (jsdouble)d; + char temp[255]; + printf("DOUBLE:%f\n",num); + sprintf(temp,"%f",(int)num); num will be casted to integer, if num is not an integer, like 3.5, cast will create 0.0. why not just use sprintf(temp,"%f",num) instead? + printf("CONVERTED:%s\n", temp); + numStr = temp;
Comment 19•18 years ago
|
||
eh? No, that would just truncate the value, yielding 3. but printing that as a %f is probably not such a good idea...
Comment 20•18 years ago
|
||
(In reply to comment #19) > eh? No, that would just truncate the value, yielding 3. but printing that as a > %f is probably not such a good idea... > It's strange, when I use TestGtkEmbed to open http://maps.google.com , the message makes me confused: == start of message == jsnum.c: Calling hack4 for JS_dtostr(buf, sizeof numBuf, DTOSTR_STANDARD, 0, d=3.500000) DOUBLE:3.500000 CONVERTED:0.000000 jsnum.c: Calling hack4 for JS_dtostr(buf, sizeof numBuf, DTOSTR_STANDARD, 0, d=5.500000) DOUBLE:5.500000 CONVERTED:0.000000 jsnum.c: Calling hack4 for JS_dtostr(buf, sizeof numBuf, DTOSTR_STANDARD, 0, d=5.500000) DOUBLE:5.500000 CONVERTED:0.000000 jsnum.c: Calling hack4 for JS_dtostr(buf, sizeof numBuf, DTOSTR_STANDARD, 0, d=5.500000) DOUBLE:5.500000 CONVERTED:0.000000 jsnum.c: Calling hack4 for JS_dtostr(buf, sizeof numBuf, DTOSTR_STANDARD, 0, d=0.000000) DOUBLE:0.000000 CONVERTED:0.000000 == End of messages == This is the reason I remove the cast of num in hack 4. But even the convert look correct, my google map also can't show anything :-(
Updated•18 years ago
|
QA Contact: chofmann → nspr
Comment 22•3 years ago
|
||
Reporter is gone.
Is there a way to determine whether this issue still exists?
Flags: needinfo?(kaie)
Comment 23•3 years ago
|
||
Are there any recent crash reports pointing to PR_dtoa ?
Flags: needinfo?(kaie)
Comment 24•3 years ago
|
||
a few version 78
- https://crash-stats.mozilla.org/signature/?proto_signature=~PR_dtoa&product=Thunderbird&signature=__pthread_kill%20%7C%20abort%20%7C%20gpusGenerateCrashLog.cold.1&date=%3E%3D2021-09-04T23%3A49%3A00.000Z&date=%3C2021-10-04T23%3A49%3A00.000Z&_columns=date&_columns=product&_columns=version&_columns=build_id&_columns=platform&_columns=reason&_columns=address&_columns=install_time&_columns=startup_crash&_sort=-date&page=1#summary
- https://crash-stats.mozilla.org/signature/?proto_signature=~PR_dtoa&product=Thunderbird&signature=__pthread_kill%20%7C%20pthread_kill%20%7C%20abort%20%7C%20gpusGenerateCrashLog.cold.1&date=%3E%3D2021-09-04T23%3A49%3A00.000Z&date=%3C2021-10-04T23%3A49%3A00.000Z&_columns=date&_columns=product&_columns=version&_columns=build_id&_columns=platform&_columns=reason&_columns=address&_columns=install_time&_columns=startup_crash&_sort=-date&page=1#reports
- bp-811a6fa7-aba5-48c0-861b-ca5da0210920
Comment 25•2 years ago
|
||
In the process of migrating remaining bugs to the new severity system, the severity for this bug cannot be automatically determined. Please retriage this bug using the new severity system.
Severity: major → --
Comment 26•5 months ago
|
||
The severity field is not set for this bug.
:KaiE, could you have a look please?
For more information, please visit BugBot documentation.
Flags: needinfo?(kaie)
Updated•5 months ago
|
Severity: -- → S3
Flags: needinfo?(kaie)
Updated•4 months ago
|
Status: NEW → RESOLVED
Closed: 4 months ago
Resolution: --- → WONTFIX
You need to log in
before you can comment on or make changes to this bug.
Description
•