Fix: Nodes tagged with RootTag throw ReturnException

steve-s · steve-s · commit bf84216f67cb · 2020-01-22T10:21:29.000+01:00
diff --git a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/FunctionBodyNode.java b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/FunctionBodyNode.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2020, 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
@@ -23,18 +23,22 @@
 package com.oracle.truffle.r.nodes.function;
 
 import com.oracle.truffle.api.CompilerDirectives;
+import com.oracle.truffle.api.CompilerDirectives.CompilationFinal;
 import com.oracle.truffle.api.frame.FrameDescriptor;
 import com.oracle.truffle.api.frame.FrameSlot;
 import com.oracle.truffle.api.frame.FrameSlotKind;
 import com.oracle.truffle.api.frame.VirtualFrame;
 import com.oracle.truffle.api.nodes.Node;
 import com.oracle.truffle.api.profiles.BranchProfile;
+import com.oracle.truffle.api.profiles.ConditionProfile;
 import com.oracle.truffle.api.source.SourceSection;
 import com.oracle.truffle.r.runtime.RArguments;
 import com.oracle.truffle.r.runtime.RArguments.DispatchArgs;
 import com.oracle.truffle.r.runtime.RArguments.S3Args;
 import com.oracle.truffle.r.runtime.RArguments.S4Args;
+import com.oracle.truffle.r.runtime.RInternalError;
 import com.oracle.truffle.r.runtime.RRuntime;
+import com.oracle.truffle.r.runtime.ReturnException;
 import com.oracle.truffle.r.runtime.RootBodyNode;
 import com.oracle.truffle.r.runtime.env.frame.FrameSlotChangeMonitor;
 import com.oracle.truffle.r.runtime.nodes.RNode;
@@ -46,16 +50,34 @@ public final class FunctionBodyNode extends Node implements RootBodyNode {
     @Child private SetupS3ArgsNode setupS3Args;
     @Child private SetupS4ArgsNode setupS4Args;
 
+    // Profiling for catching ReturnException
+    private final BranchProfile returnExceptionProfile = BranchProfile.create();
+    @CompilationFinal private ConditionProfile returnTopLevelProfile;
+
     public FunctionBodyNode(SaveArgumentsNode saveArguments, RNode body) {
         this.body = body;
         this.saveArguments = saveArguments;
     }
 
     @Override
     public Object visibleExecute(VirtualFrame frame) {
-        setupDispatchSlots(frame);
-        saveArguments.execute(frame);
-        return body.visibleExecute(frame);
+        try {
+            setupDispatchSlots(frame);
+            saveArguments.execute(frame);
+            return body.visibleExecute(frame);
+        } catch (ReturnException ex) {
+            returnExceptionProfile.enter();
+            if (profileReturnToTopLevel(ex.getTarget() == RArguments.getCall(frame))) {
+                Object result = ex.getResult();
+                if (CompilerDirectives.inInterpreter() && result == null) {
+                    throw RInternalError.shouldNotReachHere("invalid null from ReturnException.getResult() of " + this);
+                }
+                return result;
+            } else {
+                // re-thrown until it reaches its target
+                throw ex;
+            }
+        }
     }
 
     @Override
@@ -68,6 +90,14 @@ public SourceSection getSourceSection() {
         return body.getSourceSection();
     }
 
+    public boolean profileReturnToTopLevel(boolean condition) {
+        if (returnTopLevelProfile == null) {
+            CompilerDirectives.transferToInterpreterAndInvalidate();
+            returnTopLevelProfile = ConditionProfile.createBinaryProfile();
+        }
+        return returnTopLevelProfile.profile(condition);
+    }
+
     private void setupDispatchSlots(VirtualFrame frame) {
         DispatchArgs dispatchArgs = RArguments.getDispatchArgs(frame);
         if (dispatchArgs == null) {
diff --git a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/FunctionDefinitionNode.java b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/FunctionDefinitionNode.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2013, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2013, 2020, 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
@@ -132,10 +132,8 @@ public final class FunctionDefinitionNode extends RRootNode implements RSyntaxNo
     @CompilationFinal private FrameSlot handlerStackSlot;
     @CompilationFinal private FrameSlot restartStackSlot;
 
-    /**
-     * Profiling for catching {@link ReturnException}s.
-     */
-    private final ConditionProfile returnTopLevelProfile = ConditionProfile.createBinaryProfile();
+    // Profiling for catching ReturnException thrown from an exit handler
+    @CompilationFinal private ConditionProfile returnTopLevelProfile;
 
     public static FunctionDefinitionNode create(TruffleRLanguage language, SourceSection src, FrameDescriptor frameDesc, SourceSection[] argSourceSections, SaveArgumentsNode saveArguments,
                     RSyntaxNode body,
@@ -288,15 +286,9 @@ public Object execute(VirtualFrame frame) {
             }
             return result;
         } catch (ReturnException ex) {
-            if (returnTopLevelProfile.profile(ex.getTarget() == RArguments.getCall(frame))) {
-                Object result = ex.getResult();
-                if (CompilerDirectives.inInterpreter() && result == null) {
-                    throw RInternalError.shouldNotReachHere("invalid null from ReturnException.getResult() of " + this);
-                }
-                return result;
-            } else {
-                throw ex;
-            }
+            // here we just re-throw, the check whether this function is the target of the return is
+            // done in the function body node
+            throw ex;
         } catch (BreakException e) {
             breakProfile.enter();
             throw e;
@@ -391,7 +383,7 @@ public Object execute(VirtualFrame frame) {
                             }
                         }
                     } catch (ReturnException ex) {
-                        if (returnTopLevelProfile.profile(ex.getTarget() == RArguments.getCall(frame))) {
+                        if (profileReturnToTopLevel(ex.getTarget() == RArguments.getCall(frame))) {
                             return ex.getResult();
                         } else {
                             throw ex;
@@ -405,6 +397,14 @@ public Object execute(VirtualFrame frame) {
         }
     }
 
+    public boolean profileReturnToTopLevel(boolean condition) {
+        if (returnTopLevelProfile == null) {
+            CompilerDirectives.transferToInterpreterAndInvalidate();
+            returnTopLevelProfile = ConditionProfile.createBinaryProfile();
+        }
+        return returnTopLevelProfile.profile(condition);
+    }
+
     private static RPairList getCurrentOnExitList(VirtualFrame frame, FrameSlot slot) {
         try {
             return (RPairList) FrameSlotChangeMonitor.getObject(slot, frame);
diff --git a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/tck/FastRDebugTest.java b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/tck/FastRDebugTest.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2020, 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
@@ -594,6 +594,38 @@ public void testActiveBinding() throws Throwable {
         assertExecutedOK();
     }
 
+    @Test
+    public void testStepOut() throws Throwable {
+        final Source source = sourceFromText("fnc <- function() {\n" +
+                        "  x <- 10L\n" +
+                        "  return(x + 10)\n" +
+                        "}\n" +
+                        "fnc() + fnc()\n",
+                        "test.r");
+        context.eval(source);
+        run.addLast(() -> {
+            assertNull(suspendedEvent);
+            assertNotNull(debuggerSession);
+            debuggerSession.setSteppingFilter(SuspensionFilter.newBuilder().ignoreLanguageContextInitialization(true).build());
+            debuggerSession.suspendNextExecution();
+        });
+        assertLocation(1, "fnc <- function() {");
+        stepOver(1);
+        assertLocation(5, "fnc() + fnc()");
+        stepInto(1);
+        assertLocation(2, "x <- 10L");
+        stepOver(1);
+        assertLocation(3, "return(x + 10)");
+        stepOut();
+        assertLocation(5, "fnc()");
+        stepInto(1);
+        assertLocation(2, "x <- 10L");
+        continueExecution();
+        performWork();
+        context.eval(source);
+        assertExecutedOK();
+    }
+
     private void performWork() {
         try {
             if (ex == null && !run.isEmpty()) {
