[GR-20709] [GR-20837] [GR-19473] [GR-20976] Bug fixes for 20.0.

PullRequest: truffleruby/1334

Author: eregon

.gitattributes

@@ -1,6 +1,5 @@
 # Merge options
 
-/CHANGELOG.md merge=union
 
 # Rules for GitHub's Linguist language-classification system. We're abusing the
 # 'vendored' attribute to exclude files as a lot of this isn't really vendored,

CHANGELOG.md

@@ -5,6 +5,7 @@ New features:
 * Enable and document `--coverage` option (#1840, @chrisseaton).
 * Update the internal LLVM toolchain to LLVM 9 and reduce its download size.
 * Updated to Ruby 2.6.5 (#1749).
+* Automatically set `PKG_CONFIG_PATH` as needed for compiling OpenSSL on macOS (#1830).
 
 Bug fixes:
 
@@ -76,6 +77,8 @@ Bug fixes:
 * Fixed `IO.try_convert` parameter conversion.
 * Fixed linking of always-inline C API functions with `-std=gnu90` (#1837, #1879).
 * Avoid race conditions during `gem install` by using a single download thread.
+* `RSTRING_PTR()` now always returns a native pointer, resolving two bugs `memcpy`ing to (#1822) and from (#1772) Ruby Strings.
+* Do not use gems precompiled for MRI on TruffleRuby (#1837).
 
 Compatibility:
 

ci.jsonnet

@@ -76,7 +76,7 @@ local part_definitions = {
       environment+: { path+:: ["$MAVEN_HOME/bin"] },
     },
 
-    build: {
+    build_no_clean: {
       setup+: [["mx", "sversions"]] +
               # aot-build.log is used for the build-stats metrics, in other cases it does no harm
               jt(["build", "--env", self.mx_env] + self.jt_build_options + ["--"] + self.mx_build_options + ["|", "tee", "aot-build.log"]) +
@@ -86,6 +86,11 @@ local part_definitions = {
               ],
     },
 
+    build: $.use.build_no_clean + {
+      # Clean build results to make sure nothing refers to them while testing
+      setup+: jt(["mx", "--env", self.mx_env, "clean"]),
+    },
+
     clone_enterprise: {
       setup+: [["mx", "sversions"]] + jt(["checkout_enterprise_revision"]),
     },
@@ -273,18 +278,19 @@ local part_definitions = {
 
   run: {
     test_unit_tck_specs: {
-      run+: jt(["test", "unit"]) +
-            jt(["test", "tck"]) +
-            jt(["test", "specs"]) +
-            jt(["test", "specs", ":next"]),
+      run+: jt(["test", "specs"]) +
+            jt(["test", "specs", ":next"]) +
+            jt(["build"]) + # We need mx distributions to run unit tests
+            jt(["test", "unit"]) +
+            jt(["test", "tck"]),
     },
 
     test_fast: {
       run+: jt(["test", "fast"]),
     },
 
     lint: {
-      is_after:: ["$.use.build"],
+      is_after:: ["$.use.build_no_clean"],
       downloads+: {
         JDT: { name: "ecj", version: "4.5.1", platformspecific: false },
         ECLIPSE: { version: "4.5.2", name: "eclipse", platformspecific: true },
@@ -427,7 +433,7 @@ local composition_environment = utils.add_inclusion_tracking(part_definitions, "
 
   test_builds:
     {
-      "ruby-lint": $.platform.linux + $.cap.gate + $.jdk.v8 + $.use.common + $.env.jvm + $.use.build + $.run.lint + { timelimit: "30:00" },
+      "ruby-lint": $.platform.linux + $.cap.gate + $.jdk.v8 + $.use.common + $.env.jvm + $.use.build_no_clean + $.run.lint + { timelimit: "30:00" },
     } +
 
     {

lib/cext/include/truffleruby/truffleruby.h

@@ -158,7 +158,7 @@ int rb_encdb_alias(const char *alias, const char *orig);
 VALUE rb_ivar_lookup(VALUE object, const char *name, VALUE default_value);
 
 // Additional macro to make sure the RSTRING_PTR and the bytes are in native memory, for testing.
-#define NATIVE_RSTRING_PTR(str) ((char*) polyglot_as_i64(polyglot_invoke(RSTRING_PTR(str), "polyglot_address")))
+#define NATIVE_RSTRING_PTR(str) ((char*) polyglot_as_i64(RUBY_CEXT_INVOKE_NO_WRAP("NATIVE_RSTRING_PTR", str)))
 
 // Inline implementations
 

lib/truffle/rubygems/defaults/truffleruby.rb

@@ -16,4 +16,9 @@ module Gem
   def self.default_dir
     @default_dir ||= "#{Truffle::Boot.ruby_home or raise 'TruffleRuby home not found'}/lib/gems"
   end
+
+  # Only report the RUBY platform as supported to make sure gems precompiled for MRI are not used.
+  # TruffleRuby has a different ABI and cannot reuse gems precompiled for MRI.
+  # See https://github.com/rubygems/rubygems/issues/2945
+  Gem.platforms = [Gem::Platform::RUBY]
 end

lib/truffle/truffle/cext.rb

@@ -149,31 +149,31 @@ def initialize(string)
     end
 
     def size
-      Truffle::CExt.string_pointer_size(@string)
+      TrufflePrimitive.string_pointer_size(@string)
     end
 
     def polyglot_pointer?
       true
     end
 
     def polyglot_address
-      @address ||= Truffle::CExt.string_pointer_to_native(@string)
+      @address ||= TrufflePrimitive.string_pointer_to_native(@string)
     end
 
     # Every IS_POINTER object should also have TO_NATIVE
     def polyglot_to_native
     end
 
     def [](index)
-      Truffle::CExt.string_pointer_read(@string, index)
+      TrufflePrimitive.string_pointer_read(@string, index)
     end
 
     def []=(index, value)
-      Truffle::CExt.string_pointer_write(@string, index, value)
+      TrufflePrimitive.string_pointer_write(@string, index, value)
     end
 
     def native?
-      Truffle::CExt.string_pointer_is_native?(@string)
+      TrufflePrimitive.string_pointer_is_native?(@string)
     end
 
     alias_method :to_str, :string
@@ -231,7 +231,7 @@ def polyglot_pointer?
     end
 
     def polyglot_address
-      @address ||= Truffle::CExt.string_pointer_to_native(@string) + @string.bytesize
+      @address ||= TrufflePrimitive.string_pointer_to_native(@string) + @string.bytesize
     end
 
     # Every IS_POINTER object should also have TO_NATIVE
@@ -1846,10 +1846,18 @@ def rb_enc_from_native_encoding(rb_encoding)
     RbEncoding.get_encoding_from_native(rb_encoding)
   end
 
+  def native_string?(string)
+    TrufflePrimitive.string_pointer_is_native?(string)
+  end
+
   def RSTRING_PTR(string)
     RStringPtr.new(string)
   end
 
+  def NATIVE_RSTRING_PTR(string)
+    TrufflePrimitive.string_pointer_to_native(string)
+  end
+
   def RSTRING_END(string)
     RStringEndPtr.new(string)
   end

lib/truffle/truffle/lazy-rubygems.rb

@@ -10,6 +10,9 @@
 # Otherwise, --disable-gems would degrade startup which is counter-intuitive.
 Truffle::Boot.delay do
   if Truffle::Boot.get_option 'rubygems'
+    module Truffle::LazyRubyGems
+    end
+
     module Kernel
       # Take this alias name so RubyGems will reuse this copy
       # and skip the method below once RubyGems is loaded.
@@ -19,11 +22,19 @@ module Kernel
         begin
           gem_original_require(path)
         rescue LoadError
-          require 'rubygems'
-          require path
+          gem_original_require 'rubygems'
+
+          # Check that #require was redefined by RubyGems, otherwise we would end up in infinite recursion
+          new_require = ::Kernel.instance_method(:require)
+          if new_require == Truffle::LazyRubyGems::LAZY_REQUIRE
+            raise 'RubyGems did not redefine #require as expected, make sure $LOAD_PATH and home are set correctly'
+          end
+          new_require.bind(self).call(path)
         end
       end
 
+      Truffle::LazyRubyGems::LAZY_REQUIRE = instance_method(:require)
+
       private def gem(*args)
         require 'rubygems'
         gem(*args)

lib/truffle/truffle/openssl-prefix.rb

@@ -13,12 +13,26 @@
 macOS = RbConfig::CONFIG['host_os'].include?('darwin')
 
 if macOS && !ENV['OPENSSL_PREFIX']
-  homebrew_prefix = `brew --prefix openssl 2>/dev/null`.chomp
-  if $?.success? and Dir.exist?(homebrew_prefix) # Homebrew
-    ENV['OPENSSL_PREFIX'] = homebrew_prefix
+  homebrew = `brew --prefix 2>/dev/null`.strip
+  unless $?.success? and !homebrew.empty? and Dir.exist?(homebrew)
+    homebrew = nil
+  end
+
+  if homebrew and prefix = "#{homebrew}/opt/openssl@1.1" and Dir.exist?(prefix)
+    ENV['OPENSSL_PREFIX'] = prefix
+  elsif homebrew and prefix = "#{homebrew}/opt/openssl" and Dir.exist?(prefix)
+    ENV['OPENSSL_PREFIX'] = prefix
   elsif Dir.exist?('/opt/local/include/openssl') # MacPorts
     ENV['OPENSSL_PREFIX'] = '/opt/local'
   else
     abort 'Could not find OpenSSL headers, install via Homebrew or MacPorts or set OPENSSL_PREFIX'
   end
 end
+
+if openssl_prefix = ENV['OPENSSL_PREFIX']
+  Truffle::Debug.log_config("Found OpenSSL in #{openssl_prefix}")
+
+  # We need to set PKG_CONFIG_PATH too, see https://github.com/oracle/truffleruby/issues/1830
+  # OpenSSL's extconf.rb calls the pkg_config() helper.
+  ENV['PKG_CONFIG_PATH'] = ["#{openssl_prefix}/lib/pkgconfig", *ENV['PKG_CONFIG_PATH']].join(':')
+end

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

@@ -284,6 +284,26 @@ VALUE string_spec_RSTRING_PTR_iterate(VALUE self, VALUE str) {
   return Qnil;
 }
 
+VALUE string_spec_RSTRING_PTR_iterate_uint32(VALUE self, VALUE str) {
+  int i;
+  uint32_t* ptr;
+  int l = RSTRING_LEN(str) / sizeof(uint32_t);
+
+  ptr = (uint32_t *)RSTRING_PTR(str);
+  for(i = 0; i < l; i++) {
+    rb_yield(UINT2NUM(ptr[i]));
+  }
+  return Qnil;
+}
+
+VALUE string_spec_RSTRING_PTR_short_memcpy(VALUE self, VALUE str) {
+  // Short memcpy operations may be optimised by the compiler to a single write.
+  if (RSTRING_LEN(str) >= 8) {
+    memcpy(RSTRING_PTR(str), "Infinity", 8);
+  }
+  return str;
+}
+
 VALUE string_spec_RSTRING_PTR_assign(VALUE self, VALUE str, VALUE chr) {
   int i;
   char c;
@@ -477,6 +497,8 @@ void Init_string_spec(void) {
   rb_define_method(cls, "RSTRING_LEN", string_spec_RSTRING_LEN, 1);
   rb_define_method(cls, "RSTRING_LENINT", string_spec_RSTRING_LENINT, 1);
   rb_define_method(cls, "RSTRING_PTR_iterate", string_spec_RSTRING_PTR_iterate, 1);
+  rb_define_method(cls, "RSTRING_PTR_iterate_uint32", string_spec_RSTRING_PTR_iterate_uint32, 1);
+  rb_define_method(cls, "RSTRING_PTR_short_memcpy", string_spec_RSTRING_PTR_short_memcpy, 1);
   rb_define_method(cls, "RSTRING_PTR_assign", string_spec_RSTRING_PTR_assign, 2);
   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);

spec/ruby/optional/capi/string_spec.rb

@@ -513,6 +513,24 @@ def inspect
       end
       chars.should == [55, 48, 227, 131, 145, 227, 130, 175]
     end
+
+    it "returns a pointer which can be cast and used as another type" do
+      s = "70パク".
+        encode(Encoding::UTF_16LE).
+        force_encoding(Encoding::UTF_16LE).
+        encode(Encoding::UTF_8)
+
+      ints = []
+      @s.RSTRING_PTR_iterate_uint32(s) do |i|
+        ints << i
+      end
+      ints.should == s.unpack('LL')
+    end
+
+    it "allows a short memcpy to the string which may be converted to a single write operation by the compiler" do
+      str = "        "
+      @s.RSTRING_PTR_short_memcpy(str).should == "Infinity"
+    end
   end
 
   describe "RSTRING_LEN" do

spec/truffle/capi/string_spec.rb

@@ -15,16 +15,16 @@
     @s = CApiTruffleStringSpecs.new
   end
 
-  it "does not store the String to native memory if not needed" do
+  it "stores the String to native memory even if not needed for efficiency" do
     str = "foobar"
     @s.string_ptr(str).should == "f"
-    Truffle::CExt.string_pointer_is_native?(str).should == false
+    Truffle::CExt.native_string?(str).should == true
   end
 
   it "stores the String to native memory if the address is returned" do
     str = "foobar"
     @s.string_ptr_return_address(str).should be_kind_of(Integer)
-    Truffle::CExt.string_pointer_is_native?(str).should == true
+    Truffle::CExt.native_string?(str).should == true
   end
 end
 
@@ -35,8 +35,8 @@
 
   it "ensures the String is stored in native memory" do
     str = "foobar"
-    Truffle::CExt.string_pointer_is_native?(str).should == false
+    Truffle::CExt.native_string?(str).should == false
     @s.NATIVE_RSTRING_PTR(str)
-    Truffle::CExt.string_pointer_is_native?(str).should == true
+    Truffle::CExt.native_string?(str).should == true
   end
 end

spec/truffle/rubygems/platforms_spec.rb

@@ -0,0 +1,17 @@
+# Copyright (c) 2020 Oracle and/or its affiliates. All rights reserved. This
+# code is released under a tri EPL/GPL/LGPL license. You can use it,
+# redistribute it and/or modify it under the terms of the:
+#
+# Eclipse Public License version 2.0, or
+# GNU General Public License version 2, or
+# GNU Lesser General Public License version 2.1.
+# OTHER DEALINGS IN THE SOFTWARE.
+
+require_relative '../../ruby/spec_helper'
+require 'rubygems'
+
+describe "Gem.platforms" do
+  it "returns only [RUBY] on TruffleRuby to not use gems precompiled for MRI" do
+    Gem.platforms.should == [Gem::Platform::RUBY]
+  end
+end

src/main/c/cext/ruby.c

@@ -812,7 +812,7 @@ char *rb_string_value_cstr(VALUE *value_pointer) {
 }
 
 char *RSTRING_PTR_IMPL(VALUE string) {
-  return (char *)polyglot_as_i8_array(RUBY_CEXT_INVOKE_NO_WRAP("RSTRING_PTR", string));
+  return NATIVE_RSTRING_PTR(string);
 }
 
 char *RSTRING_END(VALUE string) {

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

@@ -957,8 +957,8 @@ protected Object rbHash(Object object) {
         }
     }
 
-    @CoreMethod(names = "string_pointer_size", onSingleton = true, required = 1)
-    public abstract static class StringPointerSizeNode extends CoreMethodArrayArgumentsNode {
+    @Primitive(name = "string_pointer_size")
+    public abstract static class StringPointerSizeNode extends PrimitiveArrayArgumentsNode {
 
         @Specialization(guards = "isRubyString(string)")
         protected int size(DynamicObject string) {
@@ -1010,8 +1010,8 @@ protected NativeRope toNative(DynamicObject string,
 
     }
 
-    @CoreMethod(names = "string_pointer_to_native", onSingleton = true, required = 1)
-    public abstract static class StringPointerToNativeNode extends CoreMethodArrayArgumentsNode {
+    @Primitive(name = "string_pointer_to_native")
+    public abstract static class StringPointerToNativeNode extends PrimitiveArrayArgumentsNode {
 
         @Specialization(guards = "isRubyString(string)")
         protected long toNative(DynamicObject string,
@@ -1038,8 +1038,8 @@ protected DynamicObject toNative(DynamicObject string,
 
     }
 
-    @CoreMethod(names = "string_pointer_is_native?", onSingleton = true, required = 1)
-    public abstract static class StringPointerIsNativeNode extends CoreMethodArrayArgumentsNode {
+    @Primitive(name = "string_pointer_is_native?")
+    public abstract static class StringPointerIsNativeNode extends PrimitiveArrayArgumentsNode {
 
         @Specialization(guards = "isRubyString(string)")
         protected boolean isNative(DynamicObject string) {
@@ -1048,8 +1048,8 @@ protected boolean isNative(DynamicObject string) {
 
     }
 
-    @CoreMethod(names = "string_pointer_read", onSingleton = true, required = 2, lowerFixnum = 2)
-    public abstract static class StringPointerReadNode extends CoreMethodArrayArgumentsNode {
+    @Primitive(name = "string_pointer_read", lowerFixnum = 1)
+    public abstract static class StringPointerReadNode extends PrimitiveArrayArgumentsNode {
 
         @Specialization(guards = "isRubyString(string)")
         protected Object read(DynamicObject string, int index,
@@ -1068,8 +1068,8 @@ protected Object read(DynamicObject string, int index,
 
     }
 
-    @CoreMethod(names = "string_pointer_write", onSingleton = true, required = 3, lowerFixnum = { 2, 3 })
-    public abstract static class StringPointerWriteNode extends CoreMethodArrayArgumentsNode {
+    @Primitive(name = "string_pointer_write", lowerFixnum = { 1, 2 })
+    public abstract static class StringPointerWriteNode extends PrimitiveArrayArgumentsNode {
 
         @Specialization(guards = "isRubyString(string)")
         protected int write(DynamicObject string, int index, int value,