Always use assert-free APIs when profiling and crashing

XrXr · web-flow · commit 1a52c42e6187 · 2025-09-25T18:30:12.000-04:00
rb_profile_frames() is used by profilers in a way such that it can run on any instruction in the binary, and it crashed previously in the following situation in `RUBY_DEBUG` builds: ``` * thread #1, queue = 'com.apple.main-thread', stop reason = step over frame #0: 0x00000001002827f0 miniruby`vm_make_env_each(ec=0x0000000101866b00, cfp=0x000000080c91bee8) at vm.c:992:74 989 } 990 991 vm_make_env_each(ec, prev_cfp); -> 992 VM_FORCE_WRITE_SPECIAL_CONST(&ep[VM_ENV_DATA_INDEX_SPECVAL], VM_GUARDED_PREV_EP(prev_cfp->ep)); 993 } 994 } 995 else { (lldb) call rb_profile_frames(0, 100, $2, $3) /Users/alan/ruby/vm_core.h:1448: Assertion Failed: VM_ENV_FLAGS:FIXNUM_P(flags) ruby 3.5.0dev (2025-09-23T20:20:04Z master 06b7a70) +PRISM [arm64-darwin25] -- Crash Report log information -------------------------------------------- See Crash Report log file in one of the following locations: * ~/Library/Logs/DiagnosticReports * /Library/Logs/DiagnosticReports for more details. Don't forget to include the above Crash Report log file in bug reports. -- Control frame information ----------------------------------------------- c:0008 p:---- s:0029 e:000028 CFUNC :lambda /Users/alan/ruby/vm_core.h:1448: Assertion Failed: VM_ENV_FLAGS:FIXNUM_P(flags) ruby 3.5.0dev (2025-09-23T20:20:04Z master 06b7a70) +PRISM [arm64-darwin25] -- Crash Report log information -------------------------------------------- <snip> ``` There is a small window where the control frame is invalid and fails the assert. The double crash also shows that in `RUBY_DEBUG` builds, the crash reporter was previously not resilient to corrupt frame state. In release builds, it prints more info. Add unchecked APIs for the crash reporter and profilers so they work as well in `RUBY_DEBUG` builds as non-debug builds.
diff --git a/vm_backtrace.c b/vm_backtrace.c
@@ -1745,14 +1745,14 @@ thread_profile_frames(rb_execution_context_t *ec, int start, int limit, VALUE *b
     end_cfp = RUBY_VM_NEXT_CONTROL_FRAME(end_cfp);
 
     for (i=0; i<limit && cfp != end_cfp; cfp = RUBY_VM_PREVIOUS_CONTROL_FRAME(cfp)) {
-        if (VM_FRAME_RUBYFRAME_P(cfp) && cfp->pc != 0) {
+        if (VM_FRAME_RUBYFRAME_P_UNCHECKED(cfp) && cfp->pc != 0) {
             if (start > 0) {
                 start--;
                 continue;
             }
 
             /* record frame info */
-            cme = rb_vm_frame_method_entry(cfp);
+            cme = rb_vm_frame_method_entry_unchecked(cfp);
             if (cme && cme->def->type == VM_METHOD_TYPE_ISEQ) {
                 buff[i] = (VALUE)cme;
             }
@@ -1770,6 +1770,8 @@ thread_profile_frames(rb_execution_context_t *ec, int start, int limit, VALUE *b
                 // before entering a non-leaf method (so that `caller` will work),
                 // so only the topmost frame could possibly have an out-of-date PC.
                 // ZJIT doesn't set `cfp->jit_return`, so it's not a reliable signal.
+                // TODO(zjit): lightweight frames potentially makes more than
+                //             the top most frame invalid.
                 //
                 // Avoid passing invalid PC to calc_lineno() to avoid crashing.
                 if (cfp == top && (pc < iseq_encoded || pc > pc_end)) {
@@ -1783,7 +1785,7 @@ thread_profile_frames(rb_execution_context_t *ec, int start, int limit, VALUE *b
             i++;
         }
         else {
-            cme = rb_vm_frame_method_entry(cfp);
+            cme = rb_vm_frame_method_entry_unchecked(cfp);
             if (cme && cme->def->type == VM_METHOD_TYPE_CFUNC) {
                 if (start > 0) {
                     start--;
diff --git a/vm_core.h b/vm_core.h
@@ -1449,12 +1449,25 @@ VM_ENV_FLAGS(const VALUE *ep, long flag)
     return flags & flag;
 }
 
+static inline unsigned long
+VM_ENV_FLAGS_UNCHECKED(const VALUE *ep, long flag)
+{
+    VALUE flags = ep[VM_ENV_DATA_INDEX_FLAGS];
+    return flags & flag;
+}
+
 static inline unsigned long
 VM_FRAME_TYPE(const rb_control_frame_t *cfp)
 {
     return VM_ENV_FLAGS(cfp->ep, VM_FRAME_MAGIC_MASK);
 }
 
+static inline unsigned long
+VM_FRAME_TYPE_UNCHECKED(const rb_control_frame_t *cfp)
+{
+    return VM_ENV_FLAGS_UNCHECKED(cfp->ep, VM_FRAME_MAGIC_MASK);
+}
+
 static inline int
 VM_FRAME_LAMBDA_P(const rb_control_frame_t *cfp)
 {
@@ -1473,6 +1486,12 @@ VM_FRAME_FINISHED_P(const rb_control_frame_t *cfp)
     return VM_ENV_FLAGS(cfp->ep, VM_FRAME_FLAG_FINISH) != 0;
 }
 
+static inline int
+VM_FRAME_FINISHED_P_UNCHECKED(const rb_control_frame_t *cfp)
+{
+    return VM_ENV_FLAGS_UNCHECKED(cfp->ep, VM_FRAME_FLAG_FINISH) != 0;
+}
+
 static inline int
 VM_FRAME_BMETHOD_P(const rb_control_frame_t *cfp)
 {
@@ -1498,12 +1517,24 @@ VM_FRAME_CFRAME_P(const rb_control_frame_t *cfp)
     return cframe_p;
 }
 
+static inline int
+VM_FRAME_CFRAME_P_UNCHECKED(const rb_control_frame_t *cfp)
+{
+    return VM_ENV_FLAGS_UNCHECKED(cfp->ep, VM_FRAME_FLAG_CFRAME) != 0;
+}
+
 static inline int
 VM_FRAME_RUBYFRAME_P(const rb_control_frame_t *cfp)
 {
     return !VM_FRAME_CFRAME_P(cfp);
 }
 
+static inline int
+VM_FRAME_RUBYFRAME_P_UNCHECKED(const rb_control_frame_t *cfp)
+{
+    return !VM_FRAME_CFRAME_P_UNCHECKED(cfp);
+}
+
 static inline int
 VM_FRAME_NS_SWITCH_P(const rb_control_frame_t *cfp)
 {
@@ -1522,11 +1553,23 @@ VM_ENV_LOCAL_P(const VALUE *ep)
     return VM_ENV_FLAGS(ep, VM_ENV_FLAG_LOCAL) ? 1 : 0;
 }
 
+static inline int
+VM_ENV_LOCAL_P_UNCHECKED(const VALUE *ep)
+{
+    return VM_ENV_FLAGS_UNCHECKED(ep, VM_ENV_FLAG_LOCAL) ? 1 : 0;
+}
+
+static inline const VALUE *
+VM_ENV_PREV_EP_UNCHECKED(const VALUE *ep)
+{
+    return GC_GUARDED_PTR_REF(ep[VM_ENV_DATA_INDEX_SPECVAL]);
+}
+
 static inline const VALUE *
 VM_ENV_PREV_EP(const VALUE *ep)
 {
     VM_ASSERT(VM_ENV_LOCAL_P(ep) == 0);
-    return GC_GUARDED_PTR_REF(ep[VM_ENV_DATA_INDEX_SPECVAL]);
+    return VM_ENV_PREV_EP_UNCHECKED(ep);
 }
 
 static inline VALUE
@@ -1934,6 +1977,7 @@ void rb_gc_mark_machine_context(const rb_execution_context_t *ec);
 rb_cref_t *rb_vm_rewrite_cref(rb_cref_t *node, VALUE old_klass, VALUE new_klass);
 
 const rb_callable_method_entry_t *rb_vm_frame_method_entry(const rb_control_frame_t *cfp);
+const rb_callable_method_entry_t *rb_vm_frame_method_entry_unchecked(const rb_control_frame_t *cfp);
 
 #define sysstack_error GET_VM()->special_exceptions[ruby_error_sysstack]
 
diff --git a/vm_dump.c b/vm_dump.c
@@ -61,14 +61,14 @@ control_frame_dump(const rb_execution_context_t *ec, const rb_control_frame_t *c
     const char *magic, *iseq_name = "-", *selfstr = "-", *biseq_name = "-";
     VALUE tmp;
     const rb_iseq_t *iseq = NULL;
-    const rb_callable_method_entry_t *me = rb_vm_frame_method_entry(cfp);
+    const rb_callable_method_entry_t *me = rb_vm_frame_method_entry_unchecked(cfp);
 
     if (ep < 0 || (size_t)ep > ec->vm_stack_size) {
         ep = (ptrdiff_t)cfp->ep;
         ep_in_heap = 'p';
     }
 
-    switch (VM_FRAME_TYPE(cfp)) {
+    switch (VM_FRAME_TYPE_UNCHECKED(cfp)) {
       case VM_FRAME_MAGIC_TOP:
         magic = "TOP";
         break;
@@ -128,7 +128,9 @@ control_frame_dump(const rb_execution_context_t *ec, const rb_control_frame_t *c
                 iseq = cfp->iseq;
                 pc = cfp->pc - ISEQ_BODY(iseq)->iseq_encoded;
                 iseq_name = RSTRING_PTR(ISEQ_BODY(iseq)->location.label);
-                line = rb_vm_get_sourceline(cfp);
+                if (pc >= 0 && pc <= ISEQ_BODY(iseq)->iseq_size) {
+                    line = rb_vm_get_sourceline(cfp);
+                }
                 if (line) {
                     snprintf(posbuf, MAX_POSBUF, "%s:%d", RSTRING_PTR(rb_iseq_path(iseq)), line);
                 }
@@ -138,7 +140,7 @@ control_frame_dump(const rb_execution_context_t *ec, const rb_control_frame_t *c
             }
         }
     }
-    else if (me != NULL) {
+    else if (me != NULL && IMEMO_TYPE_P(me, imemo_ment)) {
         iseq_name = rb_id2name(me->def->original_id);
         snprintf(posbuf, MAX_POSBUF, ":%s", iseq_name);
         line = -1;
@@ -158,7 +160,7 @@ control_frame_dump(const rb_execution_context_t *ec, const rb_control_frame_t *c
     if (line) {
         kprintf(" %s", posbuf);
     }
-    if (VM_FRAME_FINISHED_P(cfp)) {
+    if (VM_FRAME_FINISHED_P_UNCHECKED(cfp)) {
         kprintf(" [FINISH]");
     }
     if (0) {
diff --git a/vm_insnhelper.c b/vm_insnhelper.c
@@ -761,6 +761,25 @@ check_method_entry(VALUE obj, int can_be_svar)
     }
 }
 
+static rb_callable_method_entry_t *
+env_method_entry_unchecked(VALUE obj, int can_be_svar)
+{
+    if (obj == Qfalse) return NULL;
+
+    switch (imemo_type(obj)) {
+      case imemo_ment:
+        return (rb_callable_method_entry_t *)obj;
+      case imemo_cref:
+        return NULL;
+      case imemo_svar:
+        if (can_be_svar) {
+            return env_method_entry_unchecked(((struct vm_svar *)obj)->cref_or_me, FALSE);
+        }
+      default:
+        return NULL;
+    }
+}
+
 const rb_callable_method_entry_t *
 rb_vm_frame_method_entry(const rb_control_frame_t *cfp)
 {
@@ -775,6 +794,20 @@ rb_vm_frame_method_entry(const rb_control_frame_t *cfp)
     return check_method_entry(ep[VM_ENV_DATA_INDEX_ME_CREF], TRUE);
 }
 
+const rb_callable_method_entry_t *
+rb_vm_frame_method_entry_unchecked(const rb_control_frame_t *cfp)
+{
+    const VALUE *ep = cfp->ep;
+    rb_callable_method_entry_t *me;
+
+    while (!VM_ENV_LOCAL_P_UNCHECKED(ep)) {
+        if ((me = env_method_entry_unchecked(ep[VM_ENV_DATA_INDEX_ME_CREF], FALSE)) != NULL) return me;
+        ep = VM_ENV_PREV_EP_UNCHECKED(ep);
+    }
+
+    return env_method_entry_unchecked(ep[VM_ENV_DATA_INDEX_ME_CREF], TRUE);
+}
+
 static const rb_iseq_t *
 method_entry_iseqptr(const rb_callable_method_entry_t *me)
 {
