Fix cloning an exception

PullRequest: truffleruby/552

Author: eregon

CHANGELOG.md

@@ -6,6 +6,8 @@ Bug fixes:
   characters (#1543).
 * Fixed an issue with `String#{encode,encode!}` double-processing strings
   using XML conversion options and a new destination encoding (#1545).
+* Fixed a bug where a raised cloned exception would be caught as the
+  original exception (#1542).
 
 Compatibility:
 

spec/ruby/core/exception/exception_spec.rb

@@ -66,6 +66,22 @@
     e2.message.should == "message"
   end
 
+  it "when raised will be rescued as the new exception" do
+    begin
+      begin
+        raised_first = StandardError.new('first')
+        raise raised_first
+      rescue => caught_first
+        raised_second = raised_first.exception('second')
+        raise raised_second
+      end
+    rescue => caught_second
+    end
+    
+    raised_first.should == caught_first
+    raised_second.should == caught_second
+  end
+
   class CustomArgumentError < StandardError
     attr_reader :val
     def initialize(val)

src/main/java/org/truffleruby/core/VMPrimitiveNodes.java

@@ -218,21 +218,22 @@ public static abstract class VMRaiseExceptionNode extends PrimitiveArrayArgument
         public DynamicObject vmRaiseException(DynamicObject exception, boolean internal,
                 @Cached("createBinaryProfile()") ConditionProfile reRaiseProfile) {
             final Backtrace backtrace = Layouts.EXCEPTION.getBacktrace(exception);
-            if (reRaiseProfile.profile(backtrace != null && backtrace.getTruffleException() != null)) {
-                // We need to rethrow the existing TruffleException, otherwise we would lose the
+            if (reRaiseProfile.profile(backtrace != null && backtrace.getRaiseException() != null)) {
+                // We need to rethrow the existing RaiseException, otherwise we would lose the
                 // TruffleStackTrace stored in it.
-                throw (RaiseException) backtrace.getTruffleException();
+                assert backtrace.getRaiseException().getException() == exception;
+                throw backtrace.getRaiseException();
             } else {
                 throw new RaiseException(getContext(), exception, internal);
             }
         }
 
         public static void reRaiseException(RubyContext context, DynamicObject exception) {
             final Backtrace backtrace = Layouts.EXCEPTION.getBacktrace(exception);
-            if (backtrace != null && backtrace.getTruffleException() != null) {
-                // We need to rethrow the existing TruffleException, otherwise we would lose the
+            if (backtrace != null && backtrace.getRaiseException() != null) {
+                // We need to rethrow the existing RaiseException, otherwise we would lose the
                 // TruffleStackTrace stored in it.
-                throw (RaiseException) backtrace.getTruffleException();
+                throw backtrace.getRaiseException();
             } else {
                 throw new RaiseException(context, exception, false);
             }

src/main/java/org/truffleruby/core/exception/ExceptionNodes.java

@@ -77,7 +77,12 @@ public Object initializeCopySelfIsSameAsFrom(DynamicObject self, DynamicObject f
 
         @Specialization(guards = { "self != from", "isRubyException(from)" })
         public Object initializeCopy(DynamicObject self, DynamicObject from) {
-            Layouts.EXCEPTION.setBacktrace(self, Layouts.EXCEPTION.getBacktrace(from));
+            Backtrace backtrace = Layouts.EXCEPTION.getBacktrace(from);
+            if (backtrace != null) {
+                Layouts.EXCEPTION.setBacktrace(self, backtrace.copy(getContext(), self));
+            } else {
+                Layouts.EXCEPTION.setBacktrace(self, backtrace);
+            }
             Layouts.EXCEPTION.setFormatter(self, Layouts.EXCEPTION.getFormatter(from));
             Layouts.EXCEPTION.setMessage(self, Layouts.EXCEPTION.getMessage(from));
 

src/main/java/org/truffleruby/core/kernel/KernelNodes.java

@@ -310,9 +310,7 @@ private DynamicObject innerCallerLocations(int omit, int length) {
             final Backtrace backtrace = getContext().getCallStack().getBacktrace(this, omitted);
             final int limit = (length == GetBacktraceException.UNLIMITED) ? GetBacktraceException.UNLIMITED : omitted + length;
 
-            backtrace.setTruffleException(new GetBacktraceException(this, limit));
-
-            int locationsCount = backtrace.getActivations().length;
+            int locationsCount = backtrace.getActivations(new GetBacktraceException(this, limit)).length;
 
             if (length != GetBacktraceException.UNLIMITED && length < locationsCount) {
                 locationsCount = length;

src/main/java/org/truffleruby/core/thread/ThreadManager.java

@@ -280,7 +280,7 @@ private static void setException(RubyContext context, DynamicObject thread, Dyna
 
         // We materialize the backtrace eagerly here, as the exception escapes the thread and needs
         // to capture the backtrace from this thread.
-        final TruffleException truffleException = Layouts.EXCEPTION.getBacktrace(exception).getTruffleException();
+        final TruffleException truffleException = Layouts.EXCEPTION.getBacktrace(exception).getRaiseException();
         if (truffleException != null) {
             TruffleStackTraceElement.fillIn((Throwable) truffleException);
         }

src/main/java/org/truffleruby/language/backtrace/Backtrace.java

@@ -21,6 +21,7 @@
 import org.truffleruby.core.exception.GetBacktraceException;
 import org.truffleruby.language.CallStackManager;
 import org.truffleruby.language.arguments.RubyArguments;
+import org.truffleruby.language.control.RaiseException;
 import org.truffleruby.language.methods.InternalMethod;
 
 import java.util.ArrayList;
@@ -30,7 +31,7 @@ public class Backtrace {
 
     private final Node location;
     private SourceSection sourceLocation;
-    private TruffleException truffleException;
+    private RaiseException raiseException;
     private Activation[] activations;
     private final int omitted;
     private final Throwable javaThrowable;
@@ -43,6 +44,20 @@ public Backtrace(Node location, SourceSection sourceLocation, int omitted, Throw
         this.javaThrowable = javaThrowable;
     }
 
+    public Backtrace copy(RubyContext context, DynamicObject exception) {
+        Backtrace copy = new Backtrace(location, sourceLocation, omitted, javaThrowable);
+        // A Backtrace is 1-1-1 with a RaiseException and a Ruby exception
+        RaiseException newRaiseException = new RaiseException(context, exception, this.raiseException.isInternalError());
+        // Copy the TruffleStackTrace
+        TruffleStackTraceElement.fillIn(this.raiseException);
+        assert this.raiseException.getCause() != null;
+        newRaiseException.initCause(this.raiseException.getCause());
+        // Another way would be to copy the activations (copy.activations = getActivations()), but
+        // then the TruffleStrackTrace would be inconsistent.
+        copy.setRaiseException(newRaiseException);
+        return copy;
+    }
+
     public Node getLocation() {
         return location;
     }
@@ -51,22 +66,23 @@ public SourceSection getSourceLocation() {
         return sourceLocation;
     }
 
-    public TruffleException getTruffleException() {
-        return truffleException;
+    public RaiseException getRaiseException() {
+        return raiseException;
     }
 
-    public void setTruffleException(TruffleException truffleException) {
-        assert this.truffleException == null : "the TruffleException of a Backtrace must not be set again, otherwise the original backtrace is lost";
-        this.truffleException = truffleException;
+    public void setRaiseException(RaiseException raiseException) {
+        assert this.raiseException == null : "the RaiseException of a Backtrace must not be set again, otherwise the original backtrace is lost";
+        this.raiseException = raiseException;
     }
 
-    @TruffleBoundary
     public Activation[] getActivations() {
+        return getActivations(this.raiseException);
+    }
+
+    @TruffleBoundary
+    public Activation[] getActivations(TruffleException truffleException) {
         if (this.activations == null) {
-            final TruffleException truffleException;
-            if (this.truffleException != null) {
-                truffleException = this.truffleException;
-            } else {
+            if (truffleException == null) {
                 truffleException = new GetBacktraceException(location, GetBacktraceException.UNLIMITED);
             }
 

src/main/java/org/truffleruby/language/control/RaiseException.java

@@ -42,7 +42,7 @@ public RaiseException(RubyContext context, DynamicObject exception, boolean inte
 
         final Backtrace backtrace = Layouts.EXCEPTION.getBacktrace(exception);
         if (backtrace != null) { // The backtrace could be null if for example a user backtrace was passed to Kernel#raise
-            backtrace.setTruffleException(this);
+            backtrace.setRaiseException(this);
         }
 
         assert !isSyntaxError() || getSourceLocation() != null;