Backport #9248

Struct methods, segmentation fault

Added by Konstantin H over 1 year ago. Updated about 1 year ago.

[ruby-core:59095]
Status:Closed
Priority:Normal
Assignee:Tomoyuki Chikanaga

Description

=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


Related issues

Duplicates Ruby trunk - Bug #8756: SEGFAULT caused by `p Struct.new(:q?).instance_methods` Closed 08/09/2013

Associated revisions

Revision 44911
Added by Tomoyuki Chikanaga about 1 year ago

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]

Revision 44975
Added by Tomoyuki Chikanaga about 1 year ago

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.   [Bug #8756]

History

#1 Updated by Eric Wong over 1 year ago

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.

#2 Updated by Eric Wong over 1 year ago

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 ,
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.

#3 Updated by Nobuyoshi Nakada over 1 year ago

  • Tracker changed from Bug to Backport
  • Assignee set to Tomoyuki Chikanaga
  • Project changed from Ruby trunk to Backport200
  • Status changed from Open to Assigned

#4 Updated by Tomoyuki Chikanaga about 1 year ago

  • 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.

#5 Updated by Tomoyuki Chikanaga about 1 year ago

  • 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]

#6 Updated by Benoit Daloze about 1 year ago

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).

#7 Updated by Nobuyoshi Nakada about 1 year ago

Please backport r44926 too.

#8 Updated by Tomoyuki Chikanaga about 1 year ago

  • Status changed from Closed to Assigned

#9 Updated by Tomoyuki Chikanaga about 1 year ago

  • 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.   [Bug #8756]

Also available in: Atom PDF