From 814eaa25e2a54b5997f0d89da1a6e2259d4fc1e9 Mon Sep 17 00:00:00 2001
From: Takashi Kokubun <takashikkbn@gmail.com>
Date: Thu, 30 May 2019 04:12:10 +0900
Subject: [PATCH] Do not use rb_iseq_path() while moving ISeq pointers

in GC.compact.

While `in_jit` is false, GC.compact is allowed to run and it may be
moving ISeq-related pointers. So calling rb_iseq_path() when `in_jit`
is true is illegal.
---
 mjit_worker.c | 40 ++++++++++++++--------------------------
 1 file changed, 14 insertions(+), 26 deletions(-)

diff --git a/mjit_worker.c b/mjit_worker.c
index f99dc66e7c..6bbcfdf8f8 100644
--- a/mjit_worker.c
+++ b/mjit_worker.c
@@ -943,19 +943,6 @@ load_func_from_so(const char *so_file, const char *funcname, struct rb_mjit_unit
     return func;
 }
 
-static void
-print_jit_result(const char *result, const struct rb_mjit_unit *unit, const double duration, long lineno, const char *c_file)
-{
-    if (unit->iseq == NULL) {
-        verbose(1, "JIT %s (%.1fms): (GCed) -> %s", result, duration, c_file);
-    }
-    else {
-        verbose(1, "JIT %s (%.1fms): %s@%s:%ld -> %s", result,
-                duration, RSTRING_PTR(unit->iseq->body->location.label),
-                RSTRING_PTR(rb_iseq_path(unit->iseq)), lineno, c_file);
-    }
-}
-
 #ifndef __clang__
 static const char *
 header_name_end(const char *s)
@@ -1075,18 +1062,18 @@ convert_unit_to_func(struct rb_mjit_unit *unit)
         return (mjit_func_t)NOT_COMPILED_JIT_ISEQ_FUNC;
     }
 
-    // FIX2INT may fallback to rb_num2long(), which is a method call and dangerous in MJIT worker. So showing the
-    // line number only when it's Fixnum. Also note that doing this while in_jit is true to avoid GC / GC.compact.
-    long lineno = 0;
+    // To make MJIT worker thread-safe against GC.compact, copy ISeq values while `in_jit` is true.
+    long iseq_lineno = 0;
     if (FIXNUM_P(unit->iseq->body->location.first_lineno))
-        lineno = FIX2LONG(unit->iseq->body->location.first_lineno);
-    {
-        VALUE s = rb_iseq_path(unit->iseq);
-        const char *label = RSTRING_PTR(unit->iseq->body->location.label);
-        const char *path = RSTRING_PTR(s);
-        verbose(2, "start compilation: %s@%s:%ld -> %s", label, path, lineno, c_file);
-        fprintf(f, "/* %s@%s:%ld */\n\n", label, path, lineno);
-    }
+        // FIX2INT may fallback to rb_num2long(), which is a method call and dangerous in MJIT worker. So using only FIX2LONG.
+        iseq_lineno = FIX2LONG(unit->iseq->body->location.first_lineno);
+    char *iseq_label = alloca(RSTRING_LEN(unit->iseq->body->location.label));
+    char *iseq_path  = alloca(RSTRING_LEN(rb_iseq_path(unit->iseq)));
+    strcpy(iseq_label, RSTRING_PTR(unit->iseq->body->location.label));
+    strcpy(iseq_path,  RSTRING_PTR(rb_iseq_path(unit->iseq)));
+
+    verbose(2, "start compilation: %s@%s:%ld -> %s", iseq_label, iseq_path, iseq_lineno, c_file);
+    fprintf(f, "/* %s@%s:%ld */\n\n", iseq_label, iseq_path, iseq_lineno);
     bool success = mjit_compile(f, unit->iseq, funcname);
 
     // release blocking mjit_gc_start_hook
@@ -1100,7 +1087,7 @@ convert_unit_to_func(struct rb_mjit_unit *unit)
     if (!success) {
         if (!mjit_opts.save_temps)
             remove_file(c_file);
-        print_jit_result("failure", unit, 0, lineno, c_file);
+        verbose(1, "JIT failure: %s@%s:%ld -> %s", iseq_label, iseq_path, iseq_lineno, c_file);
         return (mjit_func_t)NOT_COMPILED_JIT_ISEQ_FUNC;
     }
 
@@ -1138,7 +1125,8 @@ convert_unit_to_func(struct rb_mjit_unit *unit)
     if ((uintptr_t)func > (uintptr_t)LAST_JIT_ISEQ_FUNC) {
         CRITICAL_SECTION_START(3, "end of jit");
         add_to_list(unit, &active_units);
-        print_jit_result("success", unit, end_time - start_time, lineno, c_file);
+        verbose(1, "JIT success (%.1fms): %s@%s:%ld -> %s",
+                end_time - start_time, iseq_label, iseq_path, iseq_lineno, c_file);
         CRITICAL_SECTION_FINISH(3, "end of jit");
     }
     return (mjit_func_t)func;