Feature #21863: Add RBASIC_FLAGS() and RBASIC_SET_FLAGS()

• Status: Open
• Assignee: -
• Target version: -

[Eregon (Benoit Daloze)]

Currently there is RBASIC_CLASS() which is the same as RBASIC(obj)->klass on CRuby but also checks it's called on a valid object:
https://github.com/ruby/ruby/blob/86dba8cfaeabb3b86df921da24b3243b9ce4ab2a/include/ruby/internal/core/rbasic.h#L159-L170

I would like to add RBASIC_FLAGS() and RBASIC_SET_FLAGS(), which could be defined like this on CRuby:

#define RBASIC_FLAGS(obj) (RBASIC(obj)->flags)
#define RBASIC_SET_FLAGS(obj, flags_to_set) (RBASIC(obj)->flags = flags_to_set)

This would let native extensions access those flags without relying on the specific fields, layout and existence of a RBasic struct.

My main motivation here is that TruffleRuby, the alternative Ruby implementation with the most complete native extension support, cannot support having mutable fields in RBasic.
The reason is because flags are mutable, it would be impossible to find out when the user writes to the flags if they write with just RBASIC(obj)->flags = v.
And since the state related to flags is held internally on the Java object, writing to native memory would do nothing, which would be incompatible.
With these macros we can know when the user writes to the flags and update the internal representation.
In fact, TruffleRuby already implements these macros, since this comment.
I would like to make it a standard C API to increase adoption and make it easier to discover.

This would also be useful on CRuby to do additional checks when reading and writing flags:

• We could add RBIMPL_ASSERT_OR_ASSUME(!RB_SPECIAL_CONST_P(obj)); like RBASIC_CLASS does.
• We could check that the frozen flag is not removed.
• We could check that the frozen flag is consistent with the frozen flag on the object shape (if CRuby has a frozen flag in the shape).
• We could check that the shape is not changed by native extensions this way (since the shape is stored in flags on 64-bit machines).
• CRuby would be able to evolve the representation of RBasic more freely in the future.

I'll note that there are already some macros/functions to manipulate flags such as RB_FL_TEST(), however they don't allow just reading or writing the flags:
https://github.com/ruby/ruby/blob/86dba8cfaeabb3b86df921da24b3243b9ce4ab2a/include/ruby/internal/fl_type.h#L95-L107
However it seems clear that C extensions do read and write the RBasic->flags field currently and these RB_FL_*() macros/functions are not enough, so RBASIC_FLAGS() and RBASIC_SET_FLAGS() would be obvious replacements.
Here is a gem-codesearch for /RBASIC\(.+\)->flags/: https://gist.github.com/eregon/5866bd86e626ae723b5b16d935af8f94

An alternative would be to deprecate the RBasic->flags field entirely and eventually hide it, and don't provide a replacement for direct reading/writing flags, but that would break a bunch of native extensions, so it does not seem actionable.
With this proposal we won't break anything and we'll actually get finer control over accesses to flags if e.g. we want to prevent some invariant-breaking changes.

[Eregon (Benoit Daloze)]

Eregon (Benoit Daloze) wrote:

We could check that the frozen flag is consistent with the frozen flag on the object shape
(if CRuby has a frozen flag in the shape).

Yes it does, so in CRuby the frozen flag is both stored in the shape and as a regular flag in RBasic.flags.
So having RBASIC_CLASS() & RBASIC_SET_FLAGS() and also using them internally in CRuby would be a good way to check they stay consistent, which is notably important because the CRuby JITs rely on this (e.g. to avoid extra check when writing an @ivar, i.e. only check the shape and not the shape + flags, and probably the interpreter does the same if it inline caches the shape), and on the frozen flag not being cleared (ZJIT constant folds reads on frozen objects, cc @tekknolagi (Maxwell Bernstein)).

[Eregon (Benoit Daloze)]

From https://ruby.esa.io/posts/144#[Feature%20#21863]%20Add%20RBASIC_FLAGS()%20and%20RBASIC_SET_FLAGS()%20(eregon)

@nobu (Nobuyoshi Nakada): why are RB_FL_SET, RB_FL_TEST, RB_FL_UNSET not enough? They can be used for bit flags in flags field. Other data in flags (such as object type) must not be tweaked.

I posted a list of gems using it: https://gist.github.com/eregon/5866bd86e626ae723b5b16d935af8f94.
It's not so many but I noticed the usage e.g.

• in numo-narray, where RB_FL_* seems usable: https://github.com/ruby-numo/numo-narray/blob/95c05257349b954c027c2284834408736244662a/ext/numo/narray/narray.c#L937-L950
• in allocation_tracer, where I don't see an obvious replacement, cc @ko1 (Koichi Sasada): https://github.com/ko1/allocation_tracer/blob/f4ac1e8c15f827c293dc885cb0d52b89b3df7fe8/ext/allocation_tracer/allocation_tracer.c#L255
• in heap_dump
• in hpricot

How about deprecating the field then and indicating to use RB_FL_* instead?