From 3aadff8dcd53d5a66a07495a57f9856db4b790c1 Mon Sep 17 00:00:00 2001
From: Lukas Stadler <lukas.stadler@oracle.com>
Date: Thu, 22 Dec 2016 15:41:25 +0100
Subject: [PATCH] prevent NaN to 0L conversion in subset/subscript specials
---
.../builtin/base/infix/SpecialsUtils.java | 34 ++++++++-----------
.../r/nodes/builtin/base/infix/Subscript.java | 6 ++--
.../r/nodes/builtin/base/infix/Subset.java | 8 ++---
.../builtin/base/infix/UpdateSubscript.java | 6 ++--
.../builtin/base/infix/UpdateSubset.java | 6 ++--
.../truffle/r/nodes/test/SpecialCallTest.java | 2 +-
6 files changed, 28 insertions(+), 34 deletions(-)
diff --git a/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/infix/SpecialsUtils.java b/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/infix/SpecialsUtils.java
index cae45bca9a..75434227a5 100644
--- a/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/infix/SpecialsUtils.java
+++ b/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/infix/SpecialsUtils.java
@@ -22,6 +22,7 @@
*/
package com.oracle.truffle.r.nodes.builtin.base.infix;
+import com.oracle.truffle.api.CompilerDirectives;
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
import com.oracle.truffle.api.dsl.NodeChild;
import com.oracle.truffle.api.dsl.NodeChildren;
@@ -30,7 +31,6 @@ import com.oracle.truffle.api.dsl.TypeSystemReference;
import com.oracle.truffle.api.frame.VirtualFrame;
import com.oracle.truffle.api.nodes.NodeCost;
import com.oracle.truffle.api.nodes.NodeInfo;
-import com.oracle.truffle.api.profiles.ConditionProfile;
import com.oracle.truffle.api.profiles.ValueProfile;
import com.oracle.truffle.r.nodes.EmptyTypeSystemFlatLayout;
import com.oracle.truffle.r.nodes.attributes.SpecialAttributesFunctions.GetDimAttributeNode;
@@ -176,14 +176,6 @@ class SpecialsUtils {
@TypeSystemReference(EmptyTypeSystemFlatLayout.class)
public abstract static class ConvertIndex extends RNode {
- private final boolean isSubset;
- private final ConditionProfile zeroProfile;
-
- ConvertIndex(boolean isSubset) {
- this.isSubset = isSubset;
- this.zeroProfile = isSubset ? null : ConditionProfile.createBinaryProfile();
- }
-
protected abstract RNode getDelegate();
@Specialization
@@ -191,14 +183,20 @@ class SpecialsUtils {
return value;
}
- @Specialization
+ @Specialization(rewriteOn = IllegalArgumentException.class)
protected int convertDouble(double value) {
- // Conversion from double to an index differs in subscript and subset.
int intValue = (int) value;
- if (isSubset) {
- return intValue;
+ if (intValue == 0) {
+ /*
+ * Conversion from double to an index differs in subscript and subset for values in
+ * the ]0..1[ range (subscript interprets 0.1 as 1, whereas subset treats it as 0).
+ * We avoid this special case by simply going to the more generic case for this
+ * range. Additionally, (int) Double.NaN is 0, which is also caught by this case.
+ */
+ CompilerDirectives.transferToInterpreterAndInvalidate();
+ throw new IllegalArgumentException();
} else {
- return zeroProfile.profile(intValue == 0) ? (value == 0 ? 0 : 1) : intValue;
+ return intValue;
}
}
@@ -217,11 +215,7 @@ class SpecialsUtils {
return new ProfiledValue(value);
}
- public static ConvertIndex convertSubscript(RNode value) {
- return ConvertIndexNodeGen.create(false, value);
- }
-
- public static ConvertIndex convertSubset(RNode value) {
- return ConvertIndexNodeGen.create(true, value);
+ public static ConvertIndex convertIndex(RNode value) {
+ return ConvertIndexNodeGen.create(value);
}
}
diff --git a/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/infix/Subscript.java b/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/infix/Subscript.java
index 4c8fe6acd3..639d98960d 100644
--- a/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/infix/Subscript.java
+++ b/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/infix/Subscript.java
@@ -22,7 +22,7 @@
*/
package com.oracle.truffle.r.nodes.builtin.base.infix;
-import static com.oracle.truffle.r.nodes.builtin.base.infix.SpecialsUtils.convertSubscript;
+import static com.oracle.truffle.r.nodes.builtin.base.infix.SpecialsUtils.convertIndex;
import static com.oracle.truffle.r.nodes.builtin.base.infix.SpecialsUtils.profile;
import static com.oracle.truffle.r.runtime.RDispatch.INTERNAL_GENERIC;
import static com.oracle.truffle.r.runtime.builtins.RBehavior.PURE;
@@ -193,9 +193,9 @@ public abstract class Subscript extends RBuiltinNode {
public static RNode special(ArgumentsSignature signature, RNode[] arguments, boolean inReplacement) {
if (signature.getNonNullCount() == 0) {
if (arguments.length == 2) {
- return SubscriptSpecialNodeGen.create(inReplacement, profile(arguments[0]), convertSubscript(arguments[1]));
+ return SubscriptSpecialNodeGen.create(inReplacement, profile(arguments[0]), convertIndex(arguments[1]));
} else if (arguments.length == 3) {
- return SubscriptSpecial2NodeGen.create(inReplacement, profile(arguments[0]), convertSubscript(arguments[1]), convertSubscript(arguments[2]));
+ return SubscriptSpecial2NodeGen.create(inReplacement, profile(arguments[0]), convertIndex(arguments[1]), convertIndex(arguments[2]));
}
}
return null;
diff --git a/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/infix/Subset.java b/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/infix/Subset.java
index 05954ea72e..b36cbf5a3e 100644
--- a/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/infix/Subset.java
+++ b/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/infix/Subset.java
@@ -22,7 +22,7 @@
*/
package com.oracle.truffle.r.nodes.builtin.base.infix;
-import static com.oracle.truffle.r.nodes.builtin.base.infix.SpecialsUtils.convertSubset;
+import static com.oracle.truffle.r.nodes.builtin.base.infix.SpecialsUtils.convertIndex;
import static com.oracle.truffle.r.nodes.builtin.base.infix.SpecialsUtils.profile;
import static com.oracle.truffle.r.runtime.RDispatch.INTERNAL_GENERIC;
import static com.oracle.truffle.r.runtime.builtins.RBehavior.PURE;
@@ -81,7 +81,7 @@ abstract class SubsetSpecial extends SubscriptSpecialBase {
}
@Specialization(guards = {"simpleVector(vector)", "!inReplacement"})
- protected static Object access(VirtualFrame frame, RAbstractVector vector, Object index,
+ protected Object access(VirtualFrame frame, RAbstractVector vector, Object index,
@Cached("createAccess()") ExtractVectorNode extract) {
return extract.apply(frame, vector, new Object[]{index}, RRuntime.LOGICAL_TRUE, RLogical.TRUE);
}
@@ -124,11 +124,11 @@ public abstract class Subset extends RBuiltinNode {
public static RNode special(ArgumentsSignature signature, RNode[] args, boolean inReplacement) {
if (signature.getNonNullCount() == 0 && (args.length == 2 || args.length == 3)) {
ProfiledValue profiledVector = profile(args[0]);
- ConvertIndex index = convertSubset(args[1]);
+ ConvertIndex index = convertIndex(args[1]);
if (args.length == 2) {
return SubsetSpecialNodeGen.create(inReplacement, profiledVector, index);
} else {
- return SubsetSpecial2NodeGen.create(inReplacement, profiledVector, index, convertSubset(args[2]));
+ return SubsetSpecial2NodeGen.create(inReplacement, profiledVector, index, convertIndex(args[2]));
}
}
return null;
diff --git a/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/infix/UpdateSubscript.java b/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/infix/UpdateSubscript.java
index 4825f4efd3..1488822ff5 100644
--- a/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/infix/UpdateSubscript.java
+++ b/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/infix/UpdateSubscript.java
@@ -22,7 +22,7 @@
*/
package com.oracle.truffle.r.nodes.builtin.base.infix;
-import static com.oracle.truffle.r.nodes.builtin.base.infix.SpecialsUtils.convertSubscript;
+import static com.oracle.truffle.r.nodes.builtin.base.infix.SpecialsUtils.convertIndex;
import static com.oracle.truffle.r.nodes.builtin.base.infix.SpecialsUtils.profile;
import static com.oracle.truffle.r.runtime.RDispatch.INTERNAL_GENERIC;
import static com.oracle.truffle.r.runtime.builtins.RBehavior.PURE;
@@ -168,11 +168,11 @@ public abstract class UpdateSubscript extends RBuiltinNode {
public static RNode special(ArgumentsSignature signature, RNode[] args, boolean inReplacement) {
if (SpecialsUtils.isCorrectUpdateSignature(signature) && (args.length == 3 || args.length == 4)) {
ProfiledValue vector = profile(args[0]);
- ConvertIndex index = convertSubscript(args[1]);
+ ConvertIndex index = convertIndex(args[1]);
if (args.length == 3) {
return UpdateSubscriptSpecialNodeGen.create(inReplacement, vector, index, args[2]);
} else {
- return UpdateSubscriptSpecial2NodeGen.create(inReplacement, vector, index, convertSubscript(args[2]), args[3]);
+ return UpdateSubscriptSpecial2NodeGen.create(inReplacement, vector, index, convertIndex(args[2]), args[3]);
}
}
return null;
diff --git a/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/infix/UpdateSubset.java b/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/infix/UpdateSubset.java
index 820e2ebcf5..4e9fd8094a 100644
--- a/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/infix/UpdateSubset.java
+++ b/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/infix/UpdateSubset.java
@@ -22,7 +22,7 @@
*/
package com.oracle.truffle.r.nodes.builtin.base.infix;
-import static com.oracle.truffle.r.nodes.builtin.base.infix.SpecialsUtils.convertSubset;
+import static com.oracle.truffle.r.nodes.builtin.base.infix.SpecialsUtils.convertIndex;
import static com.oracle.truffle.r.nodes.builtin.base.infix.SpecialsUtils.profile;
import static com.oracle.truffle.r.runtime.RDispatch.INTERNAL_GENERIC;
import static com.oracle.truffle.r.runtime.builtins.RBehavior.PURE;
@@ -57,11 +57,11 @@ public abstract class UpdateSubset extends RBuiltinNode {
public static RNode special(ArgumentsSignature signature, RNode[] args, boolean inReplacement) {
if (SpecialsUtils.isCorrectUpdateSignature(signature) && (args.length == 3 || args.length == 4)) {
ProfiledValue vector = profile(args[0]);
- ConvertIndex index = convertSubset(args[1]);
+ ConvertIndex index = convertIndex(args[1]);
if (args.length == 3) {
return UpdateSubscriptSpecialNodeGen.create(inReplacement, vector, index, args[2]);
} else {
- return UpdateSubscriptSpecial2NodeGen.create(inReplacement, vector, index, convertSubset(args[2]), args[3]);
+ return UpdateSubscriptSpecial2NodeGen.create(inReplacement, vector, index, convertIndex(args[2]), args[3]);
}
}
return null;
diff --git a/com.oracle.truffle.r.nodes.test/src/com/oracle/truffle/r/nodes/test/SpecialCallTest.java b/com.oracle.truffle.r.nodes.test/src/com/oracle/truffle/r/nodes/test/SpecialCallTest.java
index c32dd569e8..24af4e1560 100644
--- a/com.oracle.truffle.r.nodes.test/src/com/oracle/truffle/r/nodes/test/SpecialCallTest.java
+++ b/com.oracle.truffle.r.nodes.test/src/com/oracle/truffle/r/nodes/test/SpecialCallTest.java
@@ -234,7 +234,7 @@ public class SpecialCallTest extends TestBase {
assertCallCounts("a <- c(1,2,3,4)", "a[[4]] <- 1", 1, 0, 2, 0);
assertCallCounts("a <- list(c(1,2,3,4),2,3)", "a[[1]] <- 1", 1, 0, 2, 0);
assertCallCounts("a <- list(a=c(1,2,3,4),2,3)", "a[[1]] <- 1", 1, 0, 2, 0);
- assertCallCounts("a <- c(1,2,3,4)", "a[[0.1]] <- 1", 1, 0, 2, 0);
+ assertCallCounts("a <- c(1,2,3,4)", "a[[0.1]] <- 1", 1, 0, 1, 1);
assertCallCounts("a <- c(1,2,3,4)", "a[[5]] <- 1", 1, 0, 1, 1);
assertCallCounts("a <- c(1,2,3,4)", "a[[0]] <- 1", 1, 0, 1, 1);
assertCallCounts("a <- c(1,2,3,4); b <- -1", "a[[b]] <- 1", 1, 0, 1, 1);
--
GitLab