Bug #18189: `rb_cString` can be NULL during `Init_Object`

• Status: Closed
• Assignee: -
• Target version: -
• Backport: 2.6: UNKNOWN, 2.7: UNKNOWN, 3.0: UNKNOWN

[ioquatix (Samuel Williams)]

It's possible for rb_cString to be NULL during Init_Object and thus Init_class_hierarchy which means that rb_fstring_lit, which invokes setup_fake_str, invokes RBASIC_SET_CLASS_RAW(..., NULL) (or possibly just something totally random if it's not zero initialized!).

Later on in register_fstring we have an assertion which also fails to detect the abnormality:

assert(RBASIC_CLASS(args.fstr) == rb_cString);

Because both are NULL. Oops.

It seems that later on, rb_cString is set on that specific fstring. But in my own usage of rb_define_module_under during InitVM_Object, this creates invalid class names which fail when passed into Ruby land.

[ioquatix (Samuel Williams)]

• Description updated (diff)

[ioquatix (Samuel Williams)]

The order of operations between here, for anything involving strings, could be a problem:

    CALL(Object);
    CALL(top_self);
    CALL(Encoding);
    CALL(Comparable);
    CALL(Enumerable);
    CALL(String);

[ioquatix (Samuel Williams)]

I see, there is some kind of patch up:

    rb_cString  = rb_define_class("String", rb_cObject);
    assert(rb_vm_fstring_table());
    st_foreach(rb_vm_fstring_table(), fstring_set_class_i, rb_cString);

So for some things it's working but for rb_define_module_under it seems not working.

[nobu (Nobuyoshi Nakada)]

With adding the following lines after Init_class_hierarchy() in InitVM_Object, it seems working.

    VALUE rb_mImmutable = rb_define_module("Immutable");
    VALUE rb_mImmutableObject = rb_define_module_under(rb_mImmutable, "Object");

$ ./ruby -e 'class X; include Immutable::Object; end; p X.new'
#<X:0x0000000108e7c300>

[ioquatix (Samuel Williams)]

@nobu (Nobuyoshi Nakada) with your changes you made, please try running make test. It fails.

Then, use the sample I gave:

> ./ruby -e 'p Immutable::Object.to_s'
-e:1:in `p': method `inspect' called on hidden T_STRING object (0x0000000108b6fa90 flags=0x844005) (NotImplementedError)
	from -e:1:in `<main>'

This seems invalid, no?

[nobu (Nobuyoshi Nakada)]

I see.

diff --git i/variable.c w/variable.c
index aa1fdd022eb..c6e1012bddc 100644
--- i/variable.c
+++ w/variable.c
@@ -202,8 +202,7 @@ build_const_pathname(VALUE head, VALUE tail)
     VALUE path = rb_str_dup(head);
     rb_str_cat2(path, "::");
     rb_str_append(path, tail);
-    OBJ_FREEZE(path);
-    return path;
+    return rb_fstring(path);
 }
 
 static VALUE

[ioquatix (Samuel Williams)]

@nobu (Nobuyoshi Nakada) that makes total sense.

I also had one other idea.

We obviously have a lot of:

VALUE rb_cString;

We initialize this dynamically.

Why not initialize it statically?

struct RClass _cString = ...;
VALUE rb_cString = & _cString;

It seems (1) performance improvement and (2) more predictable usage at least for strings.

[ioquatix (Samuel Williams)]

Thanks to @nobu's suggestion here is a PR: https://github.com/ruby/ruby/pull/4892/commits/f0be5d27d5dde1b6dfeb174f00fcac66d9a0184d

[ioquatix (Samuel Williams)]

• Status changed from Open to Closed

It's fixed.