diff --git a/.github/workflows/zjit-macos.yml b/.github/workflows/zjit-macos.yml index 94741386c23a16..1e63e8b3115c00 100644 --- a/.github/workflows/zjit-macos.yml +++ b/.github/workflows/zjit-macos.yml @@ -98,7 +98,7 @@ jobs: rustup install ${{ matrix.rust_version }} --profile minimal rustup default ${{ matrix.rust_version }} - - uses: taiki-e/install-action@1beb33eee6d086258184383af9a538940be190ed # v2.85.6 + - uses: taiki-e/install-action@67729d5c413db75907f0ad1e39bb04b9c868ff60 # v2.85.7 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 2f30e8b9f60a4a..bac24c26439cbf 100644 --- a/.github/workflows/zjit-ubuntu.yml +++ b/.github/workflows/zjit-ubuntu.yml @@ -152,7 +152,7 @@ jobs: ruby-version: '3.1' bundler: none - - uses: taiki-e/install-action@1beb33eee6d086258184383af9a538940be190ed # v2.85.6 + - uses: taiki-e/install-action@67729d5c413db75907f0ad1e39bb04b9c868ff60 # v2.85.7 with: tool: nextest@0.9 if: ${{ matrix.test_task == 'zjit-check' }} diff --git a/array.c b/array.c index ed0c8e10bc48bb..2e0cfe5b03f16d 100644 --- a/array.c +++ b/array.c @@ -228,6 +228,15 @@ rb_ary_embeddable_p(VALUE ary) return !(ARY_SHARED_ROOT_P(ary) || OBJ_FROZEN(ary) || ARY_SHARED_P(ary)); } +/* True when other arrays may read this array's elements out of its own slot, so the + * slot contents must stay valid for as long as the object does. A frozen array is + * handed out as a shared root as it is, without the shared root flag. */ +bool +rb_ary_embedded_shared_root_p(VALUE ary) +{ + return ARY_EMBED_P(ary) && OBJ_FROZEN(ary); +} + size_t rb_ary_size_as_embedded(VALUE ary) { diff --git a/compile.c b/compile.c index 95efcefb8b69ea..0e73114c35572d 100644 --- a/compile.c +++ b/compile.c @@ -12695,7 +12695,7 @@ pinned_list_fetch(VALUE list, long offset) TypedData_Get_Struct(list, struct pinned_list, &pinned_list_type, ptr); - if (offset >= ptr->size) { + if (offset < 0 || offset >= ptr->size) { rb_raise(rb_eIndexError, "object index out of range: %ld", offset); } @@ -12709,7 +12709,7 @@ pinned_list_store(VALUE list, long offset, VALUE object) TypedData_Get_Struct(list, struct pinned_list, &pinned_list_type, ptr); - if (offset >= ptr->size) { + if (offset < 0 || offset >= ptr->size) { rb_raise(rb_eIndexError, "object index out of range: %ld", offset); } diff --git a/gc.c b/gc.c index be3d82d46f13ee..0664ae6ccfd046 100644 --- a/gc.c +++ b/gc.c @@ -3220,7 +3220,15 @@ rb_gc_mark_children(void *objspace, VALUE obj) case T_ARRAY: if (ARY_SHARED_P(obj)) { VALUE root = ARY_SHARED_ROOT(obj); - gc_mark_internal(root); + if (RB_TYPE_P(root, T_ARRAY)) { + gc_mark_internal(root); + } + else { + /* Ractor#send(move: true) hollowed the root out in place. If it was + * embedded our elements are still in its slot, and nothing says so any + * more, so it must not move (gc_ref_update_array cannot re-point us). */ + gc_mark_and_pin_internal(root); + } } else { long len = RARRAY_LEN(obj); @@ -3624,8 +3632,10 @@ gc_ref_update_array(void *objspace, VALUE v) UPDATE_IF_MOVED(objspace, RARRAY(v)->as.heap.aux.shared_root); VALUE new_root = RARRAY(v)->as.heap.aux.shared_root; + // A root hollowed out by a move is no longer an array, and it is pinned rather + // than re-pointed (see the marking of a shared root). // If the root is embedded and its location has changed - if (ARY_EMBED_P(new_root) && new_root != old_root) { + if (RB_TYPE_P(new_root, T_ARRAY) && ARY_EMBED_P(new_root) && new_root != old_root) { size_t offset = (size_t)(RARRAY(v)->as.heap.ptr - RARRAY(old_root)->as.ary); GC_ASSERT(RARRAY(v)->as.heap.ptr >= RARRAY(old_root)->as.ary); RARRAY(v)->as.heap.ptr = RARRAY(new_root)->as.ary + offset; diff --git a/include/ruby/st.h b/include/ruby/st.h index 8b2951fc933a41..8ac34468844f39 100644 --- a/include/ruby/st.h +++ b/include/ruby/st.h @@ -78,7 +78,7 @@ struct st_table_entry; /* defined in st.c */ struct st_table { /* Cached features of the table -- see st.c for more details. */ - unsigned char entry_power, bin_power, size_ind; + unsigned char entry_power, bin_power, size_ind, entries_start; /* How many times the table was rebuilt. */ unsigned int rebuilds_num; const struct st_hash_type *type; @@ -87,7 +87,7 @@ struct st_table { /* Start and bound index of entries in array entries. entries_starts and entries_bound are in interval [0,allocated_entries]. */ - st_index_t entries_start, entries_bound; + st_index_t entries_bound; /* Array of size 2^entry_power. Optionally followed by an array of bins used for access by keys. */ st_table_entry *entries; diff --git a/internal/array.h b/internal/array.h index c7085f431fa31e..9fffd9b6bc558a 100644 --- a/internal/array.h +++ b/internal/array.h @@ -37,6 +37,7 @@ void rb_ary_cancel_sharing(VALUE ary); size_t rb_ary_size_as_embedded(VALUE ary); void rb_ary_make_embedded(VALUE ary); bool rb_ary_embeddable_p(VALUE ary); +bool rb_ary_embedded_shared_root_p(VALUE ary); VALUE rb_ary_diff(VALUE ary1, VALUE ary2); VALUE rb_ary_compact_bang(VALUE ary); VALUE rb_ary_modify_expand(VALUE ary, long expand); diff --git a/internal/set_table.h b/internal/set_table.h index 3876a8935e1bf9..8c568d4fbf30c9 100644 --- a/internal/set_table.h +++ b/internal/set_table.h @@ -9,7 +9,7 @@ typedef struct set_table_entry set_table_entry; struct set_table { /* Cached features of the table -- see st.c for more details. */ - unsigned char entry_power, bin_power, size_ind; + unsigned char entry_power, bin_power, size_ind, entries_start; /* How many times the table was rebuilt. */ unsigned int rebuilds_num; const struct st_hash_type *type; @@ -19,7 +19,7 @@ struct set_table { /* Start and bound index of entries in array entries. entries_starts and entries_bound are in interval [0,allocated_entries]. */ - st_index_t entries_start, entries_bound; + st_index_t entries_bound; /** * Array of size 2^entry_power. diff --git a/lib/uri/generic.rb b/lib/uri/generic.rb index 6a0f638d764173..fce8a8a52eacdd 100644 --- a/lib/uri/generic.rb +++ b/lib/uri/generic.rb @@ -1021,9 +1021,21 @@ def merge_path(base, rel) # RFC2396, Section 5.2, 6), a) base_path << '' if base_path.last == '..' - while i = base_path.index('..') - base_path.slice!(i - 1, 2) + # Remove "/.." pairs in a single left-to-right pass (O(n)). + # This deliberately differs from the relative-path stack handling + # below: a leading ".." (one with no preceding segment left to + # cancel) discards the whole base path, reproducing the previous + # index/slice! implementation exactly. + reduced = [] + base_path.each do |seg| + if seg == '..' + break if reduced.empty? + reduced.pop + else + reduced << seg + end end + base_path = reduced if (first = rel_path.first) and first.empty? base_path.clear diff --git a/ractor.c b/ractor.c index 4e741f000df0b6..556d663ba13db0 100644 --- a/ractor.c +++ b/ractor.c @@ -8,6 +8,7 @@ #include "vm_core.h" #include "vm_sync.h" #include "ractor_core.h" +#include "internal/array.h" #include "internal/complex.h" #include "internal/error.h" #include "internal/gc.h" @@ -2125,10 +2126,20 @@ move_leave(VALUE obj, struct obj_traverse_replace_data *data) VALUE flags = T_OBJECT | FL_FREEZE | (RBASIC(obj)->flags & FL_PROMOTED); shape_id_t shape_id = (RBASIC_SHAPE_ID(obj) & SHAPE_ID_CAPACITY_MASK) | ROOT_SHAPE_ID | SHAPE_ID_LAYOUT_ROBJECT | SHAPE_ID_FL_FROZEN; - // A copy-on-write sharer reads its bytes straight out of an embedded root's slot - // (String#dup of a frozen string), and it outlives the move, so that body has to - // survive as it is. - bool wipe_body = !(RB_TYPE_P(obj, T_STRING) && rb_str_embedded_shared_root_p(obj)); + // A copy-on-write sharer reads its payload straight out of an embedded root's slot + // (String#dup of a frozen string, Array#[] of a frozen array), and it outlives the + // move, so that body has to survive as it is. + bool wipe_body = true; + switch (BUILTIN_TYPE(obj)) { + case T_STRING: + wipe_body = !rb_str_embedded_shared_root_p(obj); + break; + case T_ARRAY: + wipe_body = !rb_ary_embedded_shared_root_p(obj); + break; + default: + break; + } // Avoid mutations using bind_call, etc. size_t slot_size = rb_gc_obj_slot_size(obj); diff --git a/st.c b/st.c index 550bcc6325dd7c..3bedde661f2124 100644 --- a/st.c +++ b/st.c @@ -131,6 +131,8 @@ #define ATTRIBUTE_UNUSED #endif +#define MAX_ENTRIES_START ((unsigned char)-1) + /* The type of hashes. */ typedef st_index_t st_hash_t; @@ -1187,12 +1189,13 @@ st_get_key(st_table *tab, st_data_t key, st_data_t *result) /* Check the table and rebuild it if it is necessary. */ static inline void -rebuild_table_if_necessary (st_table *tab) +rebuild_table_if_necessary(st_table *tab) { st_index_t bound = tab->entries_bound; - if (bound == get_allocated_entries(tab)) + if (bound == get_allocated_entries(tab) || tab->entries_start == MAX_ENTRIES_START) { rebuild_table(tab); + } } /* Insert (KEY, VALUE) into table TAB and return zero. If there is @@ -1301,7 +1304,7 @@ st_insert2(st_table *tab, st_data_t key, st_data_t value, hash_value = do_hash(key, tab); retry: - rebuild_table_if_necessary (tab); + rebuild_table_if_necessary(tab); if (!st_has_bins(tab)) { bin = find_entry(tab, hash_value, key); if (EXPECT(bin == REBUILT_TABLE_ENTRY_IND, 0)) @@ -1384,6 +1387,9 @@ update_range_for_deleted(st_table *tab, st_index_t n) st_index_t bound = tab->entries_bound; st_table_entry *entries = tab->entries; while (start < bound && DELETED_ENTRY_P(&entries[start])) start++; + if (start > MAX_ENTRIES_START) { + start = MAX_ENTRIES_START; + } tab->entries_start = start; } } @@ -2989,12 +2995,13 @@ set_table_lookup(set_table *tab, st_data_t key) /* Check the table and rebuild it if it is necessary. */ static inline void -set_rebuild_table_if_necessary (set_table *tab) +set_rebuild_table_if_necessary(set_table *tab) { st_index_t bound = tab->entries_bound; - if (bound == set_get_allocated_entries(tab)) + if (bound == set_get_allocated_entries(tab) || tab->entries_start == MAX_ENTRIES_START) { set_rebuild_table(tab); + } } /* Insert KEY into table TAB and return zero. If there is @@ -3079,6 +3086,9 @@ set_update_range_for_deleted(set_table *tab, st_index_t n) st_index_t bound = tab->entries_bound; set_table_entry *entries = tab->entries; while (start < bound && DELETED_ENTRY_P(&entries[start])) start++; + if (start > MAX_ENTRIES_START) { + start = MAX_ENTRIES_START; + } tab->entries_start = start; } } diff --git a/test/ruby/test_ractor.rb b/test/ruby/test_ractor.rb index 20e86317e90791..f56b7aed8d18a3 100644 --- a/test/ruby/test_ractor.rb +++ b/test/ruby/test_ractor.rb @@ -509,4 +509,29 @@ def test_move_string_sharing_its_embedded_bytes end RUBY end + + # A frozen array is handed out as a shared root as it is, so a subseq of an embedded + # one reads the elements out of its slot. Moving the original must leave that slot + # alone, and must not let it move afterwards: the sharer has no other copy. + def test_move_array_sharing_its_embedded_elements + assert_ractor(<<~'RUBY', timeout: 60) + [8, 20, 40].each do |len| + r = Ractor.new { Ractor.receive } + ary = Array.new(len) { |i| i + 1 } # embedded + ary.instance_variable_set(:@iv, []) # unshareable, so it is moved + ary.freeze + sharer = ary[1, len - 2] # reads ary's elements in place + r.send(ary, move: true) + assert_equal (2..len - 1).to_a, sharer, "corrupted for length #{len}" + + begin + GC.verify_compaction_references(expand_heap: true, toward: :empty) + rescue NotImplementedError + # no compaction on this platform + end + assert_equal (2..len - 1).to_a, sharer, "corrupted by compaction, length #{len}" + r.value + end + RUBY + end end diff --git a/test/uri/test_generic.rb b/test/uri/test_generic.rb index 94eea71b511161..90061253a9cb05 100644 --- a/test/uri/test_generic.rb +++ b/test/uri/test_generic.rb @@ -278,6 +278,43 @@ def test_merge assert_equal(u0, u1) end + def test_merge_path_dot_dot_removal + # Base-path ".." removal (RFC2396 5.2 6a) is handled by a single + # left-to-right pass. These lock in the exact, historically observed + # semantics, which differ from the relative-path stack: a leading ".." + # (or a ".." exposed as leading after earlier cancellations) discards + # the remaining base path rather than being kept. + { + 'http://h/a/../../b' => { 'x' => 'http://h/x' }, + 'http://h/../a' => { 'x' => 'http://h/x' }, + 'http://h/a/..' => { 'x' => 'http://h/x' }, + 'http://h/../x' => { 'y' => 'http://h/y' }, + 'http://h/foo/bar/..' => { './' => 'http://h/foo/' }, + 'http://h/foo/bar/../..' => { './' => 'http://h/' }, + 'http://h/a/b/c' => { + '../../g' => 'http://h/g', + '../../../g' => 'http://h/g', + '../../../../g' => 'http://h/g', + }, + 'http://h/p//q/..' => { 'r' => 'http://h/p//r' }, + 'http://h/a/../..//y' => { 'z' => 'http://h/z' }, + }.each { |base, map| + map.each { |rel, expected| + assert_equal(expected, URI.parse(base).merge(rel).to_s, + "<#{base}> + #{rel.inspect}") + } + } + end + + def test_merge_path_dot_dot_removal_is_linear + # Regression guard for the previous O(n^2) base-path ".." removal: + # merging a base full of "a/../" segments must scale linearly. + pre = ->(n) {URI.parse('http://example.com/' + 'a/../' * n)} + assert_linear_performance((1..5).map {|i| 10 ** i}, pre: pre) do |base| + assert_equal('http://example.com/x', base.merge('x').to_s) + end + end + def test_merge_authority u = URI.parse('http://user:pass@example.com:8080') u0 = URI.parse('http://new.example.org/path')