476936 - Math.pow(x,y) gives wrong answers on Solaris for y<0

Status: RESOLVED INACTIVE

(Core :: JavaScript Engine, defect)

Description

[Wesley W. Garland]

Attachment: 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 trace-test.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 cross-compilation). 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.

Comment 1

[Jim Blandy :jimb]

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 well-known double constants available in cx->runtime, to avoid allocation.

Comment 2

[Wesley W. Garland]

This bug also causes the following trace-test.py tests to fail on Solaris, and probably Solaris/x86:

    /export/home/wes/hg/tracemonkey/js/src/trace-test/tests/basic/math-trace-tests.js
    /export/home/wes/hg/tracemonkey/js/src/trace-test/tests/sunspider/check-math-partial-sums.js

New patch coming soon. Where does the time go?!

Comment 3

[Wesley W. Garland]

Attachment: 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
  trace-test/tests/basic/math-trace-tests.js
   
The other test I mentioned, trace-test/tests/sunspider/check-math-partial-sums.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.

Comment 4

[Wesley W. Garland]

Review ping?

Comment 5

[Jim Blandy :jimb]

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()?

Comment 6

[André Bargull [:anba]]

Old bug without any actions, closing as INACTIVE.