Status
()
People
(Reporter: Wesley W. Garland, Assigned: Wesley W. Garland)
Tracking
Firefox Tracking Flags
(Not tracked)
Details
Attachments
(1 attachment, 1 obsolete attachment)
Created attachment 360580 [details] [diff] [review] Patch to fix Math.pow(0,1) when libm.so gets it wrong Math.pow(0,1) gives Infinite rather than Infinite on Solaris. (Also Cygwin, but I don't know if that's worth mentioning). This problem has probably existed for a long time: Netscape Navigator 4.79 has the same issue, although it's possible that it worked with an older libm.so. The problem was originally found running tracetest.js on a Solaris/sparc machine. The problem traces down, at least for the specific case of pow(0,1) on Solaris , to the fact that libm's pow() throws a math err signal and junk(?) data back, setting errno to indicate a Pole Error. I went back through some very old (1992) SVR4 docs, and I suspect there are other SVR4 platforms which might still suffer this same problem. Attached is a patch which detects when pow(0,1) gives the wrong answer via AC_TRY_RUN in js/src/configure.in. (Note that this does not work for crosscompilation). If the right answer is not detected it sets precompiler #define LIBM_POW_ZERO_TO_NEG_POWER_IS_BROKEN. With that guard inside jsmath.cpp, this patch evaluates each of the special conditions in ES3 15.8.2.13 when y<0 (except for when y==0) and returns the correct answer. If no special condition is found, it falls back to returning the answer provided by libm. I have tested this against js/tests/ecma/Math/15.8.2.13.js and observed failure for the following cases without the patch: FAILED! Math.pow(0, 1) = Infinity expected: Infinity FAILED! Math.pow(0, 0.5) = Infinity expected: Infinity FAILED! Math.pow(0, 1000) = Infinity expected: Infinity FAILED! Math.pow(0, 2) = Infinity expected: Infinity All tests pass with the patch applied.
Attachment #360580 
Flags: review?(jim)
Comment 1•9 years ago


Comment on attachment 360580 [details] [diff] [review] Patch to fix Math.pow(0,1) when libm.so gets it wrong Check out the autoconf 2.13 documentation on "Caching Results". You should be using AC_CACHE_CHECK to cache the results of running this test, and print messages appropriately. It seems to me that similar checks would be needed in math_pow_tn. Would it be possible to pull this code out into a new 'math_pow_kernel' function, that works strictly with numeric types and can be used by both math_pow and math_pow_tn, as was done with 'math_atan2_kernel'? You wouldn't have a context to work with in math_pow_kernel, but you might be able to get by leaning more on the stuff in jsnum.h. Also, math_pow might want to check the value returned by math_pow_kernel against wellknown double constants available in cx>runtime, to avoid allocation.
Attachment #360580 
Flags: review?(jim) → review
(Assignee)  
Comment 2•8 years ago


This bug also causes the following tracetest.py tests to fail on Solaris, and probably Solaris/x86: /export/home/wes/hg/tracemonkey/js/src/tracetest/tests/basic/mathtracetests.js /export/home/wes/hg/tracemonkey/js/src/tracetest/tests/sunspider/checkmathpartialsums.js New patch coming soon. Where does the time go?!
(Assignee)  
Comment 3•8 years ago


Created attachment 451974 [details] [diff] [review] Patch to fix Math.pow(0,1) when libm.so gets it wrong, take 2 This patch is pretty much the same as the previous one, except the autoconf test is now cached, and the code in jsmath.cpp has been refactored to work on doubles instead of jsvals, then made into a math_pow_kernel so that it can be called from from both math_pow and math_pow_tn. I have run a version of this on Solaris and Leopard against the test suite. No new regressions were noted. The following tests now pass: tests/ecma/Math/15.8.2.13.js tracetest/tests/basic/mathtracetests.js The other test I mentioned, tracetest/tests/sunspider/checkmathpartialsums.js, still fails when the tracing JIT is enabled. I smell a bug in Math.cos() or Math.sin(); will investigate further when I have the opportunity.
Assignee: general → wes
Attachment #360580 
Attachment is obsolete: true
Attachment #451974 
Flags: review?(jim)
(Assignee)  
Comment 4•7 years ago


Review ping?
Comment 5•6 years ago


Comment on attachment 451974 [details] [diff] [review] Patch to fix Math.pow(0,1) when libm.so gets it wrong, take 2 I'm going through my review queue; apologies for dropping this. The autoconf test looks perfect. I think this needs to be refreshed for the deletion of TraceMonkey. Did you ever manage to look into Math.cos() and Math.sin()?
Attachment #451974 
Attachment description: Patch to fix Math.pow(0,1) when libm.so gets it wrong, take 2 → Patch to fix Math.pow(0,1) when libm.so gets it wrong, take 2
Attachment #451974 
Flags: review?(jimb)
You need to log in
before you can comment on or make changes to this bug.
Description
•