[GR-35532] Node#replace should not create call target, set RootNode#c…

…allTarget before instrumentation, Insight fixes.

PullRequest: graal/10475

compiler/src/org.graalvm.compiler.truffle.runtime/src/org/graalvm/compiler/truffle/runtime/GraalRuntimeSupport.java

@@ -62,15 +62,26 @@ public RootCallTarget newCallTarget(CallTarget source, RootNode rootNode) {
         assert GraalRuntimeAccessor.NODES.getCallTargetWithoutInitialization(rootNode) == null : "CallTarget for root node already initialized.";
 
         CompilerAsserts.neverPartOfCompilation();
-        OptimizedCallTarget target = GraalTruffleRuntime.getRuntime().createOptimizedCallTarget((OptimizedCallTarget) source, rootNode);
+        return GraalTruffleRuntime.getRuntime().createOptimizedCallTarget((OptimizedCallTarget) source, rootNode);
+    }
+
+    @Override
+    public boolean isLoaded(CallTarget callTarget) {
+        return ((OptimizedCallTarget) callTarget).isLoaded();
+    }
+
+    @Override
+    public void notifyOnLoad(CallTarget callTarget) {
+        CompilerAsserts.neverPartOfCompilation();
+        OptimizedCallTarget target = (OptimizedCallTarget) callTarget;
         GraalRuntimeAccessor.INSTRUMENT.onLoad(target.getRootNode());
         if (target.engine.compileAOTOnCreate) {
             if (target.prepareForAOT()) {
                 target.compile(true);
             }
         }
         TruffleSplittingStrategy.newTargetCreated(target);
-        return target;
+        target.setLoaded();
     }
 
     @ExplodeLoop

compiler/src/org.graalvm.compiler.truffle.runtime/src/org/graalvm/compiler/truffle/runtime/OptimizedCallTarget.java

@@ -134,6 +134,7 @@ public abstract class OptimizedCallTarget implements CompilableTruffleAST, RootC
 
     /** Whether this call target was cloned, compiled or called. */
     @CompilationFinal protected volatile boolean initialized;
+    @CompilationFinal private volatile boolean loaded;
 
     /**
      * The call threshold is counted up for each real call until it reaches a
@@ -1630,6 +1631,15 @@ final void setNonTrivialNodeCount(int nonTrivialNodeCount) {
         this.cachedNonTrivialNodeCount = nonTrivialNodeCount;
     }
 
+    final boolean isLoaded() {
+        return loaded;
+    }
+
+    final void setLoaded() {
+        CompilerAsserts.neverPartOfCompilation();
+        this.loaded = true;
+    }
+
     public final boolean prepareForAOT() {
         if (wasExecuted()) {
             throw new IllegalStateException("Cannot prepare for AOT if call target was already executed.");

graal-common.json

@@ -1,7 +1,7 @@
 {
   "README": "This file contains definitions that are useful for the hocon and jsonnet CI files of the graal and graal-enterprise repositories.",
   "ci": {
-    "overlay": "031ab3f1c9d415abd5d889227a632d210221b3a5"
+    "overlay": "30806c62fe3527181448edf5716b4a151634262a"
   },
   "mx_version" : "HEAD"
 }

tools/src/com.oracle.truffle.tools.agentscript/src/com/oracle/truffle/tools/agentscript/impl/CurrentScopeView.java

@@ -0,0 +1,110 @@
+/*
+ * Copyright (c) 2021, 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+package com.oracle.truffle.tools.agentscript.impl;
+
+import com.oracle.truffle.api.interop.InteropLibrary;
+import com.oracle.truffle.api.interop.InvalidArrayIndexException;
+import com.oracle.truffle.api.interop.TruffleObject;
+import com.oracle.truffle.api.interop.UnsupportedMessageException;
+import com.oracle.truffle.api.library.CachedLibrary;
+import com.oracle.truffle.api.library.ExportLibrary;
+import com.oracle.truffle.api.library.ExportMessage;
+
+/**
+ * Creates a view of the current execution scope, hiding members of the parent scope.
+ */
+@ExportLibrary(value = InteropLibrary.class, delegateTo = "scope")
+final class CurrentScopeView implements TruffleObject {
+
+    final Object scope;
+
+    CurrentScopeView(Object scope) {
+        this.scope = scope;
+    }
+
+    @ExportMessage
+    @SuppressWarnings("static-method")
+    boolean hasMembers() {
+        return true;
+    }
+
+    @ExportMessage
+    Object getMembers(@SuppressWarnings("unused") boolean includeInternal,
+                    @CachedLibrary("this.scope") InteropLibrary scopeLib,
+                    @CachedLibrary(limit = "5") InteropLibrary parentScopeLib) throws UnsupportedMessageException {
+        Object allKeys = scopeLib.getMembers(scope);
+        if (scopeLib.hasScopeParent(scope)) {
+            Object parentScope = scopeLib.getScopeParent(scope);
+            Object parentKeys = parentScopeLib.getMembers(parentScope);
+            return new SubtractedKeys(allKeys, parentKeys);
+        }
+        return allKeys;
+    }
+
+    @ExportLibrary(InteropLibrary.class)
+    static final class SubtractedKeys implements TruffleObject {
+
+        final Object allKeys;
+        private final long allSize;
+        private final long removedSize;
+
+        SubtractedKeys(Object allKeys, Object removedKeys) throws UnsupportedMessageException {
+            this.allKeys = allKeys;
+            this.allSize = InteropLibrary.getUncached().getArraySize(allKeys);
+            this.removedSize = InteropLibrary.getUncached().getArraySize(removedKeys);
+        }
+
+        @ExportMessage
+        @SuppressWarnings("static-method")
+        boolean hasArrayElements() {
+            return true;
+        }
+
+        @ExportMessage
+        long getArraySize() {
+            return allSize - removedSize;
+        }
+
+        @ExportMessage
+        Object readArrayElement(long index,
+                        @CachedLibrary("this.allKeys") InteropLibrary interop) throws InvalidArrayIndexException, UnsupportedMessageException {
+            if (0 <= index && index < getArraySize()) {
+                return interop.readArrayElement(allKeys, index);
+            } else {
+                throw InvalidArrayIndexException.create(index);
+            }
+        }
+
+        @ExportMessage
+        boolean isArrayElementReadable(long index,
+                        @CachedLibrary("this.allKeys") InteropLibrary interop) {
+            if (0 <= index && index < getArraySize()) {
+                return interop.isArrayElementReadable(allKeys, index);
+            } else {
+                return false;
+            }
+        }
+    }
+}

tools/src/com.oracle.truffle.tools.agentscript/src/com/oracle/truffle/tools/agentscript/impl/EventContextObject.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2021, 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
@@ -29,6 +29,7 @@
 import com.oracle.truffle.api.frame.Frame;
 import com.oracle.truffle.api.frame.FrameInstance;
 import com.oracle.truffle.api.instrumentation.EventContext;
+import com.oracle.truffle.api.instrumentation.InstrumentableNode;
 import com.oracle.truffle.api.interop.ArityException;
 import com.oracle.truffle.api.interop.InteropException;
 import com.oracle.truffle.api.interop.InteropLibrary;
@@ -128,9 +129,10 @@ private static Object iterateFrames(Object[] args, EventContextObject obj) throw
                 return null;
             }
             final Frame frame = frameInstance.getFrame(FrameInstance.FrameAccess.READ_WRITE);
-            if (lib.hasScope(n, frame)) {
+            Node instrumentableNode = InstrumentableNode.findInstrumentableParent(n);
+            if (instrumentableNode != null && lib.hasScope(instrumentableNode, frame)) {
                 try {
-                    Object frameVars = lib.getScope(n, frame, false);
+                    Object frameVars = new CurrentScopeView(lib.getScope(instrumentableNode, frame, false));
                     Object ret = iop.execute(callback, location, frameVars);
                     return iop.isNull(ret) ? null : ret;
                 } catch (UnsupportedMessageException | UnsupportedTypeException | ArityException ex) {

truffle/src/com.oracle.truffle.api.interop/src/com/oracle/truffle/api/interop/DefaultNodeExports.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2020, 2021, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * The Universal Permissive License (UPL), Version 1.0
@@ -62,23 +62,25 @@
 @SuppressWarnings("static-method")
 final class DefaultNodeExports {
 
+    @TruffleBoundary
     @ExportMessage
     @SuppressWarnings("unused")
     static boolean hasScope(Node node, Frame frame) {
         RootNode root = node.getRootNode();
         TruffleLanguage<?> language = InteropAccessor.NODES.getLanguage(root);
-        return language != null;
+        return language != null && (node == root || InteropAccessor.ACCESSOR.instrumentSupport().isInstrumentable(node));
     }
 
+    @TruffleBoundary
     @ExportMessage
     @SuppressWarnings({"unchecked", "unused"})
     static Object getScope(Node node, Frame frame, boolean nodeEnter) throws UnsupportedMessageException {
         RootNode root = node.getRootNode();
         TruffleLanguage<?> language = InteropAccessor.NODES.getLanguage(root);
-        if (language == null) {
-            throw UnsupportedMessageException.create();
+        if (language != null && (node == root || InteropAccessor.ACCESSOR.instrumentSupport().isInstrumentable(node))) {
+            return createDefaultScope(root, frame, (Class<? extends TruffleLanguage<?>>) language.getClass());
         }
-        return createDefaultScope(root, frame, (Class<? extends TruffleLanguage<?>>) language.getClass());
+        throw UnsupportedMessageException.create();
     }
 
     private static boolean isInternal(Object identifier) {

truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/ReplaceObserver.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2013, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2013, 2021, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * The Universal Permissive License (UPL), Version 1.0
@@ -43,7 +43,8 @@
 import com.oracle.truffle.api.nodes.Node;
 
 /**
- * An observer that is notified whenever a child node is replaced.
+ * An observer that is notified whenever a child node is replaced. Note this is <b>not</b> called if
+ * the RootNode does not have a CallTarget yet.
  * 
  * @since 0.8 or earlier
  */

truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/impl/Accessor.java

@@ -989,6 +989,10 @@ protected RuntimeSupport(Object permission) {
 
         public abstract RootCallTarget newCallTarget(CallTarget source, RootNode rootNode);
 
+        public abstract boolean isLoaded(CallTarget callTarget);
+
+        public abstract void notifyOnLoad(CallTarget callTarget);
+
         public ThreadLocalHandshake getThreadLocalHandshake() {
             return DefaultThreadLocalHandshake.SINGLETON;
         }

truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/impl/DefaultCallTarget.java

@@ -59,6 +59,7 @@ public final class DefaultCallTarget implements RootCallTarget {
     public static final String CALL_BOUNDARY_METHOD = "callDirectOrIndirect";
     private final RootNode rootNode;
     private volatile boolean initialized;
+    private volatile boolean loaded;
 
     DefaultCallTarget(RootNode function) {
         this.rootNode = function;
@@ -112,4 +113,12 @@ private void initialize() {
             }
         }
     }
+
+    boolean isLoaded() {
+        return loaded;
+    }
+
+    void setLoaded() {
+        this.loaded = true;
+    }
 }

truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/impl/DefaultRuntimeAccessor.java

@@ -42,17 +42,17 @@
 
 import java.util.function.Function;
 
-import com.oracle.truffle.api.frame.Frame;
-import com.oracle.truffle.api.frame.VirtualFrame;
-import com.oracle.truffle.api.nodes.BytecodeOSRNode;
 import org.graalvm.options.OptionDescriptors;
 import org.graalvm.options.OptionValues;
 
 import com.oracle.truffle.api.CallTarget;
 import com.oracle.truffle.api.RootCallTarget;
 import com.oracle.truffle.api.TruffleLogger;
+import com.oracle.truffle.api.frame.Frame;
+import com.oracle.truffle.api.frame.VirtualFrame;
 import com.oracle.truffle.api.nodes.BlockNode;
 import com.oracle.truffle.api.nodes.BlockNode.ElementExecutor;
+import com.oracle.truffle.api.nodes.BytecodeOSRNode;
 import com.oracle.truffle.api.nodes.Node;
 import com.oracle.truffle.api.nodes.RootNode;
 
@@ -79,9 +79,19 @@ static final class DefaultRuntimeSupport extends RuntimeSupport {
 
         @Override
         public RootCallTarget newCallTarget(CallTarget sourceCallTarget, RootNode rootNode) {
-            DefaultCallTarget target = new DefaultCallTarget(rootNode);
-            DefaultRuntimeAccessor.INSTRUMENT.onLoad(rootNode);
-            return target;
+            return new DefaultCallTarget(rootNode);
+        }
+
+        @Override
+        public boolean isLoaded(CallTarget callTarget) {
+            return ((DefaultCallTarget) callTarget).isLoaded();
+        }
+
+        @Override
+        public void notifyOnLoad(CallTarget callTarget) {
+            DefaultCallTarget target = (DefaultCallTarget) callTarget;
+            DefaultRuntimeAccessor.INSTRUMENT.onLoad(target.getRootNode());
+            target.setLoaded();
         }
 
         @Override

truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/nodes/Node.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2012, 2021, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * The Universal Permissive License (UPL), Version 1.0
@@ -419,7 +419,9 @@ private void reportReplace(Node oldNode, Node newNode, CharSequence reason) {
             } else if (node instanceof BytecodeOSRNode) {
                 NodeAccessor.RUNTIME.onOSRNodeReplaced((BytecodeOSRNode) node, oldNode, newNode, reason);
             } else if (node instanceof RootNode) {
-                CallTarget target = ((RootNode) node).getCallTarget();
+                // Avoid creating a CallTarget here if replace() is called before this RootNode has
+                // a CallTarget
+                CallTarget target = ((RootNode) node).getCallTargetWithoutInitialization();
                 if (target instanceof ReplaceObserver) {
                     consumed = ((ReplaceObserver) target).nodeReplaced(oldNode, newNode, reason);
                 }