summaryrefslogtreecommitdiff
path: root/vm_insnhelper.c
diff options
context:
space:
mode:
authorYusuke Endoh <mame@ruby-lang.org>2022-10-12 19:02:11 +0900
committerYusuke Endoh <mame@ruby-lang.org>2022-10-12 20:28:24 +0900
commit7a9f865a1d855109c7990b5fee21f92cc951ce60 (patch)
tree80904cc6b1895409ec3a395b68aadbbf7fb19600 /vm_insnhelper.c
parent66a650ec41a583ec58003737b23b42ec8f069920 (diff)
Do not read cached_id from callcache on stack
The inline cache is initialized by vm_cc_attr_index_set only when vm_cc_markable(cc). However, vm_getivar attempted to read the cache even if the cc is not vm_cc_markable. This caused a condition that depends on uninitialized value. Here is an output of valgrind: ``` ==10483== Conditional jump or move depends on uninitialised value(s) ==10483== at 0x4C1D60: vm_getivar (vm_insnhelper.c:1171) ==10483== by 0x4C1D60: vm_call_ivar (vm_insnhelper.c:3257) ==10483== by 0x4E8E48: vm_call_symbol (vm_insnhelper.c:3481) ==10483== by 0x4EAD8C: vm_sendish (vm_insnhelper.c:5035) ==10483== by 0x4C62B2: vm_exec_core (insns.def:820) ==10483== by 0x4DD519: rb_vm_exec (vm.c:0) ==10483== by 0x4F00B3: invoke_block (vm.c:1417) ==10483== by 0x4F00B3: invoke_iseq_block_from_c (vm.c:1473) ==10483== by 0x4F00B3: invoke_block_from_c_bh (vm.c:1491) ==10483== by 0x4D42B6: rb_yield (vm_eval.c:0) ==10483== by 0x259128: rb_ary_each (array.c:2733) ==10483== by 0x4E8730: vm_call_cfunc_with_frame (vm_insnhelper.c:3227) ==10483== by 0x4EAD8C: vm_sendish (vm_insnhelper.c:5035) ==10483== by 0x4C6254: vm_exec_core (insns.def:801) ==10483== by 0x4DD519: rb_vm_exec (vm.c:0) ==10483== ``` In fact, the CI on FreeBSD 12 started failing since ad63b668e22e21c352b852f3119ae98a7acf99f1. ``` gmake[1]: Entering directory '/usr/home/chkbuild/chkbuild/tmp/build/20221011T163003Z/ruby' /usr/home/chkbuild/chkbuild/tmp/build/20221011T163003Z/ruby/lib/optparse.rb:924:in `complete': undefined method `complete' for nil:NilClass (NoMethodError) from /usr/home/chkbuild/chkbuild/tmp/build/20221011T163003Z/ruby/lib/optparse.rb:1816:in `block in visit' from /usr/home/chkbuild/chkbuild/tmp/build/20221011T163003Z/ruby/lib/optparse.rb:1815:in `reverse_each' from /usr/home/chkbuild/chkbuild/tmp/build/20221011T163003Z/ruby/lib/optparse.rb:1815:in `visit' from /usr/home/chkbuild/chkbuild/tmp/build/20221011T163003Z/ruby/lib/optparse.rb:1847:in `block in complete' from /usr/home/chkbuild/chkbuild/tmp/build/20221011T163003Z/ruby/lib/optparse.rb:1846:in `catch' from /usr/home/chkbuild/chkbuild/tmp/build/20221011T163003Z/ruby/lib/optparse.rb:1846:in `complete' from /usr/home/chkbuild/chkbuild/tmp/build/20221011T163003Z/ruby/lib/optparse.rb:1640:in `block in parse_in_order' from /usr/home/chkbuild/chkbuild/tmp/build/20221011T163003Z/ruby/lib/optparse.rb:1632:in `catch' from /usr/home/chkbuild/chkbuild/tmp/build/20221011T163003Z/ruby/lib/optparse.rb:1632:in `parse_in_order' from /usr/home/chkbuild/chkbuild/tmp/build/20221011T163003Z/ruby/lib/optparse.rb:1626:in `order!' from /usr/home/chkbuild/chkbuild/tmp/build/20221011T163003Z/ruby/lib/optparse.rb:1732:in `permute!' from /usr/home/chkbuild/chkbuild/tmp/build/20221011T163003Z/ruby/lib/optparse.rb:1757:in `parse!' from ./ext/extmk.rb:359:in `parse_args' from ./ext/extmk.rb:396:in `<main>' ``` This change adds a guard to read the cache only when vm_cc_markable(cc). It might be better to initialize the cache as INVALID_SHAPE_ID when the cc is not vm_cc_markable.
Notes
Notes: Merged: https://github.com/ruby/ruby/pull/6530
Diffstat (limited to 'vm_insnhelper.c')
-rw-r--r--vm_insnhelper.c8
1 files changed, 7 insertions, 1 deletions
diff --git a/vm_insnhelper.c b/vm_insnhelper.c
index 2b4eda775a..9b9220a4e3 100644
--- a/vm_insnhelper.c
+++ b/vm_insnhelper.c
@@ -1160,7 +1160,13 @@ vm_getivar(VALUE obj, ID id, const rb_iseq_t *iseq, IVC ic, const struct rb_call
attr_index_t index;
if (is_attr) {
- vm_cc_atomic_shape_and_index(cc, &cached_id, &index);
+ if (vm_cc_markable(cc)) {
+ vm_cc_atomic_shape_and_index(cc, &cached_id, &index);
+ }
+ else {
+ cached_id = INVALID_SHAPE_ID;
+ index = ATTR_INDEX_NOT_SET;
+ }
}
else {
vm_ic_atomic_shape_and_index(ic, &cached_id, &index);