Fix StrictMath.copySign's behavior with NaN.
StrictMath's copySign method is defined to treat all NaNs as positive, regardless
of their sign bit. Rather than add yet more code on the Java side (which already
called down to native code three times), let's just make one call to native code.
This change fixes failures in already-committed StrictMath tests.
The only other potential instances of this class of error were in the
max and min methods, but they all handle NaNs before calling
(double|float)To(Raw)?LongBits, so the choice there really is arbitrary.
Change-Id: I439dbdff9068cb420b78a6330cde9d7d0d12c0ef
diff --git a/libcore/luni/src/main/java/java/lang/Math.java b/libcore/luni/src/main/java/java/lang/Math.java
index 26019ec..d286d20 100644
--- a/libcore/luni/src/main/java/java/lang/Math.java
+++ b/libcore/luni/src/main/java/java/lang/Math.java
@@ -1091,47 +1091,25 @@
private native static double nextafter(double x, double y);
private native static float nextafterf(float x, float y);
-
- /**
- * Answers a result of the magnitude of the first given double value and the
- * sign of the second given double value.
- *
- * @param magnitude
- * the double value whose magnitude should be used
- * @param sign
- * the double value whose sign should be used
- * @return a result of the magnitude of the first given double value and the
- * sign of the second given double value .
- *
- * @since 1.6
- * @hide
- */
- public static double copySign(double magnitude, double sign) {
- long mbits = Double.doubleToRawLongBits(magnitude);
- long sbits = Double.doubleToRawLongBits(sign);
- return Double.longBitsToDouble((mbits & ~DOUBLE_SIGN_MASK) | (sbits & DOUBLE_SIGN_MASK));
- }
/**
- * Answers a result of the magnitude of the first given float value and the
- * sign of the second given float value .
- *
- * @param magnitude
- * the float value whose magnitude should be used
- * @param sign
- * the float value whose sign should be used
- * @return a result with the magnitude of the first given float value and
- * the sign of the second given float value .
- *
+ * Returns a double with the given magnitude and the sign of {@code sign}.
+ * If {@code sign} is NaN, the sign of the result is arbitrary.
+ * If you need a determinate sign in such cases, use {@code StrictMath.copySign}.
* @since 1.6
* @hide
*/
- public static float copySign(float magnitude, float sign) {
- int mbits = Float.floatToRawIntBits(magnitude);
- int sbits = Float.floatToRawIntBits(sign);
- return Float.intBitsToFloat((mbits & ~FLOAT_SIGN_MASK) | (sbits & FLOAT_SIGN_MASK));
- }
-
+ public static native double copySign(double magnitude, double sign);
+
+ /**
+ * Returns a float with the given magnitude and the sign of {@code sign}.
+ * If {@code sign} is NaN, the sign of the result is arbitrary.
+ * If you need a determinate sign in such cases, use {@code StrictMath.copySign}.
+ * @since 1.6
+ * @hide
+ */
+ public static native float copySign(float magnitude, float sign);
+
/**
* Answers the exponent of a float.
*
diff --git a/libcore/luni/src/main/java/java/lang/StrictMath.java b/libcore/luni/src/main/java/java/lang/StrictMath.java
index 5d3ec8f..cd3c9b5 100644
--- a/libcore/luni/src/main/java/java/lang/StrictMath.java
+++ b/libcore/luni/src/main/java/java/lang/StrictMath.java
@@ -1037,41 +1037,21 @@
private native static float nextafterf(float x, float y);
/**
- * Answers a result of the magnitude of the first given double value and the
- * sign of the second given double value.
- *
- * @param magnitude
- * the double value whose magnitude should be used
- * @param sign
- * the double value whose sign should be used
- * @return a result of the magnitude of the first given double value and the
- * sign of the second given double value.
- *
+ * Returns a double with the given magnitude and the sign of {@code sign}.
+ * If {@code sign} is NaN, the sign of the result is positive.
* @since 1.6
* @hide
*/
- public static double copySign(double magnitude, double sign) {
- return Math.copySign(magnitude, sign);
- }
+ public static native double copySign(double magnitude, double sign);
/**
- * Answers a result of the magnitude of the first given float value and the
- * sign of the second given float value.
- *
- * @param magnitude
- * the float value whose magnitude should be used
- * @param sign
- * the float value whose sign should be used
- * @return a result with the magnitude of the first given float value and
- * the sign of the second given float value.
- *
+ * Returns a float with the given magnitude and the sign of {@code sign}.
+ * If {@code sign} is NaN, the sign of the result is positive.
* @since 1.6
* @hide
*/
- public static float copySign(float magnitude, float sign) {
- return Math.copySign(magnitude, sign);
- }
-
+ public static native float copySign(float magnitude, float sign);
+
/**
* Answers the exponent of a float.
*
diff --git a/libcore/luni/src/main/native/java_lang_Math.c b/libcore/luni/src/main/native/java_lang_Math.c
index a3a69dd..35754cd 100644
--- a/libcore/luni/src/main/native/java_lang_Math.c
+++ b/libcore/luni/src/main/native/java_lang_Math.c
@@ -159,42 +159,46 @@
return nextafterf(a, b);
}
+static jdouble copySign(JNIEnv* env, jclass clazz, jdouble a, jdouble b) {
+ return copysign(a, b);
+}
+
+static jfloat copySign_f(JNIEnv* env, jclass clazz, jfloat a, jfloat b) {
+ return copysignf(a, b);
+}
+
/*
* JNI registration.
*/
static JNINativeMethod gMethods[] = {
/* name, signature, funcPtr */
- { "sin", "(D)D", jsin },
- { "cos", "(D)D", jcos },
- { "tan", "(D)D", jtan },
-
- { "asin", "(D)D", jasin },
- { "acos", "(D)D", jacos },
- { "atan", "(D)D", jatan },
-
- { "exp", "(D)D", jexp },
- { "log", "(D)D", jlog },
- { "sqrt", "(D)D", jsqrt },
-
{ "IEEEremainder", "(DD)D", jieee_remainder },
-
- { "floor", "(D)D", jfloor },
- { "ceil", "(D)D", jceil },
- { "rint", "(D)D", jrint },
-
+ { "acos", "(D)D", jacos },
+ { "asin", "(D)D", jasin },
+ { "atan", "(D)D", jatan },
{ "atan2", "(DD)D", jatan2 },
- { "pow", "(DD)D", jpow },
-
- { "sinh", "(D)D", jsinh },
- { "cosh", "(D)D", jcosh },
- { "tanh", "(D)D", jtanh },
- { "log10", "(D)D", jlog10 },
{ "cbrt", "(D)D", jcbrt },
+ { "ceil", "(D)D", jceil },
+ { "copySign", "(DD)D", copySign },
+ { "copySign", "(FF)F", copySign_f },
+ { "cos", "(D)D", jcos },
+ { "cosh", "(D)D", jcosh },
+ { "exp", "(D)D", jexp },
{ "expm1", "(D)D", jexpm1 },
+ { "floor", "(D)D", jfloor },
{ "hypot", "(DD)D", jhypot },
+ { "log", "(D)D", jlog },
+ { "log10", "(D)D", jlog10 },
{ "log1p", "(D)D", jlog1p },
{ "nextafter", "(DD)D", jnextafter },
{ "nextafterf", "(FF)F", jnextafterf },
+ { "pow", "(DD)D", jpow },
+ { "rint", "(D)D", jrint },
+ { "sin", "(D)D", jsin },
+ { "sinh", "(D)D", jsinh },
+ { "sqrt", "(D)D", jsqrt },
+ { "tan", "(D)D", jtan },
+ { "tanh", "(D)D", jtanh },
};
int register_java_lang_Math(JNIEnv* env)
diff --git a/libcore/luni/src/main/native/java_lang_StrictMath.c b/libcore/luni/src/main/native/java_lang_StrictMath.c
index 7d335f7..3839c21 100644
--- a/libcore/luni/src/main/native/java_lang_StrictMath.c
+++ b/libcore/luni/src/main/native/java_lang_StrictMath.c
@@ -8,7 +8,7 @@
#include <stdlib.h>
/* This static way is the "best" way to integrate fdlibm without a conflict
- * into the android envoirement
+ * into the android environment.
*/
/* #include "fltconst.h" */
@@ -20,6 +20,8 @@
#include "../../external/fdlibm/fdlibm.h"
_LIB_VERSION_TYPE _LIB_VERSION = _IEEE_;
+#include <math.h>
+
/* native public static double sin(double a); */
static jdouble jsin(JNIEnv* env, jclass clazz, jdouble a)
{
@@ -184,42 +186,46 @@
return arg1;
}
+static jdouble copySign(JNIEnv* env, jclass clazz, jdouble a, jdouble b) {
+ return copysign(a, isnan(b) ? 1.0 : b);
+}
+
+static jfloat copySign_f(JNIEnv* env, jclass clazz, jfloat a, jfloat b) {
+ return copysignf(a, isnan(b) ? 1.0f : b);
+}
+
/*
* JNI registration.
*/
static JNINativeMethod gMethods[] = {
/* name, signature, funcPtr */
- { "sin", "(D)D", jsin },
- { "cos", "(D)D", jcos },
- { "tan", "(D)D", jtan },
-
- { "asin", "(D)D", jasin },
- { "acos", "(D)D", jacos },
- { "atan", "(D)D", jatan },
-
- { "exp", "(D)D", jexp },
- { "log", "(D)D", jlog },
- { "sqrt", "(D)D", jsqrt2 },
-
{ "IEEEremainder", "(DD)D", jieee_remainder },
-
- { "floor", "(D)D", jfloor },
- { "ceil", "(D)D", jceil },
- { "rint", "(D)D", jrint },
-
+ { "acos", "(D)D", jacos },
+ { "asin", "(D)D", jasin },
+ { "atan", "(D)D", jatan },
{ "atan2", "(DD)D", jatan2 },
- { "pow", "(DD)D", jpow },
-
- { "sinh", "(D)D", jsinh },
- { "cosh", "(D)D", jcosh },
- { "tanh", "(D)D", jtanh },
- { "log10", "(D)D", jlog10 },
{ "cbrt", "(D)D", jcbrt },
+ { "ceil", "(D)D", jceil },
+ { "copySign", "(DD)D", copySign },
+ { "copySign", "(FF)F", copySign_f },
+ { "cos", "(D)D", jcos },
+ { "cosh", "(D)D", jcosh },
+ { "exp", "(D)D", jexp },
{ "expm1", "(D)D", jexpm1 },
+ { "floor", "(D)D", jfloor },
{ "hypot", "(DD)D", jhypot },
+ { "log", "(D)D", jlog },
+ { "log10", "(D)D", jlog10 },
{ "log1p", "(D)D", jlog1p },
{ "nextafter", "(DD)D", jnextafter },
{ "nextafterf", "(FF)F", jnextafterf },
+ { "pow", "(DD)D", jpow },
+ { "rint", "(D)D", jrint },
+ { "sin", "(D)D", jsin },
+ { "sinh", "(D)D", jsinh },
+ { "sqrt", "(D)D", jsqrt2 },
+ { "tan", "(D)D", jtan },
+ { "tanh", "(D)D", jtanh },
};
int register_java_lang_StrictMath(JNIEnv* env)
