[GR-32332] Backports for 21.2 batch 3

PullRequest: truffleruby/2793

Author: eregon

CHANGELOG.md

@@ -12,6 +12,9 @@ Bug fixes:
 * Fix `File.{atime, mtime, ctime}` to include nanoseconds (#2337).
 * Fix `Array#[a, b] = "frozen string literal".freeze` (#2355).
 * `rb_funcall()` now releases the C-extension lock (similar to MRI).
+* Fix `rb_str_modify_expand` to preserve existing bytes (#2392).
+* Fix `Marshal.load` of multiple `Symbols` with an explicit encoding (#1624).
+* Fix `String#scrub` when replacement is frozen (#2398, @LillianZ).
 
 Compatibility:
 
@@ -37,6 +40,7 @@ Compatibility:
 * Implement `rb_backref_set`.
 * Fix `Float#<=>` when comparing `Infinity` to other `#infinite?` values.
 * Implement `date` library as a C extension to improve compatibility (#2344).
+* Update `rb_str_modify` and `rb_str_modify_expand` to raise a `FrozenError` when given a frozen string (#2392).
 
 Performance:
 

lib/cext/ABI_check.txt

@@ -1 +1 @@
-2
+4

lib/cext/ABI_version.txt

@@ -1 +1 @@
-3
+21.2.0

lib/cext/include/truffleruby/truffleruby-pre.h

@@ -69,6 +69,11 @@ extern ID (*rb_tr_sym2id)(VALUE sym);
 #define offsetof(p_type,field) ((size_t)&(((p_type *)0)->field))
 #endif
 
+// Defines
+
+// To support racc releases before https://github.com/ruby/racc/pull/165
+#define HAVE_RB_BLOCK_CALL
+
 #if defined(__cplusplus)
 }
 #endif

spec/ruby/core/marshal/dump_spec.rb

@@ -78,6 +78,20 @@
       s = "\u2192".force_encoding("binary").to_sym
       Marshal.dump(s).should == "\x04\b:\b\xE2\x86\x92"
     end
+
+    it "dumps multiple Symbols sharing the same encoding" do
+      # Note that the encoding is a link for the second Symbol
+      symbol1 = "I:\t\xE2\x82\xACa\x06:\x06ET"
+      symbol2 = "I:\t\xE2\x82\xACb\x06;\x06T"
+      value = [
+        "€a".force_encoding(Encoding::UTF_8).to_sym,
+        "€b".force_encoding(Encoding::UTF_8).to_sym
+      ]
+      Marshal.dump(value).should == "\x04\b[\a#{symbol1}#{symbol2}"
+
+      value = [*value, value[0]]
+      Marshal.dump(value).should == "\x04\b[\b#{symbol1}#{symbol2};\x00"
+    end
   end
 
   describe "with an object responding to #marshal_dump" do
@@ -343,8 +357,13 @@
     end
 
     it "dumps an extended Struct" do
-      st = Struct.new("Extended", :a, :b).new
-      Marshal.dump(st.extend(Meths)).should == "\004\be:\nMethsS:\025Struct::Extended\a:\006a0:\006b0"
+      obj = Struct.new("Extended", :a, :b).new.extend(Meths)
+      Marshal.dump(obj).should == "\004\be:\nMethsS:\025Struct::Extended\a:\006a0:\006b0"
+
+      s = 'hi'
+      obj.a = [:a, s]
+      obj.b = [:Meths, s]
+      Marshal.dump(obj).should == "\004\be:\nMethsS:\025Struct::Extended\a:\006a[\a;\a\"\ahi:\006b[\a;\000@\a"
       Struct.send(:remove_const, :Extended)
     end
   end

spec/ruby/core/marshal/fixtures/marshal_data.rb

@@ -83,7 +83,7 @@ class UserPreviouslyDefinedWithInitializedIvar
 end
 
 class UserMarshal
-  attr_reader :data
+  attr_accessor :data
 
   def initialize
     @data = 'stuff'

spec/ruby/core/marshal/shared/load.rb

@@ -309,7 +309,8 @@
 
     it "loads an extended Array object containing a user-marshaled object" do
       obj = [UserMarshal.new, UserMarshal.new].extend(Meths)
-      new_obj = Marshal.send(@method, "\x04\be:\nMeths[\ao:\x10UserMarshal\x06:\n@dataI\"\nstuff\x06:\x06ETo;\x06\x06;\aI\"\nstuff\x06;\bT")
+      dump = "\x04\be:\nMeths[\ao:\x10UserMarshal\x06:\n@dataI\"\nstuff\x06:\x06ETo;\x06\x06;\aI\"\nstuff\x06;\bT"
+      new_obj = Marshal.send(@method, dump)
 
       new_obj.should == obj
       obj_ancestors = class << obj; ancestors[1..-1]; end
@@ -399,6 +400,24 @@
       sym.should == s
       sym.encoding.should == Encoding::BINARY
     end
+
+    it "loads multiple Symbols sharing the same encoding" do
+      # Note that the encoding is a link for the second Symbol
+      symbol1 = "I:\t\xE2\x82\xACa\x06:\x06ET"
+      symbol2 = "I:\t\xE2\x82\xACb\x06;\x06T"
+      dump = "\x04\b[\a#{symbol1}#{symbol2}"
+      value = Marshal.send(@method, dump)
+      value.map(&:encoding).should == [Encoding::UTF_8, Encoding::UTF_8]
+      expected = [
+        "€a".force_encoding(Encoding::UTF_8).to_sym,
+        "€b".force_encoding(Encoding::UTF_8).to_sym
+      ]
+      value.should == expected
+
+      value = Marshal.send(@method, "\x04\b[\b#{symbol1}#{symbol2};\x00")
+      value.map(&:encoding).should == [Encoding::UTF_8, Encoding::UTF_8, Encoding::UTF_8]
+      value.should == [*expected, expected[0]]
+    end
   end
 
   describe "for a String" do
@@ -460,20 +479,23 @@
   describe "for a Struct" do
     it "loads a extended_struct having fields with same objects" do
       s = 'hi'
-      obj = Struct.new("Ure2", :a, :b).new.extend(Meths)
+      obj = Struct.new("Extended", :a, :b).new.extend(Meths)
+      dump = "\004\be:\nMethsS:\025Struct::Extended\a:\006a0:\006b0"
+      Marshal.send(@method, dump).should == obj
+
       obj.a = [:a, s]
       obj.b = [:Meths, s]
-
-      Marshal.send(@method,
-        "\004\be:\nMethsS:\021Struct::Ure2\a:\006a[\a;\a\"\ahi:\006b[\a;\000@\a"
-        ).should == obj
-      Struct.send(:remove_const, :Ure2)
+      dump = "\004\be:\nMethsS:\025Struct::Extended\a:\006a[\a;\a\"\ahi:\006b[\a;\000@\a"
+      Marshal.send(@method, dump).should == obj
+      Struct.send(:remove_const, :Extended)
     end
 
     it "loads a struct having ivar" do
       obj = Struct.new("Thick").new
       obj.instance_variable_set(:@foo, 5)
-      Marshal.send(@method, "\004\bIS:\022Struct::Thick\000\006:\t@fooi\n").should == obj
+      reloaded = Marshal.send(@method, "\004\bIS:\022Struct::Thick\000\006:\t@fooi\n")
+      reloaded.should == obj
+      reloaded.instance_variable_get(:@foo).should == 5
       Struct.send(:remove_const, :Thick)
     end
 
@@ -588,6 +610,18 @@
     end
   end
 
+  describe "for an object responding to #marshal_dump and #marshal_load" do
+    it "loads a user-marshaled object" do
+      obj = UserMarshal.new
+      obj.data = :data
+      value = [obj, :data]
+      dump = Marshal.dump(value)
+      dump.should == "\x04\b[\aU:\x10UserMarshal:\tdata;\x06"
+      reloaded = Marshal.load(dump)
+      reloaded.should == value
+    end
+  end
+
   describe "for a user object" do
     it "loads a user-marshaled extended object" do
       obj = UserMarshal.new.extend(Meths)

spec/ruby/core/string/scrub_spec.rb

@@ -119,4 +119,10 @@
     input.scrub!
     input.instance_variable_get(:@a).should == 'b'
   end
+
+  it "accepts a frozen string as a replacement" do
+    input = "a\xE2"
+    input.scrub!('.'.freeze)
+    input.should == 'a.'
+  end
 end

spec/ruby/optional/capi/ext/string_spec.c

@@ -1,6 +1,7 @@
 #include "ruby.h"
 #include "rubyspec.h"
 
+#include <fcntl.h>
 #include <string.h>
 #include <stdarg.h>
 
@@ -279,6 +280,16 @@ VALUE string_spec_rb_str_resize_RSTRING_LEN(VALUE self, VALUE str, VALUE size) {
   return INT2FIX(RSTRING_LEN(modified));
 }
 
+VALUE string_spec_rb_str_resize_copy(VALUE self, VALUE str) {
+  rb_str_modify_expand(str, 5);
+  char *buffer = RSTRING_PTR(str);
+  buffer[1] = 'e';
+  buffer[2] = 's';
+  buffer[3] = 't';
+  rb_str_resize(str, 4);
+  return str;
+}
+
 VALUE string_spec_rb_str_split(VALUE self, VALUE str) {
   return rb_str_split(str, ",");
 }
@@ -374,6 +385,20 @@ VALUE string_spec_RSTRING_PTR_after_yield(VALUE self, VALUE str) {
   return from_rstring_ptr;
 }
 
+VALUE string_spec_RSTRING_PTR_read(VALUE self, VALUE str, VALUE path) {
+  char *cpath = StringValueCStr(path);
+  int fd = open(cpath, O_RDONLY);
+  rb_str_modify_expand(str, 10);
+  char *buffer = RSTRING_PTR(str);
+  read(fd, buffer, 10);
+  rb_str_modify_expand(str, 21);
+  char *buffer2 = RSTRING_PTR(str);
+  read(fd, buffer2 + 10, 11);
+  rb_str_set_len(str, 21);
+  close(fd);
+  return str;
+}
+
 VALUE string_spec_StringValue(VALUE self, VALUE str) {
   return StringValue(str);
 }
@@ -527,6 +552,7 @@ void Init_string_spec(void) {
   rb_define_method(cls, "rb_str_modify_expand", string_spec_rb_str_modify_expand, 2);
   rb_define_method(cls, "rb_str_resize", string_spec_rb_str_resize, 2);
   rb_define_method(cls, "rb_str_resize_RSTRING_LEN", string_spec_rb_str_resize_RSTRING_LEN, 2);
+  rb_define_method(cls, "rb_str_resize_copy", string_spec_rb_str_resize_copy, 1);
   rb_define_method(cls, "rb_str_set_len", string_spec_rb_str_set_len, 2);
   rb_define_method(cls, "rb_str_set_len_RSTRING_LEN", string_spec_rb_str_set_len_RSTRING_LEN, 2);
   rb_define_method(cls, "rb_str_split", string_spec_rb_str_split, 1);
@@ -542,6 +568,7 @@ void Init_string_spec(void) {
   rb_define_method(cls, "RSTRING_PTR_set", string_spec_RSTRING_PTR_set, 3);
   rb_define_method(cls, "RSTRING_PTR_after_funcall", string_spec_RSTRING_PTR_after_funcall, 2);
   rb_define_method(cls, "RSTRING_PTR_after_yield", string_spec_RSTRING_PTR_after_yield, 1);
+  rb_define_method(cls, "RSTRING_PTR_read", string_spec_RSTRING_PTR_read, 2);
   rb_define_method(cls, "StringValue", string_spec_StringValue, 1);
   rb_define_method(cls, "SafeStringValue", string_spec_SafeStringValue, 1);
   rb_define_method(cls, "rb_str_hash", string_spec_rb_str_hash, 1);

spec/ruby/optional/capi/fixtures/read.txt

@@ -0,0 +1 @@
+fixture file contents

spec/ruby/optional/capi/string_spec.rb

@@ -598,6 +598,12 @@ def inspect
       str = "        "
       @s.RSTRING_PTR_short_memcpy(str).should == "Infinity"
     end
+
+    it "allows read to update string contents" do
+      filename = fixture(__FILE__, "read.txt")
+      str = ""
+      @s.RSTRING_PTR_read(str, filename).should == "fixture file contents"
+    end
   end
 
   describe "RSTRING_LEN" do
@@ -665,6 +671,12 @@ def inspect
     end
   end
 
+  describe "rb_str_modify" do
+    it "raises an error if the string is frozen" do
+      -> { @s.rb_str_modify("frozen".freeze) }.should raise_error(FrozenError)
+    end
+  end
+
   describe "rb_str_modify_expand" do
     it "grows the capacity to bytesize + expand, not changing the bytesize" do
       str = @s.rb_str_buf_new(256, "abcd")
@@ -684,6 +696,15 @@ def inspect
       str.bytesize.should == 3
       @s.RSTRING_LEN(str).should == 3
       @s.rb_str_capacity(str).should == 1027
+
+      @s.rb_str_modify_expand(str, 1)
+      str.bytesize.should == 3
+      @s.RSTRING_LEN(str).should == 3
+      @s.rb_str_capacity(str).should == 4
+    end
+
+    it "raises an error if the string is frozen" do
+      -> { @s.rb_str_modify_expand("frozen".freeze, 10) }.should raise_error(FrozenError)
     end
   end
 
@@ -700,6 +721,11 @@ def inspect
       @s.rb_str_resize_RSTRING_LEN("test", 2).should == 2
     end
 
+    it "copies the existing bytes" do
+      str = "t"
+      @s.rb_str_resize_copy(str).should == "test"
+    end
+
     it "increases the size of the string" do
       expected = "test".force_encoding("US-ASCII")
       str = @s.rb_str_resize(expected.dup, 12)

spec/truffle/identity_spec.rb

@@ -67,8 +67,8 @@
     RbConfig::CONFIG['RUBY_INSTALL_NAME'].should == 'truffleruby'
   end
 
-  it "RbConfig::CONFIG['ruby_version'] is the ABI version and starts with RUBY_VERSION and has an extra component" do
-    RbConfig::CONFIG['ruby_version'].should =~ /\A#{Regexp.escape RUBY_VERSION}\.\d+\z/
+  it "RbConfig::CONFIG['ruby_version'] is the ABI version and starts with RUBY_VERSION and has at least an extra component" do
+    RbConfig::CONFIG['ruby_version'].should =~ /\A#{Regexp.escape RUBY_VERSION}\.\d+(\.\d+)*\z/
   end
 
   it "RbConfig::CONFIG['RUBY_BASE_NAME'] is 'ruby'" do

src/main/c/cext/string.c

@@ -174,6 +174,7 @@ VALUE rb_str_split(VALUE string, const char *split) {
 }
 
 void rb_str_modify(VALUE string) {
+  rb_check_frozen(string);
   ENC_CODERANGE_CLEAR(string);
 }
 
@@ -337,12 +338,9 @@ void rb_str_modify_expand(VALUE str, long expand) {
     rb_raise(rb_eArgError, "string size too big");
   }
 
+  rb_check_frozen(str);
   if (expand > 0) {
-    // rb_str_modify_expand() resizes the native buffer but does not change
-    // RSTRING_LEN() (and therefore String#bytesize).
-    // TODO (eregon, 26 Apr 2018): Do this more directly.
-    rb_str_resize(str, len + expand);
-    rb_str_set_len(str, len);
+    polyglot_invoke(RUBY_CEXT, "rb_tr_str_capa_resize", rb_tr_unwrap(str), len + expand);
   }
 
   ENC_CODERANGE_CLEAR(str);

src/main/java/org/truffleruby/cext/CExtNodes.java

@@ -666,6 +666,28 @@ protected RubyString rbStrResize(RubyString string, int newByteLength,
         }
     }
 
+    @CoreMethod(names = "rb_tr_str_capa_resize", onSingleton = true, required = 2, lowerFixnum = 2)
+    public abstract static class TrStrCapaResizeNode extends CoreMethodArrayArgumentsNode {
+
+        @Specialization
+        protected RubyString trStrCapaResize(RubyString string, int newCapacity,
+                @Cached StringToNativeNode stringToNativeNode,
+                @Cached ConditionProfile asciiOnlyProfile) {
+            final NativeRope nativeRope = stringToNativeNode.executeToNative(string);
+
+            if (nativeRope.getCapacity() == newCapacity) {
+                return string;
+            } else {
+                final NativeRope newRope = nativeRope
+                        .expandCapacity(getContext(), newCapacity);
+                string.setRope(newRope);
+                return string;
+            }
+        }
+
+    }
+
+
     @CoreMethod(names = "rb_block_proc", onSingleton = true)
     public abstract static class BlockProcNode extends CoreMethodArrayArgumentsNode {
 

src/main/java/org/truffleruby/core/rope/NativeRope.java

@@ -12,6 +12,7 @@
 import com.oracle.truffle.api.profiles.ConditionProfile;
 import org.jcodings.Encoding;
 import org.jcodings.specific.ASCIIEncoding;
+import org.truffleruby.RubyContext;
 import org.truffleruby.core.FinalizationService;
 import org.truffleruby.core.string.StringAttributes;
 import org.truffleruby.core.string.StringSupport;
@@ -88,12 +89,31 @@ public NativeRope resize(FinalizationService finalizationService, int newByteLen
         assert byteLength() != newByteLength;
 
         final Pointer pointer = Pointer.malloc(newByteLength + 1);
-        pointer.writeBytes(0, this.pointer, 0, Math.min(byteLength(), newByteLength));
+        pointer.writeBytes(0, this.pointer, 0, Math.min(getNativePointer().getSize(), newByteLength));
         pointer.writeByte(newByteLength, (byte) 0); // Like MRI
         pointer.enableAutorelease(finalizationService);
         return new NativeRope(pointer, newByteLength, getEncoding(), UNKNOWN_CHARACTER_LENGTH, CodeRange.CR_UNKNOWN);
     }
 
+    /** Creates a new native rope which preserves existing bytes and byte length up to newCapacity
+     * 
+     * @param context the Ruby context
+     * @param newCapacity the size in bytes minus one of the new pointer length
+     * @return the new NativeRope */
+    public NativeRope expandCapacity(RubyContext context, int newCapacity) {
+        assert getCapacity() != newCapacity;
+        final Pointer pointer = Pointer.malloc(newCapacity + 1);
+        pointer.writeBytes(0, this.pointer, 0, Math.min(getNativePointer().getSize(), newCapacity));
+        pointer.writeByte(newCapacity, (byte) 0); // Like MRI
+        pointer.enableAutorelease(context.getFinalizationService());
+        return new NativeRope(
+                pointer,
+                byteLength(),
+                getEncoding(),
+                UNKNOWN_CHARACTER_LENGTH,
+                CodeRange.CR_UNKNOWN);
+    }
+
     @Override
     public byte[] getBytes() {
         // Always re-read bytes from the native pointer as they might have changed.