Project

General

Profile

Actions

Bug #18562

closed

throw_data passed to rescue through require

Added by jhawthorn (John Hawthorn) about 2 years ago. Updated almost 2 years ago.


Description

When we throw in a require it's incorrectly considered as a raise, making the T_IMEMO throw_data object accessible via rescue (in a contrived case). In regular usage this is unlikely to cause an issue because Module#=== immediately returns false for objects with klass == 0.

With test_throw.rb:

throw :extdep, 42

and

class Anything < Exception
  def self.===(_); true; end
end

catch(:extdep) do
  begin
    require "./test_throw"
  rescue Anything => e
    p e
  end
end

We get

in 'p': method inspect' called on unexpected T_IMEMO object (0x00007f5e1486b130 flags=0x10000301a) (NotImplementedError)

I've proposed a fix via PR https://github.com/ruby/ruby/pull/5513

Actions #1

Updated by jhawthorn (John Hawthorn) about 2 years ago

  • Status changed from Open to Closed

Applied in changeset git|c79d2e54748f52c5023b0a1ee441561df9826c17.


Fix TAG_THROW through require [Bug #18562]

Previously this was being incorrectly swapped with TAG_RAISE in the next
line. This would end up checking the T_IMEMO throw_data to the exception
handling (which calls Module#===). This happened to not break existing
tests because Module#=== returned false when klass is NULL.

This commit handles throw from require correctly by jumping to the tag
retaining the TAG_THROW state.

Updated by byroot (Jean Boussier) about 2 years ago

  • Backport changed from 2.6: UNKNOWN, 2.7: UNKNOWN, 3.0: UNKNOWN, 3.1: UNKNOWN to 2.6: DONTNEED, 2.7: DONTNEED, 3.0: DONTNEED, 3.1: DONTNEED

@jhawthorn (John Hawthorn) that was a master only issue right? I could repo it with any released version.

Updated by jhawthorn (John Hawthorn) about 2 years ago

  • Backport changed from 2.6: DONTNEED, 2.7: DONTNEED, 3.0: DONTNEED, 3.1: DONTNEED to 2.6: DONTNEED, 2.7: REQUIRED, 3.0: REQUIRED, 3.1: REQUIRED

byroot (Jean Boussier) wrote in #note-2:

@jhawthorn (John Hawthorn) that was a master only issue right? I could repo it with any released version.

Thanks for checking. I didn't realize this before, but it seems that this needs --disable-gems to reproduce, but with that I see the issue on 2.7, 3.0, and 3.1.

$ ruby --disable-gems -e 'puts RUBY_DESCRIPTION; class Anything < Exception; def self.===(_); true; end; end; catch(:extdep) do; begin; require "./test_throw"; rescue Anything => e; p e; end; end'
ruby 3.1.0p0 (2021-12-25 revision fb4df44d16) [x86_64-linux]
-e:1:in `p': method `inspect' called on unexpected T_IMEMO object (0x00007f3fb90e5a10 flags=0x301a) (NotImplementedError)
        from -e:1:in `rescue in block in <main>'
        from -e:1:in `block in <main>'
        from -e:1:in `catch'
        from -e:1:in `<main>'

I added REQUIRED for 2.7 and up, but I think it's unlikely anyone will run into this bug in real usage, so I don't feel strongly that it needs a backport if it's complicated.

Updated by naruse (Yui NARUSE) about 2 years ago

  • Backport changed from 2.6: DONTNEED, 2.7: REQUIRED, 3.0: REQUIRED, 3.1: REQUIRED to 2.6: DONTNEED, 2.7: REQUIRED, 3.0: REQUIRED, 3.1: DONE

ruby_3_1 807dd0479267a067e8208a2053b446fa13a2e66f merged revision(s) c79d2e54748f52c5023b0a1ee441561df9826c17.

Updated by nagachika (Tomoyuki Chikanaga) about 2 years ago

  • Backport changed from 2.6: DONTNEED, 2.7: REQUIRED, 3.0: REQUIRED, 3.1: DONE to 2.6: DONTNEED, 2.7: REQUIRED, 3.0: DONE, 3.1: DONE

ruby_3_0 0bd3e436e27c048933133bc19f863c954ed3e3a6 merged revision(s) c79d2e54748f52c5023b0a1ee441561df9826c17.

Actions #6

Updated by usa (Usaku NAKAMURA) almost 2 years ago

  • Backport changed from 2.6: DONTNEED, 2.7: REQUIRED, 3.0: DONE, 3.1: DONE to 2.6: DONTNEED, 2.7: DONE, 3.0: DONE, 3.1: DONE
Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0