[GR-44710] Fix String#slice with negative offset

PullRequest: truffleruby/4484

Author: andrykonchin

CHANGELOG.md

@@ -11,6 +11,7 @@ Compatibility:
 * Implement `StringScanner#{peek_byte,scan_byte,scan_integer,named_captures}` methods (#3788, @andrykonchin).
 * Support String patterns in `StringScanner#{exist?,scan_until,skip_until,check_until,search_full}` methods (@andrykonchin).
 * Implement `ObjectSpace::WeakKeyMap` (#3681, @andrykonchin).
+* Fix `String#slice` called with negative offset (@andrykonchin).
 
 Performance:
 

doc/contributor/updating-ruby.md

@@ -135,10 +135,11 @@ rm -rf lib/gems/specifications
 cd ~/.rubies/ruby-$VERSION
 cp -R lib/ruby/gems/*.0/gems $TRUFFLERUBY/lib/gems
 cp -R lib/ruby/gems/*.0/specifications $TRUFFLERUBY/lib/gems
-rm -f lib/gems/gems/**/*.{o,a,so,bundle} lib/gems/gems/**/{Makefile,extconf.h,mkmf.log} lib/gems/gems/**/*.mk
 
 cd $TRUFFLERUBY
+rm -f lib/gems/gems/**/*.{o,a,so,bundle} lib/gems/gems/**/{Makefile,extconf.h,mkmf.log} lib/gems/gems/**/*.mk
 rm -rf lib/gems/gems/typeprof-* lib/gems/specifications/typeprof-*.gemspec
+rm lib/gems/gems/rbs-*/Gemfile.lock
 ruby tool/patch-default-gemspecs.rb
 ```
 

spec/ruby/core/string/shared/slice.rb

@@ -119,6 +119,13 @@
     "hello there".send(@method, -4,-3).should == nil
   end
 
+  platform_is pointer_size: 64 do
+    it "returns nil if the length is negative big value" do
+      "hello there".send(@method, 4, -(1 << 31)).should == nil
+      "hello there".send(@method, 4, -(1 << 63)).should == nil
+    end
+  end
+
   it "calls to_int on the given index and the given length" do
     "hello".send(@method, 0.5, 1).should == "h"
     "hello".send(@method, 0.5, 2.5).should == "he"
@@ -152,6 +159,11 @@
     -> { "hello".send(@method, 0, bignum_value) }.should raise_error(RangeError)
   end
 
+  it "raises a RangeError if the index or length is too small" do
+    -> { "hello".send(@method, -bignum_value, 1) }.should raise_error(RangeError)
+    -> { "hello".send(@method, 0, -bignum_value) }.should raise_error(RangeError)
+  end
+
   it "returns String instances" do
     s = StringSpecs::MyString.new("hello")
     s.send(@method, 0,0).should be_an_instance_of(String)

spec/truffle/objectspace/weak_key_map_spec.rb

@@ -27,26 +27,6 @@
     map.getkey(key).should == nil
   end
 
-  it "has iterators methods that exclude unreferenced objects" do
-    # This spec does not pass on MRI because the garbage collector is presumably too conservative and will not get rid
-    # of the references eagerly enough.
-
-    map = ObjectSpace::WeakKeyMap.new
-    k1, k2 = %w[a b].map(&:upcase)
-    v1, v2 = %w[x y].map(&:upcase)
-    map[k1] = v1
-    map[k2] = v2
-    k2 = nil
-
-    Primitive.gc_force
-
-    map.key?(k2).should == false
-    map[k2].should == nil
-
-    map.key?(k1).should == true
-    map[k1].should == v1
-  end
-
   it "supports frozen objects" do
     map = ObjectSpace::WeakKeyMap.new
     k = "x".freeze

src/main/java/org/truffleruby/core/string/StringNodes.java

@@ -575,7 +575,13 @@ Object slice(Object string, int start, int length) {
         }
 
         @Specialization
-        Object slice(Object string, long start, long length) {
+        Object slice(Object string, long start, long length,
+                @Cached @Shared InlinedBranchProfile negativeLengthProfile) {
+            if (length < 0) {
+                negativeLengthProfile.enter(this);
+                return nil;
+            }
+
             int lengthInt = (int) length;
             if (lengthInt != length) {
                 lengthInt = Integer.MAX_VALUE; // go to end of string
@@ -589,8 +595,9 @@ Object slice(Object string, long start, long length) {
 
         @Specialization(guards = "wasProvided(length)")
         Object slice(Object string, long start, Object length,
-                @Cached @Shared ToLongNode toLongNode) {
-            return slice(string, start, toLongNode.execute(this, length));
+                @Cached @Shared ToLongNode toLongNode,
+                @Cached @Shared InlinedBranchProfile negativeLengthProfile) {
+            return slice(string, start, toLongNode.execute(this, length), negativeLengthProfile);
         }
 
         @Specialization(
@@ -600,8 +607,10 @@ Object slice(Object string, long start, Object length,
                         "isNotRubyString(start)",
                         "wasProvided(length)" })
         Object slice(Object string, Object start, Object length,
-                @Cached @Shared ToLongNode toLongNode) {
-            return slice(string, toLongNode.execute(this, start), toLongNode.execute(this, length));
+                @Cached @Shared ToLongNode toLongNode,
+                @Cached @Shared InlinedBranchProfile negativeLengthProfile) {
+            return slice(string, toLongNode.execute(this, start), toLongNode.execute(this, length),
+                    negativeLengthProfile);
         }
 
         // endregion