Project

General

Profile

Bug #15162

Encoding::Converter.search_convpath(Encoding::ASCII_8BIT, Encoding::Emacs_Mule) crashes on MinGW

Added by Eregon (Benoit Daloze) almost 2 years ago. Updated over 1 year ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Target version:
-
ruby -v:
ruby 2.6.0dev (2018-09-26) [x64-mingw32]
[ruby-core:89172]

Description

It seems to work fine with String arguments, but not Encoding objects where it SEGV, at least in some runs.
See https://github.com/ruby/ruby/commit/f00bf242724d40d59a242c6bf9e567d18c9e1872#commitcomment-30650955

cc MSP-Greg (Greg L)


Files

0001-transcode.c-add-GC-guard-on-raise.patch (1.04 KB) 0001-transcode.c-add-GC-guard-on-raise.patch h.shirosaki (Hiroshi Shirosaki), 09/26/2018 11:38 AM

Related issues

Related to Ruby master - Bug #12411: Warnings when compiling transcode.c on cygwinClosednaruse (Yui NARUSE)Actions

Updated by h.shirosaki (Hiroshi Shirosaki) almost 2 years ago

I found a smaller test to reproduce SEGV.

$ ./miniruby -ve "loop do loop do loop do Encoding::Converter.search_convpath(Encoding::ASCII_8BIT, Encoding::ASCII_8BIT) end end end"
ruby 2.6.0dev (2018-09-26 trunk 64852) [x64-mingw32]
Segmentation fault

Maybe GC guard is not sufficient? Attached patch fixes SEGV on my test.

Updated by MSP-Greg (Greg L) almost 2 years ago

hsbt (Hiroshi SHIBATA), thanks for the work on this. I (quickly) did a lambda in a block, and that didn't cause the SEGV.

Using existing ruby-loco builds (I save all of them locally), I found that the issue doesn't exist on:
ruby 2.6.0dev (2018-09-26 trunk 64845) [x64-mingw32]

but does exist on:
ruby 2.6.0dev (2018-09-26 trunk 64851) [x64-mingw32]

I'll check the patch later, Greg

Updated by MSP-Greg (Greg L) almost 2 years ago

hsbt (Hiroshi SHIBATA)

I just did twenty runs of core/encoding, then twenty runs of the full spec suite. No silent SEGV's.

Hence, the patch works. Thank you. Whoever does the commmit, please remember to remove the two .name method calls on line 30 of spec/ruby/core/encoding/converter/search_convpath_spec.rb

Benoit - I can't seem to ping you using either Eregon (Benoit Daloze) or Eregon (Benoit Daloze)

Updated by hsbt (Hiroshi SHIBATA) almost 2 years ago

MSP-Greg (Greg L)

You mistake to mention about @shirosaki, not @hsbt.

Updated by MSP-Greg (Greg L) almost 2 years ago

hsbt (Hiroshi SHIBATA) & h.shirosaki (Hiroshi Shirosaki)

I apologize for that mistake. I hope to not make it again.

We have a saying something like 'foot in mouth' referring to speech. I'm not sure what the equivalent is for written... Thanks, Greg

Updated by Eregon (Benoit Daloze) almost 2 years ago

MSP-Greg (Greg L) wrote:

Benoit - I can't seem to ping you using either Eregon (Benoit Daloze) or Eregon (Benoit Daloze)

Odd, not sure why it doesn't work.

#7

Updated by h.shirosaki (Hiroshi Shirosaki) almost 2 years ago

  • Related to Bug #12411: Warnings when compiling transcode.c on cygwin added

Updated by h.shirosaki (Hiroshi Shirosaki) almost 2 years ago

Thank you for test.
This issue seems to be related to #12411 fix.

#9

Updated by Anonymous almost 2 years ago

  • Status changed from Open to Closed

Applied in changeset trunk|r64879.


transcode.c: add GC guard on raise

  • transcode.c (econv_s_search_convpath): add GC guard to fix SEGV on raise. [Bug #15162] [ruby-core:89172]
#10

Updated by nagachika (Tomoyuki Chikanaga) almost 2 years ago

  • Backport changed from 2.3: UNKNOWN, 2.4: UNKNOWN, 2.5: UNKNOWN to 2.3: REQUIRED, 2.4: REQUIRED, 2.5: REQUIRED

Updated by nagachika (Tomoyuki Chikanaga) over 1 year ago

  • Backport changed from 2.3: REQUIRED, 2.4: REQUIRED, 2.5: REQUIRED to 2.3: REQUIRED, 2.4: REQUIRED, 2.5: DONE

ruby_2_5 r66881 merged revision(s) 64879.

Updated by usa (Usaku NAKAMURA) over 1 year ago

  • Backport changed from 2.3: REQUIRED, 2.4: REQUIRED, 2.5: DONE to 2.3: REQUIRED, 2.4: DONE, 2.5: DONE

ruby_2_4 r66961 merged revision(s) 64879.

Also available in: Atom PDF