diff --git a/.github/actions/setup/directories/action.yml b/.github/actions/setup/directories/action.yml index f1b09944607785..7b955d4ff0dab2 100644 --- a/.github/actions/setup/directories/action.yml +++ b/.github/actions/setup/directories/action.yml @@ -114,7 +114,7 @@ runs: fetch-depth: ${{ inputs.fetch-depth }} persist-credentials: false - - uses: actions/cache@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5.0.5 + - uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 with: path: ${{ inputs.srcdir }}/.downloaded-cache key: ${{ runner.os }}-${{ runner.arch }}-downloaded-cache @@ -127,7 +127,7 @@ runs: # because they are the ones that clone the complete set into gems/src; other # jobs would otherwise save a partial gems/src and poison the shared key. - if: inputs.bundled-gems-src-cache == 'true' - uses: actions/cache@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5.0.5 + uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 with: path: ${{ inputs.srcdir }}/gems/src key: ${{ runner.os }}-${{ runner.arch }}-bundled-gems-src-${{ hashFiles(format('{0}/gems/bundled_gems', inputs.srcdir)) }} diff --git a/.github/workflows/modgc.yml b/.github/workflows/modgc.yml index ff43bfb21e2575..8349cf38aa96df 100644 --- a/.github/workflows/modgc.yml +++ b/.github/workflows/modgc.yml @@ -110,7 +110,7 @@ jobs: ${SETARCH} ../src/configure -C --disable-install-doc --with-modular-gc="${MODULAR_GC_DIR}" \ ${arch:+--target=$arch-$OSTYPE --host=$arch-$OSTYPE} - - uses: actions-rust-lang/setup-rust-toolchain@46268bd060767258de96ed93c1251119784f2ab6 # v1.16.1 + - uses: actions-rust-lang/setup-rust-toolchain@166cdcfd11aee3cb47222f9ddb555ce30ddb9659 # v1.17.0 with: cache-bin: false - name: Set MMTk environment variables diff --git a/.github/workflows/tarball-windows.yml b/.github/workflows/tarball-windows.yml index 34b3187d7180ce..a472af6e9e0fbe 100644 --- a/.github/workflows/tarball-windows.yml +++ b/.github/workflows/tarball-windows.yml @@ -63,7 +63,7 @@ jobs: shell: msys2 {0} run: echo PATCH=$(cygpath -wa $(command -v patch)) >> $GITHUB_ENV if: ${{ steps.setup-msys2.outcome == 'success' }} - - uses: actions/cache@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5.0.5 + - uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 with: path: C:\vcpkg\installed key: ${{ runner.os }}-vcpkg-installed-${{ env.OS_VER }}-${{ github.sha }} @@ -82,7 +82,7 @@ jobs: run: 7z x pkg/*.zip working-directory: - - uses: actions/cache@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5.0.5 + - uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 with: path: snapshot-*/.downloaded-cache key: downloaded-cache diff --git a/.github/workflows/windows.yml b/.github/workflows/windows.yml index 4aa55366d3030b..2059285e07fa0d 100644 --- a/.github/workflows/windows.yml +++ b/.github/workflows/windows.yml @@ -90,7 +90,7 @@ jobs: - name: Restore vcpkg artifact id: restore-vcpkg - uses: actions/cache/restore@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5.0.5 + uses: actions/cache/restore@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 with: path: src\vcpkg_installed key: windows-${{ matrix.os }}-vcpkg-${{ hashFiles('src/vcpkg.json') }} @@ -104,7 +104,7 @@ jobs: if: ${{ ! steps.restore-vcpkg.outputs.cache-hit }} - name: Save vcpkg artifact - uses: actions/cache/save@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5.0.5 + uses: actions/cache/save@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 with: path: src\vcpkg_installed key: windows-${{ matrix.os }}-vcpkg-${{ hashFiles('src/vcpkg.json') }} diff --git a/.github/workflows/zjit-macos.yml b/.github/workflows/zjit-macos.yml index 8cd40023ee790c..105a87045ee0b2 100644 --- a/.github/workflows/zjit-macos.yml +++ b/.github/workflows/zjit-macos.yml @@ -58,7 +58,7 @@ jobs: env: GITPULLOPTIONS: --no-tags origin ${{ github.ref }} RUN_OPTS: ${{ matrix.run_opts }} - SPECOPTS: ${{ matrix.specopts }} + SPECOPTS: ${{ matrix.specopts }} -B../src/spec/zjit.mspec TESTOPTS: ${{ matrix.testopts }} RUST_BACKTRACE: 1 ZJIT_RB_BUG: 1 @@ -98,7 +98,7 @@ jobs: rustup install ${{ matrix.rust_version }} --profile minimal rustup default ${{ matrix.rust_version }} - - uses: taiki-e/install-action@9e1e5806d4a4822de933115878265be9aaa786d9 # v2.82.2 + - uses: taiki-e/install-action@9bcaee1dcae34154180f412e2fa69355a7cda9f6 # v2.82.6 with: tool: nextest@0.9 if: ${{ matrix.test_task == 'zjit-check' }} diff --git a/.github/workflows/zjit-ubuntu.yml b/.github/workflows/zjit-ubuntu.yml index 729e1eb542de62..3f6a26c60fdf1e 100644 --- a/.github/workflows/zjit-ubuntu.yml +++ b/.github/workflows/zjit-ubuntu.yml @@ -99,7 +99,7 @@ jobs: GITPULLOPTIONS: --no-tags origin ${{ github.ref }} RUN_OPTS: ${{ matrix.run_opts }} YJIT_BENCH_OPTS: ${{ matrix.yjit_bench_opts }} - SPECOPTS: ${{ matrix.specopts }} + SPECOPTS: ${{ matrix.specopts }} -B../src/spec/zjit.mspec TESTOPTS: ${{ matrix.testopts }} RUBY_DEBUG: ci BUNDLE_JOBS: 8 # for yjit-bench @@ -130,7 +130,7 @@ jobs: ruby-version: '3.1' bundler: none - - uses: taiki-e/install-action@9e1e5806d4a4822de933115878265be9aaa786d9 # v2.82.2 + - uses: taiki-e/install-action@9bcaee1dcae34154180f412e2fa69355a7cda9f6 # v2.82.6 with: tool: nextest@0.9 if: ${{ matrix.test_task == 'zjit-check' }} diff --git a/NEWS.md b/NEWS.md index 1ac45e2da77769..004e151df07bd9 100644 --- a/NEWS.md +++ b/NEWS.md @@ -130,7 +130,7 @@ releases. * 0.1.12 to [v0.1.13][repl_type_completor-v0.1.13], [v0.1.14][repl_type_completor-v0.1.14], [v0.1.15][repl_type_completor-v0.1.15] * pstore 0.2.1 * 0.2.0 to [v0.2.1][pstore-v0.2.1] -* rdoc 7.2.0 +* rdoc 8.0.0 * 7.0.3 to [v7.0.4][rdoc-v7.0.4], [v7.1.0][rdoc-v7.1.0], [v7.2.0][rdoc-v7.2.0] * win32ole 1.9.3 * 1.9.2 to [v1.9.3][win32ole-v1.9.3] diff --git a/common.mk b/common.mk index 3c32e9fb70b693..45666a5570506c 100644 --- a/common.mk +++ b/common.mk @@ -1649,7 +1649,30 @@ yes-test-bundled-gems-spec: yes-test-all-precheck $(PREPARE_BUNDLED_GEMS) no-test-bundled-gems-spec: -test-syntax-suggest: +test-syntax-suggest-precheck: $(TEST_RUNNABLE)-test-syntax-suggest-precheck +no-test-syntax-suggest-precheck: +yes-test-syntax-suggest-precheck: main + +test-syntax-suggest-prepare: $(TEST_RUNNABLE)-test-syntax-suggest-prepare +no-test-syntax-suggest-prepare: no-test-syntax-suggest-precheck +yes-test-syntax-suggest-prepare: yes-test-syntax-suggest-precheck + $(ACTIONS_GROUP) + $(XRUBY) -C "$(srcdir)" bin/gem install --no-document \ + --install-dir .bundle --conservative "rspec:~> 3" + $(ACTIONS_ENDGROUP) + +RSPECOPTS = +SYNTAX_SUGGEST_SPECS = +PREPARE_SYNTAX_SUGGEST = $(TEST_RUNNABLE)-test-syntax-suggest-prepare +test-syntax-suggest: $(TEST_RUNNABLE)-test-syntax-suggest +yes-test-syntax-suggest: $(PREPARE_SYNTAX_SUGGEST) + $(ACTIONS_GROUP) + $(XRUBY) -C $(srcdir) -Ispec/syntax_suggest$(PATH_SEPARATOR)spec/lib .bundle/bin/rspec \ + --require rspec/expectations \ + --require spec_helper --require formatter_overrides --require spec_coverage \ + $(RSPECOPTS) spec/syntax_suggest/$(SYNTAX_SUGGEST_SPECS) + $(ACTIONS_ENDGROUP) +no-test-syntax-suggest: check: $(DOT_WAIT) $(PREPARE_SYNTAX_SUGGEST) test-syntax-suggest diff --git a/doc/distribution/windows.md b/doc/distribution/windows.md index 26a727d7adb26a..304e3451547fc6 100644 --- a/doc/distribution/windows.md +++ b/doc/distribution/windows.md @@ -149,6 +149,11 @@ sh ../../ruby/configure -C --disable-install-doc --with-opt-dir=C:\Users\usernam of `cmd.exe`. If you want to enable it explicitly, run `cmd.exe` with `/E:ON` option. +7. Some tests in `nmake check` create symbolic links. Enable + [Developer Mode](https://learn.microsoft.com/windows/apps/get-started/developer-mode-features-and-debugging) + (Settings > System > For developers) so that they do not fail with + `Permission denied @ rb_file_s_symlink`. + ### How to compile and install 1. Execute `win32\configure.bat` on your build directory. diff --git a/ext/json/parser/parser.c b/ext/json/parser/parser.c index 4d9fa25baa5f93..a6154c83ec52de 100644 --- a/ext/json/parser/parser.c +++ b/ext/json/parser/parser.c @@ -1133,6 +1133,13 @@ static inline VALUE json_decode_float(JSON_ParserConfig *config, uint64_t mantis } if (RB_UNLIKELY(mantissa_digits > 18 || mantissa_digits + exponent < -307)) { + // If the value is so small that it definitely underflows to 0.0, return early + // to avoid triggering a "Float out of range" warning from rb_cstr_to_dbl. + // When mantissa_digits + exponent < -324, value < 10^(-324) < DBL_TRUE_MIN/2, + // so it rounds to 0 in IEEE 754 round-to-nearest. + if (RB_UNLIKELY(mantissa_digits + exponent < -324)) { + return rb_float_new(negative ? -0.0 : 0.0); + } return json_decode_large_float(start, end - start); } diff --git a/file.c b/file.c index 1231848c1035f0..5473ee910b4e4a 100644 --- a/file.c +++ b/file.c @@ -1837,14 +1837,24 @@ rb_file_pipe_p(VALUE obj, VALUE fname) } /* + * :markup: markdown + * * call-seq: - * File.symlink?(filepath) -> true or false + * File.symlink?(path) -> true or false * - * Returns +true+ if +filepath+ points to a symbolic link, +false+ otherwise: + * Returns whether the entry at `path` is a symbolic link: * - * symlink = File.symlink('t.txt', 'symlink') - * File.symlink?('symlink') # => true - * File.symlink?('t.txt') # => false + * ```ruby + * # Create paths. + * file_path = 'doc/extension.rdoc' # => "doc/extension.rdoc" + * target_path = File.join('..', file_path) # => "../doc/extension.rdoc" + * link_path = 'lib/u.tmp' # => "lib/u.tmp" + * File.symlink?(link_path) # => false + * # Create link and verify. + * File.symlink(target_path, link_path) + * File.symlink?(link_path) # => true + * File.delete(link_path) # Clean up. + * ``` * */ @@ -3511,15 +3521,27 @@ rb_file_s_link(VALUE klass, VALUE from, VALUE to) #ifdef HAVE_SYMLINK /* + * :markup: markdown + * * call-seq: - * File.symlink(old_name, new_name) -> 0 + * File.symlink(path, link_path) -> 0 * - * Creates a symbolic link called new_name for the existing file - * old_name. Raises a NotImplemented exception on - * platforms that do not support symbolic links. + * Not supported on some platforms. * - * File.symlink("testfile", "link2test") #=> 0 + * Creates a symbolic link at `link_path` to the entry at `path`: * + * ```ruby + * # Create paths. + * file_path = 'doc/extension.rdoc' # => "doc/extension.rdoc" + * target_path = File.join('..', file_path) # => "../doc/extension.rdoc" + * link_path = 'lib/u.tmp' # => "lib/u.tmp" + * # Create link and verify. + * File.symlink(target_path, link_path) + * File.read(file_path) == File.read(link_path) # => true + * File.delete(link_path) # Clean up. + * ``` + * + * See also: ::read, ::readlink, ::symlink?. */ static VALUE @@ -3541,14 +3563,23 @@ rb_file_s_symlink(VALUE klass, VALUE from, VALUE to) #ifdef HAVE_READLINK /* + * :markup: markdown + * * call-seq: - * File.readlink(link_name) -> file_name + * File.readlink(link_path) -> path * - * Returns the name of the file referenced by the given link. - * Not available on all platforms. + * Returns the string path to the entry referenced by the given `link_path`: + * + * ```ruby + * # Create paths. + * file_path = 'doc/extension.rdoc' # => "doc/extension.rdoc" + * target_path = File.join('..', file_path) # => "../doc/extension.rdoc" + * link_path = 'lib/u.tmp' # => "lib/u.tmp" + * File.symlink(target_path, link_path) + * File.readlink(link_path) # => "../doc/extension.rdoc" + * File.delete(link_path) # Clean up. + * ``` * - * File.symlink("testfile", "link2test") #=> 0 - * File.readlink("link2test") #=> "testfile" */ static VALUE @@ -6415,18 +6446,24 @@ rb_stat_p(VALUE obj) } /* + * :markup: markdown + * * call-seq: - * stat.symlink? -> true or false + * symlink? -> true or false * - * Returns true if stat is a symbolic link, - * false if it isn't or if the operating system doesn't - * support this feature. As File::stat automatically follows symbolic - * links, #symlink? will always be false for an object - * returned by File::stat. + * Returns whether the entry in `self` is a symbolic link: * - * File.symlink("testfile", "alink") #=> 0 - * File.stat("alink").symlink? #=> false - * File.lstat("alink").symlink? #=> true + * ```ruby + * path = 'doc/t.tmp' + * link_path = 'lib/u.tmp' + * File.write(path, 'foo') + * File.symlink(path, link_path) + * File.stat(path).symlink? # => false + * File.stat(link_path).symlink? # Raises Errno::ENOENT; entry is not a file. + * File.lstat(link_path).symlink? # => true + * File.delete(path) + * File.delete(link_path) + * ``` * */ diff --git a/gc/default/default.c b/gc/default/default.c index bb889fecf873da..7a54f30b29b61f 100644 --- a/gc/default/default.c +++ b/gc/default/default.c @@ -9663,7 +9663,7 @@ gc_profile_disable(VALUE _) return Qnil; } -void +static void rb_gc_verify_internal_consistency(void) { gc_verify_internal_consistency(rb_gc_get_objspace()); diff --git a/gems/bundled_gems b/gems/bundled_gems index d67684d6e5d672..d9a6273a043d9b 100644 --- a/gems/bundled_gems +++ b/gems/bundled_gems @@ -37,7 +37,7 @@ ostruct 0.6.3 https://github.com/ruby/ostruct pstore 0.2.1 https://github.com/ruby/pstore benchmark 0.5.0 https://github.com/ruby/benchmark logger 1.7.0 https://github.com/ruby/logger -rdoc 7.2.0 https://github.com/ruby/rdoc a8df5c5d03b63cf05425bf676644688ac673a329 +rdoc 8.0.0 https://github.com/ruby/rdoc win32ole 1.9.3 https://github.com/ruby/win32ole irb 1.18.0 https://github.com/ruby/irb reline 0.6.3 https://github.com/ruby/reline diff --git a/imemo.c b/imemo.c index 9154f6bc6ac280..3814fbd4538d85 100644 --- a/imemo.c +++ b/imemo.c @@ -645,11 +645,7 @@ rb_imemo_free(VALUE obj) case imemo_callinfo:{ const struct rb_callinfo *ci = ((const struct rb_callinfo *)obj); - if (ci->kwarg) { - if (RUBY_ATOMIC_FETCH_SUB(((struct rb_callinfo_kwarg *)ci->kwarg)->references, 1) == 1) { - ruby_xfree_sized((void *)ci->kwarg, rb_callinfo_kwarg_bytes(ci->kwarg->keyword_len)); - } - } + rb_callinfo_kwarg_release((struct rb_callinfo_kwarg *)ci->kwarg); RB_DEBUG_COUNTER_INC(obj_imemo_callinfo); break; diff --git a/include/ruby/internal/core/robject.h b/include/ruby/internal/core/robject.h index df1901eb1e98a5..b8743dcb739116 100644 --- a/include/ruby/internal/core/robject.h +++ b/include/ruby/internal/core/robject.h @@ -116,32 +116,4 @@ struct RObject { } as; }; -RBIMPL_ATTR_PURE_UNLESS_DEBUG() -RBIMPL_ATTR_ARTIFICIAL() -/** - * Queries the instance variables. - * - * @param[in] obj Object in question. - * @return Its instance variables, in C array. - * @pre `obj` must be an instance of ::RObject. - * - * @internal - * - * @shyouhei finds no reason for this to be visible from extension libraries. - */ -static inline VALUE * -ROBJECT_FIELDS(VALUE obj) -{ - RBIMPL_ASSERT_TYPE(obj, RUBY_T_OBJECT); - - struct RObject *const ptr = ROBJECT(obj); - - if (RB_UNLIKELY(RB_FL_ANY_RAW(obj, ROBJECT_HEAP))) { - return ptr->as.heap.fields; - } - else { - return ptr->as.ary; - } -} - #endif /* RBIMPL_ROBJECT_H */ diff --git a/internal/object.h b/internal/object.h index 99aa1f524bab5d..837dd7a457f6fd 100644 --- a/internal/object.h +++ b/internal/object.h @@ -61,6 +61,21 @@ RBASIC_SET_CLASS(VALUE obj, VALUE klass) RB_OBJ_WRITTEN(obj, oldv, klass); } +static inline VALUE * +ROBJECT_FIELDS(VALUE obj) +{ + RBIMPL_ASSERT_TYPE(obj, RUBY_T_OBJECT); + + struct RObject *const ptr = ROBJECT(obj); + + if (RB_UNLIKELY(RB_FL_ANY_RAW(obj, ROBJECT_HEAP))) { + return ptr->as.heap.fields; + } + else { + return ptr->as.ary; + } +} + static inline size_t rb_obj_embedded_size(uint32_t fields_count) { diff --git a/lib/rubygems/util/atomic_file_writer.rb b/lib/rubygems/util/atomic_file_writer.rb index 32767c6a79081d..9af6d54b549843 100644 --- a/lib/rubygems/util/atomic_file_writer.rb +++ b/lib/rubygems/util/atomic_file_writer.rb @@ -24,7 +24,23 @@ def self.open(file_name) tmp_suffix = ".tmp.#{SecureRandom.hex}" dirname = File.dirname(file_name) basename = File.basename(file_name) - tmp_path = File.join(dirname, ".#{basename.byteslice(0, 254 - tmp_suffix.bytesize)}#{tmp_suffix}") + base_slice = basename.byteslice(0, 254 - tmp_suffix.bytesize) + tmp_path = File.join(dirname, ".#{base_slice}#{tmp_suffix}") + + # The temporary name is longer than the final one, so on Windows a + # writable destination can still map to a path beyond the 260-character + # MAX_PATH limit. Trim the random suffix first, keeping at least 8 hex + # characters to avoid collisions, then shorten the basename if that is not + # enough. Recomputing the basename from the trimmed suffix would just let + # it grow back, so trim the already-sliced basename instead. + if tmp_path.length >= 260 && Gem.win_platform? + overflow = tmp_path.length - 259 + trim = [tmp_suffix.bytesize - (".tmp.".bytesize + 8), overflow].min + tmp_suffix = tmp_suffix.byteslice(0, tmp_suffix.bytesize - trim) + overflow -= trim + base_slice = base_slice.byteslice(0, [base_slice.bytesize - overflow, 0].max) if overflow > 0 + tmp_path = File.join(dirname, ".#{base_slice}#{tmp_suffix}") + end flags = File::RDWR | File::CREAT | File::EXCL | File::BINARY flags |= File::SHARE_DELETE if defined?(File::SHARE_DELETE) diff --git a/pathname_builtin.rb b/pathname_builtin.rb index b12432cfb286fe..9e510eddb239a9 100644 --- a/pathname_builtin.rb +++ b/pathname_builtin.rb @@ -1196,7 +1196,37 @@ def birthtime() File.birthtime(@path) end # def ctime() File.ctime(@path) end - # See File.mtime. Returns last modification time. + # :markup: markdown + # + # call-seq: + # mtime -> time + # + # Returns a Time object containing the time of the most recent + # modification to the entry represented by `self`; + # see {File System Timestamps}[rdoc-ref:file/timestamps.md]: + # + # ```ruby + # # A directory and its Pathname. + # dir_path = 'doc/foo' + # dir_pn = Pathname(dir_path) + # # Create directory; directory mtime established. + # dir_pn.mkdir + # dir_pn.mtime # => 2026-06-28 16:38:02.675780521 -0500 + # # A file therein and its Pathname. + # file_path = dir_pn.join('t.tmp') + # file_pn = Pathname(file_path) + # # Create file; file mtime established; directory mtime updated. + # file_pn.write('foo') + # dir_pn.mtime # => 2026-06-28 16:41:23.107750483 -0500 + # file_pn.mtime # => 2026-06-28 16:41:23.107750483 -0500 + # # Modify file; file mtime updated; directory mtime unchanged. + # file_pn.write('bar') + # dir_pn.mtime # => 2026-06-28 16:41:23.107750483 -0500 + # file_pn.mtime # => 2026-06-28 16:42:48.869163049 -0500 + # # Clean up. + # dir_pn.rmtree + # ``` + # def mtime() File.mtime(@path) end @@ -1421,7 +1451,23 @@ def open(...) # :yield: file File.open(@path, ...) end - # See File.readlink. Read symbolic link. + # :markup: markdown + # + # call-seq: + # readlink -> new_pathname + # + # Returns a new pathname containing the string path to the entry referenced by `self`: + # + # ```ruby + # # Create Pathnames. + # file_pn = Pathname('doc/extension.rdoc') # => # + # target_pn = Pathname('..').join(file_pn) # => # + # link_pn = Pathname('lib/u.tmp') # => # + # link_pn.make_symlink(target_pn) + # link_pn.readlink # => # + # link_pn.delete + # ``` + # def readlink() self.class.new(File.readlink(@path)) end # See File.rename. Rename the file. @@ -1457,7 +1503,25 @@ def stat() File.stat(@path) end # def lstat() File.lstat(@path) end - # See File.symlink. Creates a symbolic link. + # :markup: markdown + # + # call-seq: + # make_symlink(path) -> 0 + # + # Creates a symbolic link at the path in `self` to the entry at `path`: + # + # ```ruby + # # Create Pathnames. + # file_pn = Pathname('doc/extension.rdoc') # => # + # target_pn = Pathname('..').join(file_pn) # => # + # link_pn = Pathname('lib/u.tmp') # => # + # # Create link and verify. + # link_pn.make_symlink(target_pn) + # file_pn.read == link_pn.read # => true + # link_pn.delete # Clean up. + # ``` + # + # See also: #read, #readlink, #symlink?. def make_symlink(old) File.symlink(old, @path) end # See File.truncate. Truncate the file to +length+ bytes. @@ -1844,11 +1908,28 @@ def size() FileTest.size(@path) end # See FileTest.size?. def size?() FileTest.size?(@path) end - # See FileTest.sticky?. def sticky?() FileTest.sticky?(@path) end - # See FileTest.symlink?. + # :markup: markdown + # + # call-seq: + # symlink? -> true or false + # + # Returns whether the entry at the path in `self` is a symbolic link: + # + # ```ruby + # # Create Pathnames. + # file_pn = Pathname('doc/extension.rdoc') # => # + # target_pn = Pathname('..').join(file_pn) # => # + # link_pn = Pathname('lib/u.tmp') # => # + # link_pn.symlink? # => false + # # Create link. + # link_pn.make_symlink(target_pn) + # link_pn.symlink? # => true + # link_pn.delete # Clean up. + # ``` + # def symlink?() FileTest.symlink?(@path) end # See FileTest.writable?. diff --git a/prism/prism.c b/prism/prism.c index e3137d3167e7db..d997c63d166e06 100644 --- a/prism/prism.c +++ b/prism/prism.c @@ -15290,6 +15290,12 @@ parse_block(pm_parser_t *parser, uint16_t depth) { statements = UP(parse_statements(parser, PM_CONTEXT_BLOCK_BRACES, (uint16_t) (depth + 1))); } + /* Pop before consuming the closing `}` so the following token (e.g. a + * `do`) is lexed in the enclosing context rather than as a block + * belonging to this block's interior. Otherwise a `do` block would + * wrongly bind to a command whose argument ends in a brace block, as in + * `foo(m a { } do end)`. */ + pm_accepts_block_stack_pop(parser); expect1_opening(parser, PM_TOKEN_BRACE_RIGHT, PM_ERR_BLOCK_TERM_BRACE, &opening); } else { if (!match1(parser, PM_TOKEN_KEYWORD_END)) { @@ -15305,6 +15311,8 @@ parse_block(pm_parser_t *parser, uint16_t depth) { } } + /* As with the brace case above, pop before consuming `end`. */ + pm_accepts_block_stack_pop(parser); expect1_opening(parser, PM_TOKEN_KEYWORD_END, PM_ERR_BLOCK_TERM_END, &opening); } @@ -15313,7 +15321,6 @@ parse_block(pm_parser_t *parser, uint16_t depth) { pm_node_t *parameters = parse_blocklike_parameters(parser, UP(block_parameters), &opening, &parser->previous); pm_parser_scope_pop(parser); - pm_accepts_block_stack_pop(parser); return pm_block_node_create(parser, &locals, &opening, parameters, statements, &parser->previous); } diff --git a/prism_compile.c b/prism_compile.c index 353e018d878493..e70477a51c786e 100644 --- a/prism_compile.c +++ b/prism_compile.c @@ -3887,6 +3887,8 @@ pm_compile_call(rb_iseq_t *iseq, const pm_call_node_t *call_node, LINK_ANCHOR *c ELEM_INSERT_NEXT(opt_new_prelude, &new_insn_body(iseq, location.line, location.node_id, BIN(putnil), 0)->link); } + rb_callinfo_kwarg_retain(kw_arg); + // Jump unless the receiver uses the "basic" implementation of "new" VALUE ci; if (flags & VM_CALL_FORWARDING) { @@ -3909,6 +3911,8 @@ pm_compile_call(rb_iseq_t *iseq, const pm_call_node_t *call_node, LINK_ANCHOR *c PUSH_LABEL(ret, not_basic_new_finish); PUSH_INSN(ret, location, pop); + + rb_callinfo_kwarg_release(kw_arg); } else { PUSH_SEND_R(ret, location, method_id, INT2FIX(orig_argc), block_iseq, INT2FIX(flags), kw_arg); diff --git a/ruby.c b/ruby.c index b9648bf155a1ad..f33b86419d589c 100644 --- a/ruby.c +++ b/ruby.c @@ -2227,7 +2227,7 @@ prism_script(ruby_cmdline_options_t *opt, pm_parse_result_t *result) const bool read_stdin = (strcmp(opt->script, "-") == 0); if (read_stdin) { - pm_options_encoding_set(options, rb_enc_name(rb_locale_encoding())); + pm_options_encoding_set(options, rb_enc_name(IF_UTF8_PATH(rb_utf8_encoding(), rb_locale_encoding()))); } if (opt->src.enc.name != 0) { pm_options_encoding_set(options, StringValueCStr(opt->src.enc.name)); @@ -2844,7 +2844,7 @@ load_file_internal(VALUE argp_v) enc = rb_enc_from_index(opt->src.enc.index); } else if (f == rb_stdin) { - enc = rb_locale_encoding(); + enc = IF_UTF8_PATH(rb_utf8_encoding(), rb_locale_encoding()); } else { enc = rb_utf8_encoding(); diff --git a/set.c b/set.c index 094f39bff2eacc..203a0abfd1bb82 100644 --- a/set.c +++ b/set.c @@ -621,16 +621,12 @@ set_inspect(VALUE set, VALUE dummy, int recur) /* * call-seq: - * inspect -> new_string + * inspect -> string * - * Returns a new string containing the set entries: + * Returns a string representation of +self+: * - * s = Set.new - * s.inspect # => "Set[]" - * s.add(1) - * s.inspect # => "Set[1]" - * s.add(2) - * s.inspect # => "Set[1, 2]" + * Set[*%w[foo bar], {foo: 0, bar: 1}].inspect + * # => "Set[\"foo\", \"bar\", {foo: 0, bar: 1}]" * * Related: see {Methods for Converting}[rdoc-ref:Set@Methods+for+Converting]. */ @@ -696,23 +692,28 @@ set_i_to_set(VALUE set) /* * call-seq: - * join(separator = $,) -> new_string + * join(separator = $,) -> string * - * Returns the new string formed by joining the string-converted elements of +self+ + * Returns the string formed by joining the string-converted elements of +self+ * with the given +separator+ (defaults to $,): * - * $, # => nil - * Set[].join # => "" - * Set[%w[foo]].join # => "foo" - * s = Set[%w[foo bar baz]] # => Set[["foo", "bar", "baz"]] - * s.join # => "foobarbaz" - * s.join('|') # => "foo|bar|baz" - * s.join(' :|: ') # => "foo :|: bar :|: baz" + * $, # => nil + * Set[*%w[foo bar baz]].join + * # => "foobarbaz" + * Set[*%w[foo bar baz]].join(', ') + * # => "foo, bar, baz" * - * Flattens and joins nested arrays: + * Flattens nested arrays: * - * Set[[:foo, [:bar, [:baz, :bat]]]].join0 # => "foobarbazbat" + * Set[[:foo, [:bar, [:baz, :bat]]]].join + * # => "foobarbazbat" * + * Does not flatten nested sets: + * + * Set[Set[:foo, Set[:bar, Set[:baz, :bat]]]].join + * # => "Set[:foo, Set[:bar, Set[:baz, :bat]]]" + * + * Related: see {Methods for Converting}[rdoc-ref:Set@Methods+for+Converting]. */ static VALUE set_i_join(int argc, VALUE *argv, VALUE set) @@ -723,14 +724,15 @@ set_i_join(int argc, VALUE *argv, VALUE set) /* * call-seq: - * add(obj) -> self + * add(object) -> self * - * Adds the given object to the set and returns self. Use Set#merge to - * add many elements at once. + * Adds the given +object+ to +self+, returns +self+: * - * Set[1, 2].add(3) #=> Set[1, 2, 3] - * Set[1, 2].add([3, 4]) #=> Set[1, 2, [3, 4]] - * Set[1, 2].add(2) #=> Set[1, 2] + * set = Set[0, 1, 2] + * set.add(%w[a b c]) # => Set[0, 1, 2, ["a", "b", "c"]] + * set.add(0) # => Set[0, 1, 2, ["a", "b", "c"]] + * + * Related: see {Methods for Assigning}[rdoc-ref:Set@Methods+for+Assigning]. */ static VALUE set_i_add(VALUE set, VALUE item) @@ -749,14 +751,16 @@ set_i_add(VALUE set, VALUE item) /* * call-seq: - * add?(obj) -> self or nil + * add?(object) -> self or nil + * + * Like #add, but returns +nil+ if +object+ is already in +self+: * - * Adds the given object to the set and returns self. If the object is - * already in the set, returns nil. + * set = Set[0, 1, 2] + * set.add?(:foo) # => Set[0, 1, 2, :foo] + * set.add?(0..9) # => Set[0, 1, 2, :foo, 0..9] + * set.add?(2) # => nil * - * Set[1, 2].add?(3) #=> Set[1, 2, 3] - * Set[1, 2].add?([3, 4]) #=> Set[1, 2, [3, 4]] - * Set[1, 2].add?(2) #=> nil + * Related: see {Methods for Assigning}[rdoc-ref:Set@Methods+for+Assigning]. */ static VALUE set_i_add_p(VALUE set, VALUE item) @@ -775,10 +779,16 @@ set_i_add_p(VALUE set, VALUE item) /* * call-seq: - * delete(obj) -> self + * delete(object) -> self * - * Deletes the given object from the set and returns self. Use subtract - * to delete many items at once. + * Removes the given +object+ from +self+, if +self+ includes the object; + * returns +self+: + * + * set = Set[0, 'zero', :zero] + * set.delete(0) # => Set["zero", :zero] + * set.delete(:nosuch) # => Set["zero", :zero] + * + * Related: see {Methods for Deleting}[rdoc-ref:Set@Methods+for+Deleting]. */ static VALUE set_i_delete(VALUE set, VALUE item) @@ -792,10 +802,15 @@ set_i_delete(VALUE set, VALUE item) /* * call-seq: - * delete?(obj) -> self or nil + * delete?(object) -> self or nil + * + * Like #delete, but returns +nil+ if the object is not in +self+: * - * Deletes the given object from the set and returns self. If the - * object is not in the set, returns nil. + * set = Set[0, 'zero', :zero] + * set.delete?(0) # => Set["zero", :zero] + * set.delete?(0) # => nil + * + * Related: see {Methods for Deleting}[rdoc-ref:Set@Methods+for+Deleting]. */ static VALUE set_i_delete_p(VALUE set, VALUE item) @@ -816,11 +831,20 @@ set_delete_if_i(st_data_t key, st_data_t dummy) /* * call-seq: - * delete_if { |o| ... } -> self + * delete_if {|element| ... } -> self * delete_if -> enumerator * - * Deletes every element of the set for which block evaluates to - * true, and returns self. Returns an enumerator if no block is given. + * With a block given, calls the block with each element in +self+; + * removes the element if the block returns a truthy value: + * + * set = Set[*0..9] + * # => Set[0, 1, 2, 3, 4, 5, 6, 7, 8, 9] + * set.delete_if {|element| element.even? } + * # => Set[1, 3, 5, 7, 9] + * + * With no block given, returns an Enumerator. + * + * Related: {Methods for Deleting}[rdoc-ref:Set@Methods+for+Deleting]. */ static VALUE set_i_delete_if(VALUE set) @@ -874,21 +898,23 @@ set_classify_i(st_data_t key, st_data_t tmp) /* * call-seq: - * classify { |o| ... } -> hash + * classify {|element| ... } -> hash * classify -> enumerator * - * Classifies the set by the return value of the given block and - * returns a hash of {value => set of elements} pairs. The block is - * called once for each element of the set, passing the element as - * parameter. + * With a block given, calls the block with each element of +self+; + * returns a hash whose keys are the block's return values. + * The value for each key is a set containing the elements + * for which the block returned that key. * - * files = Set.new(Dir.glob("*.rb")) - * hash = files.classify { |f| File.mtime(f).year } - * hash #=> {2000 => Set["a.rb", "b.rb"], - * # 2001 => Set["c.rb", "d.rb", "e.rb"], - * # 2002 => Set["f.rb"]} + * This example classifies elements by their classes: * - * Returns an enumerator if no block is given. + * set = Set[*(5..7), *%w[foo bar]] # => Set[5, 6, 7, "foo", "bar"] + * set.classify {|element| element.class } + * # => {Integer => Set[5, 6, 7], String => Set["foo", "bar"]} + * + * With no block given, returns an Enumerator. + * + * Related: see {Methods for Converting}[rdoc-ref:Set@Methods+for+Converting]. */ static VALUE set_i_classify(VALUE set) @@ -1019,11 +1045,11 @@ set_clear_i(st_data_t key, st_data_t dummy) * call-seq: * clear -> self * - * Removes all elements and returns self. + * Returns +self+ with all elements removed: * - * set = Set[1, 'c', :s] #=> Set[1, "c", :s] - * set.clear #=> Set[] - * set #=> Set[] + * Set[1, :one, 'one', 1.0].clear # => Set[] + * + * Related: see {Methods for Deleting}[rdoc-ref:Set@Methods+for+Deleting]. */ static VALUE set_i_clear(VALUE set) @@ -1115,27 +1141,29 @@ set_i_intersection(VALUE set, VALUE other) /* * call-seq: - * include?(item) -> true or false + * include?(object) -> true or false * - * Returns true if the set contains the given object: + * Returns whether the given +object+ is an element of +self+: * - * Set[1, 2, 3].include? 2 #=> true - * Set[1, 2, 3].include? 4 #=> false + * set = [0, :zero, '0'] + * set.include?('0') # => true + * set.include?('zero') # => false * - * Note that include? and member? do not test member - * equality using == as do other Enumerables. + * Tests equality using `hash` and `eql?`. * - * This is aliased to #===, so it is usable in +case+ expressions: + * Aliased as #===, which means that sets may be used in +case+ expressions: * * case :apple * when Set[:potato, :carrot] - * "vegetable" + * 'vegetable' * when Set[:apple, :banana] - * "fruit" + * 'fruit' + * else + * 'unknown' * end * # => "fruit" * - * See also Enumerable#include? + * Related: see {Methods for Querying}[rdoc-ref:Set@Methods+for+Querying]. */ static VALUE set_i_include(VALUE set, VALUE item) @@ -1290,7 +1318,12 @@ set_i_size(VALUE set) * call-seq: * empty? -> true or false * - * Returns true if the set contains no elements. + * Returns whether +self+ contains no elements: + * + * Set[].empty? # => true + * Set[0].empty? # => false + * + * Related: see {Methods for Querying}[rdoc-ref:Set@Methods+for+Querying]. */ static VALUE set_i_empty(VALUE set) @@ -1312,14 +1345,26 @@ set_xor_i(st_data_t key, st_data_t data) /* * call-seq: - * set ^ enum -> new_set + * self ^ enumerable -> new_set + * + * Returns a new \Set object containing + * the {exclusive OR}[https://en.wikipedia.org/wiki/Exclusive_or] + * of +self+ and the given +enumerable+; + * that is, containing each element that is in either +self+ or +enumerable+, + * but not in both: + * + * set = Set[0, 1, 2] + * set ^ Set[1, 2, 3] # => Set[0, 3] + * set ^ Set[2, 1] # => Set[0] + * set ^ Set[2, *('a'..'c')] # => Set[0, 1, "a", "b", "c"] + * set ^ Set[2, 1, 0] # => Set[] + * + * For \Set +set+ and \Enumerable +enumerable+, these expressions are equivalent: * - * Returns a new set containing elements exclusive between the set and the - * given enumerable object. (set ^ enum) is equivalent to - * ((set | enum) - (set & enum)). + * set ^ enumerable + * ((set | enumerable) - (set & enumerable)) * - * Set[1, 2] ^ Set[2, 3] #=> Set[3, 1] - * Set[1, 'b', 'c'] ^ ['b', 'd'] #=> Set["d", 1, "c"] + * Related: see {Methods for Set Operations}[rdoc-ref:Set@Methods+for+Set+Operations]. */ static VALUE set_i_xor(VALUE set, VALUE other) @@ -1340,13 +1385,18 @@ set_i_xor(VALUE set, VALUE other) /* * call-seq: - * set | enum -> new_set + * self | enumerable -> new_set + * + * Returns a new \Set object containing + * the {union}[https://en.wikipedia.org/wiki/Union_(set_theory)] + * of +self+ and the given +enumerable+; + * that is, containing the elements of both +self+ and +enumerable+. * - * Returns a new set built by merging the set and the elements of the - * given enumerable object. + * set = Set[0, 1, 2] + * set | Set[2, 1, 'a'] # => Set[0, 1, 2, "a"] + * set | set # => Set[0, 1, 2] * - * Set[1, 2, 3] | Set[2, 4, 5] #=> Set[1, 2, 3, 4, 5] - * Set[1, 5, 'z'] | (1..6) #=> Set[1, 5, "z", 2, 3, 4, 6] + * Related: see {Methods for Set Operations}[rdoc-ref:Set@Methods+for+Set+Operations]. */ static VALUE set_i_union(VALUE set, VALUE other) @@ -1428,12 +1478,18 @@ set_each_i(st_data_t key, st_data_t dummy) /* * call-seq: - * each { |o| ... } -> self + * each {|element| ... } -> self * each -> enumerator * - * Calls the given block once for each element in the set, passing - * the element as parameter. Returns an enumerator if no block is - * given. + * With a block given, calls the block once for each element in the set, + * passing the element as a parameter; + * returns +self+: + * + * sum = 0 + * Set[1, 2, 3].each {|i| sum += i } + * sum => 6 + * + * With no block given, returns an Enumerator. */ static VALUE set_i_each(VALUE set) @@ -1452,11 +1508,18 @@ set_collect_i(st_data_t key, st_data_t data) /* * call-seq: - * collect! { |o| ... } -> self + * collect! {|element| ... } -> self * collect! -> enumerator * - * Replaces the elements with ones returned by +collect+. - * Returns an enumerator if no block is given. + * With a block given, calls the block with each element in +self+; + * replaces the element with the block's return value: + * + * Set[1, :one, 'one', 1.0].collect! {|element| element.class } + * # => Set[Integer, Symbol, String, Float] + * + * With no block given, returns an Enumerator. + * + * Related: see {Methods for Converting}[rdoc-ref:Set@Methods+for+Converting]. */ static VALUE set_i_collect(VALUE set) @@ -1609,10 +1672,23 @@ set_flatten_merge(VALUE set, VALUE from, VALUE hash) /* * call-seq: - * flatten -> set + * flatten -> new_set + * + * Returns a new set that is a copy of +self+, + * but with +self+ and its nested sets flattened; + * that is, their elements become elements of +self+: + * + * Set[Set[0, 1], Set[2, 3]].flatten + * # => Set[0, 1, 2, 3] + * Set[Set[0, 1], Set[Set[2, 3], Set[3, 4]]].flatten + * # => Set[0, 1, 2, 3, 4] * - * Returns a new set that is a copy of the set, flattening each - * containing set recursively. + * Does not flatten nested arrays or hashes: + * + * Set[%w[foo bar]].flatten # => Set[["foo", "bar"]] + * Set[{foo: 0, bar: 1}].flatten # => Set[{foo: 0, bar: 1}] + * + * Related: see {Methods for Converting}[rdoc-ref:Set@Methods+for+Converting]. */ static VALUE set_i_flatten(VALUE set) @@ -1634,10 +1710,21 @@ set_contains_set_i(st_data_t item, st_data_t arg) /* * call-seq: - * flatten! -> self + * flatten! -> self or nil + * + * Like #flatten, but if any changes were made + * replaces +self+ with the result and returns +self+: + * + * Set[Set[0, 1], Set[2, 3]].flatten! + * # => Set[0, 1, 2, 3] + * Set[Set[0, 1], Set[Set[2, 3], Set[3, 4]]].flatten! + * # => Set[0, 1, 2, 3, 4] + * + * Returns +nil+ if no changes were made: + * + * Set[0, 1, 2].flatten! # => nil * - * Equivalent to Set#flatten, but replaces the receiver with the - * result in place. Returns nil if no modifications were made. + * Related: see {Methods for Assigning}[rdoc-ref:Set@Methods+for+Assigning]. */ static VALUE set_i_flatten_bang(VALUE set) @@ -1743,15 +1830,16 @@ set_intersect_i(st_data_t key, st_data_t arg) /* * call-seq: - * intersect?(set) -> true or false + * intersect?(enumerable) -> true or false * - * Returns true if the set and the given enumerable have at least one - * element in common. + * Returns whether +self+ and +enumerable+ have any elements in common: * - * Set[1, 2, 3].intersect? Set[4, 5] #=> false - * Set[1, 2, 3].intersect? Set[3, 4] #=> true - * Set[1, 2, 3].intersect? 4..5 #=> false - * Set[1, 2, 3].intersect? [3, 4] #=> true + * set = Set[0, 'zero', :zero] + * set.intersect?([0, 1, 2]) # => true + * set.intersect?(%w[zero one two]) # => true + * set.intersect?(Set[3]) # => false + * + * Related: see {Methods for Querying}[rdoc-ref:Set@Methods+for+Querying]. */ static VALUE set_i_intersect(VALUE set, VALUE other) @@ -1784,15 +1872,15 @@ set_i_intersect(VALUE set, VALUE other) /* * call-seq: - * disjoint?(set) -> true or false + * disjoint?(enumerable) -> true or false + * + * Returns whether no element of +enumerable+ is present in +self+: * - * Returns true if the set and the given enumerable have no - * element in common. This method is the opposite of +intersect?+. + * set = Set[0, 'zero', :zero] + * set.disjoint?([1, 2, 3]) # => true + * set.disjoint?([0, 1, 2, 3]) # => false * - * Set[1, 2, 3].disjoint? Set[3, 4] #=> false - * Set[1, 2, 3].disjoint? Set[4, 5] #=> true - * Set[1, 2, 3].disjoint? [3, 4] #=> false - * Set[1, 2, 3].disjoint? 4..5 #=> true + * Related: see {Methods for Querying}[rdoc-ref:Set@Methods+for+Querying]. */ static VALUE set_i_disjoint(VALUE set, VALUE other) @@ -1883,9 +1971,16 @@ set_recursive_eql(VALUE set, VALUE dt, int recur) /* * call-seq: - * set == other -> true or false + * self == object -> true or false + * + * Returns whether +object+ is a set, and has the same elements as +self+: * - * Returns true if two sets are equal. + * set = Set[0, 1, 2] + * set == Set[1, 2, 0] # => true + * set == [1, 2, 3] # => false + * set == Set[1, 2, '3'] # => false + * + * Related: see {Methods for Comparing}[rdoc-ref:Set@Methods+for+Comparing]. */ static VALUE set_i_eq(VALUE set, VALUE other) diff --git a/spec/bundled_gems_spec.rb b/spec/bundled_gems_spec.rb index 45ababa9ed6588..dc0e7dde6c7bfc 100644 --- a/spec/bundled_gems_spec.rb +++ b/spec/bundled_gems_spec.rb @@ -388,6 +388,12 @@ def my end context "with bundle environment" do + # Windows has no executable bit or shebang dispatch, so running the + # script directly is rejected by bundler as "not executable". Invoke it + # through ruby there. What matters here is force_activate's behavior under + # the bundle environment, not shebang execution (covered by another spec). + let(:exec_command) { Gem.win_platform? ? "exec ruby ./script.rb" : "exec ./script.rb" } + before do code = <<-RUBY #!/usr/bin/env ruby @@ -400,13 +406,13 @@ def my it "lockfile is available" do bundle "install" - bundle "exec ./script.rb" + bundle exec_command expect(err).to include("gem install csv") end it "lockfile is not available" do - bundle "exec ./script.rb" + bundle exec_command expect(err).to include("gem install csv") end diff --git a/spec/bundler/bundler/cli_spec.rb b/spec/bundler/bundler/cli_spec.rb index 56caf9937e29f6..1e8ffa7e37c883 100644 --- a/spec/bundler/bundler/cli_spec.rb +++ b/spec/bundler/bundler/cli_spec.rb @@ -237,6 +237,7 @@ def out_with_macos_man_workaround context "when the latest version is greater than the current version" do let(:latest_version) { "222.0" } it "prints the version warning" do + skip "temp dir is on a different drive than the source tree" if tmp_and_source_on_different_drives? bundle "fail", env: { "BUNDLER_VERSION" => bundler_version }, raise_on_error: false expect(err).to start_with(<<-EOS.strip) The latest bundler is #{latest_version}, but you are currently running #{bundler_version}. @@ -264,6 +265,7 @@ def out_with_macos_man_workaround context "and is a pre-release" do let(:latest_version) { "222.0.0.pre.4" } it "prints the version warning" do + skip "temp dir is on a different drive than the source tree" if tmp_and_source_on_different_drives? bundle "fail", env: { "BUNDLER_VERSION" => bundler_version }, raise_on_error: false expect(err).to start_with(<<-EOS.strip) The latest bundler is #{latest_version}, but you are currently running #{bundler_version}. diff --git a/spec/bundler/bundler/env_spec.rb b/spec/bundler/bundler/env_spec.rb index 2b7dbde217d8e2..1501bb9eb9b8fa 100644 --- a/spec/bundler/bundler/env_spec.rb +++ b/spec/bundler/bundler/env_spec.rb @@ -117,6 +117,7 @@ def with_clear_paths(env_var, env_value) let(:output) { described_class.report(print_gemfile: true) } it "prints the config with redacted values" do + skip "temp dir is on a different drive than the source tree" if tmp_and_source_on_different_drives? expect(output).to include("https://localgemserver.test") expect(output).to include("user:[REDACTED]") expect(output).to_not include("user:pass") @@ -131,6 +132,7 @@ def with_clear_paths(env_var, env_value) let(:output) { described_class.report(print_gemfile: true) } it "prints the config with redacted values" do + skip "temp dir is on a different drive than the source tree" if tmp_and_source_on_different_drives? expect(output).to include("https://localgemserver.test") expect(output).to include("[REDACTED]:x-oauth-basic") expect(output).to_not include("api_token:x-oauth-basic") diff --git a/spec/bundler/bundler/installer/parallel_installer_spec.rb b/spec/bundler/bundler/installer/parallel_installer_spec.rb index 6a91f05bf8b48b..528dc1ae93e525 100644 --- a/spec/bundler/bundler/installer/parallel_installer_spec.rb +++ b/spec/bundler/bundler/installer/parallel_installer_spec.rb @@ -83,6 +83,11 @@ skip "This example is runnable when RubyGems::Installer implements `build_jobs`" end + # The make jobserver is a GNU make feature. On Windows extensions are built + # with nmake, which has no `-j` jobserver, so the per-gem slot count never + # appears in the build output. + skip "The make jobserver is not available on Windows (nmake)" if mswin? + # When run under a parent make that already passes `-j` (e.g. ruby/ruby's # `make test-bundler-parallel`), RubyGems' extension builder sees the # inherited MAKEFLAGS as "jobs already requested" and skips appending its diff --git a/spec/bundler/bundler/lockfile_parser_spec.rb b/spec/bundler/bundler/lockfile_parser_spec.rb index cec77b0cb4cde0..c92d8909d29e56 100644 --- a/spec/bundler/bundler/lockfile_parser_spec.rb +++ b/spec/bundler/bundler/lockfile_parser_spec.rb @@ -118,7 +118,7 @@ let(:platforms) { [Gem::Platform::RUBY] } let(:bundler_version) { Gem::Version.new("1.12.0.rc.2") } let(:ruby_version) { "ruby 2.1.3p242" } - let(:lockfile_path) { Bundler.default_lockfile.relative_path_from(Dir.pwd) } + let(:lockfile_path) { Bundler::SharedHelpers.relative_lockfile_path } let(:rake_sha256_checksum) do Bundler::Checksum.from_lock( "sha256=814828c34f1315d7e7b7e8295184577cc4e969bad6156ac069d02d63f58d82e8", diff --git a/spec/bundler/bundler/shared_helpers_spec.rb b/spec/bundler/bundler/shared_helpers_spec.rb index 41115aa667312b..ab89d280a22416 100644 --- a/spec/bundler/bundler/shared_helpers_spec.rb +++ b/spec/bundler/bundler/shared_helpers_spec.rb @@ -68,6 +68,7 @@ describe "#default_bundle_dir" do context ".bundle does not exist" do it "returns nil" do + skip "temp dir is on a different drive than the source tree" if tmp_and_source_on_different_drives? expect(subject.default_bundle_dir).to be_nil end end diff --git a/spec/bundler/commands/cache_spec.rb b/spec/bundler/commands/cache_spec.rb index e223d07f7fc1e7..b33a5a386c7449 100644 --- a/spec/bundler/commands/cache_spec.rb +++ b/spec/bundler/commands/cache_spec.rb @@ -208,6 +208,7 @@ end it "prints a warn when using legacy windows rubies" do + skip "the legacy windows platform gem is not cached for the current mswin platform" if mswin? gemfile <<-D source "https://gem.repo1" gem 'myrack', :platforms => [:ruby_20, :x64_mingw_20] diff --git a/spec/bundler/commands/exec_spec.rb b/spec/bundler/commands/exec_spec.rb index aa35685be8ff50..d744fc616bf238 100644 --- a/spec/bundler/commands/exec_spec.rb +++ b/spec/bundler/commands/exec_spec.rb @@ -72,6 +72,7 @@ end it "works when exec'ing to rubygems" do + skip "https://github.com/ruby/rubygems/issues/3351" if mswin? install_gemfile "source \"https://gem.repo1\"; gem \"myrack\"" bundle "exec #{gem_cmd} --version" expect(out).to eq(Gem::VERSION) @@ -204,6 +205,7 @@ end it "uses version provided by ruby" do + skip "https://github.com/ruby/rubygems/issues/3351" if mswin? bundle "exec erb --version" expect(stdboth).to eq(default_erb_version) @@ -632,6 +634,7 @@ describe "with gems bundled via :path with invalid gemspecs" do it "outputs the gemspec validation errors" do + skip "https://github.com/ruby/rubygems/issues/3351" if mswin? build_lib "foo" gemspec = lib_path("foo-1.0").join("foo.gemspec").to_s @@ -692,6 +695,7 @@ def bin_path(a,b,c) end it "works" do + skip "https://github.com/ruby/rubygems/issues/3351" if mswin? bundle "exec #{gem_cmd} uninstall foo" expect(out).to eq("Successfully uninstalled foo-1.0") end @@ -713,6 +717,7 @@ def bin_path(a,b,c) end it "does not load plugins outside of the bundle" do + skip "https://github.com/ruby/rubygems/issues/3351" if mswin? bundle "exec #{gem_cmd} -v" expect(out).not_to include("FAIL") end diff --git a/spec/bundler/commands/install_spec.rb b/spec/bundler/commands/install_spec.rb index 18c3fd65038c16..f8a134f231089e 100644 --- a/spec/bundler/commands/install_spec.rb +++ b/spec/bundler/commands/install_spec.rb @@ -1349,6 +1349,11 @@ def run skip "This example is runnable when RubyGems::Installer implements `build_jobs`" end + # The make jobserver is a GNU make feature. On Windows extensions are built + # with nmake, which has no `-j` jobserver (and an inherited `-j` MAKEFLAGS + # even breaks nmake), so the slot count these examples assert never appears. + skip "The make jobserver is not available on Windows (nmake)" if mswin? + @old_makeflags = ENV["MAKEFLAGS"] @gemspec = nil diff --git a/spec/bundler/commands/lock_spec.rb b/spec/bundler/commands/lock_spec.rb index 1a434009232e2b..0a1f1f8902ba9e 100644 --- a/spec/bundler/commands/lock_spec.rb +++ b/spec/bundler/commands/lock_spec.rb @@ -1301,6 +1301,11 @@ s.platform = "x64-mingw-ucrt" s.required_ruby_version = "< #{next_ruby_minor}.dev" end + + build_gem "raygun-apm", "1.0.78" do |s| + s.platform = "x64-mswin64" + s.required_ruby_version = "< #{next_ruby_minor}.dev" + end end gemfile <<-G diff --git a/spec/bundler/commands/pristine_spec.rb b/spec/bundler/commands/pristine_spec.rb index 5f80b9e5348b21..842001f6b09fc9 100644 --- a/spec/bundler/commands/pristine_spec.rb +++ b/spec/bundler/commands/pristine_spec.rb @@ -232,6 +232,7 @@ # This just verifies that the generated Makefile from the c_ext gem makes # use of the build_args from the bundle config it "applies the config when installing the gem" do + skip "the generated Makefile uses MSVC `-libpath:` syntax instead of `-L` on Windows" if mswin? bundle "pristine" makefile_contents = File.read(c_ext_dir.join("Makefile").to_s) @@ -249,6 +250,7 @@ # This just verifies that the generated Makefile from the c_ext gem makes # use of the build_args from the bundle config it "applies the config when installing the gem" do + skip "the generated Makefile uses MSVC `-libpath:` syntax instead of `-L` on Windows" if mswin? bundle "pristine" makefile_contents = File.read(c_ext_dir.join("Makefile").to_s) diff --git a/spec/bundler/install/global_cache_spec.rb b/spec/bundler/install/global_cache_spec.rb index 4cffa65b2a9d97..259e8d57ba2ebc 100644 --- a/spec/bundler/install/global_cache_spec.rb +++ b/spec/bundler/install/global_cache_spec.rb @@ -63,6 +63,7 @@ def source2_global_cache(*segments) end it "uses a shorter path for the cache to not hit filesystem limits" do + skip "Windows without long path support cannot create the long cache path" if Gem.win_platform? install_gemfile <<-G, artifice: "compact_index", verbose: true source "http://#{"a" * 255}.test" gem "myrack" @@ -82,17 +83,19 @@ def source2_global_cache(*segments) # the more verbose and explicit approach. This whole ensure block can be # removed once/if https://bugs.ruby-lang.org/issues/21177 is fixed, and # once the fix propagates to all supported rubies. - File.delete cached_gem - Dir.rmdir source_cache - - File.delete compact_index_cache_path.join(source_segment, "info", "myrack") - Dir.rmdir compact_index_cache_path.join(source_segment, "info") - File.delete compact_index_cache_path.join(source_segment, "info-etags", "myrack-92f3313ce5721296f14445c3a6b9c073") - Dir.rmdir compact_index_cache_path.join(source_segment, "info-etags") - Dir.rmdir compact_index_cache_path.join(source_segment, "info-special-characters") - File.delete compact_index_cache_path.join(source_segment, "versions") - File.delete compact_index_cache_path.join(source_segment, "versions.etag") - Dir.rmdir compact_index_cache_path.join(source_segment) + if cached_gem + File.delete cached_gem + Dir.rmdir source_cache + + File.delete compact_index_cache_path.join(source_segment, "info", "myrack") + Dir.rmdir compact_index_cache_path.join(source_segment, "info") + File.delete compact_index_cache_path.join(source_segment, "info-etags", "myrack-92f3313ce5721296f14445c3a6b9c073") + Dir.rmdir compact_index_cache_path.join(source_segment, "info-etags") + Dir.rmdir compact_index_cache_path.join(source_segment, "info-special-characters") + File.delete compact_index_cache_path.join(source_segment, "versions") + File.delete compact_index_cache_path.join(source_segment, "versions.etag") + Dir.rmdir compact_index_cache_path.join(source_segment) + end end describe "when the same gem from different sources is installed" do diff --git a/spec/bundler/realworld/fixtures/tapioca/Gemfile.lock b/spec/bundler/realworld/fixtures/tapioca/Gemfile.lock index c2df2f92299ad2..a08089a6f7b33b 100644 --- a/spec/bundler/realworld/fixtures/tapioca/Gemfile.lock +++ b/spec/bundler/realworld/fixtures/tapioca/Gemfile.lock @@ -32,7 +32,7 @@ GEM thor (>= 1.2.0) yard-sorbet thor (1.4.0) - yard (0.9.42) + yard (0.9.44) yard-sorbet (0.9.0) sorbet-runtime yard diff --git a/spec/bundler/support/path.rb b/spec/bundler/support/path.rb index 7a482912cf3760..17dafb91b70ffb 100644 --- a/spec/bundler/support/path.rb +++ b/spec/bundler/support/path.rb @@ -127,6 +127,16 @@ def tmp_root end end + # On Windows there is no relative path between different drives, and much of + # the spec setup (temp home, bundled app, caches) lives under the temp dir. + # When the temp dir is on a different drive than the source tree, examples + # that compare or look up paths across the two cannot be set up correctly. + def tmp_and_source_on_different_drives? + return false unless Gem.win_platform? + drive = ->(path) { path.to_s[/\A[a-zA-Z]:/]&.upcase } + drive[tmp_root] != drive[source_root] + end + # Bump this version whenever you make a breaking change to the spec setup # that requires regenerating tmp/. diff --git a/spec/bundler/support/platforms.rb b/spec/bundler/support/platforms.rb index 56a08430056b98..28ba84634860c9 100644 --- a/spec/bundler/support/platforms.rb +++ b/spec/bundler/support/platforms.rb @@ -28,6 +28,13 @@ def not_local_tag [:jruby, :windows, :ruby].find {|tag| tag != local_tag } end + # The mswin build uses nmake and MSVC, which differ from the mingw build in + # ways several specs need to skip (no make jobserver, MSVC Makefile syntax, + # extensionless executables, mswin-only fixtures). + def mswin? + RUBY_PLATFORM.include?("mswin") + end + def local_ruby_engine RUBY_ENGINE end diff --git a/spec/bundler/support/subprocess.rb b/spec/bundler/support/subprocess.rb index 91db80da488c6a..b03ba839dd8623 100644 --- a/spec/bundler/support/subprocess.rb +++ b/spec/bundler/support/subprocess.rb @@ -38,7 +38,7 @@ def sh(cmd, options = {}) dir = options[:dir] env = options[:env] || {} - command_execution = CommandExecution.new(cmd.to_s, timeout: options[:timeout] || 60) + command_execution = CommandExecution.new(cmd.to_s, timeout: options[:timeout] || (Gem.win_platform? ? 120 : 60)) open3_opts = {} open3_opts[:chdir] = dir if dir diff --git a/spec/ruby/core/file/atime_spec.rb b/spec/ruby/core/file/atime_spec.rb index 5c6c110eec43f3..af9393bef496b0 100644 --- a/spec/ruby/core/file/atime_spec.rb +++ b/spec/ruby/core/file/atime_spec.rb @@ -19,7 +19,7 @@ unless ENV.key?('TRAVIS') # https://bugs.ruby-lang.org/issues/17926 ## NOTE also that some Linux systems disable atime (e.g. via mount params) for better filesystem speed. it "returns the last access time for the named file with microseconds" do - supports_subseconds = Integer(`stat -c%x '#{__FILE__}'`[/\.(\d{1,6})/, 1], 10) + supports_subseconds = Integer(`stat -c%x #{__FILE__}`[/\.(\d{1,6})/, 1], 10) if supports_subseconds != 0 expected_time = Time.at(Time.now.to_i + 0.123456) File.utime expected_time, 0, @file diff --git a/spec/ruby/core/file/ctime_spec.rb b/spec/ruby/core/file/ctime_spec.rb index cf37d1f4eeca96..25058fe6820a24 100644 --- a/spec/ruby/core/file/ctime_spec.rb +++ b/spec/ruby/core/file/ctime_spec.rb @@ -16,7 +16,7 @@ platform_is :linux, :windows do it "returns the change time for the named file (the time at which directory information about the file was changed, not the file itself) with microseconds." do - supports_subseconds = Integer(`stat -c%z '#{__FILE__}'`[/\.(\d{1,6})/, 1], 10) + supports_subseconds = Integer(`stat -c%z #{__FILE__}`[/\.(\d{1,6})/, 1], 10) if supports_subseconds != 0 File.ctime(__FILE__).usec.should > 0 else diff --git a/spec/ruby/core/file/mtime_spec.rb b/spec/ruby/core/file/mtime_spec.rb index d83725e25d9ecc..2e28695d977ab1 100644 --- a/spec/ruby/core/file/mtime_spec.rb +++ b/spec/ruby/core/file/mtime_spec.rb @@ -18,7 +18,7 @@ platform_is :linux, :windows do unless ENV.key?('TRAVIS') # https://bugs.ruby-lang.org/issues/17926 it "returns the modification Time of the file with microseconds" do - supports_subseconds = Integer(`stat -c%y '#{__FILE__}'`[/\.(\d{1,6})/, 1], 10) + supports_subseconds = Integer(`stat -c%y #{__FILE__}`[/\.(\d{1,6})/, 1], 10) if supports_subseconds != 0 expected_time = Time.at(Time.now.to_i + 0.123456) File.utime 0, expected_time, @filename diff --git a/spec/ruby/core/kernel/require_spec.rb b/spec/ruby/core/kernel/require_spec.rb index 62d954c2bf7eee..28d94b0515121a 100644 --- a/spec/ruby/core/kernel/require_spec.rb +++ b/spec/ruby/core/kernel/require_spec.rb @@ -34,7 +34,7 @@ features -= %w[java.rb jruby/util.rb] when "ruby" so = RbConfig::CONFIG['DLEXT'] - features -= ["windows_1252.#{so}", "windows_31.#{so}"] + features.reject! { |feature| feature.end_with?("windows_1252.#{so}", "windows_31j.#{so}") } features.reject! { |feature| feature.end_with? "encdb.#{so}" } features.reject! { |feature| feature.end_with? "transdb.#{so}" } features.reject! { |feature| feature.include?('-fake') } diff --git a/spec/zjit.mspec b/spec/zjit.mspec new file mode 100644 index 00000000000000..570b17fcf08881 --- /dev/null +++ b/spec/zjit.mspec @@ -0,0 +1,2 @@ +# Fails with --zjit-disable-hir-opt. See https://github.com/Shopify/ruby/issues/970 +MSpec.register(:exclude, "Kernel#eval updates a local in a scope above a surrounding block scope") diff --git a/template/Makefile.in b/template/Makefile.in index 7ce612e8e9c704..18f54c6f7d9c99 100644 --- a/template/Makefile.in +++ b/template/Makefile.in @@ -733,30 +733,5 @@ yes-test-leaked-globals: yes-test-leaked-globals-precheck $(COMMONOBJS) $(LIBRUBY_FOR_LEAKED_GLOBALS:yes=$(LIBRUBY_SO)) $(ACTIONS_ENDGROUP) -test-syntax-suggest-precheck: $(TEST_RUNNABLE)-test-syntax-suggest-precheck -no-test-syntax-suggest-precheck: -yes-test-syntax-suggest-precheck: main - -test-syntax-suggest-prepare: $(TEST_RUNNABLE)-test-syntax-suggest-prepare -no-test-syntax-suggest-prepare: no-test-syntax-suggest-precheck -yes-test-syntax-suggest-prepare: yes-test-syntax-suggest-precheck - $(ACTIONS_GROUP) - $(XRUBY) -C "$(srcdir)" bin/gem install --no-document \ - --install-dir .bundle --conservative "rspec:~> 3" - $(ACTIONS_ENDGROUP) - -RSPECOPTS = -SYNTAX_SUGGEST_SPECS = -PREPARE_SYNTAX_SUGGEST = $(TEST_RUNNABLE)-test-syntax-suggest-prepare -test-syntax-suggest: $(TEST_RUNNABLE)-test-syntax-suggest -yes-test-syntax-suggest: $(PREPARE_SYNTAX_SUGGEST) - $(ACTIONS_GROUP) - $(XRUBY) -C $(srcdir) -Ispec/syntax_suggest:spec/lib .bundle/bin/rspec \ - --require rspec/expectations \ - --require spec_helper --require formatter_overrides --require spec_coverage \ - $(RSPECOPTS) spec/syntax_suggest/$(SYNTAX_SUGGEST_SPECS) - $(ACTIONS_ENDGROUP) -no-test-syntax-suggest: - yesterday: $(GIT_IN_SRC) reset --hard `TZ=UTC-9 $(GIT_LOG_FORMAT)%H -1 --before=00:00` diff --git a/test/.excludes-zjit/TestKeywordArguments.rb b/test/.excludes-zjit/TestKeywordArguments.rb new file mode 100644 index 00000000000000..93e137669232e3 --- /dev/null +++ b/test/.excludes-zjit/TestKeywordArguments.rb @@ -0,0 +1,3 @@ +# See . +# This tests fail with --zjit-disable-hir-opt +exclude(:test_required_keyword_with_newline, 'local assignment within eval') diff --git a/test/.excludes-zjit/TestParse.rb b/test/.excludes-zjit/TestParse.rb new file mode 100644 index 00000000000000..b1b622d5b7b6be --- /dev/null +++ b/test/.excludes-zjit/TestParse.rb @@ -0,0 +1,8 @@ +# See . +# These tests fail with --zjit-disable-hir-opt +exclude(:test_utf8_bom, 'local assignment within eval') +exclude(:test_pow_asgn, 'local assignment within eval') +exclude(:test_backquote, 'local assignment within eval') +exclude(:test_dot_in_next_line, 'local assignment within eval') +exclude(:test_here_document, 'local assignment within eval') +exclude(:test_magic_comment, 'local assignment within eval') diff --git a/test/.excludes-zjit/TestString.rb b/test/.excludes-zjit/TestString.rb new file mode 100644 index 00000000000000..4fb2634534f8f0 --- /dev/null +++ b/test/.excludes-zjit/TestString.rb @@ -0,0 +1,3 @@ +# See . +# This tests fail with --zjit-disable-hir-opt +exclude(:test_unknown_string_option, 'local assignment within eval') diff --git a/test/.excludes-zjit/TestString2.rb b/test/.excludes-zjit/TestString2.rb new file mode 100644 index 00000000000000..4fb2634534f8f0 --- /dev/null +++ b/test/.excludes-zjit/TestString2.rb @@ -0,0 +1,3 @@ +# See . +# This tests fail with --zjit-disable-hir-opt +exclude(:test_unknown_string_option, 'local assignment within eval') diff --git a/test/prism/errors/do_block_after_command_block.txt b/test/prism/errors/do_block_after_command_block.txt new file mode 100644 index 00000000000000..0989695af0c130 --- /dev/null +++ b/test/prism/errors/do_block_after_command_block.txt @@ -0,0 +1,14 @@ +foo(m a { } do end) + ^~ unexpected 'do'; expected a `)` to close the arguments + ^~ unexpected 'do', expecting end-of-input + ^~ unexpected 'do', ignoring it + ^~~ unexpected 'end', ignoring it + ^ unexpected ')', ignoring it + +foo(m a.b { } do end) + ^~ unexpected 'do'; expected a `)` to close the arguments + ^~ unexpected 'do', expecting end-of-input + ^~ unexpected 'do', ignoring it + ^~~ unexpected 'end', ignoring it + ^ unexpected ')', ignoring it + diff --git a/test/ruby/test_file_exhaustive.rb b/test/ruby/test_file_exhaustive.rb index 6e7973897c7960..0040e8d9f00a5e 100644 --- a/test/ruby/test_file_exhaustive.rb +++ b/test/ruby/test_file_exhaustive.rb @@ -715,6 +715,28 @@ def test_symlink assert_raise(Errno::EEXIST) { File.symlink(utf8_file, utf8_file) } end + def test_symlink_to_relative_directory + # A relative target is interpreted relative to the link's directory, not the + # current directory. A relative target pointing at a directory must produce + # a directory symlink even when the current directory differs from the link's + # directory; otherwise Dir operations on the link fail (Windows). + Dir.mktmpdir(__method__.to_s) do |tmpdir| + Dir.chdir(tmpdir) do + Dir.mkdir("subdir") + Dir.mkdir(File.join("subdir", "target")) + link = File.join("subdir", "link") + begin + File.symlink("target", link) + rescue NotImplementedError, Errno::EACCES, Errno::EPERM => e + omit e.message + end + assert_file.symlink?(link) + assert_file.directory?(link) + assert(Dir.exist?(link), "relative directory symlink should be a directory") + end + end + end + def test_utime t = Time.local(2000) File.utime(t + 1, t + 2, zerofile) @@ -805,10 +827,11 @@ def test_readlink_junction def test_realpath_mount_point vol = IO.popen(["mountvol", DRIVE, "/l"], &:read).strip Dir.mkdir(mnt = File.join(@dir, mntpnt = "mntpnt")) - system("mountvol", mntpnt, vol, chdir: @dir) + err = IO.popen(%W"mountvol #{mntpnt} #{vol}", chdir: @dir, err: %i[child out], &:read) + omit err unless $?.success? assert_equal(mnt, File.realpath(mnt)) ensure - system("mountvol", mntpnt, "/d", chdir: @dir) + system("mountvol", mntpnt, "/d", chdir: @dir, out: IO::NULL, err: IO::NULL) end end diff --git a/test/ruby/test_io_m17n.rb b/test/ruby/test_io_m17n.rb index 83d4fb0c7b525e..1736d01f78e3fd 100644 --- a/test/ruby/test_io_m17n.rb +++ b/test/ruby/test_io_m17n.rb @@ -404,8 +404,17 @@ def test_dup_undef end def test_stdin - assert_equal(Encoding.default_external, STDIN.external_encoding) - assert_equal(nil, STDIN.internal_encoding) + encoding = Encoding.default_external + internal = nil + if /mswin|mingw/ =~ RUBY_PLATFORM and STDIN.tty? + # Interactive console input on Windows is read in the locale (console + # code page) encoding and transcoded to the default external encoding. + encoding = Encoding.find("locale") + internal = Encoding.default_internal || Encoding.default_external + internal = nil if internal == encoding + end + assert_equal(encoding, STDIN.external_encoding) + assert_equal(internal, STDIN.internal_encoding) end def test_stdout diff --git a/test/ruby/test_settracefunc.rb b/test/ruby/test_settracefunc.rb index e2cda4dfb82af7..b4af8fa98625b8 100644 --- a/test/ruby/test_settracefunc.rb +++ b/test/ruby/test_settracefunc.rb @@ -352,7 +352,7 @@ def test_break # [ruby-core:27606] [Bug #2610] 2: events << [event, lineno, mid, klass] if file == name 3: }) 4: [1,2,3].any? {|n| n} - 8: set_trace_func(nil) + 5: set_trace_func(nil) EOF [["c-return", 1, :set_trace_func, Kernel], diff --git a/test/ruby/test_zjit.rb b/test/ruby/test_zjit.rb index 198301d9332e30..18baf3832abb72 100644 --- a/test/ruby/test_zjit.rb +++ b/test/ruby/test_zjit.rb @@ -2,6 +2,11 @@ # # This set of tests can be run with: # make test-all TESTS=test/ruby/test_zjit.rb +# +# Instead of adding new tests here, you should probably +# be adding tests that run under the Rust test harness, +# say, in `codegen_tests.rs`. It parallelizes better and +# allows for easy inspection of VM internal states. require 'test/unit' require 'envutil' diff --git a/tool/fake.rb b/tool/fake.rb index 2c458985d8e1d3..a8d727db2150ee 100644 --- a/tool/fake.rb +++ b/tool/fake.rb @@ -27,7 +27,15 @@ class File if $extmk $ruby = "$(topdir)/miniruby -I'$(topdir)' -I'$(top_srcdir)/lib' -I'$(extout)/$(arch)' -I'$(extout)/common'" else - $ruby = baseruby + # `CROSS_COMPILING` holds the platform of the ruby that loaded this fake. + # When it matches the built ruby's platform we are not really cross + # compiling, so the just-built ruby runs on this host and matches the build + # tree. Prefer it over baseruby, which may be an older release whose version + # check rejects the freshly built standard library when building gems with + # native extensions (e.g. the bundler spec's test gems run via + # `make test-bundler[-parallel]`). + native = defined?(CROSS_COMPILING) && CROSS_COMPILING == RUBY_PLATFORM + $ruby = native && File.exist?($builtruby) ? $builtruby : baseruby end $static = static untrace_var(:$ruby, posthook) diff --git a/tool/lib/_tmpdir.rb b/tool/lib/_tmpdir.rb index ac5b9be792ec63..f4ac29ffb64fd9 100644 --- a/tool/lib/_tmpdir.rb +++ b/tool/lib/_tmpdir.rb @@ -21,10 +21,18 @@ end # warn "tmpdir(#{tmpdir.size}) = #{tmpdir}" +# Bundler's spec helpers walk up the directory tree looking for `.bundle`, +# stopping once they see a `tmp` directory (Bundler::SharedHelpers#search_up). +# On Windows the temp dir lives under the user home (%LOCALAPPDATA%\Temp), so +# without a `tmp` marker the search escapes the sandbox and picks up the real +# ~/.bundle. Create one so the search stops at the temp root. +Dir.mkdir(File.join(tmpdir, "tmp")) + pid = $$ END { if pid == $$ begin + Dir.rmdir(File.join(tmpdir, "tmp")) Dir.rmdir(tmpdir) rescue Errno::ENOENT rescue Errno::ENOTEMPTY diff --git a/tool/lib/leakchecker.rb b/tool/lib/leakchecker.rb index 7bd479bbd3f83d..b1f94ea5d62d6b 100644 --- a/tool/lib/leakchecker.rb +++ b/tool/lib/leakchecker.rb @@ -122,18 +122,24 @@ def check_fd_leak(test_name) end if header = open_list&.shift columns = header.split - fd_index, node_index = columns.index('FD'), columns.index('NODE') + fd_index, type_index, node_index = %w'FD TYPE NODE'.map {|n| columns.index(n)} open_list.reject! do |of| of = of.chomp.split(' ', node_index + 2) if of[node_index] == 'TCP' and of.last.end_with?('(CLOSE_WAIT)', '(CLOSED)') + # Sometimes TCP sockets still live in the kernel space + # but have been closed in the user space. + skip = true + elsif of[type_index] == 'systm' + # AF_SYSTEM on macOS is kept alive + skip = true + end + if skip fd = of[fd_index].to_i inspect.delete(fd) h.delete(fd) live2.delete(fd) - true - else - false end + skip end puts(header, open_list) unless open_list.empty? end diff --git a/tool/outdate-bundled-gems.rb b/tool/outdate-bundled-gems.rb index b272c448c6608b..ec8762f5e0138c 100755 --- a/tool/outdate-bundled-gems.rb +++ b/tool/outdate-bundled-gems.rb @@ -101,7 +101,7 @@ def default_gem?(spec) (@defaults ||= {}).fetch(spec) do File.open(prefixed(spec)) do |f| if /^# default: (\S+) (\d+\.\d+)/ =~ f.gets("") - File.mtime(prefixed($1)) <= Time.at(Rational($2)) + File.mtime($1) <= Time.at(Rational($2)) else false end diff --git a/tool/runruby.rb b/tool/runruby.rb index ec63d1008a2348..1f7268bd4114b7 100755 --- a/tool/runruby.rb +++ b/tool/runruby.rb @@ -140,6 +140,24 @@ def File.realpath(*args) # See: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=271490 env['LD_BIND_NOW'] = 'yes' if /freebsd/ =~ RUBY_PLATFORM +# On Windows the freshly built ruby often has no usable default CA bundle (its +# OpenSSL's OPENSSLDIR does not exist), which breaks HTTPS in tests such as +# test-bundler. Fall back to the CA bundle of baseruby (the installed ruby the +# build was bootstrapped with, recorded in the fake script), unless the caller +# already configured one. +if /mswin|mingw/ =~ RUBY_PLATFORM and !ENV["SSL_CERT_FILE"] and !ENV["SSL_CERT_DIR"] + fake = File.join(abs_archdir, "#{config['arch']}-fake.rb") + if File.exist?(fake) and /^baseruby\s*=\s*"([^"\n]+)"/ =~ File.read(fake) + baseruby = $1 + script = "f = OpenSSL::X509::DEFAULT_CERT_FILE; print f if File.exist?(f)" + begin + cert = IO.popen([baseruby, "-ropenssl", "-e", script], err: File::NULL, &:read) + env["SSL_CERT_FILE"] = cert if $?.success? and !cert.empty? + rescue SystemCallError + end + end +end + ENV.update env if debugger diff --git a/tool/test-bundled-gems.rb b/tool/test-bundled-gems.rb index b603cc09d7a530..706f665b95de51 100644 --- a/tool/test-bundled-gems.rb +++ b/tool/test-bundled-gems.rb @@ -16,6 +16,35 @@ 'irb', 'csv', ] : [] + +# minitest's assertion tests compare against unified diff output produced by +# the `diff` command, so they fail spuriously when it is not available. +diff_available = ENV["PATH"].to_s.split(File::PATH_SEPARATOR).any? do |dir| + next false if dir.empty? + exe = File.join(dir, "diff") + File.executable?(exe) || (/mswin|mingw/ =~ RUBY_PLATFORM && File.file?("#{exe}.exe")) +end +DEFAULT_ALLOWED_FAILURES << 'minitest' unless diff_available + +# rake's TestBacktraceSuppression#test_system_dir_suppressed expects rake to +# suppress RbConfig's rubylibprefix from backtraces. In an uninstalled +# out-of-tree build it is a POSIX "/usr"-style prefix that File.expand_path +# turns into a drive-prefixed path on Windows, which no longer matches rake's +# suppression pattern, so the test fails. +if /mswin|mingw/ =~ RUBY_PLATFORM && RbConfig::CONFIG["rubylibprefix"] !~ /\A[a-zA-Z]:/ + DEFAULT_ALLOWED_FAILURES << 'rake' +end + +# rbs's stdlib Resolv tests need to resolve "localhost"; allow its failures on +# hosts where the Resolv library cannot resolve it. +begin + require 'resolv' + Resolv.getaddress('localhost') +rescue LoadError +rescue Resolv::ResolvError + DEFAULT_ALLOWED_FAILURES << 'rbs' +end + allowed_failures = ENV['TEST_BUNDLED_GEMS_ALLOW_FAILURES'] || '' allowed_failures = allowed_failures.split(',').concat(DEFAULT_ALLOWED_FAILURES).uniq.reject(&:empty?) diff --git a/vcpkg.json b/vcpkg.json index c2caad14cddf8a..2c30ac0fdd3713 100644 --- a/vcpkg.json +++ b/vcpkg.json @@ -7,5 +7,5 @@ "openssl", "zlib" ], - "builtin-baseline": "f3e10653cc27d62a37a3763cd84b38bca07c6075" + "builtin-baseline": "cd61e1e26a038e82d6550a3ebbe0fbbfe7da78e3" } \ No newline at end of file diff --git a/vm_callinfo.h b/vm_callinfo.h index 9a6c69deaee35d..0ff0d89d1a5617 100644 --- a/vm_callinfo.h +++ b/vm_callinfo.h @@ -61,6 +61,20 @@ rb_callinfo_kwarg_bytes(int keyword_len) rb_eRuntimeError); } +static inline void +rb_callinfo_kwarg_retain(struct rb_callinfo_kwarg *kwarg) +{ + if (kwarg) RUBY_ATOMIC_INC(kwarg->references); +} + +static inline void +rb_callinfo_kwarg_release(struct rb_callinfo_kwarg *kwarg) +{ + if (kwarg && RUBY_ATOMIC_FETCH_SUB(kwarg->references, 1) == 1) { + ruby_xfree_sized(kwarg, rb_callinfo_kwarg_bytes(kwarg->keyword_len)); + } +} + // imemo_callinfo struct rb_callinfo { VALUE flags; diff --git a/win32/win32.c b/win32/win32.c index e3a3df71f6ce34..00a87ff226d60c 100644 --- a/win32/win32.c +++ b/win32/win32.c @@ -5284,8 +5284,33 @@ w32_symlink(UINT cp, const char *src, const char *link) MultiByteToWideChar(cp, 0, src, -1, wsrc, len1); MultiByteToWideChar(cp, 0, link, -1, wlink, len2); translate_wchar(wsrc, L'/', L'\\'); + translate_wchar(wlink, L'/', L'\\'); - atts = GetFileAttributesW(wsrc); + /* A relative target is interpreted relative to the directory of the link, + not the current directory. Resolve it there to decide whether to create + a directory symlink; otherwise a relative target pointing at a directory + would wrongly become a file symlink when the current directory differs + from the link's directory. */ + { + WCHAR *sep; + int independent = + (((wsrc[0] >= L'A' && wsrc[0] <= L'Z') || + (wsrc[0] >= L'a' && wsrc[0] <= L'z')) && wsrc[1] == L':') || + wsrc[0] == L'\\'; + if (!independent && (sep = wcsrchr(wlink, L'\\')) != NULL) { + VALUE buf2; + size_t dirlen = sep - wlink + 1; + size_t srclen = wcslen(wsrc) + 1; + WCHAR *fullsrc = ALLOCV_N(WCHAR, buf2, dirlen + srclen); + MEMCPY(fullsrc, wlink, WCHAR, dirlen); + MEMCPY(fullsrc + dirlen, wsrc, WCHAR, srclen); + atts = GetFileAttributesW(fullsrc); + ALLOCV_END(buf2); + } + else { + atts = GetFileAttributesW(wsrc); + } + } if (atts != -1 && atts & FILE_ATTRIBUTE_DIRECTORY) flag = SYMBOLIC_LINK_FLAG_DIRECTORY; ret = CreateSymbolicLinkW(wlink, wsrc, flag |= create_flag); diff --git a/zjit.c b/zjit.c index bc03deafeec665..082a9038066723 100644 --- a/zjit.c +++ b/zjit.c @@ -29,7 +29,8 @@ STATIC_ASSERT(pointer_tagging_scheme, USE_FLONUM); enum zjit_struct_offsets { - ISEQ_BODY_OFFSET_PARAM = offsetof(struct rb_iseq_constant_body, param) + ISEQ_BODY_OFFSET_PARAM = offsetof(struct rb_iseq_constant_body, param), + ISEQ_BODY_OFFSET_OUTER_VARIABLES = offsetof(struct rb_iseq_constant_body, outer_variables) }; // Special JITFrame used by all C method calls. We don't control the native diff --git a/zjit/bindgen/src/main.rs b/zjit/bindgen/src/main.rs index 3e43d40efd6c74..836f105d720209 100644 --- a/zjit/bindgen/src/main.rs +++ b/zjit/bindgen/src/main.rs @@ -297,6 +297,7 @@ fn main() { .allowlist_function("rb_zjit_iseq_inspect") .allowlist_function("rb_zjit_iseq_insn_set") .allowlist_function("rb_zjit_local_id") + .allowlist_function("rb_id_table_lookup") .allowlist_function("rb_set_cfp_(pc|sp)") .allowlist_function("rb_c_method_tracing_currently_enabled") .allowlist_function("rb_zjit_method_tracing_currently_enabled") @@ -450,6 +451,9 @@ fn main() { .blocklist_type("ID") .blocklist_type("rb_iseq_constant_body") + // We only need id_table as an opaque pointer to pass to its APIs + .opaque_type("rb_id_table") + // Avoid binding to stuff we don't use .blocklist_item("rb_thread_struct.*") .opaque_type("rb_thread_struct.*") diff --git a/zjit/src/backend/lir.rs b/zjit/src/backend/lir.rs index 60f6cbb5805c67..675c1c004b8ea0 100644 --- a/zjit/src/backend/lir.rs +++ b/zjit/src/backend/lir.rs @@ -1,9 +1,10 @@ +use std::cell::RefCell; use std::collections::{BTreeSet, HashMap, HashSet}; use std::fmt; use std::mem::take; use std::rc::Rc; use crate::bitset::BitSet; -use crate::codegen::{local_size_and_idx_to_ep_offset, perf_symbol_range_start, perf_symbol_range_end}; +use crate::codegen::{local_size_and_idx_to_ep_offset, perf_symbol_range_start, perf_symbol_range_end, register_with_perf}; use crate::cruby::{IseqPtr, RUBY_OFFSET_CFP_ISEQ, RUBY_OFFSET_CFP_JIT_RETURN, RUBY_OFFSET_CFP_PC, RUBY_OFFSET_CFP_SP, SIZEOF_VALUE_I32, VALUE, ZJIT_STACK_MAP_SHIFT, ZJIT_STACK_MAP_VREG_TAG, vm_stack_canary, YarvInsnIdx, zjit_jit_frame}; use crate::hir::{Invariant, SideExitReason}; use crate::hir; @@ -549,22 +550,18 @@ pub struct SideExit { pub stack: Vec, pub locals: Vec, pub iseq: IseqPtr, - /// If set, the side exit will call the recompile function with these arguments - /// to profile the send and invalidate the ISEQ for recompilation. + /// If set, the side exit will profile the current instruction and invalidate + /// the compiled ISEQ for recompilation. pub recompile: Option, } -/// Arguments for the recompile callback on side exit. +/// Metadata for the recompile callback on side exit. #[derive(Clone, Debug, Eq, Hash, PartialEq)] pub struct SideExitRecompile { - /// The frame's own iseq, where the runtime profile is recorded at `insn_idx`. - /// For an exit out of inlined code this is the inlined callee. - pub frame_iseq: Opnd, /// The compiled unit whose version must be invalidated to force a recompile. For inlined /// methods, this will be the outer function it was inlined into. pub compiled_iseq: Opnd, pub insn_idx: u32, - pub strategy: hir::Recompile, } /// Branch target (something that we can jump to) @@ -1697,6 +1694,43 @@ impl Assembler } pub fn linearize_instructions(&self) -> Vec { + // Wrap instructions emitted by `push_insns` with PosMarkers and record + // the emitted byte range under `symbol_name` in the perf map. + fn push_insns_with_perf_symbol( + insns: &mut Vec, + symbol_name: &str, + push_insns: impl FnOnce(&mut Vec), + ) { + // ISEQ perf symbols cover the whole compiled ISEQ, including this + // padding. HIR perf needs a separate symbol because the padding + // doesn't belong to any HIR instruction. + if get_option!(perf) != Some(PerfMap::HIR) { + push_insns(insns); + return; + } + + let symbol_name = symbol_name.to_string(); + let start = Rc::new(RefCell::new(None)); + let current = start.clone(); + insns.push(Insn::PosMarker(Rc::new(move |code_ptr, _| { + let mut current = current.borrow_mut(); + assert!(current.is_none(), "perf symbol range already open"); + *current = Some(code_ptr); + }))); + + push_insns(insns); + + insns.push(Insn::PosMarker(Rc::new(move |end, cb| { + if let Some(start) = start.borrow_mut().take() { + let start_addr = start.raw_addr(cb); + let end_addr = end.raw_addr(cb); + if start_addr < end_addr { + register_with_perf(symbol_name.clone(), start_addr, end_addr - start_addr); + } + } + }))); + } + // Emit instructions with labels, expanding branch parameters let mut insns = Vec::with_capacity(ASSEMBLER_INSNS_CAPACITY); @@ -1708,7 +1742,9 @@ impl Assembler // Entry blocks shouldn't ever be preceded by something that can // stomp on this block. if !block.is_entry { - insns.push(Insn::PadPatchPoint); + push_insns_with_perf_symbol(&mut insns, "PadPatchPoint", |insns| { + insns.push(Insn::PadPatchPoint); + }); } // Process each instruction, expanding branch params if needed @@ -1730,7 +1766,9 @@ impl Assembler // Make sure we don't stomp on the next function if block_id.0 == num_blocks - 1 { - insns.push(Insn::PadPatchPoint); + push_insns_with_perf_symbol(&mut insns, "PadPatchPoint", |insns| { + insns.push(Insn::PadPatchPoint); + }); } } insns @@ -2588,15 +2626,10 @@ impl Assembler let payload = get_or_create_iseq_payload(exit.iseq); payload.reset_profiles_remaining(recompile.insn_idx as YarvInsnIdx); use crate::codegen::exit_recompile; - let (profile_kind, profile_payload) = recompile.strategy.to_c_args(); asm_comment!(asm, "profile and maybe recompile"); asm_ccall!(asm, exit_recompile, EC, - recompile.frame_iseq, - recompile.compiled_iseq, - recompile.insn_idx.into(), - profile_kind.into(), - profile_payload.into() + recompile.compiled_iseq ); } } diff --git a/zjit/src/codegen.rs b/zjit/src/codegen.rs index b5697047a3a918..154fe09adff84d 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -23,12 +23,9 @@ use crate::backend::lir::{self, Assembler, C_ARG_OPNDS, C_RET_OPND, CFP, EC, NAT use crate::hir::{iseq_to_hir, BlockId, Invariant, RangeType, SideExitReason::{self, *}, SpecialBackrefSymbol, SpecialObjectType}; use crate::hir::{BlockHandler, Const, FieldName, FrameState, Function, Insn, InsnId, Recompile, SendFallbackReason}; use crate::hir_type::{types, Type}; -use crate::options::{get_option, InlineDepth, PerfMap}; +use crate::options::{get_option, InlineDepth, PerfMap, DEFAULT_MAX_VERSIONS}; use crate::cast::IntoUsize; -/// Default maximum number of compiled versions per ISEQ. -const DEFAULT_MAX_VERSIONS: usize = 2; - /// Maximum number of compiled versions per ISEQ. /// Configurable via --zjit-max-versions (default: 2). pub fn max_iseq_versions() -> usize { @@ -276,7 +273,7 @@ pub fn gen_iseq_call(cb: &mut CodeBlock, iseq_call: &IseqCallRef) -> Result<(), } /// Write an entry to the perf map in /tmp -fn register_with_perf(symbol_name: String, start_ptr: usize, code_size: usize) { +pub(crate) fn register_with_perf(symbol_name: String, start_ptr: usize, code_size: usize) { use std::io::Write; let perf_map = format!("/tmp/perf-{}.map", std::process::id()); let Ok(file) = std::fs::OpenOptions::new().create(true).append(true).open(&perf_map) else { @@ -290,6 +287,16 @@ fn register_with_perf(symbol_name: String, start_ptr: usize, code_size: usize) { }; } +/// Register the code emitted from `start` through the current write pointer +/// under `symbol_name` in the perf map, if perf output is enabled. +fn register_current_code_range_with_perf(cb: &CodeBlock, symbol_name: &str, start: CodePtr) { + if get_option!(perf).is_some() { + let start_ptr = start.raw_addr(cb); + let end_ptr = cb.get_write_ptr().raw_addr(cb); + register_with_perf(symbol_name.to_string(), start_ptr, end_ptr - start_ptr); + } +} + /// Compile a shared JIT entry trampoline pub fn gen_entry_trampoline(cb: &mut CodeBlock) -> Result { // Set up registers for CFP, EC, SP, and basic block arguments @@ -309,12 +316,7 @@ pub fn gen_entry_trampoline(cb: &mut CodeBlock) -> Result let (code_ptr, gc_offsets) = asm.compile(cb)?; assert!(gc_offsets.is_empty()); - if get_option!(perf).is_some() { - let start_ptr = code_ptr.raw_addr(cb); - let end_ptr = cb.get_write_ptr().raw_addr(cb); - let code_size = end_ptr - start_ptr; - register_with_perf("entry trampoline".into(), start_ptr, code_size); - } + register_current_code_range_with_perf(cb, "entry trampoline", code_ptr); Ok(code_ptr) } @@ -3177,11 +3179,9 @@ fn side_exit(jit: &JITState, state: &FrameState, reason: SideExitReason) -> Targ /// Build a Target::SideExit that optionally triggers exit_recompile on the exit path. fn side_exit_with_recompile(jit: &JITState, state: &FrameState, reason: SideExitReason, recompile: Option) -> Target { let mut exit = build_side_exit(jit, state); - exit.recompile = recompile.map(|strategy| SideExitRecompile { - frame_iseq: Opnd::Value(VALUE::from(state.iseq)), + exit.recompile = recompile.map(|_| SideExitRecompile { compiled_iseq: Opnd::Value(VALUE::from(jit.iseq())), insn_idx: state.insn_idx() as u32, - strategy, }); Target::SideExit { exit, reason } } @@ -3228,19 +3228,13 @@ pub(crate) use c_callable; c_callable! { /// Called from JIT side-exit code to profile operands and trigger recompilation. - /// `profile_kind` selects what to profile; `profile_payload` carries kind-specific data. /// Once enough profiles are gathered, invalidates the compiled unit for recompilation. /// - /// Two iseqs are passed because they diverge for inlined code. `frame_iseq_raw` is - /// the frame's own iseq, where the runtime profile is recorded; for an exit out of - /// an inlined callee this is the callee, which typically has no compiled version of - /// its own. `compiled_iseq_raw` is the function that was actually compiled (the - /// inliner folds the callee's body into it), so its version is the one holding the - /// failing guard and the one we must invalidate to force a recompile. For - /// non-inlined code the two are identical. - pub(crate) fn exit_recompile(ec: EcPtr, frame_iseq_raw: VALUE, compiled_iseq_raw: VALUE, insn_idx: u32, profile_kind: i32, profile_payload: i32) { - let recompile = Recompile::from_c_args(profile_kind, profile_payload); - + /// `compiled_iseq_raw` is the ISEQ that was actually compiled. For an exit out + /// of inlined code, the inliner folds the callee's body into the outer ISEQ, so + /// the outer ISEQ's version holds the failing guard and must be invalidated to + /// force a recompile. For non-inlined code, it is the same as the frame ISEQ. + pub(crate) fn exit_recompile(ec: EcPtr, compiled_iseq_raw: VALUE) { // Fast check before taking the VM lock: skip if the compiled unit is already // invalidated or at the version limit. This avoids expensive lock acquisition // on every shape guard exit after the recompile has already been triggered. @@ -3257,37 +3251,10 @@ c_callable! { } with_vm_lock(src_loc!(), || { - let frame_iseq: IseqPtr = frame_iseq_raw.as_iseq(); let compiled_iseq: IseqPtr = compiled_iseq_raw.as_iseq(); - // For no-profile sends, skip if already profiled at this insn_idx. - // For shape guard exits, always re-profile because the - // original YARV profiles were monomorphic but runtime showed new shapes. - if matches!(recompile, Recompile::ProfileSend { .. }) && - get_or_create_iseq_payload(frame_iseq).profile.done_profiling_at(insn_idx as usize) { - return; - } - let should_recompile = with_time_stat(Counter::profile_time_ns, || { - let cfp = unsafe { get_ec_cfp(ec) }; - let payload = get_or_create_iseq_payload(frame_iseq); - - match recompile { - Recompile::ProfileSend { argc } => { - let sp = unsafe { get_cfp_sp(cfp) }; - // Profile the receiver and arguments for this send instruction - payload.profile.profile_send_at(frame_iseq, insn_idx as usize, sp, argc as usize) - } - Recompile::ProfileSelf => { - // Profile self for shape guard exits - let self_val = unsafe { get_cfp_self(cfp) }; - payload.profile.profile_self_at(frame_iseq, insn_idx as usize, self_val) - } - Recompile::ProfileBlockHandler => { - // Profile the block handler for this getblockparamproxy instruction - payload.profile.profile_getblockparamproxy_at(frame_iseq, insn_idx as usize, cfp) - } - } + crate::profile::profile_recompile_insn(ec) }); // Once we have enough profiles, invalidate the compiled unit so it @@ -3578,6 +3545,7 @@ pub fn gen_function_stub_hit_trampoline(cb: &mut CodeBlock) -> Result Result asm.compile(cb).map(|(code_ptr, gc_offsets)| { assert_eq!(gc_offsets.len(), 0); + register_current_code_range_with_perf(cb, "exit trampoline", code_ptr); code_ptr }) } @@ -3615,6 +3584,7 @@ pub fn gen_materialize_exit_trampoline(cb: &mut CodeBlock, exit_trampoline: Code asm.compile(cb).map(|(code_ptr, gc_offsets)| { assert_eq!(gc_offsets.len(), 0); + register_current_code_range_with_perf(cb, "materialize_exit trampoline", code_ptr); code_ptr }) } @@ -3630,6 +3600,7 @@ pub fn gen_materialize_exit_trampoline_with_counter(cb: &mut CodeBlock, material asm.compile(cb).map(|(code_ptr, gc_offsets)| { assert_eq!(gc_offsets.len(), 0); + register_current_code_range_with_perf(cb, "materialize_exit_with_counter trampoline", code_ptr); code_ptr }) } diff --git a/zjit/src/codegen_tests.rs b/zjit/src/codegen_tests.rs index 92913cec16c93e..93106844a15639 100644 --- a/zjit/src/codegen_tests.rs +++ b/zjit/src/codegen_tests.rs @@ -735,6 +735,26 @@ fn test_send_with_local_written_by_blockiseq() { "), @"[1, 2]"); } +#[test] +fn test_send_does_not_reload_local_untouched_by_blockiseq() { + // https://github.com/Shopify/ruby/issues/976: a call with a block must not + // reload locals the block never assigns, otherwise it reads a stale stack + // slot and clobbers the correct SSA value (here, `a`). + eval(" + def foo(&block) = 1 + + def test + a = 1 + foo {} + a + end + + test + "); + assert_contains_opcode("test", YARVINSN_send); + assert_snapshot!(assert_compiles("test"), @"1"); +} + #[test] fn test_no_ep_escape_patch_point_after_send_does_not_repeat_send() { eval(r#" diff --git a/zjit/src/cruby.rs b/zjit/src/cruby.rs index d8bd1d95574a98..f981b3fd69f72f 100644 --- a/zjit/src/cruby.rs +++ b/zjit/src/cruby.rs @@ -93,6 +93,7 @@ use std::ffi::{c_void, CString, CStr}; use std::fmt::{Debug, Display, Formatter}; use std::os::raw::{c_char, c_int, c_long, c_uint}; use std::panic::{catch_unwind, UnwindSafe}; +use std::ptr::NonNull; use crate::cast::IntoUsize as _; @@ -749,9 +750,42 @@ impl VALUE { pub type IseqParameters = rb_iseq_constant_body_rb_iseq_parameters; +/// How a block iseq refers to a variable in an enclosing scope, as recorded in +/// `ISEQ_BODY(blockiseq)->outer_variables`. `compile.c` aggregates accesses from +/// nested blocks up the chain, and the same table backs `Ractor.shareable_proc`'s +/// isolation checks. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum OuterLocalAccess { + /// The variable is read but never assigned to. + ReadOnly, + /// The variable is assigned to and maybe also read. + ReadWrite, +} + +/// Wrapper over an iseq's `outer_variables` table, which describes +/// how a block iseq refers to a variable in an enclosing scope. +#[derive(Clone, Copy)] +pub struct OuterVariables(Option>); + +impl OuterVariables { + /// Look up how the enclosing-scope local `id` is accessed by the iseq (or any + /// iseq nested within it). Returns `None` when the variable isn't referenced. + pub fn local_access(self, id: ID) -> Option { + let table = self.0?; + let mut write = Qfalse; + // Non-zero return means there's a table entry, i.e. the variable is referenced. + if unsafe { rb_id_table_lookup(table.as_ptr(), id, &mut write) } == 0 { + return None; + } + // Truthy means write + Some(if write.test() { OuterLocalAccess::ReadWrite } else { OuterLocalAccess::ReadOnly }) + } +} + /// Extension trait to enable method calls on [`IseqPtr`] pub trait IseqAccess { unsafe fn params<'a>(self) -> &'a IseqParameters; + unsafe fn outer_variables(self) -> OuterVariables; } impl IseqAccess for IseqPtr { @@ -760,6 +794,13 @@ impl IseqAccess for IseqPtr { use crate::cast::IntoUsize; unsafe { &*((*self).body.byte_add(ISEQ_BODY_OFFSET_PARAM.to_usize()) as *const IseqParameters) } } + + /// The iseq's `outer_variables` table. See [`OuterVariables`]. + unsafe fn outer_variables(self) -> OuterVariables { + use crate::cast::IntoUsize; + let field = unsafe { (*self).body.byte_add(ISEQ_BODY_OFFSET_OUTER_VARIABLES.to_usize()) } as *const *mut rb_id_table; + OuterVariables(NonNull::new(unsafe { *field })) + } } impl IseqParameters { diff --git a/zjit/src/cruby_bindings.inc.rs b/zjit/src/cruby_bindings.inc.rs index a54f0982019afd..fd500ae898849f 100644 --- a/zjit/src/cruby_bindings.inc.rs +++ b/zjit/src/cruby_bindings.inc.rs @@ -419,17 +419,10 @@ pub const BOP_LAST_: ruby_basic_operators = 35; pub type ruby_basic_operators = u32; pub type rb_serial_t = ::std::os::raw::c_ulonglong; #[repr(C)] -#[derive(Debug, Copy, Clone)] -pub struct rb_id_item { - _unused: [u8; 0], -} -#[repr(C)] +#[repr(align(8))] #[derive(Debug, Copy, Clone)] pub struct rb_id_table { - pub capa: ::std::os::raw::c_int, - pub num: ::std::os::raw::c_int, - pub used: ::std::os::raw::c_int, - pub items: *mut rb_id_item, + pub _bindgen_opaque_blob: [u64; 3usize], } pub const imemo_env: imemo_type = 0; pub const imemo_cref: imemo_type = 1; @@ -1931,6 +1924,7 @@ pub struct zjit_jit_frame { pub stack: __IncompleteArrayField, } pub const ISEQ_BODY_OFFSET_PARAM: zjit_struct_offsets = 16; +pub const ISEQ_BODY_OFFSET_OUTER_VARIABLES: zjit_struct_offsets = 288; pub type zjit_struct_offsets = u32; pub const ROBJECT_OFFSET_AS_HEAP_FIELDS: jit_bindgen_constants = 16; pub const ROBJECT_OFFSET_AS_ARY: jit_bindgen_constants = 16; @@ -2082,6 +2076,11 @@ unsafe extern "C" { ) -> VALUE; pub fn rb_vm_top_self() -> VALUE; pub static mut rb_vm_insn_count: u64; + pub fn rb_id_table_lookup( + tbl: *mut rb_id_table, + id: ID, + valp: *mut VALUE, + ) -> ::std::os::raw::c_int; pub fn rb_method_entry_at(obj: VALUE, id: ID) -> *const rb_method_entry_t; pub fn rb_callable_method_entry(klass: VALUE, id: ID) -> *const rb_callable_method_entry_t; pub fn rb_callable_method_entry_or_negative( diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index e6fc1d74cebb85..e4b40f281b9a78 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -567,41 +567,9 @@ pub enum SideExitReason { InvokeBlockNotIfunc, } -/// Controls how a side exit triggers recompilation. -pub const RECOMPILE_PROFILE_SEND: i32 = 0; -pub const RECOMPILE_PROFILE_SELF: i32 = 1; -pub const RECOMPILE_PROFILE_BLOCK_HANDLER: i32 = 2; - +/// Marks a side exit as triggering profiling and recompilation. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] -pub enum Recompile { - /// Profile receiver + arguments from the stack (for sends without profile data). - ProfileSend { argc: i32 }, - /// Profile self from the CFP (for shape guard failures). - ProfileSelf, - /// Profile the block handler from the CFP (for getblockparamproxy guard failures). - ProfileBlockHandler, -} - -impl Recompile { - /// Convert to primitive arguments for passing across the C ABI. - pub fn to_c_args(self) -> (i32, i32) { - match self { - Recompile::ProfileSend { argc } => (RECOMPILE_PROFILE_SEND, argc), - Recompile::ProfileSelf => (RECOMPILE_PROFILE_SELF, 0), - Recompile::ProfileBlockHandler => (RECOMPILE_PROFILE_BLOCK_HANDLER, 0), - } - } - - /// Reconstruct from primitive arguments received across the C ABI. - pub fn from_c_args(kind: i32, payload: i32) -> Self { - match kind { - RECOMPILE_PROFILE_SEND => Recompile::ProfileSend { argc: payload }, - RECOMPILE_PROFILE_SELF => Recompile::ProfileSelf, - RECOMPILE_PROFILE_BLOCK_HANDLER => Recompile::ProfileBlockHandler, - _ => unreachable!("unknown recompile profile kind: {kind}"), - } - } -} +pub struct Recompile; #[derive(Debug, Clone, Copy)] pub enum MethodType { @@ -3865,8 +3833,7 @@ impl Function { // Add GuardType for profiled receiver if let Some(profiled_type) = profiled_type { - let argc = unsafe { vm_ci_argc(ci) } as i32; - recv = self.push_insn(block, Insn::GuardType { val: recv, guard_type: Type::from_profiled_type(profiled_type), state, recompile: Some(Recompile::ProfileSend { argc }) }); + recv = self.push_insn(block, Insn::GuardType { val: recv, guard_type: Type::from_profiled_type(profiled_type), state, recompile: Some(Recompile) }); } let replacement = self.try_inline_send_direct(block, Insn::SendDirect { recv, cd, cme, iseq, args: processed_args, kw_bits, state: send_state, block: send_block }); @@ -3909,8 +3876,7 @@ impl Function { self.push_insn(block, Insn::PatchPoint { invariant: Invariant::MethodRedefined { klass, method: mid, cme }, state }); if let Some(profiled_type) = profiled_type { - let argc = unsafe { vm_ci_argc(ci) } as i32; - recv = self.push_insn(block, Insn::GuardType { val: recv, guard_type: Type::from_profiled_type(profiled_type), state, recompile: Some(Recompile::ProfileSend{ argc }) }); + recv = self.push_insn(block, Insn::GuardType { val: recv, guard_type: Type::from_profiled_type(profiled_type), state, recompile: Some(Recompile) }); } let replacement = self.try_inline_send_direct(block, Insn::SendDirect { recv, cd, cme, iseq, args: processed_args, kw_bits, state: send_state, block: None }); @@ -3932,8 +3898,7 @@ impl Function { let id = unsafe { get_cme_def_body_attr_id(cme) }; if let Some(profiled_type) = profiled_type { - let argc = unsafe { vm_ci_argc(ci) } as i32; - recv = self.push_insn(block, Insn::GuardType { val: recv, guard_type: Type::from_profiled_type(profiled_type), state, recompile: Some(Recompile::ProfileSend{ argc }) }); + recv = self.push_insn(block, Insn::GuardType { val: recv, guard_type: Type::from_profiled_type(profiled_type), state, recompile: Some(Recompile) }); let replacement = self.try_emit_optimized_getivar(block, recv, id, profiled_type, state).unwrap_or_else(|counter| { self.count(block, counter); @@ -3959,12 +3924,11 @@ impl Function { self.push_insn(block, Insn::PatchPoint { invariant: Invariant::MethodRedefined { klass, method: mid, cme }, state }); let id = unsafe { get_cme_def_body_attr_id(cme) }; if let Some(profiled_type) = profiled_type { - let argc = unsafe { vm_ci_argc(ci) } as i32; // TODO: attr_writer SetIvar has a null inline cache and may target a receiver // operand other than CFP self. Support it with a reprofile strategy that // profiles the receiver operand even after the send insn has finished profiling. let recompile = None; - recv = self.push_insn(block, Insn::GuardType { val: recv, guard_type: Type::from_profiled_type(profiled_type), state, recompile: Some(Recompile::ProfileSend{ argc }) }); + recv = self.push_insn(block, Insn::GuardType { val: recv, guard_type: Type::from_profiled_type(profiled_type), state, recompile: Some(Recompile) }); self.try_emit_optimized_setivar(block, recv, id, val, profiled_type, state, recompile).unwrap_or_else(|counter| { self.count(block, counter); self.push_insn(block, Insn::SetIvar { self_val: recv, id, ic: std::ptr::null(), val, state }); @@ -3990,8 +3954,7 @@ impl Function { } self.push_insn(block, Insn::PatchPoint { invariant: Invariant::MethodRedefined { klass, method: mid, cme }, state }); if let Some(profiled_type) = profiled_type { - let argc = unsafe { vm_ci_argc(ci) } as i32; - recv = self.push_insn(block, Insn::GuardType { val: recv, guard_type: Type::from_profiled_type(profiled_type), state, recompile: Some(Recompile::ProfileSend{ argc }) }); + recv = self.push_insn(block, Insn::GuardType { val: recv, guard_type: Type::from_profiled_type(profiled_type), state, recompile: Some(Recompile) }); } let kw_splat = flags & VM_CALL_KW_SPLAT != 0; let invoke_proc = self.push_insn(block, Insn::InvokeProc { recv, args: args.clone(), state, kw_splat }); @@ -4029,8 +3992,7 @@ impl Function { } self.push_insn(block, Insn::PatchPoint { invariant: Invariant::MethodRedefined { klass, method: mid, cme }, state }); if let Some(profiled_type) = profiled_type { - let argc = unsafe { vm_ci_argc(ci) } as i32; - recv = self.push_insn(block, Insn::GuardType { val: recv, guard_type: Type::from_profiled_type(profiled_type), state, recompile: Some(Recompile::ProfileSend{ argc }) }); + recv = self.push_insn(block, Insn::GuardType { val: recv, guard_type: Type::from_profiled_type(profiled_type), state, recompile: Some(Recompile) }); } // All structs from the same Struct class should have the same // length. So if our recv is embedded all runtime @@ -4902,7 +4864,7 @@ impl Function { } let self_val = self.guard_heap(block, self_val, state); let shape = self.load_shape(block, self_val); - self.guard_shape(block, shape, profiled_type.shape(), state, Some(Recompile::ProfileSelf)); + self.guard_shape(block, shape, profiled_type.shape(), state, Some(Recompile)); Ok(self.load_ivar(block, self_val, profiled_type, id)) } @@ -5000,6 +4962,63 @@ impl Function { self.push_insn(block, Insn::PatchPoint { invariant: Invariant::NoEPEscape(iseq), state: reload_exit_id }); } + /// After a call that takes a block iseq, reload the locals that the block (or any iseq nested + /// within it) may have written. This covers syntactically visible local writes where the + /// environment does not escape. Exordinary modifications through `Binding` and debug.h APIs are + /// handled via patchpoints. + fn reload_locals_modified_by_block( + &mut self, + block: BlockId, + iseq: IseqPtr, + blockiseq: IseqPtr, + state: &mut FrameState, + ep_escaped: bool, + ) { + let to_reload: &mut dyn Iterator = if ep_escaped { + // Reload everything when working with an escaped environment + &mut (0..state.locals.len()) + } else { + let params = unsafe { iseq.params() }; + let block_param_local_idx: Option = if params.flags.has_block() != 0 { + params.block_start.try_into().ok() + } else { + None + }; + let outer_variables = unsafe { blockiseq.outer_variables() }; + // When not escaped, only reload the locals the block can have modified. + &mut (0..state.locals.len()).filter(move |&local_idx| { + let id = unsafe { rb_zjit_local_id(iseq, local_idx.try_into().unwrap()) }; + let access = outer_variables.local_access(id); + if block_param_local_idx == Some(local_idx) { + // The block param slot is special: `getblockparam` is recorded as a + // read, but it materializes the captured block into this slot. So + // reload it whenever the block references it at all (read or write), + // not just on a setlocal. When the block never references it, the + // slot can't have changed, so skip the reload. + access.is_some() + } else { + access == Some(OuterLocalAccess::ReadWrite) + } + }) + }; + let mut base: Option = None; + for local_idx in to_reload { + let ep_offset = local_idx_to_ep_offset(iseq, local_idx); + let ep_offset_u32 = u32::try_from(ep_offset) + .unwrap_or_else(|_| panic!("Could not convert ep_offset {ep_offset} to u32")); + let recv = *base.get_or_insert_with(|| { + let base_insn = if !ep_escaped { Insn::LoadSP } else { Insn::GetEP { level: 0 } }; + self.push_insn(block, base_insn) + }); + let val = if !ep_escaped { + self.get_local_from_sp(block, iseq, recv, ep_offset_u32, types::BasicObject) + } else { + self.get_local_from_ep(block, iseq, recv, ep_offset_u32, 0, types::BasicObject) + }; + state.setlocal(ep_offset_u32, val); + } + } + fn count_not_inlined_cfunc(&mut self, block: BlockId, cme: *const rb_callable_method_entry_t) { let owner = unsafe { (*cme).owner }; let called_id = unsafe { (*cme).called_id }; @@ -5154,8 +5173,7 @@ impl Function { if let Some(profiled_type) = profiled_type { // Guard receiver class - let argc = unsafe { vm_ci_argc(call_info) } as i32; - recv = fun.push_insn(block, Insn::GuardType { val: recv, guard_type: Type::from_profiled_type(profiled_type), state, recompile: Some(Recompile::ProfileSend { argc }) }); + recv = fun.push_insn(block, Insn::GuardType { val: recv, guard_type: Type::from_profiled_type(profiled_type), state, recompile: Some(Recompile) }); fun.insn_types[recv.0] = fun.infer_type(recv); } @@ -5221,8 +5239,7 @@ impl Function { if let Some(profiled_type) = profiled_type { // Guard receiver class - let argc = unsafe { vm_ci_argc(call_info) } as i32; - recv = fun.push_insn(block, Insn::GuardType { val: recv, guard_type: Type::from_profiled_type(profiled_type), state, recompile: Some(Recompile::ProfileSend { argc }) }); + recv = fun.push_insn(block, Insn::GuardType { val: recv, guard_type: Type::from_profiled_type(profiled_type), state, recompile: Some(Recompile) }); fun.insn_types[recv.0] = fun.infer_type(recv); } @@ -5340,9 +5357,8 @@ impl Function { assert!(self.blocks[block.0].insns.is_empty()); for insn_id in old_insns { match self.find(insn_id) { - Insn::Send { cd, state, reason: SendFallbackReason::SendWithoutBlockNoProfiles | SendFallbackReason::SendNoProfiles, .. } => { - let argc = unsafe { vm_ci_argc((*cd).ci) } as i32; - self.push_insn(block, Insn::SideExit { state, reason: SideExitReason::NoProfileSend, recompile: Some(Recompile::ProfileSend { argc }) }); + Insn::Send { state, reason: SendFallbackReason::SendWithoutBlockNoProfiles | SendFallbackReason::SendNoProfiles, .. } => { + self.push_insn(block, Insn::SideExit { state, reason: SideExitReason::NoProfileSend, recompile: Some(Recompile) }); // SideExit is a terminator; don't add remaining instructions break; } @@ -5509,6 +5525,19 @@ impl Function { let mut new_insns = vec![]; for insn_id in old_insns { let replacement_id = match self.find(insn_id) { + // TODO (nirvdrum 2026-06-26): Folding the guard to a SideExit is a workaround, + // not a proper fix. It relies on constant folding to keep an Empty-typed value + // (see below) from reaching codegen; disabling this pass would let that value + // through and the program would fail to compile on x86-64. Compilation correctness + // should not depend on an optimization pass, so this should be replaced by a + // comprehensive fix. + Insn::GuardType { val, guard_type, state, recompile } if !self.type_of(val).could_be(guard_type) => { + // The value's type is disjoint from the guard type, so the guard can never + // pass. Every execution would side-exit here, so we replace the guard with an + // unconditional exit. The terminator handling below then drops the rest of + // the block, which is now unreachable. + self.new_insn(Insn::SideExit { state, reason: SideExitReason::GuardType(guard_type), recompile }) + } Insn::GuardType { val, guard_type, .. } if self.is_a(val, guard_type) => { self.make_equal_to(insn_id, val); // Don't bother re-inferring the type of val; we already know it. @@ -7801,7 +7830,7 @@ fn add_iseq_to_hir( .and_then(can_optimize) { self_param = fun.guard_heap(block, self_param, exit_id); let shape = fun.load_shape(block, self_param); - fun.guard_shape(block, shape, profiled_shape, exit_id, Some(Recompile::ProfileSelf)); + fun.guard_shape(block, shape, profiled_shape, exit_id, Some(Recompile)); let mut ivar_index: attr_index_t = 0; let result = if unsafe { rb_shape_get_iv_index(profiled_shape.0, id, &mut ivar_index) } { fun.push_insn(block, Insn::Const { val: Const::Value(pushval) }) @@ -8219,7 +8248,7 @@ fn add_iseq_to_hir( // So to check for either of those cases we can use: val & 0x1 == 0x1 // Bail out if the block handler is neither ISEQ nor ifunc - fun.push_insn(unmodified_block, Insn::GuardAnyBitSet { val: block_handler, mask: Const::CUInt64(0x1), mask_name: None, reason: SideExitReason::BlockParamProxyFallbackMiss, state: exit_id, recompile: Some(Recompile::ProfileBlockHandler) }); + fun.push_insn(unmodified_block, Insn::GuardAnyBitSet { val: block_handler, mask: Const::CUInt64(0x1), mask_name: None, reason: SideExitReason::BlockParamProxyFallbackMiss, state: exit_id, recompile: Some(Recompile) }); // TODO(Shopify/ruby#753): GC root, so we should be able to avoid unnecessary GC tracing let proxy_val = fun.push_insn(unmodified_block, Insn::Const { val: Const::Value(unsafe { rb_block_param_proxy }) }); let mut args = vec![proxy_val]; @@ -8232,7 +8261,7 @@ fn add_iseq_to_hir( [profiled_handler] => match profiled_handler { ProfiledBlockHandlerFamily::Nil => { let block_handler = fun.load_ep_env_field(unmodified_block, ep, FieldName::VM_ENV_DATA_INDEX_SPECVAL, VM_ENV_DATA_INDEX_SPECVAL, types::CInt64); - fun.push_insn(unmodified_block, Insn::GuardBitEquals { val: block_handler, expected: Const::CInt64(VM_BLOCK_HANDLER_NONE.into()), reason: SideExitReason::BlockParamProxyNotNil, state: exit_id, recompile: Some(Recompile::ProfileBlockHandler) }); + fun.push_insn(unmodified_block, Insn::GuardBitEquals { val: block_handler, expected: Const::CInt64(VM_BLOCK_HANDLER_NONE.into()), reason: SideExitReason::BlockParamProxyNotNil, state: exit_id, recompile: Some(Recompile) }); let nil_val = fun.push_insn(unmodified_block, Insn::Const { val: Const::Value(Qnil) }); let mut args = vec![nil_val]; if let Some(local) = original_local { @@ -8249,7 +8278,7 @@ fn add_iseq_to_hir( // So to check for either of those cases we can use: val & 0x1 == 0x1 // Bail out if the block handler is neither ISEQ nor ifunc - fun.push_insn(unmodified_block, Insn::GuardAnyBitSet { val: block_handler, mask: Const::CUInt64(0x1), mask_name: None, reason: SideExitReason::BlockParamProxyNotIseqOrIfunc, state: exit_id, recompile: Some(Recompile::ProfileBlockHandler) }); + fun.push_insn(unmodified_block, Insn::GuardAnyBitSet { val: block_handler, mask: Const::CUInt64(0x1), mask_name: None, reason: SideExitReason::BlockParamProxyNotIseqOrIfunc, state: exit_id, recompile: Some(Recompile) }); // TODO(Shopify/ruby#753): GC root, so we should be able to avoid unnecessary GC tracing let proxy_val = fun.push_insn(unmodified_block, Insn::Const { val: Const::Value(unsafe { rb_block_param_proxy }) }); let mut args = vec![proxy_val]; @@ -8269,7 +8298,7 @@ fn add_iseq_to_hir( return_type: types::BasicObject, elidable: true, }); - fun.push_insn(unmodified_block, Insn::GuardBitEquals { val: is_proc, expected: Const::Value(Qtrue), reason: SideExitReason::BlockParamProxyNotProc, state: exit_id, recompile: Some(Recompile::ProfileBlockHandler) }); + fun.push_insn(unmodified_block, Insn::GuardBitEquals { val: is_proc, expected: Const::Value(Qtrue), reason: SideExitReason::BlockParamProxyNotProc, state: exit_id, recompile: Some(Recompile) }); let mut args = vec![proc_val]; if let Some(local) = original_local { args.push(local); @@ -8681,29 +8710,12 @@ fn add_iseq_to_hir( let send = fun.push_insn(block, Insn::Send { recv, cd, block: block_handler, args, state: exit_id, reason: Uncategorized(opcode) }); state.stack_push(send); - if let Some(BlockHandler::BlockIseq(_)) = block_handler { + if let Some(BlockHandler::BlockIseq(blockiseq)) = block_handler { // Reload locals that may have been modified by the blockiseq. - // TODO: Avoid reloading locals that are not referenced by the blockiseq - // or not used after this. Max thinks we could eventually DCE them. if !ep_escaped && !state.locals.is_empty() { fun.gen_post_send_no_ep_escape_patch_point(block, &state, insn_idx); } - let mut base: Option = None; - for local_idx in 0..state.locals.len() { - let ep_offset = local_idx_to_ep_offset(iseq, local_idx); - let ep_offset_u32 = u32::try_from(ep_offset) - .unwrap_or_else(|_| panic!("Could not convert ep_offset {ep_offset} to u32")); - let recv = *base.get_or_insert_with(|| { - let base_insn = if !ep_escaped { Insn::LoadSP } else { Insn::GetEP { level: 0 } }; - fun.push_insn(block, base_insn) - }); - let val = if !ep_escaped { - fun.get_local_from_sp(block, iseq, recv, ep_offset_u32, types::BasicObject) - } else { - fun.get_local_from_ep(block, iseq, recv, ep_offset_u32, 0, types::BasicObject) - }; - state.setlocal(ep_offset_u32, val); - } + fun.reload_locals_modified_by_block(block, iseq, blockiseq, &mut state, ep_escaped); } } YARVINSN_sendforward => { @@ -8734,22 +8746,7 @@ fn add_iseq_to_hir( if !ep_escaped && !state.locals.is_empty() { fun.gen_post_send_no_ep_escape_patch_point(block, &state, insn_idx); } - let mut base: Option = None; - for local_idx in 0..state.locals.len() { - let ep_offset = local_idx_to_ep_offset(iseq, local_idx); - let ep_offset_u32 = u32::try_from(ep_offset) - .unwrap_or_else(|_| panic!("Could not convert ep_offset {ep_offset} to u32")); - let recv = *base.get_or_insert_with(|| { - let base_insn = if !ep_escaped { Insn::LoadSP } else { Insn::GetEP { level: 0 } }; - fun.push_insn(block, base_insn) - }); - let val = if !ep_escaped { - fun.get_local_from_sp(block, iseq, recv, ep_offset_u32, types::BasicObject) - } else { - fun.get_local_from_ep(block, iseq, recv, ep_offset_u32, 0, types::BasicObject) - }; - state.setlocal(ep_offset_u32, val); - } + fun.reload_locals_modified_by_block(block, iseq, blockiseq, &mut state, ep_escaped); } } YARVINSN_invokesuper => { @@ -8774,27 +8771,10 @@ fn add_iseq_to_hir( if !blockiseq.is_null() { // Reload locals that may have been modified by the blockiseq. - // TODO: Avoid reloading locals that are not referenced by the blockiseq - // or not used after this. Max thinks we could eventually DCE them. if !ep_escaped && !state.locals.is_empty() { fun.gen_post_send_no_ep_escape_patch_point(block, &state, insn_idx); } - let mut base: Option = None; - for local_idx in 0..state.locals.len() { - let ep_offset = local_idx_to_ep_offset(iseq, local_idx); - let ep_offset_u32 = u32::try_from(ep_offset) - .unwrap_or_else(|_| panic!("Could not convert ep_offset {ep_offset} to u32")); - let recv = *base.get_or_insert_with(|| { - let base_insn = if !ep_escaped { Insn::LoadSP } else { Insn::GetEP { level: 0 } }; - fun.push_insn(block, base_insn) - }); - let val = if !ep_escaped { - fun.get_local_from_sp(block, iseq, recv, ep_offset_u32, types::BasicObject) - } else { - fun.get_local_from_ep(block, iseq, recv, ep_offset_u32, 0, types::BasicObject) - }; - state.setlocal(ep_offset_u32, val); - } + fun.reload_locals_modified_by_block(block, iseq, blockiseq, &mut state, ep_escaped); } } YARVINSN_invokesuperforward => { @@ -8821,27 +8801,10 @@ fn add_iseq_to_hir( if !blockiseq.is_null() { // Reload locals that may have been modified by the blockiseq. - // TODO: Avoid reloading locals that are not referenced by the blockiseq - // or not used after this. Max thinks we could eventually DCE them. if !ep_escaped && !state.locals.is_empty() { fun.gen_post_send_no_ep_escape_patch_point(block, &state, insn_idx); } - let mut base: Option = None; - for local_idx in 0..state.locals.len() { - let ep_offset = local_idx_to_ep_offset(iseq, local_idx); - let ep_offset_u32 = u32::try_from(ep_offset) - .unwrap_or_else(|_| panic!("Could not convert ep_offset {ep_offset} to u32")); - let recv = *base.get_or_insert_with(|| { - let base_insn = if !ep_escaped { Insn::LoadSP } else { Insn::GetEP { level: 0 } }; - fun.push_insn(block, base_insn) - }); - let val = if !ep_escaped { - fun.get_local_from_sp(block, iseq, recv, ep_offset_u32, types::BasicObject) - } else { - fun.get_local_from_ep(block, iseq, recv, ep_offset_u32, 0, types::BasicObject) - }; - state.setlocal(ep_offset_u32, val); - } + fun.reload_locals_modified_by_block(block, iseq, blockiseq, &mut state, ep_escaped); } } YARVINSN_invokeblock => { @@ -8995,7 +8958,7 @@ fn add_iseq_to_hir( if let Some(profiled_type) = fun.monomorphic_summary(&profiles, self_param, exit_id) { let result = fun.try_emit_optimized_getivar(block, self_param, id, profiled_type, exit_id).unwrap_or_else(|counter| { fun.count(block, counter); - fun.push_insn(block, Insn::GetIvar { self_val: self_param, id, ic: std::ptr::null(), state: exit_id }) + fun.push_insn(block, Insn::GetIvar { self_val: self_param, id, ic, state: exit_id }) }); state.stack_push(result); } else { @@ -9020,7 +8983,7 @@ fn add_iseq_to_hir( let val = state.stack_pop()?; if let Some(profiled_type) = fun.monomorphic_summary(&profiles, self_param, exit_id) { // TODO(max): Assert ic is never null - let recompile = if ic.is_null() { None } else { Some(Recompile::ProfileSelf) }; + let recompile = (!ic.is_null()).then_some(Recompile); fun.try_emit_optimized_setivar(block, self_param, id, val, profiled_type, exit_id, recompile).unwrap_or_else(|counter| { fun.count(block, counter); fun.push_insn(block, Insn::SetIvar { self_val: self_param, id, ic, val, state: exit_id }); diff --git a/zjit/src/hir/opt_tests.rs b/zjit/src/hir/opt_tests.rs index 72a0744e982eb6..41238a7fb7be5a 100644 --- a/zjit/src/hir/opt_tests.rs +++ b/zjit/src/hir/opt_tests.rs @@ -1611,13 +1611,11 @@ mod hir_opt_tests { bb3(v9:BasicObject, v10:BasicObject): PatchPoint NoSingletonClass(C@0x1008) PatchPoint MethodRedefined(C@0x1008, fun_new_map@0x1010, cme:0x1018) - v27:ArraySubclass[class_exact:C] = GuardType v10, ArraySubclass[class_exact:C] recompile - v28:BasicObject = SendDirect v27, 0x1040, :fun_new_map (0x1050) + v25:ArraySubclass[class_exact:C] = GuardType v10, ArraySubclass[class_exact:C] recompile + v26:BasicObject = SendDirect v25, 0x1040, :fun_new_map (0x1050) PatchPoint NoEPEscape(test) - v18:CPtr = LoadSP - v19:BasicObject = LoadField v18, :o@0x1000 CheckInterrupts - Return v28 + Return v26 "); } @@ -1648,13 +1646,11 @@ mod hir_opt_tests { bb3(v9:BasicObject, v10:BasicObject): PatchPoint NoSingletonClass(C@0x1008) PatchPoint MethodRedefined(C@0x1008, bar@0x1010, cme:0x1018) - v28:ObjectSubclass[class_exact:C] = GuardType v10, ObjectSubclass[class_exact:C] recompile - v29:BasicObject = CCallWithFrame v28, :Enumerable#bar@0x1040, block=0x1048 + v26:ObjectSubclass[class_exact:C] = GuardType v10, ObjectSubclass[class_exact:C] recompile + v27:BasicObject = CCallWithFrame v26, :Enumerable#bar@0x1040, block=0x1048 PatchPoint NoEPEscape(test) - v18:CPtr = LoadSP - v19:BasicObject = LoadField v18, :o@0x1000 CheckInterrupts - Return v29 + Return v27 "); } @@ -2512,6 +2508,41 @@ mod hir_opt_tests { assert!(!insns.contains(&dead_const)); } + // A GuardType whose value type is disjoint from the guard type can never pass, so every + // execution side-exits there. fold_constants should replace the guard with an unconditional + // SideExit and drop the now-unreachable instructions that follow. + #[test] + fn test_fold_guard_type_that_can_never_pass_into_side_exit() { + let mut function = Function::new(std::ptr::null()); + let entry = function.entry_block; + + let state = function.push_insn(entry, Insn::Snapshot { state: FrameState::new(std::ptr::null()) }); + // A nil constant is a NilClass, which is disjoint from Fixnum, so the guard below can + // never pass and the optimizer infers its result as Empty. + let nil = function.push_insn(entry, Insn::Const { val: Const::Value(Qnil) }); + let guard = function.push_insn(entry, Insn::GuardType { val: nil, guard_type: types::Fixnum, state, recompile: None }); + function.push_insn(entry, Insn::StoreField { recv: nil, id: FieldName::len, offset: 0, val: guard }); + function.push_insn(entry, Insn::Return { val: guard }); + function.seal_entries(); + + function.infer_types(); + function.fold_constants(); + + let insns: Vec = function.blocks[entry.0].insns.iter().map(|&id| function.find(id)).collect(); + assert!( + insns.iter().any(|insn| matches!(insn, Insn::SideExit { .. })), + "expected the always-failing guard to be folded into a SideExit, got {insns:?}", + ); + assert!( + !insns.iter().any(|insn| matches!(insn, Insn::GuardType { .. })), + "the always-failing GuardType should have been removed, got {insns:?}", + ); + assert!( + !insns.iter().any(|insn| matches!(insn, Insn::StoreField { .. } | Insn::Return { .. })), + "instructions after the unconditional SideExit are unreachable and should have been dropped, got {insns:?}", + ); + } + #[test] fn test_eliminate_new_array() { eval(" @@ -3875,7 +3906,7 @@ mod hir_opt_tests { def foo(&block) = 1 def test a = 1 - foo {|| } + foo {|| a = 2 } a end test @@ -3913,7 +3944,7 @@ mod hir_opt_tests { def test a = 1 lambda { a } - foo {|| } + foo {|| a = 2 } a end test @@ -4540,11 +4571,8 @@ mod hir_opt_tests { v22:TrueClass = Const Value(true) v24:BasicObject = Send v12, 0x1008, :each_line, v22 # SendFallbackReason: Complex argument passing PatchPoint NoEPEscape(test) - v27:CPtr = LoadSP - v28:BasicObject = LoadField v27, :s@0x1000 - v29:BasicObject = LoadField v27, :a@0x1030 CheckInterrupts - Return v29 + Return v17 "); } @@ -9427,12 +9455,10 @@ mod hir_opt_tests { v23:ArrayExact[VALUE(0x1018)] = Const Value(VALUE(0x1018)) PatchPoint NoSingletonClass(Array@0x1020) PatchPoint MethodRedefined(Array@0x1020, zip@0x1028, cme:0x1030) - v44:BasicObject = CCallVariadic v19, :Array#zip@0x1058, v23 + v42:BasicObject = CCallVariadic v19, :Array#zip@0x1058, v23 PatchPoint NoEPEscape(test) - v28:CPtr = LoadSP - v29:BasicObject = LoadField v28, :result@0x1060 CheckInterrupts - Return v29 + Return v13 "); } @@ -16855,6 +16881,74 @@ mod hir_opt_tests { "); } + #[test] + fn test_recompile_no_profile_send_with_blockarg() { + // Test that no-profile send recompilation profiles explicit blockargs. + // The call remains a Send fallback because &block is still complex, but + // it should no longer be a NoProfileSend side exit after recompilation. + eval(" + def passthrough_recompile_blockarg(x, &block) + block.call(x) + end + + def test(flag, block) + if flag + passthrough_recompile_blockarg(42, &block) + else + 'hello' + end + end + "); + + // With call_threshold=2, num_profiles=1, the send is not profiled + // during initial profiling because flag=false skips that branch. + eval(" + block = proc { |x| x } + test(false, block) + test(false, block) + "); + + // This hits the NoProfileSend side exit, profiles the send including + // its explicit blockarg, and invalidates the ISEQ for recompilation. + eval(" + block = proc { |x| x } + test(true, block) + "); + + assert_snapshot!(hir_string("test"), @r" + fn test@:7: + bb1(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + v2:CPtr = LoadSP + v3:BasicObject = LoadField v2, :flag@0x1000 + v4:BasicObject = LoadField v2, :block@0x1001 + Jump bb3(v1, v3, v4) + bb2(): + EntryPoint JIT(0) + v7:BasicObject = LoadArg :self@0 + v8:BasicObject = LoadArg :flag@1 + v9:BasicObject = LoadArg :block@2 + Jump bb3(v7, v8, v9) + bb3(v11:BasicObject, v12:BasicObject, v13:BasicObject): + CheckInterrupts + v19:CBool = Test v12 + v20:Falsy = RefineType v12, Falsy + CondBranch v19, bb5(), bb4(v11, v20, v13) + bb5(): + v22:Truthy = RefineType v12, Truthy + v26:Fixnum[42] = Const Value(42) + v29:BasicObject = Send v11, &block, :passthrough_recompile_blockarg, v26, v13 # SendFallbackReason: Complex argument passing + CheckInterrupts + Return v29 + bb4(v34:BasicObject, v35:Falsy, v36:BasicObject): + v40:StringExact[VALUE(0x1008)] = Const Value(VALUE(0x1008)) + v41:StringExact = StringCopy v40 + CheckInterrupts + Return v41 + "); + } + #[test] fn test_no_profile_send_on_final_version() { // On the final ISEQ version (MAX_ISEQ_VERSIONS reached), no-profile sends should @@ -16865,6 +16959,7 @@ mod hir_opt_tests { // the HIR. The auto-compile creates version 1, and hir_string() creates version 2 // (= MAX_ISEQ_VERSIONS), so this is the final version. set_call_threshold(3); + set_max_versions(2); eval(" def greet_final(x) = x.to_s def test_final_version(flag) @@ -17730,6 +17825,7 @@ mod hir_opt_tests { #[test] fn test_trigger_guard_type_recompilation() { + set_max_versions(2); eval(" class C def f(x) @@ -18918,13 +19014,13 @@ mod hir_opt_tests { Jump bb3(v6, v7) bb3(v9:BasicObject, v10:BasicObject): PatchPoint MethodRedefined(Object@0x1008, with_yield@0x1010, cme:0x1018) - v27:ObjectSubclass[class_exact*:Object@VALUE(0x1008)] = GuardType v9, ObjectSubclass[class_exact*:Object@VALUE(0x1008)] recompile - PushInlineFrame v27 (0x1040), v10 - v35:BasicObject = InvokeBlock v10 # SendFallbackReason: InvokeBlock: not yet specialized + v25:ObjectSubclass[class_exact*:Object@VALUE(0x1008)] = GuardType v9, ObjectSubclass[class_exact*:Object@VALUE(0x1008)] recompile + PushInlineFrame v25 (0x1040), v10 + v33:BasicObject = InvokeBlock v10 # SendFallbackReason: InvokeBlock: not yet specialized CheckInterrupts PopInlineFrame PatchPoint NoEPEscape(test) - Return v35 + Return v33 "); } @@ -18970,27 +19066,27 @@ mod hir_opt_tests { Jump bb3(v6, v7) bb3(v9:BasicObject, v10:BasicObject): PatchPoint MethodRedefined(Object@0x1008, with_block_param@0x1010, cme:0x1018) - v27:ObjectSubclass[class_exact*:Object@VALUE(0x1008)] = GuardType v9, ObjectSubclass[class_exact*:Object@VALUE(0x1008)] recompile - v54:NilClass = Const Value(nil) - PushInlineFrame v27 (0x1040), v10 - v37:CPtr = GetEP 0 - v38:CUInt64 = LoadField v37, :VM_ENV_DATA_INDEX_FLAGS@0x1048 - v39:CBool = IsBlockParamModified v38 - CondBranch v39, bb6(), bb7() + v25:ObjectSubclass[class_exact*:Object@VALUE(0x1008)] = GuardType v9, ObjectSubclass[class_exact*:Object@VALUE(0x1008)] recompile + v52:NilClass = Const Value(nil) + PushInlineFrame v25 (0x1040), v10 + v35:CPtr = GetEP 0 + v36:CUInt64 = LoadField v35, :VM_ENV_DATA_INDEX_FLAGS@0x1048 + v37:CBool = IsBlockParamModified v36 + CondBranch v37, bb6(), bb7() bb6(): - v41:BasicObject = LoadField v37, :block@0x1049 - Jump bb8(v41, v41) + v39:BasicObject = LoadField v35, :block@0x1049 + Jump bb8(v39, v39) bb7(): - v43:CInt64 = LoadField v37, :VM_ENV_DATA_INDEX_SPECVAL@0x104a - v44:CInt64 = GuardAnyBitSet v43, CUInt64(1) recompile - v45:ObjectSubclass[BlockParamProxy] = Const Value(VALUE(0x1050)) - Jump bb8(v45, v54) - bb8(v35:BasicObject, v36:BasicObject): - v49:BasicObject = Send v35, :call, v10 # SendFallbackReason: SendWithoutBlock: unsupported optimized method type BlockCall + v41:CInt64 = LoadField v35, :VM_ENV_DATA_INDEX_SPECVAL@0x104a + v42:CInt64 = GuardAnyBitSet v41, CUInt64(1) recompile + v43:ObjectSubclass[BlockParamProxy] = Const Value(VALUE(0x1050)) + Jump bb8(v43, v52) + bb8(v33:BasicObject, v34:BasicObject): + v47:BasicObject = Send v33, :call, v10 # SendFallbackReason: SendWithoutBlock: unsupported optimized method type BlockCall CheckInterrupts PopInlineFrame PatchPoint NoEPEscape(test) - Return v49 + Return v47 "); } @@ -19034,27 +19130,27 @@ mod hir_opt_tests { Jump bb3(v6, v7) bb3(v9:BasicObject, v10:BasicObject): PatchPoint MethodRedefined(Object@0x1008, callee@0x1010, cme:0x1018) - v27:ObjectSubclass[class_exact*:Object@VALUE(0x1008)] = GuardType v9, ObjectSubclass[class_exact*:Object@VALUE(0x1008)] recompile - v55:NilClass = Const Value(nil) - PushInlineFrame v27 (0x1040), v10 - v39:CPtr = GetEP 0 - v40:CUInt64 = LoadField v39, :VM_ENV_DATA_INDEX_FLAGS@0x1048 - v41:CBool = IsBlockParamModified v40 - CondBranch v41, bb6(), bb7() + v25:ObjectSubclass[class_exact*:Object@VALUE(0x1008)] = GuardType v9, ObjectSubclass[class_exact*:Object@VALUE(0x1008)] recompile + v53:NilClass = Const Value(nil) + PushInlineFrame v25 (0x1040), v10 + v37:CPtr = GetEP 0 + v38:CUInt64 = LoadField v37, :VM_ENV_DATA_INDEX_FLAGS@0x1048 + v39:CBool = IsBlockParamModified v38 + CondBranch v39, bb6(), bb7() bb6(): - v43:BasicObject = LoadField v39, :block@0x1049 - Jump bb8(v43, v43) + v41:BasicObject = LoadField v37, :block@0x1049 + Jump bb8(v41, v41) bb7(): - v45:CInt64 = LoadField v39, :VM_ENV_DATA_INDEX_SPECVAL@0x104a - v46:CInt64 = GuardAnyBitSet v45, CUInt64(1) recompile - v47:ObjectSubclass[BlockParamProxy] = Const Value(VALUE(0x1050)) - Jump bb8(v47, v55) - bb8(v37:BasicObject, v38:BasicObject): - v50:BasicObject = Send v27, &block, :inner, v10, v37 # SendFallbackReason: Complex argument passing + v43:CInt64 = LoadField v37, :VM_ENV_DATA_INDEX_SPECVAL@0x104a + v44:CInt64 = GuardAnyBitSet v43, CUInt64(1) recompile + v45:ObjectSubclass[BlockParamProxy] = Const Value(VALUE(0x1050)) + Jump bb8(v45, v53) + bb8(v35:BasicObject, v36:BasicObject): + v48:BasicObject = Send v25, &block, :inner, v10, v35 # SendFallbackReason: Complex argument passing CheckInterrupts PopInlineFrame PatchPoint NoEPEscape(test) - Return v50 + Return v48 "); } diff --git a/zjit/src/hir/tests.rs b/zjit/src/hir/tests.rs index 0d44650486b4d6..e6769054a222ae 100644 --- a/zjit/src/hir/tests.rs +++ b/zjit/src/hir/tests.rs @@ -1,6 +1,21 @@ #[cfg(test)] use super::*; +#[cfg(test)] +mod size_tests { + use super::*; + + #[test] + fn test_size_of_insn() { + assert_eq!(std::mem::size_of::(), 120); + } + + #[test] + fn test_size_of_type() { + assert_eq!(std::mem::size_of::(), 24); + } +} + #[cfg(test)] mod snapshot_tests { use super::*; @@ -2009,13 +2024,286 @@ pub(crate) mod hir_build_tests { bb3(v9:BasicObject, v10:BasicObject): v15:BasicObject = Send v10, 0x1008, :each # SendFallbackReason: Uncategorized(send) PatchPoint NoEPEscape(test) - v18:CPtr = LoadSP - v19:BasicObject = LoadField v18, :a@0x1000 CheckInterrupts Return v15 "); } + #[test] + fn test_send_with_block_reloads_only_written_locals() { + eval(" + def foo = yield + def test + a = 1 + b = 2 + foo { a = 3 } + a + b + end + test + "); + assert_contains_opcode("test", YARVINSN_send); + // Only `a` is reloaded after the call; `b` is never written by the block, + // so it keeps its SSA value (the Fixnum constant) and is not reloaded. + assert_snapshot!(hir_string("test"), @" + fn test@:4: + bb1(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + v2:NilClass = Const Value(nil) + v3:NilClass = Const Value(nil) + Jump bb3(v1, v2, v3) + bb2(): + EntryPoint JIT(0) + v6:BasicObject = LoadArg :self@0 + v7:NilClass = Const Value(nil) + v8:NilClass = Const Value(nil) + Jump bb3(v6, v7, v8) + bb3(v10:BasicObject, v11:NilClass, v12:NilClass): + v16:Fixnum[1] = Const Value(1) + v20:Fixnum[2] = Const Value(2) + v25:BasicObject = Send v10, 0x1000, :foo # SendFallbackReason: Uncategorized(send) + PatchPoint NoEPEscape(test) + v28:CPtr = LoadSP + v29:BasicObject = LoadField v28, :a@0x1028 + PatchPoint NoEPEscape(test) + v38:BasicObject = Send v29, :+, v20 # SendFallbackReason: Uncategorized(opt_plus) + CheckInterrupts + Return v38 + "); + } + + #[test] + fn test_send_with_block_does_not_reload_read_only_local() { + eval(" + def foo = yield + def test + a = 1 + foo { a } + a + end + test + "); + assert_contains_opcode("test", YARVINSN_send); + // The block only reads `a`; it never assigns it, so `a` keeps its SSA value + // and is not reloaded after the call. + assert_snapshot!(hir_string("test"), @" + fn test@:4: + bb1(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + v2:NilClass = Const Value(nil) + Jump bb3(v1, v2) + bb2(): + EntryPoint JIT(0) + v5:BasicObject = LoadArg :self@0 + v6:NilClass = Const Value(nil) + Jump bb3(v5, v6) + bb3(v8:BasicObject, v9:NilClass): + v13:Fixnum[1] = Const Value(1) + v18:BasicObject = Send v8, 0x1000, :foo # SendFallbackReason: Uncategorized(send) + PatchPoint NoEPEscape(test) + PatchPoint NoEPEscape(test) + CheckInterrupts + Return v13 + "); + } + + #[test] + fn test_send_reloads_referenced_block_param() { + eval(" + def take(x) = x + def consume = yield + def test(&block) + consume { take(block) } + block + end + test { 1 } + "); + assert_contains_opcode("test", YARVINSN_send); + // The block reads `block` (passed as a regular argument), so it references the + // block param. `getblockparam` is recorded as a read, but reading the block param + // materializes the captured block into its slot, so the block param must be + // reloaded after the call (this is the lazy_load_hooks miscompile scenario). + assert_snapshot!(hir_string("test"), @" + fn test@:5: + bb1(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + v2:CPtr = LoadSP + v3:BasicObject = LoadField v2, :block@0x1000 + Jump bb3(v1, v3) + bb2(): + EntryPoint JIT(0) + v6:BasicObject = LoadArg :self@0 + v7:BasicObject = LoadArg :block@1 + Jump bb3(v6, v7) + bb3(v9:BasicObject, v10:BasicObject): + v15:BasicObject = Send v9, 0x1008, :consume # SendFallbackReason: Uncategorized(send) + PatchPoint NoEPEscape(test) + v18:CPtr = LoadSP + v19:BasicObject = LoadField v18, :block@0x1000 + v24:CPtr = GetEP 0 + v25:CUInt64 = LoadField v24, :VM_ENV_DATA_INDEX_FLAGS@0x1030 + v26:CBool = IsBlockParamModified v25 + CondBranch v26, bb4(), bb5() + bb4(): + v28:BasicObject = LoadField v24, :block@0x1031 + Jump bb6(v28) + bb5(): + v30:BasicObject = GetBlockParam :block, l0, EP@3 + Jump bb6(v30) + bb6(v23:BasicObject): + CheckInterrupts + Return v23 + "); + } + + #[test] + fn test_send_does_not_reload_unreferenced_block_param() { + eval(" + def consume = yield + def test(&block) + a = 1 + consume { a } + block + end + test { 1 } + "); + assert_contains_opcode("test", YARVINSN_send); + // The block only references `a`, never the block param, so the block param + // cannot have been materialized by the call and is not reloaded. (Before the + // reload filter was refined, the block param was reloaded after every + // send-with-block, even when the block could not have touched it.) + assert_snapshot!(hir_string("test"), @" + fn test@:4: + bb1(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + v2:CPtr = LoadSP + v3:BasicObject = LoadField v2, :block@0x1000 + v4:NilClass = Const Value(nil) + Jump bb3(v1, v3, v4) + bb2(): + EntryPoint JIT(0) + v7:BasicObject = LoadArg :self@0 + v8:BasicObject = LoadArg :block@1 + v9:NilClass = Const Value(nil) + Jump bb3(v7, v8, v9) + bb3(v11:BasicObject, v12:BasicObject, v13:NilClass): + v17:Fixnum[1] = Const Value(1) + v22:BasicObject = Send v11, 0x1008, :consume # SendFallbackReason: Uncategorized(send) + PatchPoint NoEPEscape(test) + v29:CPtr = GetEP 0 + v30:CUInt64 = LoadField v29, :VM_ENV_DATA_INDEX_FLAGS@0x1030 + v31:CBool = IsBlockParamModified v30 + CondBranch v31, bb4(), bb5() + bb4(): + v33:BasicObject = LoadField v29, :block@0x1031 + Jump bb6(v33) + bb5(): + v35:BasicObject = GetBlockParam :block, l0, EP@4 + Jump bb6(v35) + bb6(v28:BasicObject): + CheckInterrupts + Return v28 + "); + } + + #[test] + fn test_send_with_anonymous_block_param() { + eval(" + def consume = yield + def test(&) + consume { consume(&) } + consume(&) + end + test { 1 } + "); + assert_contains_opcode("test", YARVINSN_send); + // An anonymous `&` block param can only be forwarded with `&`, which compiles to + // `getblockparamproxy` and reads the block from the EP. It never materializes the + // param into its slot, so the block param is read directly from the EP after the + // call and is not reloaded -- there is nothing a reload could recover. + assert_snapshot!(hir_string("test"), @" + fn test@:4: + bb1(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + v2:CPtr = LoadSP + v3:BasicObject = LoadField v2, :&@0x1000 + Jump bb3(v1, v3) + bb2(): + EntryPoint JIT(0) + v6:BasicObject = LoadArg :self@0 + v7:BasicObject = LoadArg :&@1 + Jump bb3(v6, v7) + bb3(v9:BasicObject, v10:BasicObject): + v15:BasicObject = Send v9, 0x1008, :consume # SendFallbackReason: Uncategorized(send) + PatchPoint NoEPEscape(test) + v24:CPtr = GetEP 0 + v25:CUInt64 = LoadField v24, :VM_ENV_DATA_INDEX_FLAGS@0x1030 + v26:CBool = IsBlockParamModified v25 + CondBranch v26, bb4(), bb5() + bb4(): + v28:BasicObject = LoadField v24, :&@0x1031 + Jump bb6(v28, v28) + bb5(): + v30:CInt64 = LoadField v24, :VM_ENV_DATA_INDEX_SPECVAL@0x1032 + v31:CInt64 = GuardAnyBitSet v30, CUInt64(1) recompile + v32:ObjectSubclass[BlockParamProxy] = Const Value(VALUE(0x1038)) + Jump bb6(v32, v10) + bb6(v22:BasicObject, v23:BasicObject): + v35:BasicObject = Send v9, &block, :consume, v22 # SendFallbackReason: Uncategorized(send) + CheckInterrupts + Return v35 + "); + } + + #[test] + fn test_send_reloads_local_written_by_nested_block() { + eval(" + def foo = yield + def test + a = 1 + b = 2 + foo { foo { a = 3 } } + a + b + end + test + "); + assert_contains_opcode("test", YARVINSN_send); + // `a` is assigned only from a block nested inside the block argument, but the + // outer block's outer_variables table still records that write (the compiler + // aggregates writes up the nesting chain), so `a` is reloaded while the + // untouched `b` keeps its SSA value. + assert_snapshot!(hir_string("test"), @" + fn test@:4: + bb1(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + v2:NilClass = Const Value(nil) + v3:NilClass = Const Value(nil) + Jump bb3(v1, v2, v3) + bb2(): + EntryPoint JIT(0) + v6:BasicObject = LoadArg :self@0 + v7:NilClass = Const Value(nil) + v8:NilClass = Const Value(nil) + Jump bb3(v6, v7, v8) + bb3(v10:BasicObject, v11:NilClass, v12:NilClass): + v16:Fixnum[1] = Const Value(1) + v20:Fixnum[2] = Const Value(2) + v25:BasicObject = Send v10, 0x1000, :foo # SendFallbackReason: Uncategorized(send) + PatchPoint NoEPEscape(test) + v28:CPtr = LoadSP + v29:BasicObject = LoadField v28, :a@0x1028 + PatchPoint NoEPEscape(test) + v38:BasicObject = Send v29, :+, v20 # SendFallbackReason: Uncategorized(opt_plus) + CheckInterrupts + Return v38 + "); + } + #[test] fn test_intern_interpolated_symbol() { eval(r#" @@ -2302,8 +2590,6 @@ pub(crate) mod hir_build_tests { bb3(v9:BasicObject, v10:BasicObject): v16:BasicObject = InvokeSuperForward v9, 0x1008, v10 # SendFallbackReason: InvokeSuperForward: not yet specialized PatchPoint NoEPEscape(test) - v19:CPtr = LoadSP - v20:BasicObject = LoadField v19, :...@0x1000 CheckInterrupts Return v16 "); diff --git a/zjit/src/hir_type/mod.rs b/zjit/src/hir_type/mod.rs index 6fb0483e4487c3..ef67de616e0c34 100644 --- a/zjit/src/hir_type/mod.rs +++ b/zjit/src/hir_type/mod.rs @@ -593,6 +593,9 @@ impl Type { } pub fn num_bytes(&self) -> u8 { + assert!(!self.bit_equal(types::Empty), + "a value of type Empty is unreachable and should have been eliminated before codegen"); + if self.is_subtype(types::CUInt8) || self.is_subtype(types::CInt8) { return 1; } if self.is_subtype(types::CUInt16) || self.is_subtype(types::CInt16) { return 2; } if self.is_subtype(types::CUInt32) || self.is_subtype(types::CInt32) { return 4; } diff --git a/zjit/src/options.rs b/zjit/src/options.rs index bf09235002dd8d..c7e75a54987d2b 100644 --- a/zjit/src/options.rs +++ b/zjit/src/options.rs @@ -15,6 +15,9 @@ pub enum PerfMap { HIR, } +/// Default maximum number of compiled versions per ISEQ. +pub const DEFAULT_MAX_VERSIONS: usize = 4; + /// Default --zjit-num-profiles const DEFAULT_NUM_PROFILES: NumProfiles = 5; pub type NumProfiles = u16; @@ -212,7 +215,7 @@ impl Default for Options { perf: None, allowed_iseqs: None, log_compiled_iseqs: None, - max_versions: 2, + max_versions: DEFAULT_MAX_VERSIONS, inline_threshold: DEFAULT_INLINE_THRESHOLD, inline_budget: DEFAULT_INLINE_BUDGET as InlineBudget, inline_deny: HashSet::new(), @@ -634,6 +637,13 @@ pub fn set_call_threshold(call_threshold: CallThreshold) { update_profile_threshold(); } +/// Update --zjit-max-versions for testing +#[cfg(test)] +pub fn set_max_versions(max_versions: usize) { + rb_zjit_prepare_options(); + unsafe { OPTIONS.as_mut().unwrap().max_versions = max_versions; } +} + /// Update --zjit-inline-threshold for testing #[cfg(test)] pub fn set_inline_threshold(inline_threshold: InlineThreshold) { diff --git a/zjit/src/profile.rs b/zjit/src/profile.rs index fd830c35a308f7..22e6835a6ac071 100644 --- a/zjit/src/profile.rs +++ b/zjit/src/profile.rs @@ -59,9 +59,11 @@ pub extern "C" fn rb_zjit_profile_insn(bare_opcode: u32, ec: EcPtr) { } /// Profile a YARV instruction -fn profile_insn(bare_opcode: ruby_vminsn_type, ec: EcPtr) { - let profiler = &mut Profiler::new(ec); - let profile = &mut get_or_create_iseq_payload(profiler.iseq).profile; +fn profile_insn_sample( + bare_opcode: ruby_vminsn_type, + profiler: &mut Profiler, + profile: &mut IseqProfile, +) -> bool { match bare_opcode { YARVINSN_opt_nil_p => profile_operands(profiler, profile, 1), YARVINSN_opt_plus => profile_operands(profiler, profile, 2), @@ -100,9 +102,18 @@ fn profile_insn(bare_opcode: ruby_vminsn_type, ec: EcPtr) { profile_operands(profiler, profile, argc + 1); } YARVINSN_splatkw => profile_operands(profiler, profile, 2), - _ => {} + _ => return false, } + true +} + +/// Profile a YARV instruction +fn profile_insn(bare_opcode: ruby_vminsn_type, ec: EcPtr) { + let profiler = &mut Profiler::new(ec); + let profile = &mut get_or_create_iseq_payload(profiler.iseq).profile; + let _ = profile_insn_sample(bare_opcode, profiler, profile); + // Once we profile the instruction enough times, we stop profiling it. let entry = profile.entry_mut(profiler.insn_idx); entry.profiles_remaining = entry.profiles_remaining.saturating_sub(1); @@ -111,6 +122,38 @@ fn profile_insn(bare_opcode: ruby_vminsn_type, ec: EcPtr) { } } +/// Profile the instruction at the current CFP for a recompile side exit. +pub fn profile_recompile_insn(ec: EcPtr) -> bool { + let profiler = &mut Profiler::new(ec); + let pc = unsafe { get_cfp_pc(profiler.cfp) }; + let bare_opcode = unsafe { + rb_zjit_insn_to_bare_insn(rb_iseq_opcode_at_pc(profiler.iseq, pc)) + } as ruby_vminsn_type; + let profile = &mut get_or_create_iseq_payload(profiler.iseq).profile; + + let is_send = matches!(bare_opcode, YARVINSN_send | YARVINSN_opt_send_without_block); + // For now, send recompile exits only fill in missing profiles. Once the send site + // has finished profiling, don't recompile it on later exits. + if is_send && profile.done_profiling_at(profiler.insn_idx) { + return false; + } + // For now, non-send recompile exits reset the profiling counter before requesting recompilation + // so that we can collect enough samples. + if !is_send && profile.done_profiling_at(profiler.insn_idx) { + profile.entry_mut(profiler.insn_idx) + .set_profiles_remaining(get_option!(num_profiles)); + } + + // If this opcode can't be sampled here, this exit has no profile data to collect. + if !profile_insn_sample(bare_opcode, profiler, profile) { + return false; + } + + let entry = profile.entry_mut(profiler.insn_idx); + entry.profiles_remaining = entry.profiles_remaining.saturating_sub(1); + entry.profiles_remaining == 0 +} + /// Return the argc as stated in the calldata plus: /// * 1 if there is an explicit blockarg, since that will be passed on the stack pub fn num_arguments_on_stack(cd: *const rb_call_data) -> usize { @@ -426,70 +469,6 @@ impl IseqProfile { self.entry(insn_idx).map_or(false, |e| e.profiles_remaining == 0) } - /// Profile send operands from the stack at runtime. - /// `sp` is the current stack pointer (after the args and receiver). - /// `argc` is the number of arguments (not counting receiver). - /// Returns true if enough profiles have been gathered and the ISEQ should be recompiled. - pub fn profile_send_at(&mut self, iseq: IseqPtr, insn_idx: YarvInsnIdx, sp: *const VALUE, argc: usize) -> bool { - let n = argc + 1; // args + receiver - let entry = self.entry_mut(insn_idx); - if entry.opnd_types.is_empty() { - entry.opnd_types.resize(n, TypeDistribution::new()); - } - for i in 0..n { - let obj = unsafe { *sp.offset(i as isize - n as isize) }; - let ty = ProfiledType::new(obj); - VALUE::from(iseq).write_barrier(ty.class()); - entry.opnd_types[i].observe(ty); - } - entry.profiles_remaining = entry.profiles_remaining.saturating_sub(1); - entry.profiles_remaining == 0 - } - - /// Profile self for a shape guard exit at runtime. - /// This may be called on an instruction that was already profiled by YARV, - /// so we reset the counter to re-profile with the new shapes seen at runtime. - /// Returns true if enough profiles have been gathered and the ISEQ should be recompiled. - pub fn profile_self_at(&mut self, iseq: IseqPtr, insn_idx: YarvInsnIdx, self_val: VALUE) -> bool { - let entry = self.entry_mut(insn_idx); - // Reset profiling if the previous round already finished (stale YARV profiles). - // This ensures we collect num_profiles samples of the new shapes before recompiling. - if entry.profiles_remaining == 0 { - entry.profiles_remaining = get_option!(num_profiles); - } - if entry.opnd_types.is_empty() { - entry.opnd_types.resize(1, TypeDistribution::new()); - } - let ty = ProfiledType::new(self_val); - VALUE::from(iseq).write_barrier(ty.class()); - entry.opnd_types[0].observe(ty); - entry.profiles_remaining = entry.profiles_remaining.saturating_sub(1); - entry.profiles_remaining == 0 - } - - /// Profile the block handler for a getblockparamproxy guard exit at runtime. - pub fn profile_getblockparamproxy_at(&mut self, iseq: IseqPtr, insn_idx: YarvInsnIdx, cfp: CfpPtr) -> bool { - let pc = unsafe { rb_iseq_pc_at_idx(iseq, insn_idx as u32) }; - let level = unsafe { pc.add(2).read() }.as_u32(); - - let entry = self.entry_mut(insn_idx); - if entry.profiles_remaining == 0 { - entry.profiles_remaining = get_option!(num_profiles); - } - if entry.opnd_types.is_empty() { - entry.opnd_types.resize(1, TypeDistribution::new()); - } - let ep = unsafe { get_cfp_ep_level(cfp, level) }; - let block_handler = unsafe { *ep.offset(VM_ENV_DATA_INDEX_SPECVAL as isize) }; - let untagged = unsafe { rb_vm_untag_block_handler(block_handler) }; - - let ty = ProfiledType::object(untagged); - VALUE::from(iseq).write_barrier(ty.class()); - entry.opnd_types[0].observe(ty); - entry.profiles_remaining = entry.profiles_remaining.saturating_sub(1); - entry.profiles_remaining == 0 - } - /// Get profiled operand types for a given instruction index pub fn get_operand_types(&self, insn_idx: YarvInsnIdx) -> Option<&[TypeDistribution]> { self.entry(insn_idx).map(|e| e.opnd_types.as_slice()).filter(|s| !s.is_empty())