From 9a0e857c3590412b5de6d029966d0aa67344c450 Mon Sep 17 00:00:00 2001 From: Satoshi Tagomori Date: Tue, 7 Oct 2025 18:36:08 +0900 Subject: [PATCH 1/7] Stop displaying current namespace when it crashed To avoid crashes during displaying crash reports. --- internal/namespace.h | 2 ++ namespace.c | 9 +++++++++ vm.c | 37 +++++++++++++++++++++++-------------- vm_dump.c | 27 ++++++++++++++++++++------- 4 files changed, 54 insertions(+), 21 deletions(-) diff --git a/internal/namespace.h b/internal/namespace.h index 9ffc9a5c8be8bb..c5a495fdf003ba 100644 --- a/internal/namespace.h +++ b/internal/namespace.h @@ -54,6 +54,7 @@ typedef struct rb_namespace_struct rb_namespace_t; RUBY_EXTERN bool ruby_namespace_enabled; RUBY_EXTERN bool ruby_namespace_init_done; +RUBY_EXTERN bool ruby_namespace_crashed; static inline bool rb_namespace_available(void) @@ -65,6 +66,7 @@ const rb_namespace_t * rb_root_namespace(void); const rb_namespace_t * rb_main_namespace(void); const rb_namespace_t * rb_current_namespace(void); const rb_namespace_t * rb_loading_namespace(void); +const rb_namespace_t * rb_current_namespace_in_crash_report(void); void rb_namespace_entry_mark(void *); void rb_namespace_gc_update_references(void *ptr); diff --git a/namespace.c b/namespace.c index 32249d00bc161b..059642dd94c451 100644 --- a/namespace.c +++ b/namespace.c @@ -50,6 +50,7 @@ static bool tmp_dir_has_dirsep; bool ruby_namespace_enabled = false; // extern bool ruby_namespace_init_done = false; // extern +bool ruby_namespace_crashed = false; // extern, changed only in vm.c VALUE rb_resolve_feature_path(VALUE klass, VALUE fname); static VALUE rb_namespace_inspect(VALUE obj); @@ -97,6 +98,14 @@ rb_loading_namespace(void) return rb_vm_loading_namespace(GET_EC()); } +const rb_namespace_t * +rb_current_namespace_in_crash_report(void) +{ + if (ruby_namespace_crashed) + return NULL; + return rb_current_namespace(); +} + static long namespace_id_counter = 0; static long diff --git a/vm.c b/vm.c index c9bf3dce36d76c..76c6f869eb86ae 100644 --- a/vm.c +++ b/vm.c @@ -95,6 +95,16 @@ rb_vm_search_cf_from_ep(const rb_execution_context_t *ec, const rb_control_frame } } +#if VM_CHECK_MODE > 0 +// ruby_namespace_crashed defined in internal/namespace.h +#define VM_NAMESPACE_CRASHED() {ruby_namespace_crashed = true;} +#define VM_NAMESPACE_ASSERT(expr, msg) \ + if (!(expr)) { ruby_namespace_crashed = true; rb_bug(msg); } +#else +#define VM_NAMESPACE_CRASHED() {} +#define VM_NAMESPACE_ASSERT(expr, msg) ((void)0) +#endif + static const VALUE * VM_EP_RUBY_LEP(const rb_execution_context_t *ec, const rb_control_frame_t *current_cfp) { @@ -109,23 +119,22 @@ VM_EP_RUBY_LEP(const rb_execution_context_t *ec, const rb_control_frame_t *curre while (VM_ENV_FLAGS(ep, VM_FRAME_FLAG_CFRAME) != 0) { if (!cfp) { cfp = rb_vm_search_cf_from_ep(ec, checkpoint_cfp, ep); - VM_ASSERT(cfp, "rb_vm_search_cf_from_ep should return a valid cfp for the ep"); - VM_ASSERT(cfp->ep == ep); + VM_NAMESPACE_ASSERT(cfp, "rb_vm_search_cf_from_ep should return a valid cfp for the ep, but NULL"); + VM_NAMESPACE_ASSERT(cfp->ep == ep, "rb_vm_search_cf_from_ep returns an unmatched cfp"); } if (!cfp) { return NULL; } - VM_ASSERT(cfp->ep); - VM_ASSERT(cfp->ep == ep); + VM_NAMESPACE_ASSERT(cfp->ep, "cfp->ep == NULL"); + VM_NAMESPACE_ASSERT(cfp->ep == ep, "cfp->ep != ep"); + + VM_NAMESPACE_ASSERT(!VM_FRAME_FINISHED_P(cfp), "CFUNC frame should not FINISHED"); - if (VM_FRAME_FINISHED_P(cfp)) { - rb_bug("CFUNC frame should not FINISHED"); - } cfp = RUBY_VM_PREVIOUS_CONTROL_FRAME(cfp); if (cfp >= eocfp) { return NULL; } - VM_ASSERT(cfp, "CFUNC should have a valid previous control frame"); + VM_NAMESPACE_ASSERT(cfp, "CFUNC should have a valid previous control frame"); ep = cfp->ep; if (!ep) { return NULL; @@ -3061,17 +3070,17 @@ current_namespace_on_cfp(const rb_execution_context_t *ec, const rb_control_fram rb_callable_method_entry_t *cme; const rb_namespace_t *ns; const VALUE *lep = VM_EP_RUBY_LEP(ec, cfp); - VM_ASSERT(lep); - VM_ASSERT(rb_namespace_available()); + VM_NAMESPACE_ASSERT(lep, "lep should be valid"); + VM_NAMESPACE_ASSERT(rb_namespace_available(), "namespace should be available here"); if (VM_ENV_FRAME_TYPE_P(lep, VM_FRAME_MAGIC_METHOD)) { cme = check_method_entry(lep[VM_ENV_DATA_INDEX_ME_CREF], TRUE); - VM_ASSERT(cme); - VM_ASSERT(cme->def); + VM_NAMESPACE_ASSERT(cme, "cme should be valid"); + VM_NAMESPACE_ASSERT(cme->def, "cme->def shold be valid"); return cme->def->ns; } else if (VM_ENV_FRAME_TYPE_P(lep, VM_FRAME_MAGIC_TOP) || VM_ENV_FRAME_TYPE_P(lep, VM_FRAME_MAGIC_CLASS)) { - VM_ASSERT(VM_ENV_LOCAL_P(lep)); + VM_NAMESPACE_ASSERT(VM_ENV_LOCAL_P(lep), "lep should be local on MAGIC_TOP or MAGIC_CLASS frames"); return VM_ENV_NAMESPACE(lep); } else if (VM_ENV_FRAME_TYPE_P(lep, VM_FRAME_MAGIC_DUMMY)) { @@ -3083,6 +3092,7 @@ current_namespace_on_cfp(const rb_execution_context_t *ec, const rb_control_fram return rb_root_namespace(); } else { + VM_NAMESPACE_CRASHED(); rb_bug("BUG: Local ep without cme/namespace, flags: %08lX", (unsigned long)lep[VM_ENV_DATA_INDEX_FLAGS]); } UNREACHABLE_RETURN(0); @@ -3091,7 +3101,6 @@ current_namespace_on_cfp(const rb_execution_context_t *ec, const rb_control_fram const rb_namespace_t * rb_vm_current_namespace(const rb_execution_context_t *ec) { - VM_ASSERT(rb_namespace_available()); return current_namespace_on_cfp(ec, ec->cfp); } diff --git a/vm_dump.c b/vm_dump.c index 03f573a516fe61..f5aaaeeabf37c4 100644 --- a/vm_dump.c +++ b/vm_dump.c @@ -1150,8 +1150,16 @@ rb_vm_bugreport(const void *ctx, FILE *errout) enum {other_runtime_info = 0}; #endif const rb_vm_t *const vm = GET_VM(); - const rb_namespace_t *ns = rb_current_namespace(); + const rb_namespace_t *current_ns = rb_current_namespace_in_crash_report(); const rb_execution_context_t *ec = rb_current_execution_context(false); + VALUE loaded_features; + + if (current_ns) { + loaded_features = current_ns->loaded_features; + } + else { + loaded_features = rb_root_namespace()->loaded_features; + } if (vm && ec) { rb_vmdebug_stack_dump_raw(ec, ec->cfp, errout); @@ -1201,17 +1209,22 @@ rb_vm_bugreport(const void *ctx, FILE *errout) } if (rb_namespace_available()) { kprintf("* Namespace: enabled\n"); - kprintf("* Current namespace id: %ld, type: %s\n", - ns->ns_id, - NAMESPACE_USER_P(ns) ? (NAMESPACE_MAIN_P(ns) ? "main" : "user") : "root"); + if (current_ns) { + kprintf("* Current namespace id: %ld, type: %s\n", + current_ns->ns_id, + NAMESPACE_USER_P(current_ns) ? (NAMESPACE_MAIN_P(current_ns) ? "main" : "user") : "root"); + } + else { + kprintf("* Current namespace: NULL (crashed)\n"); + } } else { kprintf("* Namespace: disabled\n"); } - if (ns->loaded_features) { + if (loaded_features) { kprintf("* Loaded features:\n\n"); - for (i=0; iloaded_features); i++) { - name = RARRAY_AREF(ns->loaded_features, i); + for (i=0; i Date: Tue, 7 Oct 2025 21:04:08 +0900 Subject: [PATCH 2/7] Add a control frame column "n:xxxx" as namespace id in crash reports --- vm.c | 4 ++-- vm_core.h | 6 ++++++ vm_dump.c | 12 ++++++++++++ 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/vm.c b/vm.c index 76c6f869eb86ae..06e85a1d1a48e2 100644 --- a/vm.c +++ b/vm.c @@ -119,8 +119,8 @@ VM_EP_RUBY_LEP(const rb_execution_context_t *ec, const rb_control_frame_t *curre while (VM_ENV_FLAGS(ep, VM_FRAME_FLAG_CFRAME) != 0) { if (!cfp) { cfp = rb_vm_search_cf_from_ep(ec, checkpoint_cfp, ep); - VM_NAMESPACE_ASSERT(cfp, "rb_vm_search_cf_from_ep should return a valid cfp for the ep, but NULL"); - VM_NAMESPACE_ASSERT(cfp->ep == ep, "rb_vm_search_cf_from_ep returns an unmatched cfp"); + VM_NAMESPACE_ASSERT(cfp, "Failed to search cfp from ep"); + VM_NAMESPACE_ASSERT(cfp->ep == ep, "Searched cfp's ep is not equal to ep"); } if (!cfp) { return NULL; diff --git a/vm_core.h b/vm_core.h index da0249e567977d..5068e200575c18 100644 --- a/vm_core.h +++ b/vm_core.h @@ -1585,6 +1585,12 @@ VM_ENV_NAMESPACE(const VALUE *ep) return (const rb_namespace_t *)GC_GUARDED_PTR_REF(ep[VM_ENV_DATA_INDEX_SPECVAL]); } +static inline const rb_namespace_t * +VM_ENV_NAMESPACE_UNCHECKED(const VALUE *ep) +{ + return (const rb_namespace_t *)GC_GUARDED_PTR_REF(ep[VM_ENV_DATA_INDEX_SPECVAL]); +} + #if VM_CHECK_MODE > 0 int rb_vm_ep_in_heap_p(const VALUE *ep); #endif diff --git a/vm_dump.c b/vm_dump.c index f5aaaeeabf37c4..460a0f7b8ccd82 100644 --- a/vm_dump.c +++ b/vm_dump.c @@ -62,6 +62,7 @@ control_frame_dump(const rb_execution_context_t *ec, const rb_control_frame_t *c VALUE tmp; const rb_iseq_t *iseq = NULL; const rb_callable_method_entry_t *me = rb_vm_frame_method_entry_unchecked(cfp); + const rb_namespace_t *ns = NULL; if (ep < 0 || (size_t)ep > ec->vm_stack_size) { ep = (ptrdiff_t)cfp->ep; @@ -71,12 +72,17 @@ control_frame_dump(const rb_execution_context_t *ec, const rb_control_frame_t *c switch (VM_FRAME_TYPE_UNCHECKED(cfp)) { case VM_FRAME_MAGIC_TOP: magic = "TOP"; + ns = VM_ENV_NAMESPACE_UNCHECKED(cfp->ep); break; case VM_FRAME_MAGIC_METHOD: magic = "METHOD"; + if (me) { + ns = me->def->ns; + } break; case VM_FRAME_MAGIC_CLASS: magic = "CLASS"; + ns = VM_ENV_NAMESPACE_UNCHECKED(cfp->ep); break; case VM_FRAME_MAGIC_BLOCK: magic = "BLOCK"; @@ -156,6 +162,12 @@ control_frame_dump(const rb_execution_context_t *ec, const rb_control_frame_t *c } kprintf("s:%04"PRIdPTRDIFF" ", cfp->sp - ec->vm_stack); kprintf(ep_in_heap == ' ' ? "e:%06"PRIdPTRDIFF" " : "E:%06"PRIxPTRDIFF" ", ep % 10000); + if (ns) { + kprintf("n:%04ld ", ns->ns_id % 10000); + } + else { + kprintf("n:---- "); + } kprintf("%-6s", magic); if (line) { kprintf(" %s", posbuf); From a6938eb46a10021642213b98d648544129a7dbb3 Mon Sep 17 00:00:00 2001 From: Nobuyoshi Nakada Date: Tue, 7 Oct 2025 22:36:32 +0900 Subject: [PATCH 3/7] Skip files that are "deleted by us" "Deleted" means that file is only for the upstream but not for ruby. --- tool/sync_default_gems.rb | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/tool/sync_default_gems.rb b/tool/sync_default_gems.rb index 366f64ddfcec9b..1f6ae06b61f7f9 100755 --- a/tool/sync_default_gems.rb +++ b/tool/sync_default_gems.rb @@ -83,6 +83,10 @@ def pipe_readlines(args, rs: "\0", chomp: true) end end + def porcelain_status(*pattern) + pipe_readlines(%W"git status --porcelain -z --" + pattern) + end + def replace_rdoc_ref(file) src = File.binread(file) changed = false @@ -101,7 +105,7 @@ def replace_rdoc_ref(file) end def replace_rdoc_ref_all - result = pipe_readlines(%W"git status --porcelain -z -- *.c *.rb *.rdoc") + result = porcelain_status("*.c", "*.rb", "*.rdoc") result.map! {|line| line[/\A.M (.*)/, 1]} result.compact! return if result.empty? @@ -489,14 +493,16 @@ def commits_in_ranges(gem, repo, default_branch, ranges) def resolve_conflicts(gem, sha, edit) # Skip this commit if everything has been removed as `ignored_paths`. - changes = pipe_readlines(%W"git status --porcelain -z") + changes = porcelain_status() if changes.empty? puts "Skip empty commit #{sha}" return false end - # We want to skip DD: deleted by both. - deleted = changes.grep(/^DD /) {$'} + # We want to skip + # DD: deleted by both + # DU: deleted by us + deleted = changes.grep(/^D[DU] /) {$'} system(*%W"git rm -f --", *deleted) unless deleted.empty? # Import UA: added by them @@ -505,10 +511,9 @@ def resolve_conflicts(gem, sha, edit) # Discover unmerged files # AU: unmerged, added by us - # DU: unmerged, deleted by us # UU: unmerged, both modified # AA: unmerged, both added - conflict = changes.grep(/\A(?:.U|AA) /) {$'} + conflict = changes.grep(/\A(?:A[AU]|UU) /) {$'} # If -e option is given, open each conflicted file with an editor unless conflict.empty? if edit @@ -624,6 +629,8 @@ def pickup_commit(gem, sha, edit) # Commit cherry-picked commit if picked system(*%w"git commit --amend --no-edit") + elsif porcelain_status().empty? + system(*%w"git cherry-pick --skip") else system(*%w"git cherry-pick --continue --no-edit") end or return nil From 78dbc6c0b793805bb0d10e7d6d2fe8a5c0069a64 Mon Sep 17 00:00:00 2001 From: Nobuyoshi Nakada Date: Tue, 7 Oct 2025 23:51:00 +0900 Subject: [PATCH 4/7] Shorten timeout for csv It usually ends in a few seconds, and less than 10 seconds even on Windows. But recently it stalls 10 minutes and times out. --- tool/test-bundled-gems.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tool/test-bundled-gems.rb b/tool/test-bundled-gems.rb index dcf4d6fdf47062..86bb747d946320 100644 --- a/tool/test-bundled-gems.rb +++ b/tool/test-bundled-gems.rb @@ -73,6 +73,9 @@ when "test-unit" test_command = [ruby, "-C", "#{gem_dir}/src/#{gem}", "test/run.rb"] + when "csv" + first_timeout = 30 + when "win32ole" next unless /mswin|mingw/ =~ RUBY_PLATFORM From 40d1603e549b12808ed7f94064014658b91660e0 Mon Sep 17 00:00:00 2001 From: Nobuyoshi Nakada Date: Wed, 8 Oct 2025 00:03:48 +0900 Subject: [PATCH 5/7] [ruby/io-console] Skip emptied commits https://github.com/ruby/io-console/commit/431c3f3369 --- tool/sync_default_gems.rb | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tool/sync_default_gems.rb b/tool/sync_default_gems.rb index 1f6ae06b61f7f9..e02a2343b96f10 100755 --- a/tool/sync_default_gems.rb +++ b/tool/sync_default_gems.rb @@ -626,11 +626,14 @@ def pickup_commit(gem, sha, edit) return picked || nil # Fail unless cherry-picked end + if porcelain_status().empty? + system(*%w"git cherry-pick --skip") + return true + end + # Commit cherry-picked commit if picked system(*%w"git commit --amend --no-edit") - elsif porcelain_status().empty? - system(*%w"git cherry-pick --skip") else system(*%w"git cherry-pick --continue --no-edit") end or return nil From c951e1c4e058c3525498d227039d32c98e11062c Mon Sep 17 00:00:00 2001 From: Nobuyoshi Nakada Date: Wed, 8 Oct 2025 00:09:44 +0900 Subject: [PATCH 6/7] Return false to skip emptied commits --- tool/sync_default_gems.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tool/sync_default_gems.rb b/tool/sync_default_gems.rb index e02a2343b96f10..64ed89145123d3 100755 --- a/tool/sync_default_gems.rb +++ b/tool/sync_default_gems.rb @@ -628,7 +628,7 @@ def pickup_commit(gem, sha, edit) if porcelain_status().empty? system(*%w"git cherry-pick --skip") - return true + return false end # Commit cherry-picked commit From 7089a4e2d83a3cb1bc394c4ce3638cbc777f4cb9 Mon Sep 17 00:00:00 2001 From: Nobuyoshi Nakada Date: Wed, 8 Oct 2025 00:50:00 +0900 Subject: [PATCH 7/7] Fix not to skip necessary commits --- tool/sync_default_gems.rb | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/tool/sync_default_gems.rb b/tool/sync_default_gems.rb index 64ed89145123d3..f404b7ff5061de 100755 --- a/tool/sync_default_gems.rb +++ b/tool/sync_default_gems.rb @@ -626,14 +626,12 @@ def pickup_commit(gem, sha, edit) return picked || nil # Fail unless cherry-picked end - if porcelain_status().empty? - system(*%w"git cherry-pick --skip") - return false - end - # Commit cherry-picked commit if picked system(*%w"git commit --amend --no-edit") + elsif porcelain_status().empty? + system(*%w"git cherry-pick --skip") + return false else system(*%w"git cherry-pick --continue --no-edit") end or return nil