Bug #16184: Entry persists in catch table even though its labels were removed, which may cause [BUG]

• Status: Closed
• Assignee: -
• Target version: -
• ruby -v: ruby 2.5.5p157 (2019-03-15) [mipsel-linux]
• Backport: 2.5: UNKNOWN, 2.6: UNKNOWN

[iv-m (Ivan Melnikov)]

When remove_unreachable_chunk removes the code from within a rescue block, the catch table entry corresponding the block is not removed. Here is a simple reproducer (tested with ruby 2.5.5):

puts "BEGIN"

if false
  begin
    require "some_mad_stuff"
  rescue LoadError
    puts "no mad stuff loaded"
  end
end

puts "END"

Here is the corresponding disasm:

== disasm: #<ISeq:<main>@test2.rb:1 (1,0)-(12,10)>======================
== catch table
| catch type: rescue st: 11022376 ed: 11022516 sp: -002 cont: 0000
== disasm: #<ISeq:rescue in <main>@test2.rb:7 (7,2)-(9,2)>==============
local table (size: 1, argc: 0 [opts: 0, rest: -1, post: 0, block: -1, kw: -1@-1, kwrest: -1])
[ 1] "\#$!"
0000 getlocal_OP__WC__0 "\#$!"                                        (   7)
0002 getinlinecache   9, <is:0>
0005 getconstant      :LoadError
0007 setinlinecache   <is:0>
0009 checkmatch       3
0011 branchunless     20
0013 putself                                                          (   8)[Li]
0014 putstring        "no mad stuff loaded"
0016 opt_send_without_block <callinfo!mid:puts, argc:1, FCALL|ARGS_SIMPLE>, <callcache>
0019 leave                                                            (   7)
0020 getlocal_OP__WC__0 "\#$!"
0022 throw            0
| catch type: retry  st: 11022516 ed: 0000 sp: -001 cont: 11022376
|------------------------------------------------------------------------
0000 putself                                                          (   2)[Li]
0001 putstring        "BEGIN"
0003 opt_send_without_block <callinfo!mid:puts, argc:1, FCALL|ARGS_SIMPLE>, <callcache>
0006 pop
0007 putself                                                          (  12)[Li]
0008 putstring        "END"
0010 opt_send_without_block <callinfo!mid:puts, argc:1, FCALL|ARGS_SIMPLE>, <callcache>
0013 leave

The interesting line here is:

catch type: rescue st: 11022376 ed: 11022516 sp: -002 cont: 0000

As the instruction corresponding the begin..rescue block were optimized away, the sp filed of the continue label was still -1 (or -2) and positions of start, end and continue labels are never initialized. When any exception happens in the remaining code (requires a bit more complex reproducer, happens in real life), this may cause an interpreter to segfault.

I've discovered this issue when building net-scp gem, which has this if false; require pattern in its Rakefile: https://github.com/net-ssh/net-scp/blob/v2.0.0/Rakefile . Interpreter reproducibly segfaults when trying to run it on my MIPS32 LE machine.

[iv-m (Ivan Melnikov)]

• File ruby-2.5.5-alt-fix-crash-on-mipsel.patch ruby-2.5.5-alt-fix-crash-on-mipsel.patch added

Of course, while having some strange cache table entries would be pretty ok if they were not used (like it usually happens on x86_64). To make sure they are never used compiler should initialize the position field of the labels. I'm attaching a patch that does just that -- at least this makes segfaults irreproducible.

[alanwu (Alan Wu)]

Could you post a reproducer that reliably crashes Ruby? Interesting issue!

[nobu (Nobuyoshi Nakada)]

• Status changed from Open to Feedback

Could you show the backtrace?

[iv-m (Ivan Melnikov)]

• File crush.log crush.log added

Could you post a reproducer that reliably crashes Ruby?

I guess this is not really possible to crash Ruby reliably via this issue. Here is a reproducer that crashes on my MIPS32 LE machine one out of 20 times or so:

puts "BEGIN"

if false
  begin
    require "some_mad_stuff"
  rescue LoadError
    puts "no mad stuff loaded"
  end
end

throw "foo"

I'm attaching the interpreter output from one of the crashes.

As far as I understood, here's what happens. When "foo" is thrown from the last line of the reproducer, exception table is considered. The exception table contains only one entry. The start and end fields of the entry come from labels which position were never initialized, since the rescue block was removed before the compilation process reached iseq_set_sequence. So, the start and end fields of the cache table entry contain (somewhat processed) garbage from unitialized memory. Here are a few examples from machine where crush sometimes happens:

$ cat disasm.rb

is = RubyVM::InstructionSequence.compile_file(ARGV[0])
puts is.disasm()
$ for x in `seq 1 10`; do ruby disasm.rb test3.rb | grep ' rescue'; done
| catch type: rescue st: 0000 ed: 0001 sp: -002 cont: 0000
| catch type: rescue st: 7168352 ed: 7168492 sp: -002 cont: 0000
| catch type: rescue st: 0000 ed: 0001 sp: -002 cont: 0000
| catch type: rescue st: 9085280 ed: 9085420 sp: -002 cont: 0000
| catch type: rescue st: 0000 ed: 0001 sp: -002 cont: 0000
| catch type: rescue st: 0000 ed: 0001 sp: -002 cont: 0000
| catch type: rescue st: 0000 ed: 0001 sp: -002 cont: 0000
| catch type: rescue st: 0000 ed: 0001 sp: -002 cont: 0000
| catch type: rescue st: 7070048 ed: 7070188 sp: -002 cont: 0000
| catch type: rescue st: 10264928 ed: 10265068 sp: -002 cont: 0000

To actually crash Ruby, start should very close to 0, and end field should be large enough. In this case, Ruby considers the entry applicable and goes to the position cont field provides (which also comes from uninitialized memory), and sets sp to some very large value ( (usigned int) -1 or (usigned int) -2), and segfaults.

So whether Ruby crashes or not depends on the contents of the memory where labels are allocated by compile_rescue function. On my x86_64 machine it always has zeroes for some reason, so crashing Ruby on it is probably impossible. I'm just "lucky" to have a machine where this memory actually contains some garbage.

[iv-m (Ivan Melnikov)]

• Status changed from Feedback to Open

So whether Ruby crashes or not depends on the contents of the memory where labels are allocated by compile_rescue function.

And having position field of labels initialized (as in my attached patch) makes crash totally impossible.

[iv-m (Ivan Melnikov)]

And having position field of labels initialized (as in my attached patch) makes crash totally impossible.

I think having this field properly initialized is a good idea anyway, so I created a pull request: https://github.com/ruby/ruby/pull/2499

[nobu (Nobuyoshi Nakada)]

• Status changed from Open to Closed

Applied in changeset git|79d5332a2da0e2743a77480aebfca256a54a962e.

---

Drop eliminated catch-entries

Drop catch table entries used in eliminated block, as well as
call_infos. [Bug #16184]