[GR-17457] C tidy up

PullRequest: truffleruby/3241

Author: aardvark179

CHANGELOG.md

@@ -18,12 +18,14 @@ Compatibility:
 * `Exception#full_message` now defaults the order to `:top` like CRuby 3+ (@eregon).
 * Fix `Process.wait2` to return `nil` when the `WNOHANG` flag is given and the child process is still running (@bjfish).
 * Disable most `nokogiri` C extension patches when system libraries are not being used (#2693, @aardvark179).
+* Implement `rb_gc_mark_maybe` and `rb_global_variable` to ensure `VALUE` stay live in C extensions (@aardvark179).
 
 Performance:
 
 * Reimplement `Float#to_s` for better performance (#1584, @aardvark179).
 * Improve reference processing by making C object free functions and other finalizers more lightweight (@aardvark179).
 * Improve performance of `RSTRING_PTR` for interned strings (@aardvark179).
+* Cache constant argument formats used with `rb_scan_args_kw` (@aardvark179).
 
 Changes:
 

lib/cext/ABI_version.txt

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

lib/cext/include/ruby/internal/gc.h

@@ -36,11 +36,7 @@ void rb_gc_register_address(VALUE *valptr);
 /**
  * An alias for `rb_gc_register_address()`.
  */
-#ifdef TRUFFLERUBY
-#define rb_global_variable(address) ;
-#else
 void rb_global_variable(VALUE *);
-#endif
 
 /**
  * Inform the garbage collector that a pointer previously passed to

lib/cext/include/truffleruby/truffleruby.h

@@ -61,6 +61,24 @@ if (polyglot_as_boolean(polyglot_invoke(RUBY_CEXT, "warning?"))) { \
 } \
 })
 
+#define rb_tr_scan_args_kw(kw_flag, argc, argv, format, v1, v2, v3, v4, v5, v6, v7, v8, v9, v10) \
+  (RBIMPL_CONSTANT_P(format) ?                                             \
+     __extension__ ({ \
+         static bool evaled; \
+         static int pre; \
+         static int optional; \
+         static bool rest; \
+         static int post; \
+         static bool kwargs; \
+         static bool block; \
+         if (!evaled) { \
+             rb_tr_scan_args_kw_parse(format, &pre, &optional, &rest, &post, &kwargs, &block); \
+             evaled = true; \
+         } \
+         rb_tr_scan_args_kw_int(kw_flag, argc, argv, pre, optional, rest, post, kwargs, block, v1, v2, v3, v4, v5, v6, v7, v8, v9, v10); \
+     }) : \
+  rb_tr_scan_args_kw_non_const(kw_flag, argc, argv, format, v1, v2, v3, v4, v5, v6, v7, v8, v9, v10))
+
 #define rb_tr_scan_args_kw_1(KW_FLAG, ARGC, ARGV, FORMAT, V1) rb_tr_scan_args_kw(KW_FLAG, ARGC, ARGV, FORMAT, V1, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL)
 #define rb_tr_scan_args_kw_2(KW_FLAG, ARGC, ARGV, FORMAT, V1, V2) rb_tr_scan_args_kw(KW_FLAG, ARGC, ARGV, FORMAT, V1, V2, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL)
 #define rb_tr_scan_args_kw_3(KW_FLAG, ARGC, ARGV, FORMAT, V1, V2, V3) rb_tr_scan_args_kw(KW_FLAG, ARGC, ARGV, FORMAT, V1, V2, V3, NULL, NULL, NULL, NULL, NULL, NULL, NULL)
@@ -196,20 +214,10 @@ static inline char *rb_tr_string_value_cstr(VALUE *value_pointer) {
   return RSTRING_PTR(string);
 }
 
-ALWAYS_INLINE(static int rb_tr_scan_args_kw(int kw_flag, int argc, VALUE *argv, const char *format, VALUE *v1, VALUE *v2, VALUE *v3, VALUE *v4, VALUE *v5, VALUE *v6, VALUE *v7, VALUE *v8, VALUE *v9, VALUE *v10));
-static inline int rb_tr_scan_args_kw(int kw_flag, int argc, VALUE *argv, const char *format, VALUE *v1, VALUE *v2, VALUE *v3, VALUE *v4, VALUE *v5, VALUE *v6, VALUE *v7, VALUE *v8, VALUE *v9, VALUE *v10) {
-
-  // TODO CS 7-Feb-17 maybe we could inline cache this part?
-
-  const char *formatp = format;
-  int pre = 0;
-  int optional = 0;
-  bool rest;
-  int post = 0;
-  bool kwargs;
-  bool block;
+void rb_tr_scan_args_kw_parse(const char *format, int *pre, int *optional, bool *rest, int *post, bool *kwargs, bool *block);
 
-  // Interpret kw_flag
+ALWAYS_INLINE(static int rb_tr_scan_args_kw_int(int kw_flag, int argc, VALUE *argv, int pre, int optional, bool rest, int post, bool kwargs, bool block, VALUE *v1, VALUE *v2, VALUE *v3, VALUE *v4, VALUE *v5, VALUE *v6, VALUE *v7, VALUE *v8, VALUE *v9, VALUE *v10));
+static inline int rb_tr_scan_args_kw_int(int kw_flag, int argc, VALUE *argv, int pre, int optional, bool rest, int post, bool kwargs, bool block, VALUE *v1, VALUE *v2, VALUE *v3, VALUE *v4, VALUE *v5, VALUE *v6, VALUE *v7, VALUE *v8, VALUE *v9, VALUE *v10) {
 
   int keyword_given = 0;
   int last_hash_keyword = 0;
@@ -220,48 +228,6 @@ static inline int rb_tr_scan_args_kw(int kw_flag, int argc, VALUE *argv, const c
     case RB_SCAN_ARGS_LAST_HASH_KEYWORDS: last_hash_keyword = 1; break;
   }
 
-  // TODO CS 27-Feb-17 can LLVM constant-fold through isdigit?
-
-  if (isdigit(*formatp)) {
-    pre = *formatp - '0';
-    formatp++;
-
-    if (isdigit(*formatp)) {
-      optional = *formatp - '0';
-      formatp++;
-    }
-  }
-
-  if (*formatp == '*') {
-    rest = true;
-    formatp++;
-  } else {
-    rest = false;
-  }
-
-  if (isdigit(*formatp)) {
-    post = *formatp - '0';
-    formatp++;
-  }
-
-  if (*formatp == ':') {
-    kwargs = true;
-    formatp++;
-  } else {
-    kwargs = false;
-  }
-
-  if (*formatp == '&') {
-    block = true;
-    formatp++;
-  } else {
-    block = false;
-  }
-
-  if (*formatp != '\0') {
-    rb_raise(rb_eArgError, "bad rb_scan_args format");
-  }
-
   // Check we have enough arguments
 
   if (pre + post > argc) {
@@ -425,6 +391,22 @@ static inline int rb_tr_scan_args_kw(int kw_flag, int argc, VALUE *argv, const c
   return argc;
 }
 
+ALWAYS_INLINE(static int rb_tr_scan_args_kw_non_const(int kw_flag, int argc, VALUE *argv, const char *format, VALUE *v1, VALUE *v2, VALUE *v3, VALUE *v4, VALUE *v5, VALUE *v6, VALUE *v7, VALUE *v8, VALUE *v9, VALUE *v10));
+static inline int rb_tr_scan_args_kw_non_const(int kw_flag, int argc, VALUE *argv, const char *format, VALUE *v1, VALUE *v2, VALUE *v3, VALUE *v4, VALUE *v5, VALUE *v6, VALUE *v7, VALUE *v8, VALUE *v9, VALUE *v10) {
+
+  const char *formatp = format;
+  int pre = 0;
+  int optional = 0;
+  bool rest;
+  int post = 0;
+  bool kwargs;
+  bool block;
+
+  rb_tr_scan_args_kw_parse(format, &pre, &optional, &rest, &post, &kwargs, &block);
+
+  return rb_tr_scan_args_kw_int(kw_flag, argc, argv, pre, optional, rest, post, kwargs, block, v1, v2, v3, v4, v5, v6, v7, v8, v9, v10);
+}
+
 #define rb_iv_get(obj, name) \
   (__builtin_constant_p(name) ? \
    rb_ivar_get(obj, rb_intern(name)) : \

lib/truffle/truffle/cext.rb

@@ -106,6 +106,8 @@ module Truffle::CExt
   RUBY_ECONV_PARTIAL_INPUT = Encoding::Converter::PARTIAL_INPUT
   RUBY_ECONV_AFTER_OUTPUT = Encoding::Converter::AFTER_OUTPUT
 
+  GLOBALLY_PRESERVED_VALUES = []
+
   SET_LIBTRUFFLERUBY = -> libtruffleruby do
     LIBTRUFFLERUBY = libtruffleruby
   end
@@ -1919,4 +1921,8 @@ def rb_tr_pointer(pointer)
   def rb_exception_set_message(e, mesg)
     Primitive.exception_set_message(e, mesg)
   end
+
+  def rb_global_variable(obj)
+    GLOBALLY_PRESERVED_VALUES << obj
+  end
 end

src/main/c/cext/args.c

@@ -102,3 +102,47 @@ int rb_get_kwargs(VALUE keyword_hash, const ID *table, int required, int optiona
 
   return extracted;
 }
+
+void rb_tr_scan_args_kw_parse(const char *format, int *pre, int *optional, bool *rest, int *post, bool *kwargs, bool *block) {
+  const char *formatp = format;
+
+  if (isdigit(*formatp)) {
+    *pre = *formatp - '0';
+    formatp++;
+
+    if (isdigit(*formatp)) {
+      *optional = *formatp - '0';
+      formatp++;
+    }
+  }
+
+  if (*formatp == '*') {
+    *rest = true;
+    formatp++;
+  } else {
+    *rest = false;
+  }
+
+  if (isdigit(*formatp)) {
+    *post = *formatp - '0';
+    formatp++;
+  }
+
+  if (*formatp == ':') {
+    *kwargs = true;
+    formatp++;
+  } else {
+    *kwargs = false;
+  }
+
+  if (*formatp == '&') {
+    *block = true;
+    formatp++;
+  } else {
+    *block = false;
+  }
+
+  if (*formatp != '\0') {
+    rb_raise(rb_eArgError, "bad rb_scan_args format");
+  }
+}

src/main/c/cext/gc.c

@@ -23,6 +23,12 @@ void rb_gc_mark(VALUE ptr) {
   polyglot_invoke(RUBY_CEXT, "rb_gc_mark", ptr);
 }
 
+void rb_gc_mark_maybe(VALUE ptr) {
+  if (!RB_TYPE_P(ptr, T_NONE)) {
+    polyglot_invoke(RUBY_CEXT, "rb_gc_mark", ptr);
+  }
+}
+
 VALUE rb_gc_enable() {
   return RUBY_CEXT_INVOKE("rb_gc_enable");
 }
@@ -47,3 +53,10 @@ VALUE rb_gc_latest_gc_info(VALUE key) {
 void rb_gc_register_mark_object(VALUE obj) {
   RUBY_CEXT_INVOKE_NO_WRAP("rb_gc_register_mark_object", obj);
 }
+
+void rb_global_variable(VALUE *obj) {
+  /* TODO: This should guard the address of the pointer, not the
+     object pointed to, but we haven't yet found a good way to implement
+     that, or a real world use case where it is required. */
+  RUBY_CEXT_INVOKE_NO_WRAP("rb_global_variable", *obj);
+}