diff --git a/.document b/.document index 6106cb76134ec4..6a322ad7542c77 100644 --- a/.document +++ b/.document @@ -43,10 +43,10 @@ lib ext # GC is split between gc.rb and gc/default/default.c -gc/default +gc/default/*.c # For `prism`, ruby code is in lib and c in the prism folder -prism +prism/*.c # rdoc files NEWS.md diff --git a/.github/workflows/zjit-ubuntu.yml b/.github/workflows/zjit-ubuntu.yml index 1f4f663a744f6d..c27b1a775afbee 100644 --- a/.github/workflows/zjit-ubuntu.yml +++ b/.github/workflows/zjit-ubuntu.yml @@ -84,7 +84,6 @@ jobs: run_opts: '--zjit-inline-threshold=0 --zjit-call-threshold=1' specopts: '-T --zjit-inline-threshold=0 -T --zjit-call-threshold=1' configure: '--enable-zjit=dev' - tests: '--exclude=ruby/test_hash.rb' # The optimizer benefits from at least 1 iteration of profiling. Also, many # regression tests in bootstraptest/test_yjit.rb assume call-threshold=2. diff --git a/bootstraptest/test_block.rb b/bootstraptest/test_block.rb index cdc5960a59045a..3056f52c88399e 100644 --- a/bootstraptest/test_block.rb +++ b/bootstraptest/test_block.rb @@ -10,15 +10,18 @@ assert_equal %q{2}, %q{ [1,2,3].find{|x| x == 2} } -assert_equal %q{2}, %q{ - class E - include Enumerable - def each(&block) - [1, 2, 3].each(&block) +# TODO: Remove this clang 17 CI workaround once #17953's regression is fixed. +unless ENV["GITHUB_ACTIONS"] == "true" && ENV["INPUT_WITH_GCC"] == "clang-17" + assert_equal %q{2}, %q{ + class E + include Enumerable + def each(&block) + [1, 2, 3].each(&block) + end end - end - E.new.find {|x| x == 2 } -} + E.new.find {|x| x == 2 } + } +end assert_equal %q{6}, %q{ sum = 0 for x in [1, 2, 3] diff --git a/bootstraptest/test_flow.rb b/bootstraptest/test_flow.rb index 7a95def1e6fb15..c5b0305c5c752c 100644 --- a/bootstraptest/test_flow.rb +++ b/bootstraptest/test_flow.rb @@ -557,9 +557,12 @@ def each(&block) end e = Bug6460.new }]].each do |bug, src| - assert_equal "foo", src + %q{e.detect {true}}, bug - assert_equal "true", src + %q{e.any? {true}}, bug - assert_equal "false", src + %q{e.all? {false}}, bug + # TODO: Remove this clang 17 CI workaround once #17953's regression is fixed. + unless ENV["GITHUB_ACTIONS"] == "true" && ENV["INPUT_WITH_GCC"] == "clang-17" + assert_equal "foo", src + %q{e.detect {true}}, bug + assert_equal "true", src + %q{e.any? {true}}, bug + assert_equal "false", src + %q{e.all? {false}}, bug + end assert_equal "true", src + %q{e.include?(:foo)}, bug end diff --git a/eval_intern.h b/eval_intern.h index d323165107f119..25ea8a9ef1db24 100644 --- a/eval_intern.h +++ b/eval_intern.h @@ -106,11 +106,14 @@ extern int select_large_fdset(int, fd_set *, fd_set *, fd_set *, struct timeval EC_SAVE_TAG_CFP(_tag, _ec); \ rb_vm_tag_jmpbuf_init(&_tag.buf); \ -// Remember the CFP as of EC_PUSH_TAG so that ZJIT can materialize frames -// only up to longjmp's target CFP. When a C method does longjmp inside it, -// the target CFP may not be equal to the VM_FRAME_FLAG_FINISH frame. +// Remember the CFP and whether it was already running in ZJIT as of +// EC_PUSH_TAG. When a C method does longjmp inside it, the target CFP may not +// be equal to the VM_FRAME_FLAG_FINISH frame. If its ZJIT frame predates this +// tag, its native stack survives the jump and should not be materialized. #if USE_ZJIT -# define EC_SAVE_TAG_CFP(_tag, _ec) _tag.cfp = _ec->cfp +# define EC_SAVE_TAG_CFP(_tag, _ec) \ + _tag.cfp = _ec->cfp; \ + _tag.zjit_frame_active = CFP_ZJIT_FRAME_P(_ec->cfp) #else # define EC_SAVE_TAG_CFP(_tag, _ec) #endif @@ -166,7 +169,7 @@ rb_ec_tag_jump(const rb_execution_context_t *ec, enum ruby_tag_type st) { RUBY_ASSERT(st > TAG_NONE && st <= TAG_FATAL, ": Invalid tag jump: %d", (int)st); #if USE_ZJIT - rb_zjit_materialize_frames(ec, ec->cfp); + rb_zjit_materialize_frames_for_longjmp(ec, ec->cfp); #endif ec->tag->state = st; ruby_longjmp(RB_VM_TAG_JMPBUF_GET(ec->tag->buf), 1); diff --git a/internal/struct.h b/internal/struct.h index ffe1786a896bac..fc1041bf1852da 100644 --- a/internal/struct.h +++ b/internal/struct.h @@ -47,6 +47,8 @@ struct RStruct { VALUE rb_struct_init_copy(VALUE copy, VALUE s); VALUE rb_struct_lookup(VALUE s, VALUE idx); VALUE rb_struct_s_keyword_init(VALUE klass); +void rb_struct_define_aref_method(VALUE nstr, ID name, unsigned int off); + static inline long RSTRUCT_EMBED_LEN(VALUE st); static inline long RSTRUCT_LEN_RAW(VALUE st); static inline int RSTRUCT_LENINT(VALUE st); diff --git a/lib/bundler/cli/open.rb b/lib/bundler/cli/open.rb index f24693b84367d6..64d44e6a0b7c0e 100644 --- a/lib/bundler/cli/open.rb +++ b/lib/bundler/cli/open.rb @@ -18,12 +18,22 @@ def run Bundler.ui.info "Unable to open #{name} because it's a default gem, so the directory it would normally be installed to does not exist." else root_path = spec.full_gem_path - require "shellwords" - command = Shellwords.split(editor) << File.join([root_path, path].compact) + command = editor_command(editor) << File.join([root_path, path].compact) Bundler.with_original_env do system(*command, { chdir: root_path }) end || Bundler.ui.info("Could not run '#{command.join(" ")}'") end end + + def editor_command(editor) + # On Windows an editor is often configured with a full path such as + # C:\Program Files\Microsoft VS Code\Code.exe, which shell splitting + # would corrupt. Take a value that names an existing file as a + # single word. + return [editor] if Gem.win_platform? && File.file?(editor) + + require "shellwords" + Shellwords.split(editor) + end end end diff --git a/lib/bundler/runtime.rb b/lib/bundler/runtime.rb index 5280e72aa24b5e..d821dc022b6e32 100644 --- a/lib/bundler/runtime.rb +++ b/lib/bundler/runtime.rb @@ -141,7 +141,7 @@ def cache(custom_path = nil, local = false) end end - Dir[cache_path.join("*/.git")].each do |git_dir| + Gem::Util.glob_files_in_dir("*/.git", cache_path.to_s).each do |git_dir| FileUtils.rm_rf(git_dir) FileUtils.touch(File.expand_path("../.bundlecache", git_dir)) end @@ -159,13 +159,13 @@ def prune_cache(cache_path) end def clean(dry_run = false) - gem_bins = Dir["#{Gem.dir}/bin/*"] - git_dirs = Dir["#{Gem.dir}/bundler/gems/*"] - git_cache_dirs = Dir["#{Gem.dir}/cache/bundler/git/*"] - gem_dirs = Dir["#{Gem.dir}/gems/*"] - gem_files = Dir["#{Gem.dir}/cache/*.gem"] - gemspec_files = Dir["#{Gem.dir}/specifications/*.gemspec"] - extension_dirs = Dir["#{Gem.dir}/extensions/*/*/*"] + Dir["#{Gem.dir}/bundler/gems/extensions/*/*/*"] + gem_bins = Gem::Util.glob_files_in_dir("bin/*", Gem.dir) + git_dirs = Gem::Util.glob_files_in_dir("bundler/gems/*", Gem.dir) + git_cache_dirs = Gem::Util.glob_files_in_dir("cache/bundler/git/*", Gem.dir) + gem_dirs = Gem::Util.glob_files_in_dir("gems/*", Gem.dir) + gem_files = Gem::Util.glob_files_in_dir("cache/*.gem", Gem.dir) + gemspec_files = Gem::Util.glob_files_in_dir("specifications/*.gemspec", Gem.dir) + extension_dirs = Gem::Util.glob_files_in_dir("extensions/*/*/*", Gem.dir) + Gem::Util.glob_files_in_dir("bundler/gems/extensions/*/*/*", Gem.dir) spec_gem_paths = [] # need to keep git sources around spec_git_paths = @definition.spec_git_paths @@ -232,7 +232,7 @@ def clean(dry_run = false) private def prune_gem_cache(resolve, cache_path) - cached = Dir["#{cache_path}/*.gem"] + cached = Gem::Util.glob_files_in_dir("*.gem", cache_path.to_s) cached = cached.delete_if do |path| spec = Bundler.rubygems.spec_from_gem path @@ -257,7 +257,7 @@ def prune_gem_cache(resolve, cache_path) end def prune_git_and_path_cache(resolve, cache_path) - cached = Dir["#{cache_path}/*/.bundlecache"] + cached = Gem::Util.glob_files_in_dir("*/.bundlecache", cache_path.to_s) cached = cached.delete_if do |path| name = File.basename(File.dirname(path)) @@ -283,7 +283,7 @@ def setup_manpath # Add man/ subdirectories from activated bundles to MANPATH for man(1) manuals = $LOAD_PATH.filter_map do |path| man_subdir = path.sub(/lib$/, "man") - man_subdir unless Dir[man_subdir + "/man?/"].empty? + man_subdir unless Dir.glob("man?/", base: man_subdir).empty? end return if manuals.empty? diff --git a/lib/bundler/self_manager.rb b/lib/bundler/self_manager.rb index 82efbf56a4fb80..4e6156ffa2f56d 100644 --- a/lib/bundler/self_manager.rb +++ b/lib/bundler/self_manager.rb @@ -75,7 +75,12 @@ def restart_with(version) argv0 = File.exist?($PROGRAM_NAME) ? $PROGRAM_NAME : Process.argv0 cmd = [argv0, *ARGV] - cmd.unshift(Gem.ruby) unless File.executable?(argv0) + unless File.executable?(argv0) + # Gem.ruby is quoted if it contains whitespace, so split it into argv + # elements to keep the quotes out of the exec'd command. + require "shellwords" + cmd.unshift(*Shellwords.split(Gem.ruby)) + end Bundler.with_original_env do Kernel.exec( diff --git a/lib/bundler/shared_helpers.rb b/lib/bundler/shared_helpers.rb index 2aa8abe0a078b0..2c22103e3066b3 100644 --- a/lib/bundler/shared_helpers.rb +++ b/lib/bundler/shared_helpers.rb @@ -347,7 +347,12 @@ def set_path def set_rubyopt rubyopt = [ENV["RUBYOPT"]].compact - setup_require = "-r#{File.expand_path("setup", __dir__)}" + setup_path = File.expand_path("setup", __dir__) + # RUBYOPT is split on whitespace with no quoting mechanism, so an + # absolute path containing spaces would be torn apart. Fall back to + # requiring by feature name; set_rubylib puts our lib directory first + # on the child's load path. + setup_require = /\s/.match?(setup_path) ? "-rbundler/setup" : "-r#{setup_path}" return if !rubyopt.empty? && rubyopt.first.include?(setup_require) rubyopt.unshift setup_require Bundler::SharedHelpers.set_env "RUBYOPT", rubyopt.join(" ") diff --git a/lib/bundler/source/git.rb b/lib/bundler/source/git.rb index a002a2570a8a25..ae6fd5ecdda419 100644 --- a/lib/bundler/source/git.rb +++ b/lib/bundler/source/git.rb @@ -325,7 +325,7 @@ def humanized_ref def serialize_gemspecs_in(destination) destination = destination.expand_path(Bundler.root) if destination.relative? - Dir["#{destination}/#{@glob}"].each do |spec_path| + Gem::Util.glob_files_in_dir(@glob, destination.to_s).each do |spec_path| # Evaluate gemspecs and cache the result. Gemspecs # in git might require git or other dependencies. # The gemspecs we cache should already be evaluated. diff --git a/lib/bundler/source/rubygems.rb b/lib/bundler/source/rubygems.rb index 0a0914f6c6e328..8c21864b08280a 100644 --- a/lib/bundler/source/rubygems.rb +++ b/lib/bundler/source/rubygems.rb @@ -406,7 +406,7 @@ def cached_specs @cached_specs ||= begin idx = Index.new - Dir["#{cache_path}/*.gem"].each do |gemfile| + Gem::Util.glob_files_in_dir("*.gem", cache_path.to_s).each do |gemfile| s ||= Bundler.rubygems.spec_from_gem(gemfile) s.source = self idx << s diff --git a/lib/rubygems/basic_specification.rb b/lib/rubygems/basic_specification.rb index 0ed7fc60bbe475..61d35307f4ea13 100644 --- a/lib/rubygems/basic_specification.rb +++ b/lib/rubygems/basic_specification.rb @@ -306,9 +306,7 @@ def source_paths # Return all files in this gem that match for +glob+. def matches_for_glob(glob) # TODO: rename? - glob = File.join(lib_dirs_glob, glob) - - Dir[glob] + Gem::Util.glob_files_in_dir(File.join(lib_dirs, glob), full_gem_path) end ## @@ -323,17 +321,7 @@ def plugins # for this spec. def lib_dirs_glob - dirs = if raw_require_paths - if raw_require_paths.size > 1 - "{#{raw_require_paths.join(",")}}" - else - raw_require_paths.first - end - else - "lib" # default value for require_paths for bundler/inline - end - - "#{full_gem_path}/#{dirs}" + "#{full_gem_path}/#{lib_dirs}" end ## @@ -364,6 +352,22 @@ def this private + ## + # Returns the require_paths of this gem as a string usable in Dir.glob, + # relative to full_gem_path. + + def lib_dirs + if raw_require_paths + if raw_require_paths.size > 1 + "{#{raw_require_paths.join(",")}}" + else + raw_require_paths.first + end + else + "lib" # default value for require_paths for bundler/inline + end + end + def have_extensions? !extensions.empty? end diff --git a/lib/rubygems/commands/open_command.rb b/lib/rubygems/commands/open_command.rb index 0fe90dc8b83586..d59237d3b99bf5 100644 --- a/lib/rubygems/commands/open_command.rb +++ b/lib/rubygems/commands/open_command.rb @@ -70,7 +70,18 @@ def open_gem(name) end def open_editor(path) - system(*@editor.split(/\s+/) + [path], { chdir: path }) + system(*editor_command(@editor), path, { chdir: path }) + end + + def editor_command(editor) # :nodoc: + # On Windows an editor is often configured with a full path such as + # C:\Program Files\Microsoft VS Code\Code.exe, which shell splitting + # would corrupt. Take a value that names an existing file as a + # single word. + return [editor] if Gem.win_platform? && File.file?(editor) + + require "shellwords" + Shellwords.split(editor) end def spec_for(name) diff --git a/lib/rubygems/commands/push_command.rb b/lib/rubygems/commands/push_command.rb index 02931b30252a7b..78fb844eb9634d 100644 --- a/lib/rubygems/commands/push_command.rb +++ b/lib/rubygems/commands/push_command.rb @@ -147,6 +147,7 @@ def send_push_request_with_attestation(name, args) def attest!(name) require "open3" + require "shellwords" require "tempfile" tempfile = Tempfile.new([File.basename(name, ".*"), ".sigstore.json"]) @@ -154,9 +155,11 @@ def attest!(name) tempfile.close(false) env = defined?(Bundler.unbundled_env) ? Bundler.unbundled_env : ENV.to_h + # Gem.ruby is quoted if it contains whitespace, so split it into argv + # elements to keep the quotes out of the spawned command. out, st = Open3.capture2e( env, - Gem.ruby, "-S", "gem", "exec", "--conservative", + *Shellwords.split(Gem.ruby), "-S", "gem", "exec", "--conservative", "sigstore-cli", "sign", name, "--bundle", bundle, unsetenv_others: true ) diff --git a/lib/rubygems/commands/update_command.rb b/lib/rubygems/commands/update_command.rb index d9740d814a2e70..cb0a2660dde47d 100644 --- a/lib/rubygems/commands/update_command.rb +++ b/lib/rubygems/commands/update_command.rb @@ -183,7 +183,10 @@ def install_rubygems(spec) # :nodoc: say "Installing RubyGems #{version}" unless options[:silent] installed = preparing_gem_layout_for(version) do - system Gem.ruby, "--disable-gems", "setup.rb", *args + # Gem.ruby is quoted if it contains whitespace, so split it into argv + # elements to keep the quotes out of the spawned command. + require "shellwords" + system(*Shellwords.split(Gem.ruby), "--disable-gems", "setup.rb", *args) end unless options[:silent] diff --git a/lib/rubygems/ext/builder.rb b/lib/rubygems/ext/builder.rb index 53a02f61f31b0d..f5040d98f29e7e 100644 --- a/lib/rubygems/ext/builder.rb +++ b/lib/rubygems/ext/builder.rb @@ -32,7 +32,7 @@ def self.make(dest_path, results, make_dir = Dir.pwd, sitedir = nil, targets = [ target_rbconfig["configure_args"] =~ /with-make-prog\=(\w+)/ make_program_name = ENV["MAKE"] || ENV["make"] || $1 make_program_name ||= RUBY_PLATFORM.include?("mswin") ? "nmake" : "make" - make_program = shellsplit(make_program_name) + make_program = shellsplit_command(make_program_name) is_nmake = /\bnmake/i.match?(make_program_name) # The installation of the bundled gems is failed when DESTDIR is empty in mswin platform. @@ -102,6 +102,10 @@ def self.run(command, results, command_name = nil, dir = Dir.pwd, env = {}) require "open3" # Set $SOURCE_DATE_EPOCH for the subprocess. build_env = { "SOURCE_DATE_EPOCH" => Gem.source_date_epoch_string }.merge(env) + # A single-element command would be parsed as a shell command line, + # splitting an unquoted command path containing spaces. Use the + # [cmdname, argv0] form to keep exec semantics. + command = [[command.first, command.first]] if command.size == 1 output, status = begin Open3.popen2e(build_env, *command, chdir: dir) do |stdin, stdouterr, wait_thread| stdin.close @@ -148,6 +152,21 @@ def self.shellsplit(command) Shellwords.split(command) end + ## + # Splits a command string such as ENV["MAKE"] into an argument list. + # + # On Windows, a value that names an existing file, such as + # C:\path\nmake.exe, is taken as a single word because POSIX + # shell splitting would consume the backslashes as escape characters. + # Any other value is split like a POSIX shell command line, where a + # path containing spaces must be quoted. + + def self.shellsplit_command(command) + return [command] if Gem.win_platform? && File.file?(command) + + shellsplit(command) + end + def self.shelljoin(command) require "shellwords" diff --git a/lib/rubygems/ext/rake_builder.rb b/lib/rubygems/ext/rake_builder.rb index d702d7f3393757..fa5b55bb4d35df 100644 --- a/lib/rubygems/ext/rake_builder.rb +++ b/lib/rubygems/ext/rake_builder.rb @@ -14,13 +14,15 @@ def self.build(extension, dest_path, results, args = [], lib_dir = nil, extensio end if /mkrf_conf/i.match?(File.basename(extension)) - run([Gem.ruby, File.basename(extension), *args], results, class_name, extension_dir) + # Gem.ruby is quoted if it contains whitespace, so split it into argv + # elements to keep the quotes out of the spawned command. + run([*shellsplit(Gem.ruby), File.basename(extension), *args], results, class_name, extension_dir) end rake = ENV["rake"] if rake - rake = shellsplit(rake) + rake = shellsplit_command(rake) else begin rake = ruby << "-rrubygems" << Gem.bin_path("rake", "rake") diff --git a/lib/rubygems/util.rb b/lib/rubygems/util.rb index ee4106c6cea0c3..526c612b40c877 100644 --- a/lib/rubygems/util.rb +++ b/lib/rubygems/util.rb @@ -76,7 +76,9 @@ def self.traverse_parents(directory, &block) ## # Globs for files matching +pattern+ inside of +directory+, - # returning absolute paths to the matching files. + # returning absolute paths to the matching files. Unlike a plain + # Dir.glob with an interpolated path, glob metacharacters in + # +base_path+ are not treated as part of the pattern. def self.glob_files_in_dir(glob, base_path) Dir.glob(glob, base: base_path).map! {|f| File.expand_path(f, base_path) } diff --git a/range.c b/range.c index e751c282c32964..18d74c54a4a6e0 100644 --- a/range.c +++ b/range.c @@ -1362,13 +1362,6 @@ range_reverse_each(VALUE range) * Related: Range#first, Range#end. */ -static VALUE -range_begin(VALUE range) -{ - return RANGE_BEG(range); -} - - /* * call-seq: * self.end -> object @@ -1382,14 +1375,6 @@ range_begin(VALUE range) * Related: Range#begin, Range#last. */ - -static VALUE -range_end(VALUE range) -{ - return RANGE_END(range); -} - - static VALUE first_i(RB_BLOCK_CALL_FUNC_ARGLIST(i, cbarg)) { @@ -2980,8 +2965,8 @@ Init_Range(void) rb_define_method(rb_cRange, "%", range_percent_step, 1); rb_define_method(rb_cRange, "reverse_each", range_reverse_each, 0); rb_define_method(rb_cRange, "bsearch", range_bsearch, 0); - rb_define_method(rb_cRange, "begin", range_begin, 0); - rb_define_method(rb_cRange, "end", range_end, 0); + rb_struct_define_aref_method(rb_cRange, id_beg, 0); + rb_struct_define_aref_method(rb_cRange, id_end, 1); rb_define_method(rb_cRange, "first", range_first, -1); rb_define_method(rb_cRange, "last", range_last, -1); rb_define_method(rb_cRange, "min", range_min, -1); diff --git a/shape.c b/shape.c index 7815591d2d8877..e9037acf6a3ccf 100644 --- a/shape.c +++ b/shape.c @@ -1348,8 +1348,15 @@ rb_shape_verify_consistency(VALUE obj, shape_id_t shape_id) size_t shape_id_slot_size = shape_id_capacity * sizeof(VALUE) + sizeof(struct RBasic); size_t actual_slot_size = rb_gc_obj_slot_size(obj); - if (shape_id_slot_size != actual_slot_size) { - rb_bug("shape_id capacity flags mismatch: shape_id_slot_size=%zu, gc_slot_size=%zu\n", shape_id_slot_size, actual_slot_size); + if (shape_id_capacity == SHAPE_ID_CAPACITY_MAX) { + if (actual_slot_size < SHAPE_ID_CAPACITY_MAX) { + rb_bug("shape_id_capacity is SHAPE_ID_CAPACITY_MAX, but actual slot size is only %zu", actual_slot_size); + } + } + else { + if (shape_id_slot_size != actual_slot_size) { + rb_bug("shape_id capacity flags mismatch: shape_id_slot_size=%zu, gc_slot_size=%zu\n", shape_id_slot_size, actual_slot_size); + } } } diff --git a/shape.h b/shape.h index 14b9a00bcc8ea5..68fd4a7c9f332c 100644 --- a/shape.h +++ b/shape.h @@ -338,14 +338,26 @@ static inline size_t rb_obj_shape_slot_size(VALUE obj) { RUBY_ASSERT(!RB_TYPE_P(obj, T_IMEMO) || IMEMO_TYPE_P(obj, imemo_fields)); - return rb_shape_slot_size(RBASIC_SHAPE_ID(obj)); + + shape_id_t shape_id = RBASIC_SHAPE_ID(obj); + size_t slot_size = rb_shape_slot_size(shape_id); + + if (rb_shape_embedded_capacity(shape_id) == SHAPE_ID_CAPACITY_MAX) { + size_t gc_slot_size = rb_gc_obj_slot_size(obj); + RUBY_ASSERT(gc_slot_size >= slot_size); + return gc_slot_size; + } + + return slot_size; } static inline attr_index_t rb_shape_capacity_for_slot_size(size_t slot_size) { size_t capacity = (slot_size - sizeof(struct RBasic)) / sizeof(VALUE); - RUBY_ASSERT(capacity <= SHAPE_ID_CAPACITY_MAX); + if (capacity > SHAPE_ID_CAPACITY_MAX) { + capacity = SHAPE_ID_CAPACITY_MAX; + } return (attr_index_t)capacity; } diff --git a/spec/bundler/bundler/cli/open_spec.rb b/spec/bundler/bundler/cli/open_spec.rb new file mode 100644 index 00000000000000..bb8c2b59de106c --- /dev/null +++ b/spec/bundler/bundler/cli/open_spec.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +require "bundler/cli" +require "bundler/cli/open" + +RSpec.describe Bundler::CLI::Open do + subject { described_class.new({}, "rack") } + + describe "#editor_command" do + it "takes an editor path that names an existing file as a single word on Windows" do + editor = File.join(Dir.mktmpdir, "editor.exe") + FileUtils.touch editor + allow(Gem).to receive(:win_platform?).and_return(true) + + expect(subject.editor_command(editor)).to eq([editor]) + end + + it "keeps backslashes in a quoted path" do + expect(subject.editor_command('"C:\Program Files\Microsoft VS Code\Code.exe" -w')). + to eq(['C:\Program Files\Microsoft VS Code\Code.exe', "-w"]) + end + + it "splits an editor with arguments" do + expect(subject.editor_command("code -w")).to eq(["code", "-w"]) + end + + it "splits an existing path on POSIX" do + dir = Dir.mktmpdir + FileUtils.mkdir_p File.join(dir, "editor dir") + editor = File.join(dir, "editor dir", "editor") + FileUtils.touch editor + allow(Gem).to receive(:win_platform?).and_return(false) + + expect(subject.editor_command(editor)).to eq(editor.split(" ")) + end + end +end diff --git a/spec/bundler/bundler/self_manager_spec.rb b/spec/bundler/bundler/self_manager_spec.rb new file mode 100644 index 00000000000000..68f5dd55d9db0e --- /dev/null +++ b/spec/bundler/bundler/self_manager_spec.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +RSpec.describe Bundler::SelfManager do + describe "#restart_with" do + # When the running Ruby lives under a path containing whitespace, Gem.ruby + # returns a quoted string. That quoting must not leak into the argv passed + # to Kernel.exec, or the interpreter can't be found and the auto-switch to + # the locked bundler version fails to spawn. + it "does not embed quotes in the ruby executable when Gem.ruby is quoted" do + manager = described_class.new + + allow(Gem).to receive(:ruby).and_return('"/path with space/bin/ruby"') + + # Force the branch that prepends Gem.ruby to the command. + allow(File).to receive(:executable?).and_return(false) + allow(Bundler).to receive(:with_original_env).and_yield + + captured = nil + allow(Kernel).to receive(:exec) do |_env, *cmd| + captured = cmd + end + + manager.send(:restart_with, Gem::Version.new("1.0.0")) + + expect(captured.first).to eq("/path with space/bin/ruby") + expect(captured.first).not_to include('"') + end + end +end diff --git a/spec/bundler/bundler/shared_helpers_spec.rb b/spec/bundler/bundler/shared_helpers_spec.rb index ab89d280a22416..1619b0a14a5982 100644 --- a/spec/bundler/bundler/shared_helpers_spec.rb +++ b/spec/bundler/bundler/shared_helpers_spec.rb @@ -399,6 +399,34 @@ it_behaves_like "ENV['RUBYOPT'] gets set correctly" end + context "when bundler install path contains whitespace" do + let(:install_path) { "/opt/ruby with space/lib" } + + before do + allow(File).to receive(:expand_path).and_return("#{install_path}/bundler/setup") + allow(Gem).to receive(:bin_path).and_return("#{install_path}/bundler/setup") + end + + # RUBYOPT is split on whitespace with no quoting mechanism, so an + # absolute -r path containing spaces would make every child ruby fail + # to boot with "invalid switch in RUBYOPT". + it "requires bundler/setup by feature name instead of absolute path" do + subject.set_bundle_environment + expect(ENV["RUBYOPT"].split(" ")).to start_with("-rbundler/setup") + end + + it "does not inject the space-containing path into RUBYOPT" do + subject.set_bundle_environment + expect(ENV["RUBYOPT"]).not_to include(install_path) + end + + it "is idempotent" do + subject.set_bundle_environment + subject.set_bundle_environment + expect(ENV["RUBYOPT"].split(" ").grep(%r{\A-r.*bundler/setup\z}).length).to eq(1) + end + end + context "ENV['RUBYLIB'] does not exist" do before { ENV.delete("RUBYLIB") } diff --git a/spec/bundler/commands/clean_spec.rb b/spec/bundler/commands/clean_spec.rb index c77859d3781205..9d8bfa3b59af4b 100644 --- a/spec/bundler/commands/clean_spec.rb +++ b/spec/bundler/commands/clean_spec.rb @@ -46,6 +46,34 @@ def should_not_have_gems(*gems) expect(vendored_gems("bin/myrackup")).to exist end + it "removes unused gems when the bundle path contains glob metacharacters" do + gemfile <<-G + source "https://gem.repo1" + + gem "thin" + gem "foo" + G + + bundle_config "path vendor/dir[1]" + bundle_config "clean false" + bundle "install" + + gemfile <<-G + source "https://gem.repo1" + + gem "thin" + G + bundle "install" + + bundle :clean + + expect(out).to include("Removing foo (1.0)") + + metachar_vendored_gems = scoped_gem_path(bundled_app("vendor/dir[1]")) + expect(metachar_vendored_gems.join("gems/foo-1.0")).not_to exist + expect(metachar_vendored_gems.join("gems/thin-1.0")).to exist + end + it "removes old version of gem if unused" do gemfile <<-G source "https://gem.repo1" diff --git a/spec/bundler/install/gemfile/git_spec.rb b/spec/bundler/install/gemfile/git_spec.rb index bed0a261a95404..a648344d600d98 100644 --- a/spec/bundler/install/gemfile/git_spec.rb +++ b/spec/bundler/install/gemfile/git_spec.rb @@ -122,6 +122,26 @@ expect(file_code.strip).to eq(ruby_code) end + it "caches the evaluated gemspec when the bundle path contains glob metacharacters" do + bundle_config "path vendor/dir[1]" + install_base_gemfile + git = update_git "foo" do |s| + s.executables = ["foobar"] # we added this the first time, so keep it now + s.files = ["bin/foobar"] # updating git nukes the files list + foospec = s.to_ruby.gsub(/s\.files.*/, 's.files = `git ls-files -z`.split("\x0")') + s.write "foo.gemspec", foospec + end + + bundle "update foo" + + sha = git.ref_for("main", 11) + spec_file = scoped_gem_path(bundled_app("vendor/dir[1]")).join("bundler/gems/foo-1.0-#{sha}/foo.gemspec") + expect(spec_file).to exist + ruby_code = Gem::Specification.load(spec_file.to_s).to_ruby + file_code = File.read(spec_file) + expect(file_code).to eq(ruby_code) + end + it "does not update the git source implicitly" do install_base_gemfile update_git "foo" diff --git a/spec/bundler/support/shards.rb b/spec/bundler/support/shards.rb index 9fd67f921b4ff7..72b1a4a1b7f6dd 100644 --- a/spec/bundler/support/shards.rb +++ b/spec/bundler/support/shards.rb @@ -203,6 +203,7 @@ module Shards "spec/install/path_spec.rb", "spec/bundler/fetcher/gem_remote_fetcher_local_ssl_server_spec.rb", "spec/bundler/plugin/unloaded_source_spec.rb", + "spec/bundler/cli/open_spec.rb", ], }.freeze end diff --git a/struct.c b/struct.c index 0c6d7330dfc590..c38171b0c2fe88 100644 --- a/struct.c +++ b/struct.c @@ -285,6 +285,12 @@ define_aref_method(VALUE nstr, VALUE name, VALUE off) rb_add_method_optimized(nstr, SYM2ID(name), OPTIMIZED_METHOD_TYPE_STRUCT_AREF, FIX2UINT(off), METHOD_VISI_PUBLIC); } +void +rb_struct_define_aref_method(VALUE nstr, ID name, unsigned int off) +{ + rb_add_method_optimized(nstr, name, OPTIMIZED_METHOD_TYPE_STRUCT_AREF, off, METHOD_VISI_PUBLIC); +} + static void define_aset_method(VALUE nstr, VALUE name, VALUE off) { diff --git a/test/ruby/test_yjit.rb b/test/ruby/test_yjit.rb index 33f13511cb370a..97bf24bcb8a685 100644 --- a/test/ruby/test_yjit.rb +++ b/test/ruby/test_yjit.rb @@ -644,16 +644,15 @@ def foo(obj) RUBY end - STRUCT_MAX_EMBEDDED_MEMBERS = ( - GC::INTERNAL_CONSTANTS[:RVARGC_MAX_ALLOCATE_SIZE] - - GC::INTERNAL_CONSTANTS[:RBASIC_SIZE] - - GC::INTERNAL_CONSTANTS[:RVALUE_OVERHEAD] - ) / RbConfig::SIZEOF["void*"] - def test_spilled_struct_aref omit("FIXME: https://github.com/Shopify/ruby/issues/977") + struct_max_embedded_members = ( + GC::INTERNAL_CONSTANTS[:RVARGC_MAX_ALLOCATE_SIZE] - + GC::INTERNAL_CONSTANTS[:RBASIC_SIZE] - + GC::INTERNAL_CONSTANTS[:RVALUE_OVERHEAD] + ) / RbConfig::SIZEOF["void*"] assert_compiles(<<~RUBY) - LargeStruct = Struct.new(:foo, :bar, *(#{STRUCT_MAX_EMBEDDED_MEMBERS} - 2).times.map { :"m_\#{it}" }) + LargeStruct = Struct.new(:foo, :bar, *(#{struct_max_embedded_members} - 2).times.map { :"m_\#{it}" }) def foo(obj) foo = obj.foo diff --git a/test/rubygems/test_gem_commands_open_command.rb b/test/rubygems/test_gem_commands_open_command.rb index addc7427e2dcb5..3a774a9343c08b 100644 --- a/test/rubygems/test_gem_commands_open_command.rb +++ b/test/rubygems/test_gem_commands_open_command.rb @@ -43,6 +43,34 @@ def test_execute assert_equal "", @ui.output end + def test_editor_command_windows_path + win_platform = Gem.win_platform? + + editor = if win_platform + FileUtils.mkdir_p File.join(@tempdir, "editor dir") + File.join(@tempdir, "editor dir", "editor.exe").tr("/", "\\") + else + # backslashes and spaces are plain filename characters on POSIX + File.join(@tempdir, 'C:\editor dir\editor.exe') + end + FileUtils.touch editor + + Gem.win_platform = true + + assert_equal [editor], @cmd.editor_command(editor) + ensure + Gem.win_platform = win_platform + end + + def test_editor_command_quoted_path + assert_equal ['C:\editor dir\editor.exe', "-w"], + @cmd.editor_command('"C:\editor dir\editor.exe" -w') + end + + def test_editor_command_with_arguments + assert_equal %w[code -w], @cmd.editor_command("code -w") + end + def test_wrong_version @cmd.options[:version] = "4.0" @cmd.options[:args] = %w[foo] diff --git a/test/rubygems/test_gem_commands_push_command.rb b/test/rubygems/test_gem_commands_push_command.rb index ada95e89b4a85d..f8bb09d60062ff 100644 --- a/test/rubygems/test_gem_commands_push_command.rb +++ b/test/rubygems/test_gem_commands_push_command.rb @@ -219,6 +219,36 @@ def test_execute_attestation_skipped_on_jruby end end + # When the running Ruby lives under a path containing whitespace, Gem.ruby + # returns a quoted string. That quoting must not leak into the argv passed to + # Open3.capture2e, or signing spawns fail and push silently falls back to an + # unsigned upload. + def test_attest_unquotes_ruby + require "open3" + + fake_status = Object.new + def fake_status.success? + true + end + + captured = nil + capture_stub = lambda do |*args, **_kwargs| + captured = args + ["", fake_status] + end + Gem.stub(:ruby, '"/path with space/bin/ruby"') do + Open3.stub(:capture2e, capture_stub) do + @cmd.send(:attest!, @path) + end + end + + refute_nil captured, "signing command should have been spawned" + # captured[0] is the env hash; the interpreter follows it. + assert_equal "/path with space/bin/ruby", captured[1] + refute_includes captured[1], '"' + assert_equal "-S", captured[2] + end + def test_execute_allowed_push_host @spec, @path = util_gem "freebird", "1.0.1" do |spec| spec.metadata["allowed_push_host"] = "https://privategemserver.example" diff --git a/test/rubygems/test_gem_commands_update_command.rb b/test/rubygems/test_gem_commands_update_command.rb index 5bc36d61035e33..89b56e0f7cfd30 100644 --- a/test/rubygems/test_gem_commands_update_command.rb +++ b/test/rubygems/test_gem_commands_update_command.rb @@ -816,6 +816,36 @@ def test_update_gem_unresolved_dependency assert_empty @cmd.updated end + # When the running Ruby lives under a path containing whitespace, Gem.ruby + # returns a quoted string. That quoting must not leak into the argv passed to + # the non-shell `system` call that runs setup.rb, or the interpreter can't be + # found and `gem update --system` fails to spawn. + def test_install_rubygems_unquotes_ruby + version = Gem::Version.new("99.0.0") + update_dir = File.join @tempdir, "gems", "rubygems-update-#{version}" + FileUtils.mkdir_p update_dir + + spec = Struct.new(:version, :base_dir).new(version, @tempdir) + + captured = nil + @cmd.define_singleton_method(:system) do |*args| + captured = args + true + end + + @cmd.options[:silent] = true + + Gem.stub(:ruby, '"/path with space/bin/ruby"') do + @cmd.install_rubygems(spec) + end + + refute_nil captured, "setup.rb should have been spawned" + assert_equal "/path with space/bin/ruby", captured.first + refute_includes captured.first, '"' + assert_includes captured, "--disable-gems" + assert_includes captured, "setup.rb" + end + def test_update_rubygems_arguments @cmd.options[:system] = true diff --git a/test/rubygems/test_gem_ext_builder.rb b/test/rubygems/test_gem_ext_builder.rb index 37204f3c472a7f..040877338bb37d 100644 --- a/test/rubygems/test_gem_ext_builder.rb +++ b/test/rubygems/test_gem_ext_builder.rb @@ -106,6 +106,78 @@ def test_custom_make_with_options assert_match(/install: OK/, results) end + def test_class_shellsplit_command_windows_path + win_platform = Gem.win_platform? + + make = if win_platform + FileUtils.mkdir_p File.join(@tempdir, "make dir") + File.join(@tempdir, "make dir", "nmake.exe").tr("/", "\\") + else + # backslashes and spaces are plain filename characters on POSIX + File.join(@tempdir, 'C:\make dir\nmake.exe') + end + FileUtils.touch make + + Gem.win_platform = true + + assert_equal [make], Gem::Ext::Builder.shellsplit_command(make) + ensure + Gem.win_platform = win_platform + end + + def test_class_shellsplit_command_quoted_path + assert_equal ['C:\make dir\nmake.exe'], + Gem::Ext::Builder.shellsplit_command('"C:\make dir\nmake.exe"') + end + + def test_class_shellsplit_command_with_arguments + win_platform = Gem.win_platform? + + Gem.win_platform = true + + assert_equal %w[make -j4], Gem::Ext::Builder.shellsplit_command("make -j4") + + Gem.win_platform = false + + assert_equal %w[make -j4], Gem::Ext::Builder.shellsplit_command("make -j4") + ensure + Gem.win_platform = win_platform + end + + def test_class_shellsplit_command_existing_path_on_posix + win_platform = Gem.win_platform? + + FileUtils.mkdir_p File.join(@tempdir, "make dir") + make = File.join(@tempdir, "make dir", "make") + FileUtils.touch make + + Gem.win_platform = false + + assert_equal make.split(" "), Gem::Ext::Builder.shellsplit_command(make) + ensure + Gem.win_platform = win_platform + end + + def test_class_run_single_element_command_with_spaces + dir = File.join @tempdir, "cmd dir" + FileUtils.mkdir_p dir + + if Gem.win_platform? + command = File.join dir, "say.cmd" + File.write command, "@echo spaces-ok\r\n" + else + command = File.join dir, "say" + File.write command, "#!/bin/sh\necho spaces-ok\n" + FileUtils.chmod 0o755, command + end + + results = [] + + Gem::Ext::Builder.run([command], results) + + assert_match(/spaces-ok/, results.join) + end + def test_class_run_closes_stdin results = [] check_stdin_script = <<~'RUBY' diff --git a/test/rubygems/test_gem_ext_rake_builder.rb b/test/rubygems/test_gem_ext_rake_builder.rb index 68ad15b044f0a2..0176af3abbf370 100644 --- a/test/rubygems/test_gem_ext_rake_builder.rb +++ b/test/rubygems/test_gem_ext_rake_builder.rb @@ -101,6 +101,24 @@ def test_class_build_fail end end + # When the running Ruby lives under a path containing whitespace, Gem.ruby + # returns a quoted string. That quoting must not leak into the argv passed to + # the non-shell spawn that runs mkrf_conf.rb, or the interpreter can't be found. + def test_class_build_mkrf_conf_unquotes_ruby + commands = [] + + Gem.stub(:ruby, '"/path with space/bin/ruby"') do + Gem::Ext::RakeBuilder.stub(:run, ->(command, *) { commands << command }) do + Gem::Ext::RakeBuilder.build "mkrf_conf.rb", @dest_path, [], [], nil, @ext + end + end + + mkrf_command = commands.find {|command| command.include?("mkrf_conf.rb") } + refute_nil mkrf_command, "mkrf_conf.rb should have been spawned" + assert_equal "/path with space/bin/ruby", mkrf_command.first + refute_includes mkrf_command.first, '"' + end + def create_temp_mkrf_file(rakefile_content) File.open File.join(@ext, "mkrf_conf.rb"), "w" do |mkrf_conf| mkrf_conf.puts <<-EO_MKRF diff --git a/test/rubygems/test_gem_stub_specification.rb b/test/rubygems/test_gem_stub_specification.rb index 6c07480c7fd4d6..744ffa7d059e88 100644 --- a/test/rubygems/test_gem_stub_specification.rb +++ b/test/rubygems/test_gem_stub_specification.rb @@ -118,6 +118,31 @@ def test_matches_for_glob assert_equal code_rb, stub.matches_for_glob("code*").first end + def test_matches_for_glob_with_glob_metacharacters_in_gem_dir + base_dir = File.join @tempdir, "dir[1]" + gems_dir = File.join base_dir, "gems" + spec_dir = File.join base_dir, "specifications" + FileUtils.mkdir_p spec_dir + + spec = File.join spec_dir, "stub-2.gemspec" + File.write spec, <<~STUB + # -*- encoding: utf-8 -*- + # stub: stub 2 ruby lib + + Gem::Specification.new do |s| + s.name = 'stub' + s.version = Gem::Version.new '2' + end + STUB + + stub = Gem::StubSpecification.gemspec_stub spec, base_dir, gems_dir + code_rb = File.join stub.gem_dir, "lib", "code.rb" + FileUtils.mkdir_p File.dirname code_rb + FileUtils.touch code_rb + + assert_equal code_rb, stub.matches_for_glob("code*").first + end + def test_matches_for_glob_with_bundler_inline stub = stub_with_extension code_rb = File.join stub.gem_dir, "lib", "code.rb" diff --git a/tool/commit-email.rb b/tool/commit-email.rb index 896d5e2a02bd81..7ada82db2a7baa 100755 --- a/tool/commit-email.rb +++ b/tool/commit-email.rb @@ -280,43 +280,6 @@ def modified_dirs(info) changed_dirs('Modified', info.updated_dirs) end - def changed_dirs_info(info, uri) - (info.added_dirs.collect do |dir| - " Added: #{dir}\n" - end + info.deleted_dirs.collect do |dir| - " Deleted: #{dir}\n" - end + info.updated_dirs.collect do |dir| - " Modified: #{dir}\n" - end).join("\n") - end - - def diff_info(info, uri) - info.diffs.collect do |key, values| - [ - key, - values.collect do |type, value| - case type - when :added - rev = "?revision=#{info.revision}&view=markup" - when :modified, :property_changed - prev_revision = (info.revision.is_a?(Integer) ? info.revision - 1 : "#{info.revision}^") - rev = "?r1=#{info.revision}&r2=#{prev_revision}&diff_format=u" - when :deleted, :copied - rev = '' - else - raise "unknown diff type: #{value[:type]}" - end - - link = [uri, key.sub(/ .+/, '') || ''].join('/') + rev - - desc = '' - - [desc, link] - end - ] - end - end - def make_header(to, from, info) <<~EOS Mime-Version: 1.0 diff --git a/tool/ruby_vm/helpers/c_escape.rb b/tool/ruby_vm/helpers/c_escape.rb index 628cb0428bcbac..6737958846155b 100644 --- a/tool/ruby_vm/helpers/c_escape.rb +++ b/tool/ruby_vm/helpers/c_escape.rb @@ -9,8 +9,6 @@ # conditions mentioned in the file COPYING are met. Consult the file for # details. -require 'securerandom' - module RubyVM::CEscape module_function @@ -22,11 +20,6 @@ def commentify str return "/* #{str.gsub('*/', '*\\/').gsub('/*', '/\\*')} */" end - # Mimic gensym of CL. - def gensym prefix = 'gensym_' - return as_tr_cpp "#{prefix}#{SecureRandom.uuid}" - end - # Mimic AS_TR_CPP() of autoconf. def as_tr_cpp name q = name.b diff --git a/vm.c b/vm.c index df24fa3c300dc8..213b396bc09e50 100644 --- a/vm.c +++ b/vm.c @@ -2884,14 +2884,26 @@ vm_exec_loop(rb_execution_context_t *ec, enum ruby_tag_type state, #if USE_ZJIT // Materialize JITFrame-enabled CFP into interpreter-compatible CFP -void -rb_zjit_materialize_frames(const rb_execution_context_t *ec, rb_control_frame_t *cfp) +static void +zjit_materialize_frames(const rb_execution_context_t *ec, rb_control_frame_t *cfp, bool materialize_target) { if (!rb_zjit_enabled_p) return; const rb_control_frame_t *end_cfp = ec->tag->cfp; VM_ASSERT(cfp <= end_cfp); while (true) { + // If materialize_target is false, we skip materializing ec->tag->cfp. + // + // When JIT code calls a C function that does the same number of setjmps and + // longjmps, e.g. rb_hash_aref, it calls zjit_materialize_frames but goes + // back to the JIT code. In that case, we don't want to materialize the frame + // and clear cfp->jit_return, which will still be used by the JIT code. + // + // When JIT code calls a C function that does more longjmps than setjmps, + // it would not go back to the JIT code. So ec->tag->cfp should be materialized + // in that case. + if (cfp == end_cfp && !materialize_target) break; + if (CFP_ZJIT_FRAME_P(cfp)) { const zjit_jit_frame_t *jit_frame = CFP_ZJIT_FRAME(cfp); cfp->pc = jit_frame->pc; @@ -2931,6 +2943,20 @@ rb_zjit_materialize_frames(const rb_execution_context_t *ec, rb_control_frame_t cfp = RUBY_VM_PREVIOUS_CONTROL_FRAME(cfp); } } + +void +rb_zjit_materialize_frames(const rb_execution_context_t *ec, rb_control_frame_t *cfp) +{ + zjit_materialize_frames(ec, cfp, true); +} + +void +rb_zjit_materialize_frames_for_longjmp(const rb_execution_context_t *ec, rb_control_frame_t *cfp) +{ + // A ZJIT frame active before the tag's setjmp is below it on the native + // stack and survives longjmp. Materialize only the frames unwound above it. + zjit_materialize_frames(ec, cfp, !ec->tag->zjit_frame_active); +} #endif static inline VALUE diff --git a/vm_core.h b/vm_core.h index b189023c4c63d5..0ca0598678edc2 100644 --- a/vm_core.h +++ b/vm_core.h @@ -1029,6 +1029,12 @@ struct rb_vm_tag { #if USE_ZJIT // ec->cfp as of EC_PUSH_TAG, which is saved for materializing JITFrame. rb_control_frame_t *cfp; + // Whether cfp had a ZJIT frame before this tag's setjmp was established. + // It's used for checking if zjit_materialize_frames should materialize + // the frame or not when the tag is popped. If zjit_frame_active is true, + // we don't want to materialize cfp->jit_return, which will still be used + // by JIT code. + bool zjit_frame_active; #endif }; diff --git a/zjit.h b/zjit.h index ac6af1a9de2f4c..0b5e5f922cec1a 100644 --- a/zjit.h +++ b/zjit.h @@ -92,6 +92,7 @@ void rb_zjit_invalidate_no_singleton_class(VALUE klass); void rb_zjit_invalidate_root_box(void); void rb_zjit_jit_frame_update_references(zjit_jit_frame_t *jit_frame); void rb_zjit_materialize_frames(const rb_execution_context_t *ec, rb_control_frame_t *cfp); +void rb_zjit_materialize_frames_for_longjmp(const rb_execution_context_t *ec, rb_control_frame_t *cfp); size_t rb_zjit_hash_new_size(void); bool rb_zjit_class_allocate_instance_fastpath(VALUE klass, size_t *size_out, shape_id_t *shape_id_out); bool rb_zjit_str_resurrect_fastpath(VALUE str, bool chilled, size_t *size_out, VALUE *flags_out, long *len_out, size_t *byte_size_out); @@ -137,6 +138,7 @@ static inline void rb_zjit_invalidate_no_singleton_class(VALUE klass) {} static inline void rb_zjit_invalidate_root_box(void) {} static inline void rb_zjit_jit_frame_update_references(zjit_jit_frame_t *jit_frame) {} static inline void rb_zjit_materialize_frames(const rb_execution_context_t *ec, rb_control_frame_t *cfp) {} +static inline void rb_zjit_materialize_frames_for_longjmp(const rb_execution_context_t *ec, rb_control_frame_t *cfp) {} static inline const zjit_jit_frame_t *CFP_ZJIT_FRAME(const rb_control_frame_t *cfp) { return NULL; } #endif // #if USE_ZJIT diff --git a/zjit/src/codegen_tests.rs b/zjit/src/codegen_tests.rs index 6995daa2df489f..8d1ca36f3763e4 100644 --- a/zjit/src/codegen_tests.rs +++ b/zjit/src/codegen_tests.rs @@ -6,7 +6,7 @@ use crate::backend::lir::Assembler; use crate::codegen::max_iseq_versions; use crate::cruby::*; use crate::hir::{Insn, iseq_to_hir}; -use crate::options::{get_option, rb_zjit_prepare_options, set_call_threshold, set_inline_threshold}; +use crate::options::{get_option, rb_zjit_prepare_options, set_call_threshold, set_inline_threshold, set_max_versions}; use crate::payload::IseqVersion; use crate::hir::tests::hir_build_tests::assert_contains_opcode; use crate::payload::*; @@ -6693,6 +6693,19 @@ fn test_inlined_method_returns_correct_value() { }); } +#[test] +fn test_inlined_method_with_rest_parameter() { + with_inlining(|| { + assert_snapshot!(assert_inlines(" + def add_rest(*rest) = rest[0] + rest[1] + def test = add_rest(1, 2) + + test + test + "), @"3"); + }); +} + #[test] fn test_inlined_method_deoptimizes_on_redefinition() { with_inlining(|| { @@ -7235,6 +7248,61 @@ fn test_regression_gc_stress_with_lazy_block_code() { "#), @":ok"); } +// Hash recursion uses catch/throw internally. The target frame remains in JIT +// code after the caught throw, so longjmp must not materialize and detach it +// before a callee side exit uses its updated PC and stack map. +#[test] +fn test_keep_jit_frame_for_caught_jump() { + rb_zjit_prepare_options(); + let old_call_threshold = unsafe { crate::options::rb_zjit_call_threshold }; + let old_inline_threshold = get_option!(inline_threshold); + let old_max_versions = get_option!(max_versions); + set_call_threshold(1); + set_inline_threshold(0); + set_max_versions(2); + let result = inspect(r#" + module KeepJITFrameAssertions + def assert_receiver(*) + raise unless is_a?(KeepJITFrameBase) + end + end + + class KeepJITFrameBase + include KeepJITFrameAssertions + + def hash_class = Hash + + def test + hash = hash_class[] + recursive = [hash] + hash[:x] = recursive + object = Object.new + lookup = { hash => object } + + [recursive, [hash]].each do |key| + key = { x: key } + assert_receiver(object, lookup[key], -> { key.inspect }) + end + end + end + + class KeepJITFrameHash < Hash + end + + class KeepJITFrameSubclass < KeepJITFrameBase + def hash_class = KeepJITFrameHash + end + + KeepJITFrameBase.new.test + KeepJITFrameSubclass.new.test + :ok + "#); + set_max_versions(old_max_versions); + set_inline_threshold(old_inline_threshold); + set_call_threshold(old_call_threshold); + assert_snapshot!(result, @":ok"); +} + #[test] fn test_float_arithmetic() { set_call_threshold(1); diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index 45ab7300b3e830..298fce859e4386 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -4951,11 +4951,11 @@ impl Function { fn can_inline(callee_iseq: IseqPtr) -> bool { // Inline callees with required, optional, post-required positional, keyword, and // block parameters, including callees that dispatch to a passed block with `yield`. - // Rest params, double-splat (kwrest), and forwardable params stay out of the - // general inliner for now; rest-aware direct sends are still handled by codegen. + // Double-splat (kwrest) and forwardable params stay out of the general + // inliner for now. Rest params are already normalized to a packed Array by + // prepare_direct_send_args, so the inliner can map that Array to the rest local. let params = unsafe { callee_iseq.params() }; - if params.flags.has_rest() != 0 - || params.flags.forwardable() != 0 + if params.flags.forwardable() != 0 || params.flags.has_kwrest() != 0 { incr_counter!(inline_reject_complex_params); diff --git a/zjit/src/hir/opt_tests.rs b/zjit/src/hir/opt_tests.rs index f1faa88b9f1215..85b887983606b0 100644 --- a/zjit/src/hir/opt_tests.rs +++ b/zjit/src/hir/opt_tests.rs @@ -4025,9 +4025,14 @@ mod hir_opt_tests { v23:ArrayExact = NewArray v11, v13, v15 PatchPoint MethodRedefined(Object@0x1000, foo@0x1008, cme:0x1010) v26:ObjectSubclass[class_exact*:Object@VALUE(0x1000)] = GuardType v6, ObjectSubclass[class_exact*:Object@VALUE(0x1000)] recompile - v27:BasicObject = SendDirect v26, 0x1038, :foo (0x1048), v23 + PushInlineFrame v26 (0x1038), v23 + PatchPoint NoSingletonClass(Array@0x1040) + PatchPoint MethodRedefined(Array@0x1040, length@0x1048, cme:0x1050) + v49:CInt64 = ArrayLength v23 + v50:Fixnum = BoxFixnum v49 CheckInterrupts - Return v27 + PopInlineFrame + Return v50 "); } @@ -4060,9 +4065,14 @@ mod hir_opt_tests { v31:ArrayExact = NewArray v11, v13, v15, v17, v19, v21, v23 PatchPoint MethodRedefined(Object@0x1000, foo@0x1008, cme:0x1010) v34:ObjectSubclass[class_exact*:Object@VALUE(0x1000)] = GuardType v6, ObjectSubclass[class_exact*:Object@VALUE(0x1000)] recompile - v35:BasicObject = SendDirect v34, 0x1038, :foo (0x1048), v31 + PushInlineFrame v34 (0x1038), v31 + PatchPoint NoSingletonClass(Array@0x1040) + PatchPoint MethodRedefined(Array@0x1040, length@0x1048, cme:0x1050) + v57:CInt64 = ArrayLength v31 + v58:Fixnum = BoxFixnum v57 CheckInterrupts - Return v35 + PopInlineFrame + Return v58 "); } @@ -4091,9 +4101,19 @@ mod hir_opt_tests { v23:ArrayExact = NewArray v11, v13, v15 PatchPoint MethodRedefined(Object@0x1000, foo@0x1008, cme:0x1010) v26:ObjectSubclass[class_exact*:Object@VALUE(0x1000)] = GuardType v6, ObjectSubclass[class_exact*:Object@VALUE(0x1000)] recompile - v27:BasicObject = SendDirect v26, 0x1038, :foo (0x1048), v23 + PushInlineFrame v26 (0x1038), v23 + PatchPoint NoSingletonClass(Array@0x1040) + PatchPoint MethodRedefined(Array@0x1040, length@0x1048, cme:0x1050) + v55:CInt64 = ArrayLength v23 + v56:Fixnum = BoxFixnum v55 + v38:CPtr = GetEP 0 + v39:CInt64 = LoadField v38, :VM_ENV_DATA_INDEX_SPECVAL@0x1078 + v40:CInt64[-4] = Const CInt64(-4) + v41:CInt64 = IntAnd v39, v40 + v42:BasicObject = InvokeBlockIseqDirect (0x1080), v41, v56 CheckInterrupts - Return v27 + PopInlineFrame + Return v42 "); } @@ -4122,9 +4142,29 @@ mod hir_opt_tests { v23:ArrayExact = NewArray v11, v13, v15 PatchPoint MethodRedefined(Object@0x1000, foo@0x1008, cme:0x1010) v26:ObjectSubclass[class_exact*:Object@VALUE(0x1000)] = GuardType v6, ObjectSubclass[class_exact*:Object@VALUE(0x1000)] recompile - v27:BasicObject = SendDirect v26, 0x1038, :foo (0x1048), v23 + v57:NilClass = Const Value(nil) + PushInlineFrame v26 (0x1038), v23 + v37:CPtr = GetEP 0 + v38:CUInt64 = LoadField v37, :VM_ENV_DATA_INDEX_FLAGS@0x1040 + v39:CBool = IsBlockParamModified v38 + CondBranch v39, bb6(), bb7() + bb6(): + v41:BasicObject = LoadField v37, :block@0x1041 + Jump bb8(v41, v41) + bb7(): + v43:CInt64 = LoadField v37, :VM_ENV_DATA_INDEX_SPECVAL@0x1042 + v44:CInt64 = GuardAnyBitSet v43, CUInt64(1) recompile + v45:ObjectSubclass[BlockParamProxy] = Const Value(VALUE(0x1048)) + Jump bb8(v45, v57) + bb8(v35:BasicObject, v36:BasicObject): + PatchPoint NoSingletonClass(Array@0x1050) + PatchPoint MethodRedefined(Array@0x1050, length@0x1058, cme:0x1060) + v66:CInt64 = ArrayLength v23 + v67:Fixnum = BoxFixnum v66 + v52:BasicObject = Send v35, :call, v67 # SendFallbackReason: Send: unsupported optimized method type BlockCall CheckInterrupts - Return v27 + PopInlineFrame + Return v52 "); } @@ -4154,9 +4194,17 @@ mod hir_opt_tests { v25:ArrayExact = NewArray v13, v15 PatchPoint MethodRedefined(Object@0x1000, foo@0x1008, cme:0x1010) v28:ObjectSubclass[class_exact*:Object@VALUE(0x1000)] = GuardType v6, ObjectSubclass[class_exact*:Object@VALUE(0x1000)] recompile - v29:BasicObject = SendDirect v28, 0x1038, :foo (0x1048), v11, v25, v17 + PushInlineFrame v28 (0x1038), v11, v25, v17 + PatchPoint NoSingletonClass(Array@0x1040) + PatchPoint MethodRedefined(Array@0x1040, length@0x1048, cme:0x1050) + v61:CInt64 = ArrayLength v25 + v62:Fixnum = BoxFixnum v61 + PatchPoint MethodRedefined(Integer@0x1078, +@0x1080, cme:0x1088) + v66:Fixnum = FixnumAdd v62, v11 + v70:Fixnum = FixnumAdd v66, v17 CheckInterrupts - Return v29 + PopInlineFrame + Return v70 "); } @@ -4185,9 +4233,17 @@ mod hir_opt_tests { v23:ArrayExact = NewArray v11, v13 PatchPoint MethodRedefined(Object@0x1000, foo@0x1008, cme:0x1010) v26:ObjectSubclass[class_exact*:Object@VALUE(0x1000)] = GuardType v6, ObjectSubclass[class_exact*:Object@VALUE(0x1000)] recompile - v27:BasicObject = SendDirect v26, 0x1038, :foo (0x1048), v23, v15 + v47:Fixnum[0] = Const Value(0) + PushInlineFrame v26 (0x1038), v23, v15 + PatchPoint NoSingletonClass(Array@0x1040) + PatchPoint MethodRedefined(Array@0x1040, length@0x1048, cme:0x1050) + v56:CInt64 = ArrayLength v23 + v57:Fixnum = BoxFixnum v56 + PatchPoint MethodRedefined(Integer@0x1078, +@0x1080, cme:0x1088) + v61:Fixnum = FixnumAdd v57, v15 CheckInterrupts - Return v27 + PopInlineFrame + Return v61 "); } @@ -4216,9 +4272,17 @@ mod hir_opt_tests { v22:ArrayExact = NewArray v11, v13 PatchPoint MethodRedefined(Object@0x1000, foo@0x1008, cme:0x1010) v25:ObjectSubclass[class_exact*:Object@VALUE(0x1000)] = GuardType v6, ObjectSubclass[class_exact*:Object@VALUE(0x1000)] recompile - v26:BasicObject = SendDirect v25, 0x1038, :foo (0x1048), v22, v21 + v46:Fixnum[0] = Const Value(0) + PushInlineFrame v25 (0x1038), v22, v21 + PatchPoint NoSingletonClass(Array@0x1040) + PatchPoint MethodRedefined(Array@0x1040, length@0x1048, cme:0x1050) + v55:CInt64 = ArrayLength v22 + v56:Fixnum = BoxFixnum v55 + PatchPoint MethodRedefined(Integer@0x1078, +@0x1080, cme:0x1088) + v60:Fixnum = FixnumAdd v56, v21 CheckInterrupts - Return v26 + PopInlineFrame + Return v60 "); } @@ -4248,9 +4312,11 @@ mod hir_opt_tests { v25:ArrayExact = NewArray v15, v17 PatchPoint MethodRedefined(Object@0x1000, foo@0x1008, cme:0x1010) v28:ObjectSubclass[class_exact*:Object@VALUE(0x1000)] = GuardType v6, ObjectSubclass[class_exact*:Object@VALUE(0x1000)] recompile - v29:BasicObject = SendDirect v28, 0x1038, :foo (0x1048), jit_entry_idx=1, v11, v13, v25 + PushInlineFrame v28 (0x1038), v11, v13, v25 + v41:ArrayExact = NewArray v11, v13, v25 CheckInterrupts - Return v29 + PopInlineFrame + Return v41 "); }