Project

General

Profile

Bug #12670

Segmentation fault on `Magick::Image#get_pixels` on ruby since ruby-2.2

Added by ShockwaveNN (Pavel Lobashov) almost 3 years ago. Updated almost 2 years ago.

Status:
Closed
Priority:
Normal
Target version:
-
[ruby-core:76837]

Description

  1. Create test.rb this code
require 'rmagick'
include Magick

(0..10).each do |_|
  ImageList.new('1.bmp').get_pixels(0, 0, 1600, 800).each_slice(1600).to_a
end
  1. Create any 1.bmp file with dimension at least 1600*800 (Include zip file with it, extract it in directory with test.tb)
  2. Install ruby 2.1.9 (via RVM for example) and run
  ruby -v # ruby 2.1.9p490 (2016-03-30 revision 54437) [x86_64-linux]
  ruby test.rb # Nothing output, all good
  1. Install ruby 2.3.1 (via RVM for example) and run
  ruby 2.3.1p112 (2016-04-26 revision 54768) [x86_64-linux]
  ruby test.rb # You get a big log of Segmentation failed

I issued an problem on rmagick issue tracker (https://github.com/rmagick/rmagick/issues/212) but there is no progress at all
So I thought since both rubies use same version of rmagick I think problem may be in ruby itself.


Files

out.log (25.8 KB) out.log Segmentation fault at 0x0000000000001c ShockwaveNN (Pavel Lobashov), 08/11/2016 12:22 PM
1.bmp.zip (37.7 KB) 1.bmp.zip zip file with bmp ShockwaveNN (Pavel Lobashov), 08/11/2016 12:24 PM
core_dump.log (23.5 KB) core_dump.log ShockwaveNN (Pavel Lobashov), 11/04/2016 10:26 AM
Dockerfile (667 Bytes) Dockerfile wanabe (_ wanabe), 11/05/2016 04:41 PM
segv.c (306 Bytes) segv.c wanabe (_ wanabe), 05/22/2017 12:28 AM
segv.rb (209 Bytes) segv.rb wanabe (_ wanabe), 05/22/2017 12:28 AM
segv.c (365 Bytes) segv.c wanabe (_ wanabe), 05/22/2017 05:03 AM

Related issues

Has duplicate Ruby master - Bug #13900: Segmentation fault - 2 different machinesClosedActions

Associated revisions

Revision 0b8ab5b0
Added by ko1 (Koichi Sasada) over 2 years ago

  • gc.c (heap_page_resurrect): do not return tomb_pages when page->freelist == NULL. [Bug #12670]

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@56558 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

Revision 56558
Added by ko1 (Koichi Sasada) over 2 years ago

  • gc.c (heap_page_resurrect): do not return tomb_pages when page->freelist == NULL. [Bug #12670]

Revision 56558
Added by ko1 (Koichi Sasada) over 2 years ago

  • gc.c (heap_page_resurrect): do not return tomb_pages when page->freelist == NULL. [Bug #12670]

Revision 56558
Added by ko1 (Koichi Sasada) over 2 years ago

  • gc.c (heap_page_resurrect): do not return tomb_pages when page->freelist == NULL. [Bug #12670]

Revision 263a0f7f
Added by nobu (Nobuyoshi Nakada) about 2 years ago

test for [Bug #12670]

heap corruption by deferred free.

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@59116 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

Revision 59116
Added by nobu (Nobuyoshi Nakada) about 2 years ago

test for [Bug #12670]

heap corruption by deferred free.

Revision 59116
Added by nobu (Nobuyoshi Nakada) about 2 years ago

test for [Bug #12670]

heap corruption by deferred free.

Revision 59116
Added by nobu (Nobuyoshi Nakada) about 2 years ago

test for [Bug #12670]

heap corruption by deferred free.

Revision 7a1c10ba
Added by nobu (Nobuyoshi Nakada) about 2 years ago

gc.c: expand sorted pages

  • gc.c (heap_page_allocate): expand sorted pages before inserting allocated new page. [Bug #12670]

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@59136 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

Revision 59136
Added by nobu (Nobuyoshi Nakada) about 2 years ago

gc.c: expand sorted pages

  • gc.c (heap_page_allocate): expand sorted pages before inserting allocated new page. [Bug #12670]

Revision 59136
Added by nobu (Nobuyoshi Nakada) about 2 years ago

gc.c: expand sorted pages

  • gc.c (heap_page_allocate): expand sorted pages before inserting allocated new page. [Bug #12670]

Revision 59136
Added by nobu (Nobuyoshi Nakada) about 2 years ago

gc.c: expand sorted pages

  • gc.c (heap_page_allocate): expand sorted pages before inserting allocated new page. [Bug #12670]

Revision 687763bc
Added by usa (Usaku NAKAMURA) about 2 years ago

merge revision(s) 56558,59116,59136: [Backport #12670]

    * gc.c (heap_page_resurrect): do not return tomb_pages when
      page->freelist == NULL.
      [Bug #12670]

    test for [Bug #12670]

    heap corruption by deferred free.
    gc.c: expand sorted pages

    * gc.c (heap_page_allocate): expand sorted pages before inserting
      allocated new page.  [Bug #12670]

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/branches/ruby_2_3@59234 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

Revision 59234
Added by usa (Usaku NAKAMURA) about 2 years ago

merge revision(s) 56558,59116,59136: [Backport #12670]

* gc.c (heap_page_resurrect): do not return tomb_pages when
  page->freelist == NULL.
  [Bug #12670]

test for [Bug #12670]

heap corruption by deferred free.
gc.c: expand sorted pages

* gc.c (heap_page_allocate): expand sorted pages before inserting
  allocated new page.  [Bug #12670]

Revision e15f61a0
Added by nagachika (Tomoyuki Chikanaga) about 2 years ago

merge revision(s) 56558,59116,59136: [Backport #12670]

    * gc.c (heap_page_resurrect): do not return tomb_pages when
      page->freelist == NULL.
      [Bug #12670]

    test for [Bug #12670]

    heap corruption by deferred free.
    gc.c: expand sorted pages

    * gc.c (heap_page_allocate): expand sorted pages before inserting
      allocated new page.  [Bug #12670]

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/branches/ruby_2_4@59302 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

Revision 59302
Added by nagachika (Tomoyuki Chikanaga) about 2 years ago

merge revision(s) 56558,59116,59136: [Backport #12670]

* gc.c (heap_page_resurrect): do not return tomb_pages when
  page->freelist == NULL.
  [Bug #12670]

test for [Bug #12670]

heap corruption by deferred free.
gc.c: expand sorted pages

* gc.c (heap_page_allocate): expand sorted pages before inserting
  allocated new page.  [Bug #12670]

History

Updated by wanabe (_ wanabe) almost 3 years ago

It may be related to #10440 r48239, as git bisect shown.

Updated by wanabe (_ wanabe) almost 3 years ago

Line 5426 of gc.c at v2_3_1 https://github.com/ruby/ruby/blob/v2_3_1/gc.c#L5426 that is pointed by out.log, is about GC but r48239 is not include a change of GC, as I see.
r48239 may not be related...

Updated by wanabe (_ wanabe) almost 3 years ago

I got it.
SEGV is raised from gc_marks_continue(), line 5426: slots = heap->free_pages->free_slots,
as pointed by out.log. https://github.com/ruby/ruby/blob/v2_3_1/gc.c#L5426
My gdb shows the value of heap->free_pages is NULL when SEGV.

heap_page_resurrect() can return page even if page->freelist is NULL. It is intended - see r43461.
In the case, heap_add_freepage() does not set heap->free_pages and still keeps it NULL.

Updated by nagachika (Tomoyuki Chikanaga) almost 3 years ago

  • Assignee set to ko1 (Koichi Sasada)
  • Status changed from Open to Assigned

Updated by nagachika (Tomoyuki Chikanaga) almost 3 years ago

  • Backport changed from 2.1: UNKNOWN, 2.2: UNKNOWN, 2.3: UNKNOWN to 2.1: UNKNOWN, 2.2: UNKNOWN, 2.3: REQUIRED

wanabe san, thank you for your great investigation!

Sasada san, could you take a look at the lines wanabe san pointed out?

#6

Updated by ko1 (Koichi Sasada) over 2 years ago

  • Status changed from Assigned to Closed

Applied in changeset r56558.


  • gc.c (heap_page_resurrect): do not return tomb_pages when page->freelist == NULL. [Bug #12670]

Updated by ko1 (Koichi Sasada) over 2 years ago

  • Status changed from Closed to Feedback

I commit a fix on trunk. Could you check it?

Updated by ShockwaveNN (Pavel Lobashov) over 2 years ago

Koichi Sasada wrote:

I commit a fix on trunk. Could you check it?

Seems nothing changed. Errors looks like same

ruby -v
ruby 2.4.0dev (2016-11-04 trunk 56558) [x86_64-linux]
Attached a terminal output file.

Updated by nagachika (Tomoyuki Chikanaga) over 2 years ago

  • Status changed from Feedback to Assigned

Updated by wanabe (_ wanabe) over 2 years ago

I think the status is changed, but there is another issue.
We can see a assertion fail when I set RGENGC_CHECK_MODE as 1 in gc.c.

$ bundle exec ruby test.rb
ruby: ../gc.c:1532: heap_page_allocate: Assertion `heap_allocated_pages <= heap_pages_sorted_length' failed.
Aborted (core dumped)

This is in heap_page_allocate().
heap_pages_sorted_length, length of heap_pages_sorted[], must be greater than heap_allocated_pages when ruby call the function.
Otherwise, it causes invalid MEMMOVE() or assignment (heap_pages_sorted[hi] = page;).

Updated by ko1 (Koichi Sasada) over 2 years ago

On my linux VM, I can't reproduce with and without previous patch.

  • ruby 2.4.0dev (2016-11-05 trunk 56574) [x86_64-linux]
  • ruby 2.4.0dev (2016-11-01 trunk 56534) [x86_64-linux]

Sorry...

Updated by ShockwaveNN (Pavel Lobashov) over 2 years ago

Koichi Sasada wrote:

On my linux VM, I can't reproduce with and without previous patch.

  • ruby 2.4.0dev (2016-11-05 trunk 56574) [x86_64-linux]
  • ruby 2.4.0dev (2016-11-01 trunk 56534) [x86_64-linux]

Sorry...

I have several PC and this problem reproduces even in TravisCi. But all my PC's and travis images are Ubuntu-based. It may be important. Whis linux VM are you using?

Updated by ko1 (Koichi Sasada) over 2 years ago

I'm also using "Ubuntu 14.04.5 LTS" with gcc version 4.8.4 (Ubuntu 4.8.4-2ubuntu1~14.04.3).

Updated by wanabe (_ wanabe) over 2 years ago

The attached Dockerfile may help to reproduce.

Updated by ko1 (Koichi Sasada) about 2 years ago

Sorry for late response.

Unfortunately I have no enough disk to run docker. Try later when I get new machine.

Updated by wanabe (_ wanabe) about 2 years ago

I reduced repro codes, segv.c and segv.rb.
They cause assertion failure with RGENGC_CHECK_MODE=1.

ruby 2.5.0dev (2017-05-21 trunk 58832) [x86_64-linux]
creating Makefile
linking shared-object segv.so
0
1
2
3
4
5
6
7
8
9
10
11
12
13
14
ruby: ../../gc.c:1533: heap_page_allocate: Assertion `heap_allocated_pages <= heap_pages_sorted_length' failed.
Aborted (core dumped)

Updated by nobu (Nobuyoshi Nakada) about 2 years ago

    Data_Wrap_Struct(rb_cData, NULL, dummy_free, &i);

Wrapping the pointer to a local variable?
Obviously it is a bug.

If it is the thing what happens in RMagick, it's a bug in object lifetime management of RMagick.

Updated by wanabe (_ wanabe) about 2 years ago

nobu (Nobuyoshi Nakada) wrote:

    Data_Wrap_Struct(rb_cData, NULL, dummy_free, &i);

Wrapping the pointer to a local variable?
Obviously it is a bug.

I'm sorry that it is not RMagick but MY bug.
Please let me try it again.

Updated by nobu (Nobuyoshi Nakada) about 2 years ago

It reproduced "incorrect checksum for freed object", with a fix.

-    Data_Wrap_Struct(rb_cData, NULL, dummy_free, destroy_pixel);
+    Data_Wrap_Struct(rb_cData, NULL, destroy_pixel, pixel);
#20

Updated by nobu (Nobuyoshi Nakada) about 2 years ago

  • Status changed from Assigned to Closed

Applied in changeset trunk|r59116.


test for [Bug #12670]

heap corruption by deferred free.

#21

Updated by nobu (Nobuyoshi Nakada) about 2 years ago

  • Status changed from Closed to Assigned
#22

Updated by nobu (Nobuyoshi Nakada) about 2 years ago

  • Status changed from Assigned to Closed

Applied in changeset trunk|r59136.


gc.c: expand sorted pages

  • gc.c (heap_page_allocate): expand sorted pages before inserting allocated new page. [Bug #12670]
#23

Updated by usa (Usaku NAKAMURA) about 2 years ago

  • Backport changed from 2.1: UNKNOWN, 2.2: UNKNOWN, 2.3: REQUIRED to 2.1: UNKNOWN, 2.2: UNKNOWN, 2.3: REQUIRED, 2.4: REQUIRED

Updated by usa (Usaku NAKAMURA) about 2 years ago

  • Backport changed from 2.1: UNKNOWN, 2.2: UNKNOWN, 2.3: REQUIRED, 2.4: REQUIRED to 2.1: UNKNOWN, 2.2: UNKNOWN, 2.3: DONE, 2.4: REQUIRED

ruby_2_3 r59234 merged revision(s) 56558,59116,59136.

Updated by nagachika (Tomoyuki Chikanaga) about 2 years ago

  • Backport changed from 2.1: UNKNOWN, 2.2: UNKNOWN, 2.3: DONE, 2.4: REQUIRED to 2.1: UNKNOWN, 2.2: UNKNOWN, 2.3: DONE, 2.4: DONE

ruby_2_4 r59302 merged revision(s) 56558,59116,59136.

Updated by ShockwaveNN (Pavel Lobashov) almost 2 years ago

nagachika (Tomoyuki Chikanaga) wrote:

ruby_2_4 r59302 merged revision(s) 56558,59116,59136.

Can confirm bug is fixed.
Checked again with attached Dockerfile
Bug reproduced on:
v2_4_1
v2_3_4
v2_2_8

Bug fixed on:
v2_4_2
v2_3_5

#27

Updated by nobu (Nobuyoshi Nakada) almost 2 years ago

  • Has duplicate Bug #13900: Segmentation fault - 2 different machines added

Also available in: Atom PDF