Remove act_like_pointer workaround and add spec

PullRequest: truffleruby/526

Author: eregon

lib/truffle/truffle/cext.rb

@@ -102,7 +102,6 @@ def address
     end
 
     def cache_address
-      name.act_like_pointer = true
       addr = name.address
       ENCODING_CACHE_MUTEX.synchronize do
         NATIVE_CACHE[addr] = self
@@ -138,21 +137,28 @@ def fd
 
   class RStringPtr
     attr_reader :string
-    attr_accessor :act_like_pointer
 
     def initialize(string)
       @string = string
-      @act_like_pointer = false
     end
 
     def size
       Truffle::CExt.string_pointer_size(@string)
     end
 
+    def pointer?
+      true
+    end
+
     def address
       @address ||= Truffle::CExt.string_pointer_to_native(@string)
     end
 
+    # Every IS_POINTER object should also have TO_NATIVE
+    def to_native
+      self
+    end
+
     def [](index)
       Truffle::CExt.string_pointer_read(@string, index)
     end
@@ -167,26 +173,32 @@ def native?
 
     alias_method :to_str, :string
     alias_method :to_s, :string
-    alias_method :pointer?, :act_like_pointer
   end
 
   class RArrayPtr
     attr_reader :array
-    attr_accessor :act_like_pointer
 
     def initialize(array)
       @array = array
-      @act_like_pointer = false
     end
 
     def size
       @array.size
     end
 
+    def pointer?
+      true
+    end
+
     def address
       raise RuntimeError, 'RARRAY_PTRs cannot be converted to native pointers yet'
     end
 
+    # Every IS_POINTER object should also have TO_NATIVE
+    def to_native
+      self
+    end
+
     def [](index)
       Truffle::CExt.rb_tr_wrap(array[index])
     end
@@ -198,27 +210,29 @@ def []=(index, value)
     def native?
       false
     end
-
-    alias_method :pointer?, :act_like_pointer
   end
 
   class RStringEndPtr
-    attr_accessor :act_like_pointer
-
     def initialize(string)
       @string = string
-      @act_like_pointer = false
     end
 
     def size
       0
     end
 
+    def pointer?
+      true
+    end
+
     def address
       @address ||= Truffle::CExt.string_pointer_to_native(@string) + @string.bytesize
     end
 
-    alias_method :pointer?, :act_like_pointer
+    # Every IS_POINTER object should also have TO_NATIVE
+    def to_native
+      self
+    end
   end
 
   T_NONE     = 0x00

spec/truffle/capi/ext/truffleruby_string_spec.c

@@ -0,0 +1,42 @@
+#include "ruby.h"
+#include "rubyspec.h"
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+// Tests that Sulong does not force ptr to native
+char* function_returning_char_ptr(char* ptr) {
+  return ptr;
+}
+
+static VALUE string_ptr(VALUE self, VALUE str) {
+  char* ptr = function_returning_char_ptr(RSTRING_PTR(str));
+  char cstring[] = { ptr[0], 0 };
+  return rb_str_new_cstr(cstring);
+}
+
+static VALUE string_ptr_stored_in_native(VALUE self, VALUE str) {
+  char* ptr = RSTRING_PTR(str);
+  char** native = malloc(sizeof(char*));
+  *native = ptr;
+  free(native);
+  return str;
+}
+
+static VALUE string_ptr_return_address(VALUE self, VALUE str) {
+  char* ptr = RSTRING_PTR(str);
+  return LONG2NUM((long) ptr);
+}
+
+void Init_truffleruby_string_spec(void) {
+  VALUE cls;
+  cls = rb_define_class("CApiTruffleStringSpecs", rb_cObject);
+  rb_define_method(cls, "string_ptr", string_ptr, 1);
+  rb_define_method(cls, "string_ptr_stored_in_native", string_ptr_stored_in_native, 1);
+  rb_define_method(cls, "string_ptr_return_address", string_ptr_return_address, 1);
+}
+
+#ifdef __cplusplus
+}
+#endif

spec/truffle/capi/string_spec.rb

@@ -0,0 +1,27 @@
+require_relative '../../ruby/optional/capi/spec_helper'
+
+load_extension("truffleruby_string")
+
+describe "TruffleRuby RSTRING_PTR" do
+  before :each do
+    @s = CApiTruffleStringSpecs.new
+  end
+
+  it "does not store the String to native memory if not needed" do
+    str = "foobar"
+    @s.string_ptr(str).should == "f"
+    Truffle::CExt.string_pointer_is_native?(str).should == false
+  end
+
+  it "stores the String to native memory if stored in malloc'd memory" do
+    str = "foobar"
+    @s.string_ptr_stored_in_native(str)
+    Truffle::CExt.string_pointer_is_native?(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
+  end
+end

src/main/c/cext/ruby.c

@@ -697,25 +697,11 @@ double rb_float_value(VALUE value) {
 // String
 
 char *RSTRING_PTR_IMPL(VALUE string) {
-  char* ret = RUBY_CEXT_INVOKE_NO_WRAP("RSTRING_PTR", string);
-
-  // We start off treating RStringPtr as if it weren't a pointer for interop purposes. This is so Sulong doesn't try
-  // to convert it to an actual char* when returning from `polyglot_invoke`. Once we have a handle to the real RStringPtr
-  // object, we can instruct it to start acting like a pointer, which is necessary for pointer address comparisons.
-  polyglot_invoke(ret, "act_like_pointer=", rb_tr_unwrap(Qtrue));
-
-  return ret;
+  return RUBY_CEXT_INVOKE_NO_WRAP("RSTRING_PTR", string);
 }
 
 char *RSTRING_END(VALUE string) {
-  char* ret = RUBY_CEXT_INVOKE_NO_WRAP("RSTRING_END", string);
-
-  // We start off treating RStringPtr as if it weren't a pointer for interop purposes. This is so Sulong doesn't try
-  // to convert it to an actual char* when returning from `polyglot_invoke`. Once we have a handle to the real RStringPtr
-  // object, we can instruct it to start acting like a pointer, which is necessary for pointer address comparisons.
-  polyglot_invoke(ret, "act_like_pointer=", rb_tr_unwrap(Qtrue));
-
-  return ret;
+  return RUBY_CEXT_INVOKE_NO_WRAP("RSTRING_END", string);
 }
 
 int MBCLEN_NEEDMORE_P(int r) {

src/main/c/openssl/ossl_pkey_dh.c

@@ -262,7 +262,7 @@ ossl_dh_initialize_copy(VALUE self, VALUE other)
 	BIGNUM *pub2 = BN_dup(pub);
 	BIGNUM *priv2 = BN_dup(priv);
 
-	if (!pub2 || priv && !priv2) {
+	if (!pub2 || (priv && !priv2)) { // Truffle: add parens to silence clang warnings
 	    BN_clear_free(pub2);
 	    BN_clear_free(priv2);
 	    ossl_raise(eDHError, "BN_dup");