Skip to content

Commit f7b53a7

Browse files
committed
Do not emit shape transition warnings when YJIT is compiling
[Bug #20522] If `Warning.warn` is redefined in Ruby, emitting a warning would invoke Ruby code, which can't safely be done when YJIT is compiling.
1 parent 8627225 commit f7b53a7

File tree

6 files changed

+57
-6
lines changed

6 files changed

+57
-6
lines changed

bootstraptest/test_yjit.rb

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5013,3 +5013,37 @@ def test_body(args) = Array(1, *args)
50135013
test_body(array)
50145014
end
50155015
}
5016+
5017+
# compiling code shouldn't emit warnings as it may call into more Ruby code
5018+
assert_normal_exit %q{
5019+
# [Bug #20522]
5020+
Warning[:performance] = true
5021+
5022+
module StrictWarnings
5023+
def warn(msg, category: nil, **)
5024+
raise msg
5025+
end
5026+
end
5027+
Warning.singleton_class.prepend(StrictWarnings)
5028+
5029+
class A
5030+
def compiled_method(is_private)
5031+
@some_ivar = is_private
5032+
end
5033+
end
5034+
5035+
100.times do |i|
5036+
klass = Class.new(A)
5037+
7.times do |j|
5038+
obj = klass.new
5039+
obj.instance_variable_set("@base_#{i}", 42)
5040+
obj.instance_variable_set("@ivar_#{j}", 42)
5041+
end
5042+
obj = klass.new
5043+
obj.instance_variable_set("@base_#{i}", 42)
5044+
begin
5045+
obj.compiled_method(true)
5046+
rescue
5047+
end
5048+
end
5049+
}

shape.c

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -696,8 +696,8 @@ rb_shape_get_next_iv_shape(rb_shape_t* shape, ID id)
696696
return get_next_shape_internal(shape, id, SHAPE_IVAR, &dont_care, true);
697697
}
698698

699-
rb_shape_t *
700-
rb_shape_get_next(rb_shape_t *shape, VALUE obj, ID id)
699+
static inline rb_shape_t *
700+
shape_get_next(rb_shape_t *shape, VALUE obj, ID id, bool emit_warnings)
701701
{
702702
RUBY_ASSERT(!is_instance_id(id) || RTEST(rb_sym2str(ID2SYM(id))));
703703
if (UNLIKELY(shape->type == SHAPE_OBJ_TOO_COMPLEX)) {
@@ -730,7 +730,7 @@ rb_shape_get_next(rb_shape_t *shape, VALUE obj, ID id)
730730

731731
if (variation_created) {
732732
RCLASS_EXT(klass)->variation_count++;
733-
if (rb_warning_category_enabled_p(RB_WARN_CATEGORY_PERFORMANCE)) {
733+
if (emit_warnings && rb_warning_category_enabled_p(RB_WARN_CATEGORY_PERFORMANCE)) {
734734
if (RCLASS_EXT(klass)->variation_count >= SHAPE_MAX_VARIATIONS) {
735735
rb_category_warn(
736736
RB_WARN_CATEGORY_PERFORMANCE,
@@ -747,6 +747,18 @@ rb_shape_get_next(rb_shape_t *shape, VALUE obj, ID id)
747747
return new_shape;
748748
}
749749

750+
rb_shape_t *
751+
rb_shape_get_next(rb_shape_t *shape, VALUE obj, ID id)
752+
{
753+
return shape_get_next(shape, obj, id, true);
754+
}
755+
756+
rb_shape_t *
757+
rb_shape_get_next_no_warnings(rb_shape_t *shape, VALUE obj, ID id)
758+
{
759+
return shape_get_next(shape, obj, id, false);
760+
}
761+
750762
// Same as rb_shape_get_iv_index, but uses a provided valid shape id and index
751763
// to return a result faster if branches of the shape tree are closely related.
752764
bool

shape.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,7 @@ int rb_shape_frozen_shape_p(rb_shape_t* shape);
164164
rb_shape_t* rb_shape_transition_shape_frozen(VALUE obj);
165165
bool rb_shape_transition_shape_remove_ivar(VALUE obj, ID id, rb_shape_t *shape, VALUE * removed);
166166
rb_shape_t* rb_shape_get_next(rb_shape_t* shape, VALUE obj, ID id);
167+
rb_shape_t* rb_shape_get_next_no_warnings(rb_shape_t* shape, VALUE obj, ID id);
167168

168169
rb_shape_t * rb_shape_rebuild_shape(rb_shape_t * initial_shape, rb_shape_t * dest_shape);
169170

yjit/bindgen/src/main.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ fn main() {
100100
.allowlist_function("rb_shape_get_shape_by_id")
101101
.allowlist_function("rb_shape_id_offset")
102102
.allowlist_function("rb_shape_get_iv_index")
103-
.allowlist_function("rb_shape_get_next")
103+
.allowlist_function("rb_shape_get_next_no_warnings")
104104
.allowlist_function("rb_shape_id")
105105
.allowlist_function("rb_shape_obj_too_complex")
106106
.allowlist_var("SHAPE_ID_NUM_BITS")

yjit/src/codegen.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2970,7 +2970,7 @@ fn gen_set_ivar(
29702970
// The current shape doesn't contain this iv, we need to transition to another shape.
29712971
let new_shape = if !shape_too_complex && receiver_t_object && ivar_index.is_none() {
29722972
let current_shape = comptime_receiver.shape_of();
2973-
let next_shape = unsafe { rb_shape_get_next(current_shape, comptime_receiver, ivar_name) };
2973+
let next_shape = unsafe { rb_shape_get_next_no_warnings(current_shape, comptime_receiver, ivar_name) };
29742974
let next_shape_id = unsafe { rb_shape_id(next_shape) };
29752975

29762976
// If the VM ran out of shapes, or this class generated too many leaf,

yjit/src/cruby_bindings.inc.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1063,7 +1063,11 @@ extern "C" {
10631063
pub fn rb_shape_get_shape_id(obj: VALUE) -> shape_id_t;
10641064
pub fn rb_shape_get_iv_index(shape: *mut rb_shape_t, id: ID, value: *mut attr_index_t) -> bool;
10651065
pub fn rb_shape_obj_too_complex(obj: VALUE) -> bool;
1066-
pub fn rb_shape_get_next(shape: *mut rb_shape_t, obj: VALUE, id: ID) -> *mut rb_shape_t;
1066+
pub fn rb_shape_get_next_no_warnings(
1067+
shape: *mut rb_shape_t,
1068+
obj: VALUE,
1069+
id: ID,
1070+
) -> *mut rb_shape_t;
10671071
pub fn rb_shape_id(shape: *mut rb_shape_t) -> shape_id_t;
10681072
pub fn rb_gvar_get(arg1: ID) -> VALUE;
10691073
pub fn rb_gvar_set(arg1: ID, arg2: VALUE) -> VALUE;

0 commit comments

Comments
 (0)