From 73be9992e93072be803ffd5173e29dcf597e04ef Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Sun, 11 Jan 2026 09:54:18 +0100 Subject: [PATCH] Disambiguate private and public RSTRUCT_ helpers RSTRUCT_LEN / RSTRUCT_GET / RSTRUCT_SET all existing in two versions, one public that does type and frozens checks and one private that doesn't. The problem is that this is error prone because the public version is always accessible, but the private one require to include `internal/struct.h`. So you may have some code that rely on the public version, and later on the private header is included and changes the behavior. This already led to introducing a bug in YJIT & ZJIT: https://github.com/ruby/ruby/pull/15835 --- gc.c | 5 ++- internal/range.h | 6 ++-- internal/struct.h | 38 +++++--------------- marshal.c | 4 +-- ractor.c | 4 +-- struct.c | 88 +++++++++++++++++++++++------------------------ vm_insnhelper.c | 4 +-- 7 files changed, 64 insertions(+), 85 deletions(-) diff --git a/gc.c b/gc.c index e81c004bd8..1fe3dbf0ae 100644 --- a/gc.c +++ b/gc.c @@ -2461,9 +2461,8 @@ rb_obj_memsize_of(VALUE obj) break; case T_STRUCT: - if ((RBASIC(obj)->flags & RSTRUCT_EMBED_LEN_MASK) == 0 && - RSTRUCT(obj)->as.heap.ptr) { - size += sizeof(VALUE) * RSTRUCT_LEN(obj); + if (RSTRUCT_EMBED_LEN(obj) == 0) { + size += sizeof(VALUE) * RSTRUCT_LEN_RAW(obj); } break; diff --git a/internal/range.h b/internal/range.h index 2394937bf8..80493ce13e 100644 --- a/internal/range.h +++ b/internal/range.h @@ -18,19 +18,19 @@ static inline VALUE RANGE_EXCL(VALUE r); static inline VALUE RANGE_BEG(VALUE r) { - return RSTRUCT(r)->as.ary[0]; + return RSTRUCT_GET_RAW(r, 0); } static inline VALUE RANGE_END(VALUE r) { - return RSTRUCT_GET(r, 1); + return RSTRUCT_GET_RAW(r, 1); } static inline VALUE RANGE_EXCL(VALUE r) { - return RSTRUCT_GET(r, 2); + return RSTRUCT_GET_RAW(r, 2); } VALUE diff --git a/internal/struct.h b/internal/struct.h index 337f96a336..d3c8157393 100644 --- a/internal/struct.h +++ b/internal/struct.h @@ -49,36 +49,16 @@ struct RStruct { #define RSTRUCT(obj) ((struct RStruct *)(obj)) -#ifdef RSTRUCT_LEN -# undef RSTRUCT_LEN -#endif - -#ifdef RSTRUCT_PTR -# undef RSTRUCT_PTR -#endif - -#ifdef RSTRUCT_SET -# undef RSTRUCT_SET -#endif - -#ifdef RSTRUCT_GET -# undef RSTRUCT_GET -#endif - -#define RSTRUCT_LEN internal_RSTRUCT_LEN -#define RSTRUCT_SET internal_RSTRUCT_SET -#define RSTRUCT_GET internal_RSTRUCT_GET - /* struct.c */ VALUE rb_struct_init_copy(VALUE copy, VALUE s); VALUE rb_struct_lookup(VALUE s, VALUE idx); VALUE rb_struct_s_keyword_init(VALUE klass); static inline long RSTRUCT_EMBED_LEN(VALUE st); -static inline long RSTRUCT_LEN(VALUE st); +static inline long RSTRUCT_LEN_RAW(VALUE st); static inline int RSTRUCT_LENINT(VALUE st); static inline const VALUE *RSTRUCT_CONST_PTR(VALUE st); -static inline void RSTRUCT_SET(VALUE st, long k, VALUE v); -static inline VALUE RSTRUCT_GET(VALUE st, long k); +static inline void RSTRUCT_SET_RAW(VALUE st, long k, VALUE v); +static inline VALUE RSTRUCT_GET_RAW(VALUE st, long k); static inline long RSTRUCT_EMBED_LEN(VALUE st) @@ -89,7 +69,7 @@ RSTRUCT_EMBED_LEN(VALUE st) } static inline long -RSTRUCT_LEN(VALUE st) +RSTRUCT_LEN_RAW(VALUE st) { if (FL_TEST_RAW(st, RSTRUCT_EMBED_LEN_MASK)) { return RSTRUCT_EMBED_LEN(st); @@ -102,7 +82,7 @@ RSTRUCT_LEN(VALUE st) static inline int RSTRUCT_LENINT(VALUE st) { - return rb_long2int(RSTRUCT_LEN(st)); + return rb_long2int(RSTRUCT_LEN_RAW(st)); } static inline const VALUE * @@ -119,13 +99,13 @@ RSTRUCT_CONST_PTR(VALUE st) } static inline void -RSTRUCT_SET(VALUE st, long k, VALUE v) +RSTRUCT_SET_RAW(VALUE st, long k, VALUE v) { RB_OBJ_WRITE(st, &RSTRUCT_CONST_PTR(st)[k], v); } static inline VALUE -RSTRUCT_GET(VALUE st, long k) +RSTRUCT_GET_RAW(VALUE st, long k) { return RSTRUCT_CONST_PTR(st)[k]; } @@ -137,7 +117,7 @@ RSTRUCT_FIELDS_OBJ(VALUE st) VALUE fields_obj; if (embed_len) { RUBY_ASSERT(!FL_TEST_RAW(st, RSTRUCT_GEN_FIELDS)); - fields_obj = RSTRUCT_GET(st, embed_len); + fields_obj = RSTRUCT_GET_RAW(st, embed_len); } else { fields_obj = RSTRUCT(st)->as.heap.fields_obj; @@ -151,7 +131,7 @@ RSTRUCT_SET_FIELDS_OBJ(VALUE st, VALUE fields_obj) const long embed_len = RSTRUCT_EMBED_LEN(st); if (embed_len) { RUBY_ASSERT(!FL_TEST_RAW(st, RSTRUCT_GEN_FIELDS)); - RSTRUCT_SET(st, embed_len, fields_obj); + RSTRUCT_SET_RAW(st, embed_len, fields_obj); } else { RB_OBJ_WRITE(st, &RSTRUCT(st)->as.heap.fields_obj, fields_obj); diff --git a/marshal.c b/marshal.c index 8cd4dc6079..9d9a83097c 100644 --- a/marshal.c +++ b/marshal.c @@ -1084,7 +1084,7 @@ w_object(VALUE obj, struct dump_arg *arg, int limit) case T_STRUCT: w_class(TYPE_STRUCT, obj, arg, TRUE); { - long len = RSTRUCT_LEN(obj); + long len = RSTRUCT_LEN_RAW(obj); VALUE mem; long i; @@ -1092,7 +1092,7 @@ w_object(VALUE obj, struct dump_arg *arg, int limit) mem = rb_struct_members(obj); for (i=0; iself); - RSTRUCT_SET(args->self, i, val); + RSTRUCT_SET_RAW(args->self, i, val); } return ST_CONTINUE; } @@ -782,7 +782,7 @@ rb_struct_initialize_m(int argc, const VALUE *argv, VALUE self) rb_raise(rb_eArgError, "struct size differs"); } for (long i=0; i argc) { rb_mem_clear((VALUE *)RSTRUCT_CONST_PTR(self)+argc, n-argc); @@ -917,8 +917,8 @@ rb_struct_each(VALUE s) long i; RETURN_SIZED_ENUMERATOR(s, 0, 0, struct_enum_size); - for (i=0; i"); @@ -1051,7 +1051,7 @@ rb_struct_inspect(VALUE s) static VALUE rb_struct_to_a(VALUE s) { - return rb_ary_new4(RSTRUCT_LEN(s), RSTRUCT_CONST_PTR(s)); + return rb_ary_new4(RSTRUCT_LEN_RAW(s), RSTRUCT_CONST_PTR(s)); } /* @@ -1080,13 +1080,13 @@ rb_struct_to_a(VALUE s) static VALUE rb_struct_to_h(VALUE s) { - VALUE h = rb_hash_new_with_size(RSTRUCT_LEN(s)); + VALUE h = rb_hash_new_with_size(RSTRUCT_LEN_RAW(s)); VALUE members = rb_struct_members(s); long i; int block_given = rb_block_given_p(); - for (i=0; icc)->def->body.optimized.type == OPTIMIZED_METHOD_TYPE_STRUCT_AREF); const unsigned int off = vm_cc_cme(calling->cc)->def->body.optimized.index; - return internal_RSTRUCT_GET(recv, off); + return RSTRUCT_GET_RAW(recv, off); } static VALUE @@ -4789,7 +4789,7 @@ vm_call_opt_struct_aset0(rb_execution_context_t *ec, struct rb_calling_info *cal rb_check_frozen(recv); const unsigned int off = vm_cc_cme(calling->cc)->def->body.optimized.index; - internal_RSTRUCT_SET(recv, off, val); + RSTRUCT_SET_RAW(recv, off, val); return val; }