summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAlan Wu <XrXr@users.noreply.github.com>2021-05-21 12:13:55 -0400
committerGitHub <noreply@github.com>2021-05-21 12:13:55 -0400
commitb2fc592c3046e60fdfbb5692d52cc7cbf814b6d0 (patch)
treef38d97fe181933560c58be574b8561cc48d602c6
parent0772f1b583cedebddb493a8eb542243f6d2437d8 (diff)
Build CDHASH properly when loading iseq from binary
Before this change, CDHASH operands were built as plain hashes when loaded from binary. Without setting up the hash with the correct st_table type, the hash can sometimes be an ar_table. When the hash is an ar_table, lookups can call the `eql?` method on keys of the hash, which makes the `opt_case_dispatch` instruction not "leaf" as it implicitly declares. The following script trips the stack canary for checking the leaf attribute for `opt_case_dispatch` on VM_CHECK_MODE > 0 (enabled by default with RUBY_DEBUG). rb_vm_iseq = RubyVM::InstructionSequence iseq = rb_vm_iseq.compile(<<-EOF) case Class.new(String).new("foo") when "foo" 42 end EOF puts rb_vm_iseq.load_from_binary(iseq.to_binary).eval This commit changes the binary loading logic to build CDHASH with the right st_table type. The dumping logic and the dump format stays the same
Notes
Notes: Merged: https://github.com/ruby/ruby/pull/4511 Merged-By: XrXr
-rw-r--r--compile.c15
-rw-r--r--test/ruby/test_iseq.rb13
2 files changed, 27 insertions, 1 deletions
diff --git a/compile.c b/compile.c
index d2de521ec8..44fb53cefd 100644
--- a/compile.c
+++ b/compile.c
@@ -10761,7 +10761,6 @@ ibf_load_code(const struct ibf_load *load, rb_iseq_t *iseq, ibf_offset_t bytecod
/* operands */
for (op_index=0; types[op_index]; op_index++, code_index++) {
switch (types[op_index]) {
- case TS_CDHASH:
case TS_VALUE:
{
VALUE op = ibf_load_small_value(load, &reading_pos);
@@ -10773,6 +10772,20 @@ ibf_load_code(const struct ibf_load *load, rb_iseq_t *iseq, ibf_offset_t bytecod
}
break;
}
+ case TS_CDHASH:
+ {
+ VALUE op = ibf_load_small_value(load, &reading_pos);
+ VALUE v = ibf_load_object(load, op);
+ v = rb_hash_dup(v); // hash dumped as frozen
+ RHASH_TBL_RAW(v)->type = &cdhash_type;
+ rb_hash_rehash(v); // hash function changed
+ freeze_hide_obj(v);
+
+ code[code_index] = v;
+ RB_OBJ_WRITTEN(iseqv, Qundef, v);
+ FL_SET(iseqv, ISEQ_MARKABLE_ISEQ);
+ break;
+ }
case TS_ISEQ:
{
VALUE op = (VALUE)ibf_load_small_value(load, &reading_pos);
diff --git a/test/ruby/test_iseq.rb b/test/ruby/test_iseq.rb
index e3ca1ba926..0cde1fbb7a 100644
--- a/test/ruby/test_iseq.rb
+++ b/test/ruby/test_iseq.rb
@@ -82,6 +82,19 @@ class TestISeq < Test::Unit::TestCase
end;
end if defined?(RubyVM::InstructionSequence.load)
+ def test_cdhash_after_roundtrip
+ # CDHASH was not built properly when loading from binary and
+ # was causing opt_case_dispatch to clobber its stack canary
+ # for its "leaf" instruction attribute.
+ iseq = compile(<<~EOF)
+ case Class.new(String).new("foo")
+ when "foo"
+ 42
+ end
+ EOF
+ assert_equal(42, ISeq.load_from_binary(iseq.to_binary).eval)
+ end
+
def test_disasm_encoding
src = "\u{3042} = 1; \u{3042}; \u{3043}"
asm = compile(src).disasm