symbol.c: skip tombstones when migrating to the hash table - #7018

migrate_to_hash_table() dereferences the tombstones symbol GC leaves behind.

Symbol GC sweeps a dynamic symbol by freeing its name buffer and storing NULL in
mrb->symtbl[i] as a tombstone (phase 3 of mrb_symbol_gc()). Every other reader of
symtbl checks for that NULL:

migrate_to_hash_table() does not. It walks the whole table to build the buckets and
untags each entry unconditionally, so the first migration from linear mode to hash mode
after a sweep reads through a null pointer.

Reachability

Migration fires at mrb->symidx >= MRB_SYMBOL_LINEAR_THRESHOLD (256 by default), symbol
GC at mrb->dynamic_sym_count >= MRB_SYMBOL_MAX (4096 by default). symidx never
shrinks and symidx >= dynamic_sym_count always holds, so on a default build the table
has been in hash mode for a long time before the first sweep and migrate_to_hash_table()
is never reached again.

The bug is therefore reachable only when MRB_SYMBOL_MAX < MRB_SYMBOL_LINEAR_THRESHOLD,
which is a configuration a memory constrained target would plausibly pick: a small symbol
budget is exactly the reason to set MRB_SYMBOL_MAX at all. With MRB_SYMBOL_MAX=16:

$ ./build/host/bin/mruby -e '2000.times { |i| "filler-symbol-name-#{i}".to_sym }'

src/debug.c:91:22: runtime error: load of null pointer of type 'const uint8_t'
AddressSanitizer: SEGV on unknown address 0x000000000000
    #0 mrb_packed_int_decode      src/debug.c:91
    #1 migrate_to_hash_table      src/symbol.c:309
    #2 sym_intern                 src/symbol.c:439
    #3 mrb_intern                 src/symbol.c:464
    #4 mrb_intern_str             src/symbol.c:511
    #5 mrb_str_intern             src/string.c:2241

The failure is deterministic, not allocation dependent. Building the same tree with a
larger budget makes it go away:

Fix

Skip tombstones the way the rebuild in mrb_symbol_gc() already does. ht->symlink and
ht->buckets come from mrb_calloc(), so a skipped index keeps the zero that means
"no chain", which is the same invariant that rebuild relies on.

No test is added: the crash needs a non default MRB_SYMBOL_MAX, so it cannot be
expressed in test/t/.

Verification

Reproduced on master with a clang address,undefined build:

MRuby::Build.new('symmax16') do |conf|
  conf.toolchain :clang
  conf.gembox 'default'
  conf.cc.defines << 'MRB_SYMBOL_MAX=16'
  conf.enable_sanitizer "address,undefined"
  conf.enable_debug
end

With the patch applied the same build runs the snippet to completion, exit 0, with no
UBSan report. rake test on a plain host build passes (1942 and 105 assertions, 0 KO,
0 crash).

Summary by CodeRabbit

• Bug Fixes
  • Improved symbol-table rebuilding by safely skipping removed entries, preventing invalid or stale symbols from being restored.