From c4faee90b7931710b18ef9b4686530220ea3b264 Mon Sep 17 00:00:00 2001
From: Florian Angerer <florian.angerer@oracle.com>
Date: Fri, 18 Aug 2017 14:55:57 +0200
Subject: [PATCH] Fix: Interaction between active bindings and multi slot data.

---
 .../r/nodes/access/BaseWriteVariableNode.java | 11 ++++++----
 .../access/WriteLocalFrameVariableNode.java   |  4 ++--
 .../access/WriteSuperFrameVariableNode.java   |  2 +-
 .../env/frame/FrameSlotChangeMonitor.java     | 11 +++++-----
 .../truffle/r/test/ExpectedTestOutput.test    | 20 +++++++++----------
 .../com/oracle/truffle/r/test/S4/TestR5.java  |  4 ++--
 .../oracle/truffle/r/test/rng/TestRRNG.java   |  2 +-
 7 files changed, 28 insertions(+), 26 deletions(-)

diff --git a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/access/BaseWriteVariableNode.java b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/access/BaseWriteVariableNode.java
index e699b28a72..25dd64ff79 100644
--- a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/access/BaseWriteVariableNode.java
+++ b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/access/BaseWriteVariableNode.java
@@ -165,12 +165,14 @@ abstract class BaseWriteVariableNode extends WriteVariableNode {
      * @param value The value to set.
      * @param frameSlot The frame slot of the value.
      * @param invalidateProfile The invalidation profile.
+     * @param mode
+     * @param storedObjectProfile
      */
-    protected static Object handleActiveBinding(VirtualFrame execFrame, Frame lookupFrame, Object value, FrameSlot frameSlot, BranchProfile invalidateProfile,
-                    ConditionProfile isActiveBindingProfile) {
+    protected Object handleActiveBinding(VirtualFrame execFrame, Frame lookupFrame, Object value, FrameSlot frameSlot, BranchProfile invalidateProfile,
+                    ConditionProfile isActiveBindingProfile, ValueProfile storedObjectProfile, Mode mode) {
         Object object;
         try {
-            object = lookupFrame.getObject(frameSlot);
+            object = FrameSlotChangeMonitor.getObject(frameSlot, lookupFrame);
         } catch (FrameSlotTypeException e) {
             object = null;
         }
@@ -178,7 +180,8 @@ abstract class BaseWriteVariableNode extends WriteVariableNode {
         if (isActiveBindingProfile.profile(object != null && ActiveBinding.isActiveBinding(object))) {
             return ((ActiveBinding) object).writeValue(value);
         } else {
-            FrameSlotChangeMonitor.setObjectAndInvalidate(lookupFrame, frameSlot, value, false, invalidateProfile);
+            Object newValue = shareObjectValue(lookupFrame, frameSlot, storedObjectProfile.profile(value), mode, false);
+            FrameSlotChangeMonitor.setObjectAndInvalidate(lookupFrame, frameSlot, newValue, false, invalidateProfile);
         }
         return value;
     }
diff --git a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/access/WriteLocalFrameVariableNode.java b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/access/WriteLocalFrameVariableNode.java
index 527369c269..19b3ab595d 100644
--- a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/access/WriteLocalFrameVariableNode.java
+++ b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/access/WriteLocalFrameVariableNode.java
@@ -97,12 +97,12 @@ public abstract class WriteLocalFrameVariableNode extends BaseWriteVariableNode
             CompilerDirectives.transferToInterpreterAndInvalidate();
             containsNoActiveBinding = FrameSlotChangeMonitor.getContainsNoActiveBindingAssumption(frame.getFrameDescriptor());
         }
-        Object newValue = shareObjectValue(frame, frameSlot, storedObjectProfile.profile(value), mode, false);
         if (containsNoActiveBinding.isValid()) {
+            Object newValue = shareObjectValue(frame, frameSlot, storedObjectProfile.profile(value), mode, false);
             FrameSlotChangeMonitor.setObjectAndInvalidate(frame, frameSlot, newValue, false, invalidateProfile);
         } else {
             // it's a local variable lookup; so use 'frame' for both, executing and looking up
-            return handleActiveBinding(frame, frame, newValue, frameSlot, invalidateProfile, isActiveBindingProfile);
+            return handleActiveBinding(frame, frame, value, frameSlot, invalidateProfile, isActiveBindingProfile, storedObjectProfile, mode);
         }
         return value;
     }
diff --git a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/access/WriteSuperFrameVariableNode.java b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/access/WriteSuperFrameVariableNode.java
index a389e83a42..90f278cc2d 100644
--- a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/access/WriteSuperFrameVariableNode.java
+++ b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/access/WriteSuperFrameVariableNode.java
@@ -111,7 +111,7 @@ abstract class WriteSuperFrameVariableNode extends BaseWriteVariableNode {
                 Object newValue = shareObjectValue(profiledFrame, frameSlot, storedObjectProfile.profile(value), mode, true);
                 FrameSlotChangeMonitor.setObjectAndInvalidate(profiledFrame, frameSlot, newValue, true, invalidateProfile);
             } else {
-                handleActiveBinding(frame, profiledFrame, value, frameSlot, invalidateProfile, isActiveBindingProfile);
+                handleActiveBinding(frame, profiledFrame, value, frameSlot, invalidateProfile, isActiveBindingProfile, storedObjectProfile, mode);
             }
         }
     }
diff --git a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/env/frame/FrameSlotChangeMonitor.java b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/env/frame/FrameSlotChangeMonitor.java
index 47d82e72ec..20c8cd97be 100644
--- a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/env/frame/FrameSlotChangeMonitor.java
+++ b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/env/frame/FrameSlotChangeMonitor.java
@@ -853,6 +853,10 @@ public final class FrameSlotChangeMonitor {
 
     public static void setObjectAndInvalidate(Frame frame, FrameSlot frameSlot, Object newValue, boolean isNonLocal, BranchProfile invalidateProfile) {
         assert !ActiveBinding.isActiveBinding(newValue);
+        setAndInvalidate(frame, frameSlot, newValue, isNonLocal, invalidateProfile);
+    }
+
+    private static void setAndInvalidate(Frame frame, FrameSlot frameSlot, Object newValue, boolean isNonLocal, BranchProfile invalidateProfile) {
         FrameSlotInfoImpl info = getFrameSlotInfo(frameSlot);
         if (FastROptions.SharedContexts.getBooleanValue() && info.possibleMultiSlot() && !RContext.isSingle()) {
             info.setMultiSlot(frame, frameSlot, newValue);
@@ -877,12 +881,7 @@ public final class FrameSlotChangeMonitor {
     }
 
     public static void setActiveBinding(Frame frame, FrameSlot frameSlot, ActiveBinding newValue, boolean isNonLocal, BranchProfile invalidateProfile) {
-        frame.setObject(frameSlot, newValue);
-        FrameSlotInfoImpl info = getFrameSlotInfo(frameSlot);
-        if (info.needsInvalidation()) {
-            info.setValue(newValue, frameSlot);
-        }
-        checkAndInvalidate(frame, frameSlot, isNonLocal, invalidateProfile);
+        setAndInvalidate(frame, frameSlot, newValue, isNonLocal, invalidateProfile);
         getContainsNoActiveBindingAssumption(frame.getFrameDescriptor()).invalidate();
     }
 
diff --git a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/ExpectedTestOutput.test b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/ExpectedTestOutput.test
index 0b075e7151..085cc297b2 100644
--- a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/ExpectedTestOutput.test
+++ b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/ExpectedTestOutput.test
@@ -1,6 +1,6 @@
 ##com.oracle.truffle.r.test.S4.TestR5.testAllocation#
-#env0 <- new.env(); setRefClass('Foo17R5', where = env0); ls(topenv(parent.frame()), all.names = T); ls(env0, all.names = T)
-[1] "env0"
+#env0 <- new.env(); setRefClass('Foo17R5', where = env0); grep('Foo17R5', ls(topenv(parent.frame()), all.names = T), value = TRUE); ls(env0, all.names = T)
+character(0)
 [1] ".__C__Foo17R5"          ".__global__"            ".requireCachedGenerics"
 
 ##com.oracle.truffle.r.test.S4.TestR5.testAllocation#
@@ -31,8 +31,8 @@ In methods::initRefFields(.Object, classDef, selfEnv, list(...)) :
   unnamed arguments to $new() must be objects from a reference class; got an object of class “numeric”
 
 ##com.oracle.truffle.r.test.S4.TestR5.testAllocation#
-#{ setRefClass('Foo16R5'); ls(topenv(parent.frame()), all.names = T) }
-[1] ".__C__Foo16R5"          ".__global__"            ".requireCachedGenerics"
+#{ setRefClass('Foo16R5'); grep('Foo16R5', ls(topenv(parent.frame()), all.names = T), value = TRUE) }
+[1] ".__C__Foo16R5"
 
 ##com.oracle.truffle.r.test.S4.TestR5.testAttributes#
 #{ clazz <- setRefClass('Foo13R5'); clazz$methods() }
@@ -75123,6 +75123,10 @@ imaginary parts discarded in coercion
 #{ as.vector(list(1,2,3), mode="integer") }
 [1] 1 2 3
 
+##com.oracle.truffle.r.test.builtins.TestMiscBuiltins.testCasts#
+#{ env <- new.env(); env$x<-7; env$.y<-42; length(as.list(env, all.names=TRUE)) }
+[1] 2
+
 ##com.oracle.truffle.r.test.builtins.TestMiscBuiltins.testCasts#
 #{ k <- as.list(3:6) ; l <- as.list(1) ; list(k,l) }
 [[1]]
@@ -75261,10 +75265,6 @@ $x
 [1] 7
 
 
-##com.oracle.truffle.r.test.builtins.TestMiscBuiltins.testCasts#
-#{ x<-7; .y<-42; length(as.list(environment(), all.names=TRUE)) }
-[1] 2
-
 ##com.oracle.truffle.r.test.builtins.TestMiscBuiltins.testCasts#
 #{ x<-7; as.list(environment()) }
 $x
@@ -155997,7 +155997,7 @@ non-integer value 12345678909876543212L qualified with L; using numeric value
 [619]  -346017954 -1916029881 -1735526436  1461823277   580724746  -622095869
 [625]  -686746776   705745481
 
-##com.oracle.truffle.r.test.rng.TestRRNG.testDirectSeedAssignment#Output.IgnoreWarningContext#
+##com.oracle.truffle.r.test.rng.TestRRNG.testDirectSeedAssignment#Output.IgnoreWarningContext#Output.MayIgnoreWarningContext#
 #.Random.seed <- c(401, 1, 2); invisible(runif(3))
 Warning message:
 In runif(3) :
@@ -156007,7 +156007,7 @@ In runif(3) :
 #.Random.seed <- c(401L, 1L, 2L); runif(3)
 [1] 0.5641106 0.2932388 0.6696743
 
-##com.oracle.truffle.r.test.rng.TestRRNG.testDirectSeedAssignment#Output.IgnoreWarningContext#
+##com.oracle.truffle.r.test.rng.TestRRNG.testDirectSeedAssignment#Output.IgnoreWarningContext#Output.MayIgnoreWarningContext#
 #.Random.seed <- c(999, 1, 2); invisible(runif(3))
 Warning message:
 In runif(3) :
diff --git a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/S4/TestR5.java b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/S4/TestR5.java
index 650fb03db0..e263c2eaa1 100644
--- a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/S4/TestR5.java
+++ b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/S4/TestR5.java
@@ -49,8 +49,8 @@ public class TestR5 extends TestBase {
         assertEval("{ DummyClass2 <- setRefClass('DummyClass2'); obj <- DummyClass2$new(); is(obj, 'refObject') }");
         assertEval("{ fooClass <- setRefClass('Foo6R5', fields = list( a = 'numeric')); fooClass$new(a = 1) }");
         assertEval("{ fooClass <- setRefClass('Foo7R5', fields = list( a = 'numeric')); fooClass$new(1) }");
-        assertEval("{ setRefClass('Foo16R5'); ls(topenv(parent.frame()), all.names = T) }");
-        assertEval("env0 <- new.env(); setRefClass('Foo17R5', where = env0); ls(topenv(parent.frame()), all.names = T); ls(env0, all.names = T)");
+        assertEval("{ setRefClass('Foo16R5'); grep('Foo16R5', ls(topenv(parent.frame()), all.names = T), value = TRUE) }");
+        assertEval("env0 <- new.env(); setRefClass('Foo17R5', where = env0); grep('Foo17R5', ls(topenv(parent.frame()), all.names = T), value = TRUE); ls(env0, all.names = T)");
     }
 
     @Test
diff --git a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/rng/TestRRNG.java b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/rng/TestRRNG.java
index 0a0b86d08c..dc4faf27a8 100644
--- a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/rng/TestRRNG.java
+++ b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/rng/TestRRNG.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2017, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
-- 
GitLab