Bug #8654

SEGV in Array#count

Added by Masaki Matsushita 9 months ago. Updated 6 months ago.

[ruby-core:56072]
Status:Closed
Priority:Normal
Assignee:Masaki Matsushita
Category:core
Target version:2.1.0
ruby -v:ruby 2.1.0dev (2013-07-18 trunk 42039) [x86_64-linux] Backport:1.9.3: DONE, 2.0.0: DONE

Description

Following code causes SEGV.

a1 = []
a2 = Array.new(100) {|i| i }
a2.count do |i|
p i
a2.replace(a1) if i == 0
end

0001-array.c-rb_ary_count-check-length-to-avoid-SEGV.patch Magnifier (2.37 KB) Benoit Daloze, 07/18/2013 07:50 PM

Associated revisions

Revision 42041
Added by Benoit Daloze 9 months ago

  • array.c (rbarycount): check length to avoid SEGV while iterating. Remove other pointer loop when arg is given.
  • test/ruby/testarray.rb (testcount): add test for bug. [Bug #8654]

Revision 42047
Added by Benoit Daloze 9 months ago

  • test/ruby/testarray.rb (testcount): add a test case for #count with an argument. See Bug #8654.

History

#1 Updated by Benoit Daloze 9 months ago

What do you think of this patch?

I am not sure assertinout_err is good for segfaults checks,
but I could not reproduce so reliably when removing the "p i".

#2 Updated by Masaki Matsushita 9 months ago

What do you think of this patch?

I already fixed it on r42040, but this ticket hasn't been closed because I have commited it with wrong commit message.
It's my fault.

However, I will add your test code.
Thank you for your patch,

#3 Updated by Kenta Murata 9 months ago

  • Backport changed from 1.9.3: UNKNOWN, 2.0.0: UNKNOWN to 1.9.3: REQUIRED, 2.0.0: REQUIRED

I confirmed it is reproducible on the both head revisions of ruby193 and ruby200.

#4 Updated by Benoit Daloze 9 months ago

Glass_saga (Masaki Matsushita) wrote:

What do you think of this patch?

I already fixed it on r42040, but this ticket hasn't been closed because I have commited it with wrong commit message.
It's my fault.

However, I will add your test code.
Thank you for your patch,

Ah, I should have looked the newest commits.

It might be worth adding the second change to avoid the pointer loop (seems about the only one in array.c), RARRAY_PTR() is kind of deprecated with the new GC for these cases in core.
I will commit it as I can rebase easily if it is OK.

#5 Updated by Benoit Daloze 9 months ago

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

This issue was solved with changeset r42041.
Masaki, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.


  • array.c (rbarycount): check length to avoid SEGV while iterating. Remove other pointer loop when arg is given.
  • test/ruby/testarray.rb (testcount): add test for bug. [Bug #8654]

#6 Updated by Benoit Daloze 9 months ago

  • Status changed from Closed to Open

Reopening for backport.
(The fix will need to be a bit different as there is no RARRAY_AREF() in older versions).

#7 Updated by Benoit Daloze 9 months ago

  • Status changed from Open to Closed

This issue was solved with changeset r42047.
Masaki, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.


  • test/ruby/testarray.rb (testcount): add a test case for #count with an argument. See Bug #8654.

#8 Updated by Tomoyuki Chikanaga 7 months ago

  • Backport changed from 1.9.3: REQUIRED, 2.0.0: REQUIRED to 1.9.3: REQUIRED, 2.0.0: DONE

backported r42040, r42041 and r42047 to ruby20_0 at r43228.

#9 Updated by Tomoyuki Chikanaga 7 months ago

... and backport r42068, r42069 to suppress warning and fix failure on CI (run with -w option).

#10 Updated by Usaku NAKAMURA 6 months ago

  • Backport changed from 1.9.3: REQUIRED, 2.0.0: DONE to 1.9.3: DONE, 2.0.0: DONE

Backported to ruby19_3 at r43491.

Also available in: Atom PDF