Skip to content

Merge from upstream #24

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 547 commits into from
Sep 1, 2022
Merged

Merge from upstream #24

merged 547 commits into from
Sep 1, 2022

Conversation

chrisseaton
Copy link
Collaborator

Merge from upstream, due to a merge conflict identified by the nightly builders.

maximecb and others added 30 commits August 29, 2022 08:46
* Initial setup for aarch64

* ADDS and SUBS

* ADD and SUB for immediates

* Revert moved code

* Documentation

* Rename Arm64* to A64*

* Comments on shift types

* Share sig_imm_size and unsig_imm_size
* LDUR

* Fix up immediate masking

* Consume operands directly

* Consistency and cleanup

* More consistency and entrypoints

* Cleaner syntax for masks

* Cleaner shifting for encodings
* Remove x86-64 dependency from codegen.rs

* Port over putnil and putobject

* Port over gen_leave()

* Complete port of gen_leave()

* Fix bug in x86 instruction splitting
* MOVK instruction

* More tests for the A64 entrypoints

* Finish testing entrypoints

* MOVZ

* BR instruction
* LDADDAL instruction

* STUR

* BL instruction

* Remove num_bits from imm and uimm

* Tests for imm_fits_bits and uimm_fits_bits

* Reorder arguments to LDADDAL
maximecb and others added 24 commits August 30, 2022 14:21
`BufferedIO` is a bit inefficient for reading large responses because
it use the classic `buffer.slice!` technique which cause a lot of
unnecessary string copying.

This is particularly visible on line based protocol when reading
line by line.

Instead of repeatedly shifting the string, we can keep track of
which offset we're at, to know how many bytes are left in the buffer.

This change also open the door to further optimization by increasing
the buffer size, as previously `slice!` would get slower the larger
the buffer is.

Benchmark results:

```
=== 1k ===
Warming up --------------------------------------
                  1k     1.234k i/100ms
              1k opt     1.283k i/100ms
Calculating -------------------------------------
                  1k     12.615k (± 0.9%) i/s -     64.168k in   5.086995s
              1k opt     12.856k (± 0.9%) i/s -     65.433k in   5.090051s

Comparison:
                  1k:    12615.2 i/s
              1k opt:    12856.0 i/s - 1.02x  (± 0.00) faster

=== 10k ===
Warming up --------------------------------------
                 10k     1.165k i/100ms
             10k opt     1.269k i/100ms
Calculating -------------------------------------
                 10k     11.550k (± 2.4%) i/s -     58.250k in   5.046378s
             10k opt     12.736k (± 1.0%) i/s -     64.719k in   5.081969s

Comparison:
                 10k:    11550.3 i/s
             10k opt:    12736.3 i/s - 1.10x  (± 0.00) faster

=== 100k ===
Warming up --------------------------------------
                100k   809.000  i/100ms
            100k opt   926.000  i/100ms
Calculating -------------------------------------
                100k      8.054k (± 3.0%) i/s -     40.450k in   5.028299s
            100k opt      9.286k (± 2.2%) i/s -     47.226k in   5.088841s

Comparison:
                100k:     8053.6 i/s
            100k opt:     9285.5 i/s - 1.15x  (± 0.00) faster

=== 1M ===
Warming up --------------------------------------
                  1M   249.000  i/100ms
              1M opt   315.000  i/100ms
Calculating -------------------------------------
                  1M      2.448k (± 2.5%) i/s -     12.450k in   5.089744s
              1M opt      3.119k (± 2.6%) i/s -     15.750k in   5.053772s

Comparison:
                  1M:     2447.8 i/s
              1M opt:     3118.8 i/s - 1.27x  (± 0.00) faster
```

Profiling before (1MB responses):

```
==================================
  Mode: wall(1000)
  Samples: 5276 (0.00% miss rate)
  GC: 394 (7.47%)
==================================
     TOTAL    (pct)     SAMPLES    (pct)     FRAME
      1622  (30.7%)        1622  (30.7%)     IO#wait_readable
       777  (14.7%)         777  (14.7%)     IO#read_nonblock
       365   (6.9%)         365   (6.9%)     (sweeping)
      2705  (51.3%)         364   (6.9%)     Net::BufferedIO#rbuf_fill
       264   (5.0%)         264   (5.0%)     String#index
       223   (4.2%)         223   (4.2%)     String#sub
       221   (4.2%)         221   (4.2%)     String#slice!
       185   (3.5%)         185   (3.5%)     String#split
       108   (2.0%)         108   (2.0%)     IO#write_nonblock
       101   (1.9%)         101   (1.9%)     String#downcase
        66   (1.3%)          66   (1.3%)     Net::BufferedIO#LOG
        57   (1.1%)          57   (1.1%)     String#count
        51   (1.0%)          51   (1.0%)     String#to_s
       391   (7.4%)          50   (0.9%)     Net::HTTPGenericRequest#write_header
        50   (0.9%)          50   (0.9%)     String#capitalize
        49   (0.9%)          49   (0.9%)     Array#join
        47   (0.9%)          47   (0.9%)     String#b
       106   (2.0%)          36   (0.7%)     Net::HTTPHeader#set_field
        34   (0.6%)          34   (0.6%)     Module#===
        33   (0.6%)          33   (0.6%)     String#[]
       140   (2.7%)          29   (0.5%)     Net::BufferedIO#write0
        29   (0.5%)          29   (0.5%)     (marking)
       281   (5.3%)          27   (0.5%)     Net::BufferedIO#rbuf_consume
      1195  (22.6%)          25   (0.5%)     Net::HTTPResponse#read_body
      1024  (19.4%)          25   (0.5%)     Net::HTTPResponse.each_response_header
        86   (1.6%)          24   (0.5%)     Net::HTTPHeader#set_field
        23   (0.4%)          23   (0.4%)     Net::HTTP#proxy_uri
        51   (1.0%)          23   (0.4%)     Net::HTTPHeader#initialize_http_header
      2225  (42.2%)          22   (0.4%)     Net::BufferedIO#readuntil
        20   (0.4%)          20   (0.4%)     Regexp#===
```

Profiling after (1MB responses):

```
==================================
  Mode: wall(1000)
  Samples: 15180 (0.00% miss rate)
  GC: 1688 (11.12%)
==================================
     TOTAL    (pct)     SAMPLES    (pct)     FRAME
      4534  (29.9%)        4534  (29.9%)     IO#read_nonblock
     10650  (70.2%)        3944  (26.0%)     Net::HTTPOpt::BufferedIOOpt#rbuf_fill
      2101  (13.8%)        2101  (13.8%)     IO#wait_readable
      1442   (9.5%)        1442   (9.5%)     (sweeping)
       360   (2.4%)         360   (2.4%)     String#sub
       312   (2.1%)         312   (2.1%)     String#split
       265   (1.7%)         265   (1.7%)     String#bytesize
       246   (1.6%)         246   (1.6%)     (marking)
       151   (1.0%)         151   (1.0%)     IO#write_nonblock
       125   (0.8%)         125   (0.8%)     String#downcase
       116   (0.8%)         116   (0.8%)     String#index
       113   (0.7%)         113   (0.7%)     Module#===
       162   (1.1%)          89   (0.6%)     Net::HTTPOpt::BufferedIOOpt#rbuf_consume_all_shareable!
       158   (1.0%)          65   (0.4%)     Net::HTTPHeader#set_field
        63   (0.4%)          63   (0.4%)     String#capitalize
        63   (0.4%)          63   (0.4%)     BasicObject#equal?
        58   (0.4%)          58   (0.4%)     Regexp#match
        58   (0.4%)          58   (0.4%)     String#[]
       449   (3.0%)          56   (0.4%)     Net::HTTPGenericRequest#write_header
        53   (0.3%)          53   (0.3%)     String#to_s
        52   (0.3%)          52   (0.3%)     Net::HTTPOpt::BufferedIOOpt#LOG
        52   (0.3%)          52   (0.3%)     String#count
        44   (0.3%)          44   (0.3%)     String#byteslice
        44   (0.3%)          44   (0.3%)     Array#join
      1096   (7.2%)          42   (0.3%)     Net::HTTPResponse.each_response_header
      2617  (17.2%)          40   (0.3%)     Net::HTTPOpt::BufferedIOOpt#readuntil
       132   (0.9%)          30   (0.2%)     Net::HTTPOpt::BufferedIOOpt#rbuf_consume
        28   (0.2%)          28   (0.2%)     Regexp#===
        27   (0.2%)          27   (0.2%)     Net::HTTP#proxy_uri
      8862  (58.4%)          27   (0.2%)     Net::HTTPResponse#read_body
````

Benchmark code:

```ruby

require "fileutils"
DIR = "/tmp/www"
FileUtils.mkdir_p(DIR)
HOST = "127.0.0.1"
PORT = 8080
CONF = <<~EOS
daemon            off;
worker_processes  2;

events {
    worker_connections  128;
}

http {
    server_tokens off;
    charset       utf-8;

    server {
        server_name   localhost;
        listen        #{HOST}:#{PORT};

        keepalive_requests 10000000;
        keepalive_timeout 3600s;

        error_page    500 502 503 504  /50x.html;

        location      / {
            root      #{DIR};
        }

    }

}
EOS

File.write(File.join(DIR, "1k.txt"), 'a' * 1024)
File.write(File.join(DIR, "10k.txt"), 'a' * 1024 * 10)
File.write(File.join(DIR, "100k.txt"), 'a' * 1024 * 100)
File.write(File.join(DIR, "1M.txt"), 'a' * 1024 * 1024)

File.write(File.join(DIR, "nginx.conf"), CONF)

require "benchmark/ips"
require "net/http"

nginx_pid = Process.spawn('nginx', '-c', File.join(DIR, "nginx.conf"))

module Net
  class HTTPOpt < HTTP

    class BufferedIOOpt < ::Net::BufferedIO  #:nodoc: internal use only
      def initialize(io, read_timeout: 60, write_timeout: 60, continue_timeout: nil, debug_output: nil)
        @io = io
        @read_timeout = read_timeout
        @write_timeout = write_timeout
        @continue_timeout = continue_timeout
        @debug_output = debug_output
        @rbuf = ''.b
        @rbuf_offset = 0
      end

      attr_reader :io
      attr_accessor :read_timeout
      attr_accessor :write_timeout
      attr_accessor :continue_timeout
      attr_accessor :debug_output

      def inspect
        "#<#{self.class} io=#{@io}>"
      end

      def eof?
        @io.eof?
      end

      def closed?
        @io.closed?
      end

      def close
        @io.close
      end

      #
      # Read
      #

      public

      def read(len, dest = ''.b, ignore_eof = false)
        LOG "reading #{len} bytes..."
        read_bytes = 0
        begin
          while read_bytes + rbuf_size < len
            if s = rbuf_consume_all_shareable!
              read_bytes += s.bytesize
              dest << s
            end
            rbuf_fill
          end
          s = rbuf_consume(len - read_bytes)
          read_bytes += s.bytesize
          dest << s
        rescue EOFError
          raise unless ignore_eof
        end
        LOG "read #{read_bytes} bytes"
        dest
      end

      def read_all(dest = ''.b)
        LOG 'reading all...'
        read_bytes = 0
        begin
          while true
            if s = rbuf_consume_all_shareable!
              read_bytes += s.bytesize
              dest << s
            end
            rbuf_fill
          end
        rescue EOFError
          ;
        end
        LOG "read #{read_bytes} bytes"
        dest
      end

      def readuntil(terminator, ignore_eof = false)
        offset = @rbuf_offset
        begin
          until idx = @rbuf.index(terminator, offset)
            offset = @rbuf.bytesize
            rbuf_fill
          end
          return rbuf_consume(idx + terminator.bytesize - @rbuf_offset)
        rescue EOFError
          raise unless ignore_eof
          return rbuf_consume
        end
      end

      def readline
        readuntil("\n").chop
      end

      private

      BUFSIZE = 1024 * 16

      def rbuf_fill
        tmp = @rbuf_empty ? @rbuf : nil
        case rv = @io.read_nonblock(BUFSIZE, tmp, exception: false)
        when String
          @rbuf_empty = false
          if rv.equal?(tmp)
            @rbuf_offset = 0
          else
            @rbuf << rv
            rv.clear
          end
          return
        when :wait_readable
          (io = @io.to_io).wait_readable(@read_timeout) or raise Net::ReadTimeout.new(io)
          # continue looping
        when :wait_writable
          # OpenSSL::Buffering#read_nonblock may fail with IO::WaitWritable.
          # http://www.openssl.org/support/faq.html#PROG10
          (io = @io.to_io).wait_writable(@read_timeout) or raise Net::ReadTimeout.new(io)
          # continue looping
        when nil
          raise EOFError, 'end of file reached'
        end while true
      end

      def rbuf_flush
        if @rbuf_empty
          @rbuf.clear
          @rbuf_offset = 0
        end
        nil
      end

      def rbuf_size
        @rbuf.bytesize - @rbuf_offset
      end

      # Warning: this method may share the buffer to avoid
      # copying. The caller must no longer use the returned
      # string once rbuf_fill has been called again
      def rbuf_consume_all_shareable!
        @rbuf_empty = true
        buf = if @rbuf_offset == 0
          @rbuf
        else
          @rbuf.byteslice(@rbuf_offset..-1)
        end
        @rbuf_offset = @rbuf.bytesize
        buf
      end

      def rbuf_consume(len = nil)
        if @rbuf_offset == 0 && (len.nil? || len == @rbuf.bytesize)
          s = @rbuf
          @rbuf = ''.b
          @rbuf_offset = 0
          @rbuf_empty = true
        elsif len.nil?
          s = @rbuf.byteslice(@rbuf_offset..-1)
          @rbuf = ''.b
          @rbuf_offset = 0
          @rbuf_empty = true
        else
          s = @rbuf.byteslice(@rbuf_offset, len)
          @rbuf_offset += len
          @rbuf_empty = @rbuf_offset == @rbuf.bytesize
          rbuf_flush
        end

        @debug_output << %Q[-> #{s.dump}\n] if @debug_output
        s
      end

      #
      # Write
      #

      public

      def write(*strs)
        writing {
          write0(*strs)
        }
      end

      alias << write

      def writeline(str)
        writing {
          write0 str + "\r\n"
        }
      end

      private

      def writing
        @written_bytes = 0
        @debug_output << '<- ' if @debug_output
        yield
        @debug_output << "\n" if @debug_output
        bytes = @written_bytes
        @written_bytes = nil
        bytes
      end

      def write0(*strs)
        @debug_output << strs.map(&:dump).join if @debug_output
        orig_written_bytes = @written_bytes
        strs.each_with_index do |str, i|
          need_retry = true
          case len = @io.write_nonblock(str, exception: false)
          when Integer
            @written_bytes += len
            len -= str.bytesize
            if len == 0
              if strs.size == i+1
                return @written_bytes - orig_written_bytes
              else
                need_retry = false
                # next string
              end
            elsif len < 0
              str = str.byteslice(len, -len)
            else # len > 0
              need_retry = false
              # next string
            end
            # continue looping
          when :wait_writable
            (io = @io.to_io).wait_writable(@write_timeout) or raise Net::WriteTimeout.new(io)
            # continue looping
          end while need_retry
        end
      end

      #
      # Logging
      #

      private

      def LOG_off
        @save_debug_out = @debug_output
        @debug_output = nil
      end

      def LOG_on
        @debug_output = @save_debug_out
      end

      def LOG(msg)
        return unless @debug_output
        @debug_output << msg + "\n"
      end
    end
    BufferedIO = BufferedIOOpt

    # Unchanged from ruby 3.1.1, only allow to lookup the mofidied BufferedIO
    def connect
      if use_ssl?
        # reference early to load OpenSSL before connecting,
        # as OpenSSL may take time to load.
        @ssl_context = OpenSSL::SSL::SSLContext.new
      end

      if proxy? then
        conn_addr = proxy_address
        conn_port = proxy_port
      else
        conn_addr = conn_address
        conn_port = port
      end

      D "opening connection to #{conn_addr}:#{conn_port}..."
      begin
        s = Socket.tcp conn_addr, conn_port, @local_host, @local_port, connect_timeout: @open_timeout
      rescue => e
        e = Net::OpenTimeout.new(e) if e.is_a?(Errno::ETIMEDOUT) #for compatibility with previous versions
        raise e, "Failed to open TCP connection to " +
          "#{conn_addr}:#{conn_port} (#{e.message})"
      end
      s.setsockopt(Socket::IPPROTO_TCP, Socket::TCP_NODELAY, 1)
      D "opened"
      if use_ssl?
        if proxy?
          plain_sock = BufferedIO.new(s, read_timeout: @read_timeout,
                                      write_timeout: @write_timeout,
                                      continue_timeout: @continue_timeout,
                                      debug_output: @debug_output)
          buf = "CONNECT #{conn_address}:#{@PORT} HTTP/#{HTTPVersion}\r\n"
          buf << "Host: #{@address}:#{@PORT}\r\n"
          if proxy_user
            credential = ["#{proxy_user}:#{proxy_pass}"].pack('m0')
            buf << "Proxy-Authorization: Basic #{credential}\r\n"
          end
          buf << "\r\n"
          plain_sock.write(buf)
          HTTPResponse.read_new(plain_sock).value
          # assuming nothing left in buffers after successful CONNECT response
        end

        ssl_parameters = Hash.new
        iv_list = instance_variables
        SSL_IVNAMES.each_with_index do |ivname, i|
          if iv_list.include?(ivname)
            value = instance_variable_get(ivname)
            unless value.nil?
              ssl_parameters[SSL_ATTRIBUTES[i]] = value
            end
          end
        end
        @ssl_context.set_params(ssl_parameters)
        @ssl_context.session_cache_mode =
          OpenSSL::SSL::SSLContext::SESSION_CACHE_CLIENT |
          OpenSSL::SSL::SSLContext::SESSION_CACHE_NO_INTERNAL_STORE
        @ssl_context.session_new_cb = proc {|sock, sess| @ssl_session = sess }
        D "starting SSL for #{conn_addr}:#{conn_port}..."
        s = OpenSSL::SSL::SSLSocket.new(s, @ssl_context)
        s.sync_close = true
        # Server Name Indication (SNI) RFC 3546
        s.hostname = @address if s.respond_to? :hostname=
        if @ssl_session and
           Process.clock_gettime(Process::CLOCK_REALTIME) < @ssl_session.time.to_f + @ssl_session.timeout
          s.session = @ssl_session
        end
        ssl_socket_connect(s, @open_timeout)
        if (@ssl_context.verify_mode != OpenSSL::SSL::VERIFY_NONE) && @ssl_context.verify_hostname
          s.post_connection_check(@address)
        end
        D "SSL established, protocol: #{s.ssl_version}, cipher: #{s.cipher[0]}"
      end
      @socket = BufferedIO.new(s, read_timeout: @read_timeout,
                               write_timeout: @write_timeout,
                               continue_timeout: @continue_timeout,
                               debug_output: @debug_output)
      @last_communicated = nil
      on_connect
    rescue => exception
      if s
        D "Conn close because of connect error #{exception}"
        s.close
      end
      raise
    end
    private :connect
  end
end

begin
  sleep 0.2

  connection = Net::HTTP.start(HOST, PORT)
  connection.keep_alive_timeout = 3600
  connection_opt = Net::HTTPOpt.start(HOST, PORT)
  connection_opt.keep_alive_timeout = 3600

  unless connection.request_get("/100k.txt").body == connection_opt.request_get("/100k.txt").body
    abort("bug?")
  end

  if ARGV.first == "profile"
    require 'stackprof'
    require 'json'

    StackProf.run(mode: :wall, out: "/tmp/stackprof-net-http.dump", raw: true) do
      40_000.times do
        connection.request_get("/1M.txt").body
      end
    end
    File.write("/tmp/stackprof-net-http.json", JSON.dump(Marshal.load(File.binread("/tmp/stackprof-net-http.dump"))))
    system("stackprof", "/tmp/stackprof-net-http.rb")

    StackProf.run(mode: :wall, out: "/tmp/stackprof-net-http-opt.dump", raw: true) do
      40_000.times do
        connection_opt.request_get("/1M.txt").body
      end
    end
    File.write("/tmp/stackprof-net-http-opt.json", JSON.dump(Marshal.load(File.binread("/tmp/stackprof-net-http-opt.dump"))))
    system("stackprof", "/tmp/stackprof-net-http-opt.dump")

  else
    %w(1k 10k 100k 1M).each do |size|
      puts "=== #{size} ==="
      Benchmark.ips do |x|
        path = "/#{size}.txt"
        x.report("#{size}") { connection.request_get(path).body }
        x.report("#{size} opt") { connection_opt.request_get(path).body }
        x.compare!(order: :baseline)
      end
      puts
    end
  end
ensure
  Process.kill('TERM', nginx_pid)
  Process.wait(nginx_pid)
end

```

ruby/net-protocol@781e400389
Which restarts scanning the code range in unscanned part.
Moved the contents of `ruby_description` and `ruby_copyright` which
are never used in the other places.
* Better b.cond usage on AArch64

When we're lowering a conditional jump, we previously had a bit of
a complicated setup where we could emit a conditional jump to skip
over a jump that was the next instruction, and then write out the
destination and use a branch register.

Now instead we use the b.cond instruction if our offset fits (not
common, but not unused either) and if it doesn't we write out an
inverse condition to jump past loading the destination and
branching directly.

* Added an inverse fn for Condition (ruby#443)

Prevents the need to pass two params and potentially reduces errors.

Co-authored-by: Jimmy Miller <[email protected]>

Co-authored-by: Maxime Chevalier-Boisvert <[email protected]>
Co-authored-by: Jimmy Miller <[email protected]>
Close ruby#6307

Co-authored-by: Takashi Kokubun <[email protected]>
The "dumb" terminal is considered only on MSys tty now.  However, the
`TERM` feature has been used on many Unix-like systems for decades,
not MSys specific.

ruby/reline@53fd51ab62
@chrisseaton chrisseaton requested a review from wks September 1, 2022 09:24
@chrisseaton chrisseaton self-assigned this Sep 1, 2022
Copy link

@wks wks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@wks wks merged commit e4d3c4b into mmtk Sep 1, 2022
wks pushed a commit that referenced this pull request Aug 4, 2023
[Bug #19793]

Dummy frames are created at the top level when requiring another file.
While requiring a file, it will try to convert using encodings. Some of
these encodings will not respond to to_str. If method_missing is
redefined on Object, then it will call method_missing and attempt raise
an error. However, the iseq is invalid as it's a dummy frame so it will
write an invalid iseq to the created NoMethodError.

The following script crashes:

```
GC.stress = true

class Object
  public :method_missing
end

File.write("/tmp/empty.rb", "")
require "/tmp/empty.rb"
```

With the following backtrace:

```
frame #0: 0x00000001000fa8b8 miniruby`RVALUE_MARKED(obj=4308637824) at gc.c:1638:12
frame #1: 0x00000001000fb440 miniruby`RVALUE_BLACK_P(obj=4308637824) at gc.c:1763:12
frame #2: 0x00000001000facdc miniruby`gc_writebarrier_incremental(a=4308637824, b=4308332208, objspace=0x000000010180b000) at gc.c:8822:9
frame #3: 0x00000001000faad8 miniruby`rb_gc_writebarrier(a=4308637824, b=4308332208) at gc.c:8864:17
frame #4: 0x000000010016aff0 miniruby`rb_obj_written(a=4308637824, oldv=36, b=4308332208, filename="../iseq.c", line=1279) at gc.h:804:9
frame #5: 0x0000000100162a60 miniruby`rb_obj_write(a=4308637824, slot=0x0000000100d09888, b=4308332208, filename="../iseq.c", line=1279) at gc.h:837:5
frame #6: 0x0000000100165b0c miniruby`iseqw_new(iseq=0x0000000100d09880) at iseq.c:1279:9
frame #7: 0x0000000100165a64 miniruby`rb_iseqw_new(iseq=0x0000000100d09880) at iseq.c:1289:12
frame #8: 0x00000001000d8324 miniruby`name_err_init_attr(exc=4309777920, recv=4304780496, method=827660) at error.c:1830:35
frame #9: 0x00000001000d1b80 miniruby`name_err_init(exc=4309777920, mesg=4308332496, recv=4304780496, method=827660) at error.c:1869:12
frame #10: 0x00000001000d1bd4 miniruby`rb_nomethod_err_new(mesg=4308332496, recv=4304780496, method=827660, args=4308332448, priv=0) at error.c:1957:5
frame #11: 0x000000010039049c miniruby`rb_make_no_method_exception(exc=4304914512, format=4308332496, obj=4304780496, argc=1, argv=0x000000016fdfab00, priv=0) at vm_eval.c:959:16
frame #12: 0x00000001003b3274 miniruby`raise_method_missing(ec=0x0000000100b06f40, argc=1, argv=0x000000016fdfab00, obj=4304780496, last_call_status=MISSING_NOENTRY) at vm_eval.c:999:15
frame #13: 0x00000001003945d4 miniruby`rb_method_missing(argc=1, argv=0x000000016fdfab00, obj=4304780496) at vm_eval.c:944:5
...
frame #23: 0x000000010038f5e4 miniruby`rb_vm_call_kw(ec=0x0000000100b06f40, recv=4304780496, id=2865, argc=1, argv=0x000000016fdfab00, me=0x0000000100cbfcf0, kw_splat=0) at vm_eval.c:326:12
frame #24: 0x00000001003c18e4 miniruby`call_method_entry(ec=0x0000000100b06f40, defined_class=4304927952, obj=4304780496, id=2865, cme=0x0000000100cbfcf0, argc=1, argv=0x000000016fdfab00, kw_splat=0) at vm_method.c:2720:20
frame #25: 0x00000001003c440c miniruby`check_funcall_exec(v=6171896792) at vm_eval.c:589:12
frame #26: 0x00000001000dec00 miniruby`rb_vrescue2(b_proc=(miniruby`check_funcall_exec at vm_eval.c:587), data1=6171896792, r_proc=(miniruby`check_funcall_failed at vm_eval.c:596), data2=6171896792, args="Pȗ") at eval.c:919:18
frame #27: 0x00000001000deab0 miniruby`rb_rescue2(b_proc=(miniruby`check_funcall_exec at vm_eval.c:587), data1=6171896792, r_proc=(miniruby`check_funcall_failed at vm_eval.c:596), data2=6171896792) at eval.c:900:17
frame #28: 0x000000010039008c miniruby`check_funcall_missing(ec=0x0000000100b06f40, klass=4304923536, recv=4304780496, mid=3233, argc=0, argv=0x0000000000000000, respond=-1, def=36, kw_splat=0) at vm_eval.c:666:15
frame #29: 0x000000010038fa60 miniruby`rb_check_funcall_default_kw(recv=4304780496, mid=3233, argc=0, argv=0x0000000000000000, def=36, kw_splat=0) at vm_eval.c:703:21
frame #30: 0x000000010038fb04 miniruby`rb_check_funcall(recv=4304780496, mid=3233, argc=0, argv=0x0000000000000000) at vm_eval.c:685:12
frame #31: 0x00000001001c469c miniruby`convert_type_with_id(val=4304780496, tname="String", method=3233, raise=0, index=-1) at object.c:3061:15
frame #32: 0x00000001001c4a4c miniruby`rb_check_convert_type_with_id(val=4304780496, type=5, tname="String", method=3233) at object.c:3153:9
frame #33: 0x00000001002d59f8 miniruby`rb_check_string_type(str=4304780496) at string.c:2571:11
frame #34: 0x000000010014b7b0 miniruby`io_encoding_set(fptr=0x0000000100d09ca0, v1=4304780496, v2=4, opt=4) at io.c:11655:19
frame #35: 0x0000000100139a58 miniruby`rb_io_set_encoding(argc=1, argv=0x000000016fdfb450, io=4308334032) at io.c:13497:5
frame #36: 0x00000001003c0004 miniruby`ractor_safe_call_cfunc_m1(recv=4308334032, argc=1, argv=0x000000016fdfb450, func=(miniruby`rb_io_set_encoding at io.c:13487)) at vm_insnhelper.c:3271:12
...
frame #43: 0x0000000100390b08 miniruby`rb_funcall(recv=4308334032, mid=16593, n=1) at vm_eval.c:1137:12
frame #44: 0x00000001002a43d8 miniruby`load_file_internal(argp_v=6171899936) at ruby.c:2500:5
...
```
wks pushed a commit that referenced this pull request Dec 5, 2024
[Bug #20921]

When we create a cache entry for a constant, the following sequence of
events could happen:

- vm_track_constant_cache is called to insert a constant cache.
- In vm_track_constant_cache, we first look up the ST table for the ID
  of the constant. Assume the ST table exists because another iseq also
  holds a cache entry for this ID.
- We then insert into this ST table with the iseq_inline_constant_cache.
- However, while inserting into this ST table, it allocates memory, which
  could trigger a GC. Assume that it does trigger a GC.
- The GC frees the one and only other iseq that holds a cache entry for
  this ID.
- In remove_from_constant_cache, it will appear that the ST table is now
  empty because there are no more iseq with cache entries for this ID, so
  we free the ST table.
- We complete GC and continue our st_insert. However, this ST table has
  been freed so we now have a use-after-free.

This issue is very hard to reproduce, because it requires that the GC runs
at a very specific time. However, we can make it show up by applying this
patch which runs GC right before the st_insert to mimic the st_insert
triggering a GC:

    diff --git a/vm_insnhelper.c b/vm_insnhelper.c
    index 3cb23f0..a93998136a 100644
    --- a/vm_insnhelper.c
    +++ b/vm_insnhelper.c
    @@ -6338,6 +6338,10 @@ vm_track_constant_cache(ID id, void *ic)
            rb_id_table_insert(const_cache, id, (VALUE)ics);
        }

    +    if (id == rb_intern("MyConstant")) rb_gc();
    +
        st_insert(ics, (st_data_t) ic, (st_data_t) Qtrue);
    }

And if we run this script:

    Object.const_set("MyConstant", "Hello!")

    my_proc = eval("-> { MyConstant }")
    my_proc.call

    my_proc = eval("-> { MyConstant }")
    my_proc.call

We can see that ASAN outputs a use-after-free error:

    ==36540==ERROR: AddressSanitizer: heap-use-after-free on address 0x606000049528 at pc 0x000102f3ceac bp 0x00016d607a70 sp 0x00016d607a68
    READ of size 8 at 0x606000049528 thread T0
        #0 0x102f3cea8 in do_hash st.c:321
        #1 0x102f3ddd0 in rb_st_insert st.c:1132
        #2 0x103140700 in vm_track_constant_cache vm_insnhelper.c:6345
        #3 0x1030b91d8 in vm_ic_track_const_chain vm_insnhelper.c:6356
        #4 0x1030b8cf8 in rb_vm_opt_getconstant_path vm_insnhelper.c:6424
        #5 0x1030bc1e0 in vm_exec_core insns.def:263
        #6 0x1030b55fc in rb_vm_exec vm.c:2585
        #7 0x1030fe0ac in rb_iseq_eval_main vm.c:2851
        #8 0x102a82588 in rb_ec_exec_node eval.c:281
        #9 0x102a81fe0 in ruby_run_node eval.c:319
        #10 0x1027f3db4 in rb_main main.c:43
        #11 0x1027f3bd4 in main main.c:68
        #12 0x183900270  (<unknown module>)

    0x606000049528 is located 8 bytes inside of 56-byte region [0x606000049520,0x606000049558)
    freed by thread T0 here:
        #0 0x104174d40 in free+0x98 (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x54d40)
        #1 0x102ada89c in rb_gc_impl_free default.c:8183
        #2 0x102ada7dc in ruby_sized_xfree gc.c:4507
        #3 0x102ac4d34 in ruby_xfree gc.c:4518
        #4 0x102f3cb34 in rb_st_free_table st.c:663
        #5 0x102bd52d8 in remove_from_constant_cache iseq.c:119
        #6 0x102bbe2cc in iseq_clear_ic_references iseq.c:153
        #7 0x102bbd2a0 in rb_iseq_free iseq.c:166
        #8 0x102b32ed0 in rb_imemo_free imemo.c:564
        #9 0x102ac4b44 in rb_gc_obj_free gc.c:1407
        #10 0x102af4290 in gc_sweep_plane default.c:3546
        #11 0x102af3bdc in gc_sweep_page default.c:3634
        #12 0x102aeb140 in gc_sweep_step default.c:3906
        #13 0x102aeadf0 in gc_sweep_rest default.c:3978
        #14 0x102ae4714 in gc_sweep default.c:4155
        #15 0x102af8474 in gc_start default.c:6484
        #16 0x102afbe30 in garbage_collect default.c:6363
        #17 0x102ad37f0 in rb_gc_impl_start default.c:6816
        #18 0x102ad3634 in rb_gc gc.c:3624
        #19 0x1031406ec in vm_track_constant_cache vm_insnhelper.c:6342
        #20 0x1030b91d8 in vm_ic_track_const_chain vm_insnhelper.c:6356
        #21 0x1030b8cf8 in rb_vm_opt_getconstant_path vm_insnhelper.c:6424
        #22 0x1030bc1e0 in vm_exec_core insns.def:263
        #23 0x1030b55fc in rb_vm_exec vm.c:2585
        #24 0x1030fe0ac in rb_iseq_eval_main vm.c:2851
        #25 0x102a82588 in rb_ec_exec_node eval.c:281
        #26 0x102a81fe0 in ruby_run_node eval.c:319
        #27 0x1027f3db4 in rb_main main.c:43
        #28 0x1027f3bd4 in main main.c:68
        #29 0x183900270  (<unknown module>)

    previously allocated by thread T0 here:
        #0 0x104174c04 in malloc+0x94 (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x54c04)
        #1 0x102ada0ec in rb_gc_impl_malloc default.c:8198
        #2 0x102acee44 in ruby_xmalloc gc.c:4438
        #3 0x102f3c85c in rb_st_init_table_with_size st.c:571
        #4 0x102f3c900 in rb_st_init_table st.c:600
        #5 0x102f3c920 in rb_st_init_numtable st.c:608
        #6 0x103140698 in vm_track_constant_cache vm_insnhelper.c:6337
        #7 0x1030b91d8 in vm_ic_track_const_chain vm_insnhelper.c:6356
        #8 0x1030b8cf8 in rb_vm_opt_getconstant_path vm_insnhelper.c:6424
        #9 0x1030bc1e0 in vm_exec_core insns.def:263
        #10 0x1030b55fc in rb_vm_exec vm.c:2585
        #11 0x1030fe0ac in rb_iseq_eval_main vm.c:2851
        #12 0x102a82588 in rb_ec_exec_node eval.c:281
        #13 0x102a81fe0 in ruby_run_node eval.c:319
        #14 0x1027f3db4 in rb_main main.c:43
        #15 0x1027f3bd4 in main main.c:68
        #16 0x183900270  (<unknown module>)

This commit fixes this bug by adding a inserting_constant_cache_id field
to the VM, which stores the ID that is currently being inserted and, in
remove_from_constant_cache, we don't free the ST table for ID equal to
this one.

Co-Authored-By: Alan Wu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.