[GR-26970] Backports for 20.3 batch 2

eregon · eregon · commit d2d8607326b3 · 2020-11-04T09:50:48.000Z
PullRequest: truffleruby/2128
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -19,6 +19,9 @@ Bug fixes:
 * Fixed `SystemStackError` sometimes replaced by an internal Java `NoClassDefFoundError` on JVM (#1743).
 * Fixed constant/identifier detection in lexer for non-ASCII encodings (#2079, #2102, @ivoanjo).
 * Fixed parsing of `--jvm` as an application argument (#2108).
+* Fix `rb_rescue2` to ignore the end marker `(VALUE)0` (#2127, #2130).
+* Fix `String#{chomp, chomp!}` issue with invalid encoded strings (#2133).
+* Fix status and output when SystemExit is subclassed and raised (#2128)
 
 Compatibility:
 
diff --git a/spec/ruby/core/exception/system_exit_spec.rb b/spec/ruby/core/exception/system_exit_spec.rb
@@ -0,0 +1,17 @@
+require_relative '../../spec_helper'
+
+describe "SystemExit" do
+  it "sets the exit status and exits silently when raised" do
+    code = 'raise SystemExit.new(7)'
+    result = ruby_exe(code, args: "2>&1")
+    result.should == ""
+    $?.exitstatus.should == 7
+  end
+
+  it "sets the exit status and exits silently when raised when subclassed" do
+    code = 'class CustomExit < SystemExit; end; raise CustomExit.new(8)'
+    result = ruby_exe(code, args: "2>&1")
+    result.should == ""
+    $?.exitstatus.should == 8
+  end
+end
diff --git a/spec/ruby/core/string/chomp_spec.rb b/spec/ruby/core/string/chomp_spec.rb
@@ -55,6 +55,10 @@
       $/ = "cdef"
       "abcdef".chomp.should == "ab"
     end
+
+    it "removes one trailing newline for string with invalid encoding" do
+      "\xa0\xa1\n".chomp.should == "\xa0\xa1"
+    end
   end
 
   describe "when passed nil" do
@@ -108,6 +112,10 @@
     it "returns an empty String when self is empty" do
       "".chomp("").should == ""
     end
+
+    it "removes one trailing newline for string with invalid encoding" do
+      "\xa0\xa1\n".chomp("").should == "\xa0\xa1"
+    end
   end
 
   describe "when passed '\\n'" do
diff --git a/spec/ruby/optional/capi/kernel_spec.rb b/spec/ruby/optional/capi/kernel_spec.rb
@@ -396,12 +396,20 @@ def proc_caller
       proc = -> x { x }
       arg_error_proc = -> *_ { raise ArgumentError, '' }
       run_error_proc = -> *_ { raise RuntimeError, '' }
-      type_error_proc = -> *_ { raise TypeError, '' }
+      type_error_proc = -> *_ { raise Exception, 'custom error' }
       @s.rb_rescue2(arg_error_proc, :no_exc, proc, :exc, ArgumentError, RuntimeError).should == :exc
       @s.rb_rescue2(run_error_proc, :no_exc, proc, :exc, ArgumentError, RuntimeError).should == :exc
       -> {
         @s.rb_rescue2(type_error_proc, :no_exc, proc, :exc, ArgumentError, RuntimeError)
-      }.should raise_error(TypeError)
+      }.should raise_error(Exception, 'custom error')
+    end
+
+    ruby_bug "#17305", ""..."2.7" do
+      it "raises TypeError if one of the passed exceptions is not a Module" do
+        -> {
+          @s.rb_rescue2(-> *_ { raise RuntimeError, "foo" }, :no_exc, -> x { x }, :exc, Object.new, 42)
+        }.should raise_error(TypeError, /class or module required/)
+      end
     end
   end
 
diff --git a/spec/tags/core/exception/system_exit_tags.txt b/spec/tags/core/exception/system_exit_tags.txt
@@ -0,0 +1,2 @@
+slow:SystemExit sets the exit status and exits silently when raised
+slow:SystemExit sets the exit status and exits silently when raised when subclassed
diff --git a/src/main/c/cext/exception.c b/src/main/c/cext/exception.c
@@ -85,6 +85,10 @@ VALUE rb_rescue2(VALUE (*b_proc)(ANYARGS), VALUE data1, VALUE (*r_proc)(ANYARGS)
   // arguments using the polyglot api.
   for (;n < total; n++) {
     VALUE arg = polyglot_get_arg(n);
+    if (arg == (VALUE)0) {
+      break;
+    }
+
     rb_ary_push(rescued, arg);
   }
   return cext_rb_rescue2(b_proc, data1, r_proc, data2, rb_tr_unwrap(rescued));
diff --git a/src/main/java/org/truffleruby/core/array/ArrayNodes.java b/src/main/java/org/truffleruby/core/array/ArrayNodes.java
@@ -1591,7 +1591,7 @@ private RubyString finishPack(int formatLength, BytesResult result) {
 
         @TruffleBoundary
         protected RootCallTarget compileFormat(RubyString format) {
-            return new PackCompiler(getContext(), this).compile(format.toString());
+            return new PackCompiler(getContext(), this).compile(format.getJavaString());
         }
 
         protected int getCacheLimit() {
diff --git a/src/main/java/org/truffleruby/core/exception/CoreExceptions.java b/src/main/java/org/truffleruby/core/exception/CoreExceptions.java
@@ -89,6 +89,14 @@ public void showExceptionIfDebug(RubyClass rubyClass, Object message, Backtrace
         }
     }
 
+    public String inspectReceiver(Object receiver) {
+        RubyString rubyString = (RubyString) context.send(
+                context.getCoreLibrary().truffleExceptionOperationsModule,
+                "receiver_string",
+                receiver);
+        return rubyString.getJavaString();
+    }
+
     // ArgumentError
 
     public RubyException argumentErrorOneHashRequired(Node currentNode) {
@@ -488,17 +496,17 @@ public RubyException typeErrorCantConvertInto(Object from, String toClass, Node
 
     @TruffleBoundary
     public RubyException typeErrorIsNotA(Object value, String expectedType, Node currentNode) {
-        return typeErrorIsNotA(value.toString(), expectedType, currentNode);
+        return typeErrorIsNotA(inspectReceiver(value), expectedType, currentNode);
     }
 
     @TruffleBoundary
     public RubyException typeErrorIsNotA(String value, String expectedType, Node currentNode) {
-        return typeError(StringUtils.format("%s is not a %s", value, expectedType), currentNode);
+        return typeError(value + " is not a " + expectedType, currentNode);
     }
 
     @TruffleBoundary
     public RubyException typeErrorIsNotAClassModule(Object value, Node currentNode) {
-        return typeError(StringUtils.format("%s is not a class/module", value), currentNode);
+        return typeError(inspectReceiver(value) + " is not a class/module", currentNode);
     }
 
     @TruffleBoundary
diff --git a/src/main/java/org/truffleruby/core/kernel/AtExitManager.java b/src/main/java/org/truffleruby/core/kernel/AtExitManager.java
@@ -16,13 +16,13 @@
 
 import org.truffleruby.RubyContext;
 import org.truffleruby.core.exception.RubyException;
-import org.truffleruby.core.klass.RubyClass;
 import org.truffleruby.core.proc.ProcOperations;
 import org.truffleruby.core.proc.RubyProc;
 import org.truffleruby.language.control.ExitException;
 import org.truffleruby.language.control.RaiseException;
 
 import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
+import org.truffleruby.language.objects.IsANode;
 
 public class AtExitManager {
 
@@ -83,9 +83,9 @@ public List<RubyProc> getHandlers() {
     }
 
     public static boolean isSilentException(RubyContext context, RubyException rubyException) {
-        final RubyClass logicalClass = rubyException.getLogicalClass();
-        return logicalClass == context.getCoreLibrary().systemExitClass ||
-                logicalClass == context.getCoreLibrary().signalExceptionClass;
+        final IsANode isANode = IsANode.getUncached();
+        return isANode.executeIsA(rubyException, context.getCoreLibrary().systemExitClass) ||
+                isANode.executeIsA(rubyException, context.getCoreLibrary().signalExceptionClass);
     }
 
     private static void handleAtExitException(RubyContext context, RubyException rubyException) {
diff --git a/src/main/java/org/truffleruby/core/module/ModuleFields.java b/src/main/java/org/truffleruby/core/module/ModuleFields.java
@@ -343,6 +343,10 @@ public RubyConstant setConstant(RubyContext context, Node currentNode, String na
     @TruffleBoundary
     public void setAutoloadConstant(RubyContext context, Node currentNode, String name, RubyString filename) {
         RubyConstant autoloadConstant = setConstantInternal(context, currentNode, name, filename, true);
+        if (autoloadConstant == null) {
+            return;
+        }
+
         if (context.getOptions().LOG_AUTOLOAD) {
             RubyLanguage.LOGGER.info(() -> String.format(
                     "%s: setting up autoload %s with %s",
@@ -351,7 +355,7 @@ public void setAutoloadConstant(RubyContext context, Node currentNode, String na
                     filename));
         }
         final ReentrantLockFreeingMap<String> fileLocks = getContext().getFeatureLoader().getFileLocks();
-        final ReentrantLock lock = fileLocks.get(filename.toString());
+        final ReentrantLock lock = fileLocks.get(filename.getJavaString());
         if (lock.isLocked()) {
             // We need to handle the new autoload constant immediately
             // if Object.autoload(name, filename) is executed from filename.rb
@@ -367,10 +371,22 @@ private RubyConstant setConstantInternal(RubyContext context, Node currentNode,
 
         SharedObjects.propagate(context, rubyModule, value);
 
+        final String autoloadPath = autoload ? ((RubyString) value).getJavaString() : null;
         RubyConstant previous;
         RubyConstant newConstant;
         do {
             previous = constants.get(name);
+            if (autoload && previous != null) {
+                if (previous.hasValue()) {
+                    // abort, do not set an autoload constant, the constant already has a value
+                    return null;
+                } else if (previous.isAutoload() &&
+                        previous.getAutoloadConstant().getAutoloadPath().equals(autoloadPath)) {
+                    // already an autoload constant with the same path,
+                    // do nothing so we don't replace the AutoloadConstant#autoloadLock which might be already acquired
+                    return null;
+                }
+            }
             newConstant = newConstant(currentNode, name, value, autoload, previous);
         } while (!ConcurrentOperations.replace(constants, name, previous, newConstant));
 
diff --git a/src/main/java/org/truffleruby/core/module/ModuleNodes.java b/src/main/java/org/truffleruby/core/module/ModuleNodes.java
@@ -580,11 +580,7 @@ protected Object autoload(RubyModule module, String name, RubyString filename) {
                 throw new RaiseException(getContext(), coreExceptions().argumentError("empty file name", this));
             }
 
-            final RubyConstant constant = module.fields.getConstant(name);
-            if (constant == null || !constant.hasValue()) {
-                module.fields.setAutoloadConstant(getContext(), this, name, filename);
-            }
-
+            module.fields.setAutoloadConstant(getContext(), this, name, filename);
             return nil;
         }
     }
diff --git a/src/main/java/org/truffleruby/core/regexp/MatchDataNodes.java b/src/main/java/org/truffleruby/core/regexp/MatchDataNodes.java
@@ -389,11 +389,11 @@ private int getBackRefFromRope(RubyMatchData matchData, Object index, Rope value
                         0,
                         value.byteLength(),
                         matchData.region);
-            } catch (final ValueException e) {
+            } catch (ValueException e) {
                 throw new RaiseException(
                         getContext(),
                         coreExceptions().indexError(
-                                StringUtils.format("undefined group name reference: %s", index.toString()),
+                                StringUtils.format("undefined group name reference: %s", index),
                                 this));
             }
         }
diff --git a/src/main/java/org/truffleruby/core/rope/ManagedRope.java b/src/main/java/org/truffleruby/core/rope/ManagedRope.java
@@ -54,12 +54,14 @@ public final byte[] getBytes() {
 
     @Override
     public final String toString() {
-        assert ALLOW_TO_STRING;
-
-        final byte[] bytesBefore = bytes;
-        final String string = RopeOperations.decodeOrEscapeBinaryRope(this, RopeOperations.flattenBytes(this));
-        assert bytes == bytesBefore : "bytes should not be modified by Rope#toString() as otherwise inspecting a Rope would have a side effect";
-        return string;
+        if (DEBUG_ROPE_BYTES) {
+            final byte[] bytesBefore = bytes;
+            final String string = RopeOperations.decodeOrEscapeBinaryRope(this, RopeOperations.flattenBytes(this));
+            assert bytes == bytesBefore : "bytes should not be modified by Rope#toString() as otherwise inspecting a Rope would have a side effect";
+            return string;
+        } else {
+            return RopeOperations.decodeRope(this);
+        }
     }
 
 }
diff --git a/src/main/java/org/truffleruby/core/rope/NativeRope.java b/src/main/java/org/truffleruby/core/rope/NativeRope.java
@@ -184,7 +184,6 @@ public int hashCode() {
 
     @Override
     public String toString() {
-        assert ALLOW_TO_STRING;
         return toLeafRope().toString();
     }
 
diff --git a/src/main/java/org/truffleruby/core/rope/Rope.java b/src/main/java/org/truffleruby/core/rope/Rope.java
@@ -20,8 +20,8 @@ public abstract class Rope implements Comparable<Rope> {
     // NativeRope, RepeatingRope, 3 LeafRope, ConcatRope, SubstringRope, 1 LazyRope
     public static final int NUMBER_OF_CONCRETE_CLASSES = 8;
 
-    // Useful for debugging. Setting to false allow to catch wrong usages.
-    protected static final boolean ALLOW_TO_STRING = true;
+    // Useful for debugging. Setting to true avoids ManagedRope#toString to populate bytes as a side-effect of the debugger calling toString().
+    protected static final boolean DEBUG_ROPE_BYTES = false;
 
     public final Encoding encoding;
     private final int byteLength;
diff --git a/src/main/java/org/truffleruby/core/thread/ThreadManager.java b/src/main/java/org/truffleruby/core/thread/ThreadManager.java
@@ -315,6 +315,8 @@ private void threadMain(RubyThread thread, Node currentNode, Supplier<Object> ta
             final String message = StringUtils
                     .format("%s terminated with internal error:", Thread.currentThread().getName());
             final RuntimeException runtimeException = new RuntimeException(message, e);
+            // Immediately print internal exceptions, in case they would cause a deadlock
+            runtimeException.printStackTrace();
             rethrowOnMainThread(currentNode, runtimeException);
             setThreadValue(context, thread, Nil.INSTANCE);
         } finally {
diff --git a/src/main/java/org/truffleruby/language/SafepointManager.java b/src/main/java/org/truffleruby/language/SafepointManager.java
@@ -209,10 +209,12 @@ private void driveArrivalAtPhaser() {
     }
 
     private void printStacktracesOfBlockedThreads() {
+        final Thread drivingThread = Thread.currentThread();
+
         System.err.println("Dumping stacktraces of all threads:");
         for (Entry<Thread, StackTraceElement[]> entry : Thread.getAllStackTraces().entrySet()) {
             final Thread thread = entry.getKey();
-            if (thread != Thread.currentThread() && runningThreads.contains(thread)) {
+            if (runningThreads.contains(thread)) {
                 final StackTraceElement[] stackTrace = entry.getValue();
                 boolean blocked = true;
 
@@ -225,7 +227,8 @@ private void printStacktracesOfBlockedThreads() {
                     }
                 }
 
-                System.err.println((blocked ? "BLOCKED: " : "IN SAFEPOINT: ") + thread);
+                String kind = thread == drivingThread ? "DRIVER" : (blocked ? "BLOCKED" : "IN SAFEPOINT");
+                System.err.println(kind + ": " + thread);
                 for (StackTraceElement stackTraceElement : stackTrace) {
                     System.err.println(stackTraceElement);
                 }
diff --git a/src/main/java/org/truffleruby/language/exceptions/TopLevelRaiseHandler.java b/src/main/java/org/truffleruby/language/exceptions/TopLevelRaiseHandler.java
@@ -20,6 +20,7 @@
 import org.truffleruby.language.dispatch.DispatchNode;
 
 import com.oracle.truffle.api.CompilerDirectives;
+import org.truffleruby.language.objects.IsANodeGen;
 
 public class TopLevelRaiseHandler extends RubyContextNode {
 
@@ -66,7 +67,7 @@ public int execute(Runnable body) {
     }
 
     private int statusFromException(RubyException exception) {
-        if (exception.getLogicalClass() == coreLibrary().systemExitClass) {
+        if (IsANodeGen.getUncached().executeIsA(exception, coreLibrary().systemExitClass)) {
             final Object status = DynamicObjectLibrary.getUncached().getOrDefault(exception, "@status", null);
             return IntegerCastNodeGen.getUncached().executeCastInt(status);
         } else {
diff --git a/src/main/java/org/truffleruby/language/objects/IsANode.java b/src/main/java/org/truffleruby/language/objects/IsANode.java
@@ -28,6 +28,10 @@ public static IsANode create() {
         return IsANodeGen.create();
     }
 
+    public static IsANode getUncached() {
+        return IsANodeGen.getUncached();
+    }
+
     public abstract boolean executeIsA(Object self, RubyModule module);
 
     @Specialization(
diff --git a/src/main/java/org/truffleruby/parser/parser/RubyParser.java b/src/main/java/org/truffleruby/parser/parser/RubyParser.java
diff --git a/src/main/java/org/truffleruby/parser/parser/RubyParser.y b/src/main/java/org/truffleruby/parser/parser/RubyParser.y
@@ -1259,7 +1259,7 @@ rel_expr        : arg relop arg   %prec tGT {
                      $$ = support.getOperatorCallNode($1, $2, $3, lexer.getPosition());
                 }
                 | rel_expr relop arg   %prec tGT {
-                     support.warning(lexer.getPosition(), "comparison '" + $2 + "' after comparison");
+                     support.warning(lexer.getPosition(), "comparison '" + $2.getString() + "' after comparison");
                      $$ = support.getOperatorCallNode($1, $2, $3, lexer.getPosition());
                 }
  
diff --git a/src/main/ruby/truffleruby/core/string.rb b/src/main/ruby/truffleruby/core/string.rb
@@ -844,7 +844,7 @@ def chomp!(sep=undefined)
         if j = Primitive.string_previous_byte_index(self, bytes)
           chr = Primitive.string_chr_at(self, j)
 
-          if chr.ord == 13
+          if !Primitive.nil?(chr) && chr.ord == 13
             bytes = j
           end
         end
@@ -857,13 +857,13 @@ def chomp!(sep=undefined)
 
       while i = Primitive.string_previous_byte_index(self, bytes)
         chr = Primitive.string_chr_at(self, i)
-        break unless chr.ord == 10
+        break unless !Primitive.nil?(chr) && chr.ord == 10
 
         bytes = i
 
         if j = Primitive.string_previous_byte_index(self, i)
           chr = Primitive.string_chr_at(self, j)
-          if chr.ord == 13
+          if !Primitive.nil?(chr) && chr.ord == 13
             bytes = j
           end
         end
diff --git a/src/main/ruby/truffleruby/core/truffle/exception_operations.rb b/src/main/ruby/truffleruby/core/truffle/exception_operations.rb
diff --git a/test/truffle/integration/safepoints.sh b/test/truffle/integration/safepoints.sh

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+slow:SystemExit sets the exit status and exits silently when raised`
	`2`	`+slow:SystemExit sets the exit status and exits silently when raised when subclassed`
Original file line number	Diff line number	Diff line change
`@@ -1591,7 +1591,7 @@ private RubyString finishPack(int formatLength, BytesResult result) {`
`1591`	`1591`
`1592`	`1592`	`@TruffleBoundary`
`1593`	`1593`	`protected RootCallTarget compileFormat(RubyString format) {`
`1594`		`- return new PackCompiler(getContext(), this).compile(format.toString());`
	`1594`	`+ return new PackCompiler(getContext(), this).compile(format.getJavaString());`
`1595`	`1595`	`}`
`1596`	`1596`
`1597`	`1597`	`protected int getCacheLimit() {`
Original file line number	Diff line number	Diff line change
`@@ -89,6 +89,14 @@ public void showExceptionIfDebug(RubyClass rubyClass, Object message, Backtrace`
`89`	`89`	`}`
`90`	`90`	`}`
`91`	`91`
	`92`	`+ public String inspectReceiver(Object receiver) {`
	`93`	`+ RubyString rubyString = (RubyString) context.send(`
	`94`	`+ context.getCoreLibrary().truffleExceptionOperationsModule,`
	`95`	`+ "receiver_string",`
	`96`	`+ receiver);`
	`97`	`+ return rubyString.getJavaString();`
	`98`	`+ }`
	`99`	`+`
`92`	`100`	`// ArgumentError`
`93`	`101`
`94`	`102`	`public RubyException argumentErrorOneHashRequired(Node currentNode) {`
`@@ -488,17 +496,17 @@ public RubyException typeErrorCantConvertInto(Object from, String toClass, Node`
`488`	`496`
`489`	`497`	`@TruffleBoundary`
`490`	`498`	`public RubyException typeErrorIsNotA(Object value, String expectedType, Node currentNode) {`
`491`		`- return typeErrorIsNotA(value.toString(), expectedType, currentNode);`
	`499`	`+ return typeErrorIsNotA(inspectReceiver(value), expectedType, currentNode);`
`492`	`500`	`}`
`493`	`501`
`494`	`502`	`@TruffleBoundary`
`495`	`503`	`public RubyException typeErrorIsNotA(String value, String expectedType, Node currentNode) {`
`496`		`- return typeError(StringUtils.format("%s is not a %s", value, expectedType), currentNode);`
	`504`	`+ return typeError(value + " is not a " + expectedType, currentNode);`
`497`	`505`	`}`
`498`	`506`
`499`	`507`	`@TruffleBoundary`
`500`	`508`	`public RubyException typeErrorIsNotAClassModule(Object value, Node currentNode) {`
`501`		`- return typeError(StringUtils.format("%s is not a class/module", value), currentNode);`
	`509`	`+ return typeError(inspectReceiver(value) + " is not a class/module", currentNode);`
`502`	`510`	`}`
`503`	`511`
`504`	`512`	`@TruffleBoundary`