Backport #9248: Struct methods, segmentation fault

• Status: Closed
• Assignee: nagachika (Tomoyuki Chikanaga)

[Soilent (Konstantin x)]

=begin
The following snippet causes segmentation fault:

(({p Struct.new(:method_with_questionmark?).new.methods}))

This happens when accessor method name contains question mark.
=end

[normalperson (Eric Wong)]

Odd, doesn't happen with trunk, but I can reproduce it with 2.0.0-p353
on x86_64-linux. Will try to investigate before falling asleep.

[normalperson (Eric Wong)]

This is due to inspect of symbol inside the array.

Somehow the rb_id2str() call in sym_inspect is returning zero.
I'm not sure how that's happening, yet. It seems like the global
symbol table is missing entries somehow, and I fail to see how struct
handles ? methods differently. And I'm too sleepy to dig deeper.

#5 0x00007effeefc904f in rb_str_symname_p (sym=0) at string.c:7817
#6 0x00007effeefc92c6 in sym_inspect (self=5303308) at string.c:7874
#7 0x00007effef00de45 in call_cfunc_0 (func=0x7effeefc9292 <sym_inspect>,
recv=5303308, argc=0, argv=0x0) at vm_insnhelper.c:1336

Work-in-progress patch to clarify the backtrace and reproduce the issue
on 2.0.0:

--- a/string.c
+++ b/string.c
@@ -7861,12 +7861,13 @@ rb_id_quote_unprintable(ID id)
*/

static VALUE
-sym_inspect(VALUE sym)
+sym_inspect(VALUE self)
{
VALUE str;
const char *ptr;

• VALUE sym;
  long len;

• ID id = SYM2ID(sym);

• ID id = SYM2ID(self);
  char *dest;

  sym = rb_id2str(id);
  --- a/test/ruby/test_struct.rb
  +++ b/test/ruby/test_struct.rb
  @@ -281,4 +281,9 @@ class TestStruct < Test::Unit::TestCase
  o = klass.new(1, 2, 3, 4, 5, 6)
  assert_equal({a:1, b:2, c:3, d:4, e:5, f:6}, o.to_h)
  end

• def test_struct_question_mark

• klass = Struct.new(:a?)

• assert_equal [:a?], klass.new.methods.inspect, "should not segfault"

• end
  end

Oddly: klass.new.methods[0].inspect # does not segfault.
So recursive inspect seems to have something to do with it.

[nobu (Nobuyoshi Nakada)]

• Tracker changed from Bug to Backport
• Project changed from Ruby master to Backport200
• Status changed from Open to Assigned
• Assignee set to nagachika (Tomoyuki Chikanaga)

[nagachika (Tomoyuki Chikanaga)]

• ruby -v set to 2.0.0p353

#8756 seems related with this issue.
I'll backport them and add testcase test_struct_question_mark.
Eric thank you for your report. Have a good sleep.

[nagachika (Tomoyuki Chikanaga)]

• Status changed from Assigned to Closed
• % Done changed from 0 to 100

Applied in changeset r44911.

---

merge revision(s) r42479,r42490,r42509,r43083,r43084,r43085: [Backport #8756] [Backport #9248]

* parse.y (rb_enc_symname_type): allow ID_ATTRSET for ID_INSTANCE,
  ID_GLOBAL, ID_CLASS, ID_JUNK too.  [Bug #8756]

* parse.y (rb_id_attrset): fix inconsistency with literals, allow
  ID_ATTRSET and return it itself, but ID_JUNK cannot make ID_ATTRSET.
  and raise a NameError instead of rb_bug() for invalid argument.

* parse.y (rb_id_attrset, intern_str): allow junk attrset ID for
  Struct.

* parse.y (rb_id_attrset): check if the argument is valid type as an
  attribute.

* parse.y (rb_id_attrset): allow other than ID_ATTRSET.

* parse.y (intern_str): ditto.  try stem ID for ID_INSTANCE,
  ID_GLOBAL, ID_CLASS, ID_JUNK too.  [Bug #8756]

[Eregon (Benoit Daloze)]

This still seems problematic:

c = Struct.new(:a?)

o = c.new(42)

o.send("a?=", 3) rescue p $! # => #<NoMethodError: undefined method `a?=' for #<struct :a?=42>>
p o.a?

p o.methods # removing the "p" here would produce a NoMethodError below, somehow this "fixes" everything

o.send("a?=", 3)
p o.a?

IMHO, adding methods not callable by default because they do not respect the symbol syntax is bad. I would much prefer just to forbid attributes with a final question mark in Struct (or make them behave as the attribute without the "?" and just an extra "#{attr}?" predicate).

[nobu (Nobuyoshi Nakada)]

Please backport r44926 too.

[nagachika (Tomoyuki Chikanaga)]

• Status changed from Closed to Assigned

[nagachika (Tomoyuki Chikanaga)]

• Status changed from Assigned to Closed

Applied in changeset r44975.

---

merge revision(s) r44926: [Backport #8756] [Backport #9248]

* parse.y (IDSET_ATTRSET_FOR_INTERN): fix off-by-one bug.

* parse.y (rb_enc_symname_type): junk ID succeeded by '=' is also
  attrset ID.  [ruby-core:60668] [Bug #8756]