Always use assert-free APIs when profiling and crashing

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 06b7a70837) +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 06b7a70837) +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.
This commit is contained in:
Alan Wu 2025-09-25 18:30:12 -04:00 committed by GitHub
parent 62430c19c9
commit 1a52c42e61
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 90 additions and 9 deletions

View File

@ -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--;

View File

@ -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]

View File

@ -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) {

View File

@ -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)
{