Skip to content

Commit acb2915

Browse files
committed
ZJIT: fix gen_new_hash to use proper shape_id in flags
If the shape capacity isn't set on an object, one thing that fails is moving it across Ractors. The new object created in the dst Ractor needs to be big enough to embed the contents if the src was embedded.
1 parent e0314f0 commit acb2915

5 files changed

Lines changed: 46 additions & 11 deletions

File tree

hash.c

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
#include "internal/class.h"
3333
#include "internal/cont.h"
3434
#include "internal/error.h"
35+
#include "internal/gc.h"
3536
#include "internal/hash.h"
3637
#include "internal/object.h"
3738
#include "internal/proc.h"
@@ -44,6 +45,7 @@
4445
#include "ruby/st.h"
4546
#include "ruby/util.h"
4647
#include "ruby_assert.h"
48+
#include "shape.h"
4749
#include "symbol.h"
4850
#include "ruby/thread_native.h"
4951
#include "ruby/ractor.h"
@@ -1463,9 +1465,14 @@ hash_alloc(VALUE klass)
14631465

14641466
#if USE_ZJIT
14651467
size_t
1466-
rb_zjit_hash_new_size(void)
1467-
{
1468-
return hash_slot_size(sizeof(st_table) > sizeof(ar_table));
1468+
rb_zjit_hash_new_size(VALUE *flags_out)
1469+
{
1470+
size_t size = hash_slot_size(sizeof(st_table) > sizeof(ar_table));
1471+
// mimic rb_newobj()
1472+
shape_id_t shape_id = rb_shape_transition_slot_size(ROOT_SHAPE_ID | SHAPE_ID_LAYOUT_OTHER,
1473+
rb_gc_size_slot_size(size));
1474+
*flags_out = T_HASH | ((VALUE)shape_id << SHAPE_FLAG_SHIFT);
1475+
return size;
14691476
}
14701477
#endif
14711478

zjit.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ void rb_zjit_invalidate_root_box(void);
9393
void rb_zjit_jit_frame_update_references(zjit_jit_frame_t *jit_frame);
9494
void rb_zjit_materialize_frames(const rb_execution_context_t *ec, rb_control_frame_t *cfp);
9595
void rb_zjit_materialize_frames_for_longjmp(const rb_execution_context_t *ec, rb_control_frame_t *cfp);
96-
size_t rb_zjit_hash_new_size(void);
96+
size_t rb_zjit_hash_new_size(VALUE *flags_out);
9797
bool rb_zjit_class_allocate_instance_fastpath(VALUE klass, size_t *size_out, shape_id_t *shape_id_out);
9898
bool rb_zjit_str_resurrect_fastpath(VALUE str, bool chilled, size_t *size_out, VALUE *flags_out, long *len_out, size_t *byte_size_out);
9999
bool rb_zjit_array_dup_can_fastpath(VALUE ary, size_t *alloc_size_out, VALUE *flags_out, long *len_out);

zjit/src/codegen.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2475,11 +2475,11 @@ fn gen_new_hash(
24752475
if elements.is_empty() {
24762476
gen_prepare_leaf_call_with_gc(asm, state);
24772477

2478-
let alloc_size = unsafe { rb_zjit_hash_new_size() };
2479-
let flags = RUBY_T_HASH as u64;
2478+
let mut flags = VALUE(0);
2479+
let alloc_size = unsafe { rb_zjit_hash_new_size(&mut flags) };
24802480
let klass = unsafe { rb_cHash };
24812481

2482-
gc_fastpath::gc_fastpath_new_obj(jit, asm, alloc_size, flags, klass,
2482+
gc_fastpath::gc_fastpath_new_obj(jit, asm, alloc_size, flags.into(), klass,
24832483
|asm, hash| {
24842484
asm.store(Opnd::mem(VALUE_BITS, hash, RUBY_OFFSET_RHASH_IFNONE), Qnil.into());
24852485
},
@@ -2494,11 +2494,11 @@ fn gen_new_hash(
24942494

24952495
let num_pairs = elements.len() / 2;
24962496
let hash = if num_pairs <= RUBY_RHASH_AR_TABLE_MAX_SIZE as usize {
2497-
let alloc_size = unsafe { rb_zjit_hash_new_size() };
2498-
let flags = RUBY_T_HASH as u64;
2497+
let mut flags = VALUE(0);
2498+
let alloc_size = unsafe { rb_zjit_hash_new_size(&mut flags) };
24992499
let klass = unsafe { rb_cHash };
25002500

2501-
gc_fastpath::gc_fastpath_new_obj(jit, asm, alloc_size, flags, klass,
2501+
gc_fastpath::gc_fastpath_new_obj(jit, asm, alloc_size, flags.into(), klass,
25022502
|asm, hash| {
25032503
asm.store(Opnd::mem(VALUE_BITS, hash, RUBY_OFFSET_RHASH_IFNONE), Qnil.into());
25042504
},

zjit/src/codegen_tests.rs

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3351,6 +3351,34 @@ fn test_new_hash_dynamic_sym_keys_gc_stress() {
33513351
"#), @r#"[Hash, 2, [3], [3]]"#);
33523352
}
33533353

3354+
// The NewHash inline-alloc fast path must bake the slot-size shape_id into the
3355+
// object flags. Without it, a cross-ractor move sizes the destination object
3356+
// from a zero shape_id, so the moved hash is allocated too small and its keys
3357+
// are corrupted.
3358+
#[test]
3359+
fn test_new_hash_sym_keys_ractor_move() {
3360+
eval("
3361+
def create_hash
3362+
{ an_object: Array.new, hi: true, bonjour: true }
3363+
end
3364+
");
3365+
assert_contains_opcode("create_hash", YARVINSN_newhash);
3366+
assert_snapshot!(inspect("
3367+
r = Ractor.new do
3368+
h = receive
3369+
30.times { |i| h[i] = true }
3370+
h.keys.delete_if { |k| Integer === k }
3371+
end
3372+
3373+
create_hash
3374+
create_hash
3375+
3376+
h = create_hash
3377+
r.send(h, move: true)
3378+
r.value
3379+
"), @"[:an_object, :hi, :bonjour]");
3380+
}
3381+
33543382
#[test]
33553383
fn test_object_alloc_gc_stress() {
33563384
eval("

zjit/src/cruby_bindings.inc.rs

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)