diff --git a/.github/workflows/check_sast.yml b/.github/workflows/check_sast.yml index 90ed8a5bc9767f..d94a19fd2dbede 100644 --- a/.github/workflows/check_sast.yml +++ b/.github/workflows/check_sast.yml @@ -45,7 +45,7 @@ jobs: persist-credentials: false - name: Run zizmor - uses: zizmorcore/zizmor-action@70fb788f84895a7701f5643d103d587e460b5c99 # v0.6.3 + uses: zizmorcore/zizmor-action@cc914d7f3750a2d13d75c7f184a1060aa0e9d482 # v0.6.4 continue-on-error: true analyze: @@ -78,14 +78,14 @@ jobs: persist-credentials: false - name: Initialize CodeQL - uses: github/codeql-action/init@cdf488f595d80d6e07e03d4674febd5ab45fa938 # v4.37.9 + uses: github/codeql-action/init@b96794f015dfd88f77b49b1c93e0fa7110f94c63 # v4.38.0 with: languages: ${{ matrix.language }} build-mode: none config-file: .github/codeql/codeql-config.yml - name: Perform CodeQL Analysis - uses: github/codeql-action/analyze@cdf488f595d80d6e07e03d4674febd5ab45fa938 # v4.37.9 + uses: github/codeql-action/analyze@b96794f015dfd88f77b49b1c93e0fa7110f94c63 # v4.38.0 with: category: '/language:${{ matrix.language }}' upload: False @@ -127,7 +127,7 @@ jobs: continue-on-error: true - name: Upload SARIF - uses: github/codeql-action/upload-sarif@cdf488f595d80d6e07e03d4674febd5ab45fa938 # v4.37.9 + uses: github/codeql-action/upload-sarif@b96794f015dfd88f77b49b1c93e0fa7110f94c63 # v4.38.0 with: sarif_file: sarif-results/${{ matrix.language }}.sarif continue-on-error: true diff --git a/.github/workflows/modgc.yml b/.github/workflows/modgc.yml index 26bde7f894c148..1baaa9036106b5 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@166cdcfd11aee3cb47222f9ddb555ce30ddb9659 # v1.17.0 + - uses: actions-rust-lang/setup-rust-toolchain@ecabd13d1c56bd1345c230e542e9144811ad706f # v2.0.0 with: cache-bin: false - name: Set MMTk environment variables diff --git a/.github/workflows/scorecards.yml b/.github/workflows/scorecards.yml index 8f3ed45b13cb0f..9c2adf56600433 100644 --- a/.github/workflows/scorecards.yml +++ b/.github/workflows/scorecards.yml @@ -73,6 +73,6 @@ jobs: # Upload the results to GitHub's code scanning dashboard (optional). # Commenting out will disable upload of results to your repo's Code Scanning dashboard - name: "Upload to code-scanning" - uses: github/codeql-action/upload-sarif@cdf488f595d80d6e07e03d4674febd5ab45fa938 # v4.37.9 + uses: github/codeql-action/upload-sarif@b96794f015dfd88f77b49b1c93e0fa7110f94c63 # v4.38.0 with: sarif_file: results.sarif diff --git a/.github/workflows/zjit-macos.yml b/.github/workflows/zjit-macos.yml index 246d0c8722a0c1..8ee5f12ecead35 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@84f5ac3124727fb3d284d4d22ee9ab3654fd09a6 # v2.87.7 + - uses: taiki-e/install-action@9534c84618278caac52cb373bb164ed464dbd8af # v2.87.11 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 9cb401f8a7f2ea..8e0ebddea901b3 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@84f5ac3124727fb3d284d4d22ee9ab3654fd09a6 # v2.87.7 + - uses: taiki-e/install-action@9534c84618278caac52cb373bb164ed464dbd8af # v2.87.11 with: tool: nextest@0.9 if: ${{ matrix.test_task == 'zjit-check' }} diff --git a/NEWS.md b/NEWS.md index 3e1111cecfe318..6741268187b100 100644 --- a/NEWS.md +++ b/NEWS.md @@ -139,6 +139,11 @@ Note: We're only listing outstanding class updates. * `Symbol#to_s` now returns a frozen string. [[Feature #22137]] +* Thread + + * Assigning `nil` with `Thread#thread_variable_set` now deletes the + thread-local variable. [[Feature #19333]] + * Thread::Backtrace::Location * `Thread::Backtrace::Location#source_range` is added. It returns a diff --git a/compile.c b/compile.c index c539f8e91e4715..be83950c55f1cb 100644 --- a/compile.c +++ b/compile.c @@ -9394,6 +9394,9 @@ compile_builtin_attr_symbol(rb_iseq_t *iseq, VALUE symbol) else if (rb_streql_lit(string, "without_interrupts")) { ISEQ_BODY(iseq)->builtin_attrs |= BUILTIN_ATTR_WITHOUT_INTERRUPTS; } + else if (rb_streql_lit(string, "caller_user_box")) { + ISEQ_BODY(iseq)->builtin_attrs |= BUILTIN_ATTR_CALLER_USER_BOX; + } else { return COMPILE_NG; } diff --git a/cont.c b/cont.c index b999db478ff4e1..c4d57d92ba1a0d 100644 --- a/cont.c +++ b/cont.c @@ -2346,6 +2346,7 @@ rb_fiber_storage_aref(VALUE class, VALUE key) * * Assign +value+ to the fiber storage variable identified by +key+. * The variable is created if it doesn't exist. + * Assigning +nil+ deletes the variable. * * +key+ must be a Symbol, otherwise a TypeError is raised. * diff --git a/coroutine/asyncify/Context.h b/coroutine/asyncify/Context.h index 4e1a804923a5ef..38cd07dc45c2a1 100644 --- a/coroutine/asyncify/Context.h +++ b/coroutine/asyncify/Context.h @@ -62,9 +62,6 @@ static inline void coroutine_initialize(struct coroutine_context *context, corou static inline struct coroutine_context * coroutine_transfer(struct coroutine_context * current, struct coroutine_context * target) { if (ASYNCIFY_CORO_DEBUG) fprintf(stderr, "[%s] entry (current = %p, target = %p)\n", __func__, current, target); -#ifndef COROUTINE_TARGET_MAY_BE_FREED - struct coroutine_context * previous = target->from; -#endif target->from = current; if (ASYNCIFY_CORO_DEBUG) fprintf(stderr, "[%s] current->current_sp = %p -> %p\n", __func__, current->current_sp, rb_wasm_get_stack_pointer()); @@ -79,13 +76,8 @@ static inline struct coroutine_context * coroutine_transfer(struct coroutine_con rb_wasm_set_stack_pointer(current->current_sp); -#ifndef COROUTINE_TARGET_MAY_BE_FREED - /* from is read only by coroutine_trampoline, when target starts, which has - * happened before we get here. */ - target->from = previous; -#endif - - return target; + /* current may have been resumed by a context other than target. */ + return current->from; } static inline void coroutine_destroy(struct coroutine_context * context) diff --git a/coroutine/emscripten/Context.h b/coroutine/emscripten/Context.h index c26b4b23bbde92..83d1a36acadf41 100644 --- a/coroutine/emscripten/Context.h +++ b/coroutine/emscripten/Context.h @@ -60,19 +60,11 @@ static inline void coroutine_initialize( static inline struct coroutine_context * coroutine_transfer(struct coroutine_context * current, struct coroutine_context * target) { -#ifndef COROUTINE_TARGET_MAY_BE_FREED - struct coroutine_context * previous = target->from; -#endif - target->from = current; emscripten_fiber_swap(¤t->state, &target->state); -#ifndef COROUTINE_TARGET_MAY_BE_FREED - /* from is read only by coroutine_trampoline, when target starts, which has - * happened before we get here. */ - target->from = previous; -#endif - return target; + /* current may have been resumed by a context other than target. */ + return current->from; } static inline void coroutine_destroy(struct coroutine_context * context) diff --git a/coroutine/pthread/Context.c b/coroutine/pthread/Context.c index 014abbd4b59b30..e42f4b9bb42ad8 100644 --- a/coroutine/pthread/Context.c +++ b/coroutine/pthread/Context.c @@ -193,6 +193,7 @@ struct coroutine_context * coroutine_transfer(struct coroutine_context * current assert(current->shared); struct coroutine_context * previous = target->from; + int result = 0; target->from = current; if (DEBUG) fprintf(stderr, "coroutine_transfer:pthread_mutex_lock(guard=%p is_locked=%d)\n", ¤t->shared->guard, is_locked(¤t->shared->guard)); @@ -204,38 +205,42 @@ struct coroutine_context * coroutine_transfer(struct coroutine_context * current target->shared = current->shared; if (DEBUG) fprintf(stderr, "coroutine_transfer:coroutine_create_thread...\n"); - if (coroutine_create_thread(target)) { + result = coroutine_create_thread(target); + if (result != 0) { if (DEBUG) fprintf(stderr, "coroutine_transfer:coroutine_create_thread failed\n"); target->shared = NULL; target->from = previous; - return NULL; } } else { if (DEBUG) fprintf(stderr, "coroutine_transfer:pthread_cond_signal(target)\n"); pthread_cond_signal(&target->schedule); } - // A side effect of acting upon a cancellation request while in a condition wait is that the mutex is (in effect) re-acquired before calling the first cancellation cleanup handler. If cancelled, pthread_cond_wait immediately invokes cleanup handlers. - if (DEBUG) fprintf(stderr, "coroutine_transfer:pthread_cond_wait(schedule=%p, guard=%p, is_locked=%d)\n", ¤t->schedule, ¤t->shared->guard, is_locked(¤t->shared->guard)); - check("coroutine_transfer:pthread_cond_wait", - pthread_cond_wait(¤t->schedule, ¤t->shared->guard) - ); + if (result == 0) { + // A side effect of acting upon a cancellation request while in a condition wait is that the mutex is (in effect) re-acquired before calling the first cancellation cleanup handler. If cancelled, pthread_cond_wait immediately invokes cleanup handlers. + if (DEBUG) fprintf(stderr, "coroutine_transfer:pthread_cond_wait(schedule=%p, guard=%p, is_locked=%d)\n", ¤t->schedule, ¤t->shared->guard, is_locked(¤t->shared->guard)); + check("coroutine_transfer:pthread_cond_wait", + pthread_cond_wait(¤t->schedule, ¤t->shared->guard) + ); + } if (DEBUG) fprintf(stderr, "coroutine_transfer:pthread_cleanup_pop\n"); pthread_cleanup_pop(1); + /* Keep the push/pop pair in the same lexical scope and unlock the guard + * before reporting a setup failure. */ + if (result != 0) { + errno = result; + return NULL; + } + #ifdef __FreeBSD__ // Apparently required for FreeBSD: pthread_testcancel(); #endif -#ifndef COROUTINE_TARGET_MAY_BE_FREED - /* from is read only by coroutine_trampoline, when target starts, which has - * happened before we get here. */ - target->from = previous; -#endif - - return target; + /* current may have been resumed by a context other than target. */ + return current->from; } static diff --git a/coroutine/ucontext/Context.h b/coroutine/ucontext/Context.h index 0063f72b77beee..8d916c235cede1 100644 --- a/coroutine/ucontext/Context.h +++ b/coroutine/ucontext/Context.h @@ -60,19 +60,16 @@ static inline void coroutine_initialize( static inline struct coroutine_context * coroutine_transfer(struct coroutine_context * current, struct coroutine_context * target) { -#ifndef COROUTINE_TARGET_MAY_BE_FREED struct coroutine_context * previous = target->from; -#endif target->from = current; - swapcontext(¤t->state, &target->state); -#ifndef COROUTINE_TARGET_MAY_BE_FREED - /* from is read only by coroutine_trampoline, when target starts, which has - * happened before we get here. */ - target->from = previous; -#endif + if (swapcontext(¤t->state, &target->state) == -1) { + target->from = previous; + return NULL; + } - return target; + /* current may have been resumed by a context other than target. */ + return current->from; } static inline void coroutine_destroy(struct coroutine_context * context) diff --git a/ext/erb/escape/escape.c b/ext/erb/escape/escape.c index 66cb77ed332a7d..98d83ea035e00b 100644 --- a/ext/erb/escape/escape.c +++ b/ext/erb/escape/escape.c @@ -45,7 +45,7 @@ escaped_length(VALUE str) #endif #endif -#if defined(__ARM_NEON) || defined(__ARM_NEON__) || defined(__aarch64__) || defined(_M_ARM64) +#if defined(__aarch64__) || defined(_M_ARM64) #define HAVE_SIMD 1 #define HAVE_SIMD_NEON 1 #include @@ -172,11 +172,93 @@ find_next_match_neon(search_state *search) // uint64_t >>= 64 is undefined behaviour RUBY_ASSERT(trailing_zeros < 64); search->matches_bitmap >>= trailing_zeros; - search->cstr += trailing_zeros / 4; + search->cstr += trailing_zeros; RUBY_ASSERT(search->cstr <= search->end); return true; } +// This 16-byte lookup table is indexed into by using the +// low nibble of each input byte. +// Note: index 0 is intentionally set to a character that will not match +// the NULL byte. +static const uint8x16_t escape_char_by_low_nibble = { + '\'', 0, '"', 0, + 0, 0, '&', '\'', + 0, 0, 0, 0, + '<', 0, '>', 0, +}; + +static inline uint8x16_t +neon_escape_matches(const uint8x16_t bytes) +{ + // An example to demonstrate how this works. The goal is to get a uint8x16_t + // with each lane to equal 0xFF if the corresponding byte in 'bytes' needs + // to be escaped, or 0x00 otherwise. + // + // To keep things very simple, I'm going to assume a vector of length 6, in + // reality, the vector would be 16 bytes wide. + // + // Assume the string is: "
" + // Converted to integers: + // [0x3c 0x62 0x72 0x20 0x2f 0x3e] + // + // Next, we mask off the top nibble so we are left only with the low nibble + // of each byte. We do this by AND'ing each byte with 0x0F. + // + // The result: + // [0x0c 0x02 0x02 0x00 0x0f 0x0e] + // + // Now, we use these low nibbles as indexes into the + // escape_char_by_low_nibble array and find the full byte + // value we expect to match in the input. + // + // The result: + // [0x3c 0x22 0x22 0x27 0x00 0x3e] + // + // Finally, we compare the bytes we expect with the actual input bytes. + // + // The result: + // [0xFF 0x00 0x00 0x00 0x00 0xFF] + const uint8x16_t low_nibbles = vandq_u8(bytes, vdupq_n_u8(0x0F)); + const uint8x16_t looked_up = vqtbl1q_u8(escape_char_by_low_nibble, low_nibbles); + return vceqq_u8(looked_up, bytes); +} + +static inline uint64_t +neon_matches_to_bitmap16(const uint8x16_t matches) +{ + static const uint8x16_t bit_mask = { + 0x01, 0x02, 0x04, 0x08, 0x10, 0x20, 0x40, 0x80, + 0x01, 0x02, 0x04, 0x08, 0x10, 0x20, 0x40, 0x80, + }; + + uint8x16_t folded = vandq_u8(matches, bit_mask); + folded = vpaddq_u8(folded, folded); + folded = vpaddq_u8(folded, folded); + folded = vpaddq_u8(folded, folded); + + return vgetq_lane_u16(vreinterpretq_u16_u8(folded), 0); +} + +static inline uint64_t +neon_matches_to_bitmap64(const uint8x16_t m0, const uint8x16_t m1, const uint8x16_t m2, const uint8x16_t m3) +{ + static const uint8x16_t bit_mask = { + 0x01, 0x02, 0x04, 0x08, 0x10, 0x20, 0x40, 0x80, + 0x01, 0x02, 0x04, 0x08, 0x10, 0x20, 0x40, 0x80, + }; + + const uint8x16_t t0 = vandq_u8(m0, bit_mask); + const uint8x16_t t1 = vandq_u8(m1, bit_mask); + const uint8x16_t t2 = vandq_u8(m2, bit_mask); + const uint8x16_t t3 = vandq_u8(m3, bit_mask); + + uint8x16_t folded = vpaddq_u8(vpaddq_u8(t0, t1), vpaddq_u8(t2, t3)); + folded = vpaddq_u8(folded, folded); + + return vgetq_lane_u64(vreinterpretq_u64_u8(folded), 0); +} + static inline bool find_next_neon(search_state *search) { @@ -184,30 +266,34 @@ find_next_neon(search_state *search) return find_next_match_neon(search); } - const uint8x16_t single_quote = vdupq_n_u8('\''); - const uint8x16_t double_quote = vdupq_n_u8('"'); - const uint8x16_t ampersand = vdupq_n_u8('&'); - const uint8x16_t lt = vdupq_n_u8('<'); - const uint8x16_t gt = vdupq_n_u8('>'); + while ((size_t)(search->end - search->cstr) >= sizeof(uint8x16x4_t)) { + const uint8x16_t bytes0 = vld1q_u8(search->cstr + 0); + const uint8x16_t bytes1 = vld1q_u8(search->cstr + 16); + const uint8x16_t bytes2 = vld1q_u8(search->cstr + 32); + const uint8x16_t bytes3 = vld1q_u8(search->cstr + 48); - while ((size_t)(search->end - search->cstr) >= sizeof(uint8x16_t)) { - const uint8x16_t bytes = vld1q_u8(search->cstr); - const uint8x16_t match1 = vceqq_u8(bytes, single_quote); - const uint8x16_t match2 = vceqq_u8(bytes, double_quote); - const uint8x16_t match3 = vceqq_u8(bytes, ampersand); - const uint8x16_t match4 = vceqq_u8(bytes, lt); - const uint8x16_t match5 = vceqq_u8(bytes, gt); + const uint8x16_t m0 = neon_escape_matches(bytes0); + const uint8x16_t m1 = neon_escape_matches(bytes1); + const uint8x16_t m2 = neon_escape_matches(bytes2); + const uint8x16_t m3 = neon_escape_matches(bytes3); - const uint8x16_t mask1 = vorrq_u8(match1, match2); - const uint8x16_t mask2 = vorrq_u8(match3, match4); - const uint8x16_t mask3 = vorrq_u8(mask1, match5); - const uint8x16_t matches = vorrq_u8(mask2, mask3); + const uint64_t bitmap = neon_matches_to_bitmap64(m0, m1, m2, m3); - const uint8x8_t res = vshrn_n_u16(vreinterpretq_u16_u8(matches), 4); - const uint64_t bitmap = vget_lane_u64(vreinterpret_u64_u8(res), 0); + if (bitmap) { + search->matches_bitmap = bitmap; + return find_next_match_neon(search); + } + + search->cstr += 64; + } + + while ((size_t)(search->end - search->cstr) >= sizeof(uint8x16_t)) { + const uint8x16_t bytes = vld1q_u8(search->cstr); + const uint8x16_t matches = neon_escape_matches(bytes); + const uint64_t bitmap = neon_matches_to_bitmap16(matches); if (bitmap) { - search->matches_bitmap = bitmap & 0x8888888888888888ull; + search->matches_bitmap = bitmap; return find_next_match_neon(search); } search->cstr += sizeof(uint8x16_t); diff --git a/ext/erb/escape/extconf.rb b/ext/erb/escape/extconf.rb index 8e1d482d52b127..63a534bed51a43 100644 --- a/ext/erb/escape/extconf.rb +++ b/ext/erb/escape/extconf.rb @@ -8,7 +8,7 @@ unless ENV["ERB_DISABLE_SIMD"] case RbConfig::CONFIG['host_cpu'] - when /^(arm|aarch64)/ + when /^(arm64|aarch64)/ # Try to compile a small program using NEON instructions header, type, init, extra = 'arm_neon.h', 'uint8x16_t', 'vdupq_n_u8(32)', nil when /^(x86_64|x64)/ diff --git a/marshal.rb b/marshal.rb index 9ff74dd8b248cc..0b8a9305a1a8c9 100644 --- a/marshal.rb +++ b/marshal.rb @@ -31,6 +31,7 @@ module Marshal # # => [1039360, 1039380, 1039360, 1039380] -- only 2 different objects, object_ids repeating # def self.load(source, proc = nil, freeze: false) + Primitive.attr! :caller_user_box Primitive.marshal_load(source, proc, freeze) end diff --git a/ractor_sync.c b/ractor_sync.c index 45c319d9c1c1c4..3eb83a6550b30e 100644 --- a/ractor_sync.c +++ b/ractor_sync.c @@ -15,7 +15,6 @@ static VALUE rb_cRactorPort; static VALUE ractor_receive(rb_execution_context_t *ec, const struct ractor_port *rp, const rb_hrtime_t *end); static VALUE ractor_send(rb_execution_context_t *ec, const struct ractor_port *rp, VALUE obj, VALUE move); -static struct ractor_basket *ractor_basket_new_ref(VALUE shareable); static struct ractor_basket *ractor_basket_new_exit(VALUE sender, VALUE token); static void ractor_send_basket(rb_execution_context_t *ec, const struct ractor_port *rp, struct ractor_basket *b, bool raise_on_error); static void ractor_add_port(rb_ractor_t *r, st_data_t id); @@ -1670,28 +1669,10 @@ ractor_send_basket(rb_execution_context_t *ec, const struct ractor_port *rp, str } } -/* A shareable payload needs no preparation, so this skips the tag ractor_basket_new - * pushes. The exit tokens travel this way: they are sent from a thread whose EC has - * already lost its VM stack, and EC_PUSH_TAG reads ec->cfp under ZJIT. */ -static struct ractor_basket * -ractor_basket_new_ref(VALUE shareable) -{ - struct ractor_basket *b = ractor_basket_alloc(); - - b->type = basket_type_ref; - b->sender = Qnil; - b->p.v = shareable; - b->p.exception = false; - b->p.marshaled = false; - b->p.courier = NULL; - b->p.mbuf = NULL; - b->p.mlen = 0; - - return b; -} - /* sender is the ractor the token is about; both it and the token are shareable, - * so nothing is copied until the receiver builds the pair. */ + * so nothing is copied until the receiver builds the pair. It also skips the tag + * ractor_basket_new pushes: the tokens go out from a thread whose EC has already + * lost its VM stack, and EC_PUSH_TAG reads ec->cfp under ZJIT. */ static struct ractor_basket * ractor_basket_new_exit(VALUE sender, VALUE token) { diff --git a/spec/ruby/core/thread/element_set_spec.rb b/spec/ruby/core/thread/element_set_spec.rb index 97d6c23980411f..1c80a536ad033b 100644 --- a/spec/ruby/core/thread/element_set_spec.rb +++ b/spec/ruby/core/thread/element_set_spec.rb @@ -28,6 +28,15 @@ t2[:value].should == 2 end + it "removes a key if the value is nil" do + thread = Thread.new {}.join + thread[:value] = 1 + thread[:value] = nil + + thread.key?(:value).should == false + thread.keys.should_not.include?(:value) + end + it "converts a key that is neither String nor Symbol with #to_str" do key = mock('value') key.should_receive(:to_str).and_return('value') diff --git a/spec/ruby/core/thread/thread_variable_set_spec.rb b/spec/ruby/core/thread/thread_variable_set_spec.rb index f8d25364ae9156..ceb91a0c31808d 100644 --- a/spec/ruby/core/thread/thread_variable_set_spec.rb +++ b/spec/ruby/core/thread/thread_variable_set_spec.rb @@ -41,8 +41,12 @@ it "removes a key if the value is nil" do @t.thread_variable_set(:a, 52) - @t.thread_variable_set(:a, nil) + @t.thread_variable_set(:a, nil).should == nil @t.thread_variable?(:a).should == false + + ruby_version_is "4.1" do + @t.thread_variables.should_not.include?(:a) + end end it "raises a FrozenError if the thread is frozen" do diff --git a/test/ruby/test_box.rb b/test/ruby/test_box.rb index 5d46906021f55f..ed9896bff1219e 100644 --- a/test/ruby/test_box.rb +++ b/test/ruby/test_box.rb @@ -439,6 +439,45 @@ def test_prepend_to_builtin_module_in_box assert_equal 42, 42.itself end; end + + def test_marshal_round_trip_in_main_box + assert_separately([ENV_ENABLE_BOX], __FILE__, __LINE__, "#{<<~"begin;"}\n#{<<~'end;'}", ignore_stderr: true) + begin; + class BoxMarshalFoo + attr_reader :value + def initialize(value) + @value = value + end + end + obj = Marshal.load(Marshal.dump(BoxMarshalFoo.new(42))) # [Bug #22090] + assert_instance_of BoxMarshalFoo, obj + assert_equal 42, obj.value + assert_instance_of BoxMarshalFoo, Marshal.load(Marshal.dump(BoxMarshalFoo.new(1)), freeze: true) + end; + end + + def test_marshal_resolves_classes_in_the_caller_user_box + setup_box + + obj = @box.eval("class BoxMarshalBar; end; Marshal.load(Marshal.dump(BoxMarshalBar.new))") + assert_equal "BoxMarshalBar", obj.class.name + + # a class defined only in the box is invisible from the main box + dump = @box.eval("Marshal.dump(BoxMarshalBar.new)") + assert_raise_with_message(ArgumentError, /undefined class\/module BoxMarshalBar/) do + Marshal.load(dump) + end + end + + def test_marshal_skips_root_box_frames_in_the_caller_stack + assert_separately([ENV_ENABLE_BOX], __FILE__, __LINE__, "#{<<~"begin;"}\n#{<<~'end;'}", ignore_stderr: true) + begin; + class BoxMarshalBaz; end + # the proc runs in the root box, where BoxMarshalBaz is invisible + loader = Ruby::Box.root.eval("->(dump) { Marshal.load(dump) }") + assert_instance_of BoxMarshalBaz, loader.call(Marshal.dump(BoxMarshalBaz.new)) + end; + end end class TestBoxDescendantsMain diff --git a/thread.c b/thread.c index 4b64e8ed92d49c..2a64366ec79e0d 100644 --- a/thread.c +++ b/thread.c @@ -4230,6 +4230,7 @@ rb_thread_local_aset(VALUE thread, ID id, VALUE val) * * Attribute Assignment---Sets or creates the value of a fiber-local variable, * using either a symbol or a string. + * Assigning +nil+ deletes the variable. * * See also Thread#[]. * @@ -4291,19 +4292,33 @@ rb_thread_variable_get(VALUE thread, VALUE key) * Sets a thread local with +key+ to +value+. Note that these are local to * threads, and not to fibers. Please see Thread#thread_variable_get and * Thread#[] for more information. + * Assigning +nil+ deletes the variable. */ static VALUE rb_thread_variable_set(VALUE thread, VALUE key, VALUE val) { VALUE locals; + VALUE symbol; if (OBJ_FROZEN(thread)) { rb_frozen_error_raise(thread, "can't modify frozen thread locals"); } + symbol = rb_to_symbol(key); + + if (NIL_P(val)) { + if (LIKELY(!THREAD_LOCAL_STORAGE_INITIALISED_P(thread))) { + return Qnil; + } + + locals = rb_thread_local_storage(thread); + rb_hash_delete(locals, symbol); + return Qnil; + } + locals = rb_thread_local_storage(thread); - return rb_hash_aset(locals, rb_to_symbol(key), val); + return rb_hash_aset(locals, symbol, val); } /* diff --git a/thread_pthread.c b/thread_pthread.c index a10570317919c0..8f1564753ceed1 100644 --- a/thread_pthread.c +++ b/thread_pthread.c @@ -67,10 +67,6 @@ static const void *const condattr_monotonic = NULL; // otherwise the caller has to restate the deadline in the condvar's clock. #define RB_NATIVE_COND_HRTIME_DEADLINE_P() (condattr_monotonic != NULL) -/* A retiring shared native thread frees its own context while the threads it - * parked are still suspended with that context as their target. */ -#define COROUTINE_TARGET_MAY_BE_FREED 1 - #include COROUTINE_H #ifndef HAVE_SYS_EVENT_H diff --git a/tool/mk_builtin_loader.rb b/tool/mk_builtin_loader.rb index 7c93cccd6a10df..87335015dd942e 100644 --- a/tool/mk_builtin_loader.rb +++ b/tool/mk_builtin_loader.rb @@ -7,7 +7,7 @@ SUBLIBS = {} REQUIRED = {} -BUILTIN_ATTRS = %w[leaf inline_block use_block c_trace without_interrupts] +BUILTIN_ATTRS = %w[leaf inline_block use_block c_trace without_interrupts caller_user_box] module CompileWarning @@warnings = 0 diff --git a/vm.c b/vm.c index f9d9d8ef0272c8..426ffcd8a3c875 100644 --- a/vm.c +++ b/vm.c @@ -3317,6 +3317,31 @@ rb_vm_frame_flag_set_box_require(const rb_execution_context_t *ec) VM_ENV_FLAGS_SET(ec->cfp->ep, VM_FRAME_FLAG_BOX_REQUIRE); } +static const rb_box_t *current_box_on_cfp(const rb_execution_context_t *ec, const rb_control_frame_t *cfp); + +/** + * Returns the nearest user box in the caller frames, or NULL if there is none. + * + * Builtin methods written in Ruby are defined in the master box, so their own + * frame tells nothing about the caller. Those marked with + * `Primitive.attr! :caller_user_box` need the box owning the caller code, and the + * frames in between may belong to the master or the root box, e.g. when another + * builtin method or a proc made in the root box calls them. + */ +static const rb_box_t * +caller_user_box_on_cfp(const rb_execution_context_t *ec, const rb_control_frame_t *cfp) +{ + const rb_control_frame_t * const eocfp = RUBY_VM_END_CONTROL_FRAME(ec); + + while (RUBY_VM_VALID_CONTROL_FRAME_P(cfp, eocfp)) { + const rb_box_t *box = current_box_on_cfp(ec, cfp); + if (BOX_USER_P(box)) + return box; + cfp = RUBY_VM_PREVIOUS_CONTROL_FRAME(cfp); + } + return NULL; +} + static const rb_box_t * current_box_on_cfp(const rb_execution_context_t *ec, const rb_control_frame_t *cfp) { @@ -3330,6 +3355,15 @@ current_box_on_cfp(const rb_execution_context_t *ec, const rb_control_frame_t *c cme = check_method_entry(lep[VM_ENV_DATA_INDEX_ME_CREF], TRUE); VM_BOX_ASSERT(cme, "cme should be valid"); VM_BOX_ASSERT(cme->def, "cme->def shold be valid"); + if (cme->def->type == VM_METHOD_TYPE_ISEQ && + (ISEQ_BODY(cme->def->body.iseq.iseqptr)->builtin_attrs & BUILTIN_ATTR_CALLER_USER_BOX)) { + const rb_control_frame_t *owner_cfp = rb_vm_search_cf_from_ep(ec, cfp, lep); + if (owner_cfp) { + box = caller_user_box_on_cfp(ec, RUBY_VM_PREVIOUS_CONTROL_FRAME(owner_cfp)); + if (box) + return box; + } + } return cme->def->box; } else if (VM_ENV_FRAME_TYPE_P(lep, VM_FRAME_MAGIC_TOP) || VM_ENV_FRAME_TYPE_P(lep, VM_FRAME_MAGIC_CLASS)) { diff --git a/vm_core.h b/vm_core.h index 8372f7ca4aa799..4754feae6f6c17 100644 --- a/vm_core.h +++ b/vm_core.h @@ -407,6 +407,8 @@ enum rb_builtin_attr { BUILTIN_ATTR_C_TRACE = 0x08, // The iseq uses noint branch/jump opcodes that skip interrupt checking. BUILTIN_ATTR_WITHOUT_INTERRUPTS = 0x10, + // The iseq operates on the nearest user box in its caller frames. + BUILTIN_ATTR_CALLER_USER_BOX = 0x20, }; typedef VALUE (*rb_jit_func_t)(struct rb_execution_context_struct *, struct rb_control_frame_struct *); diff --git a/yjit/src/cruby_bindings.inc.rs b/yjit/src/cruby_bindings.inc.rs index c4f7d85ed283e3..be4653d19662ba 100644 --- a/yjit/src/cruby_bindings.inc.rs +++ b/yjit/src/cruby_bindings.inc.rs @@ -487,6 +487,7 @@ pub const BUILTIN_ATTR_SINGLE_NOARG_LEAF: rb_builtin_attr = 2; pub const BUILTIN_ATTR_INLINE_BLOCK: rb_builtin_attr = 4; pub const BUILTIN_ATTR_C_TRACE: rb_builtin_attr = 8; pub const BUILTIN_ATTR_WITHOUT_INTERRUPTS: rb_builtin_attr = 16; +pub const BUILTIN_ATTR_CALLER_USER_BOX: rb_builtin_attr = 32; pub type rb_builtin_attr = u32; #[repr(C)] #[derive(Debug, Copy, Clone)] diff --git a/zjit/src/cruby_bindings.inc.rs b/zjit/src/cruby_bindings.inc.rs index 023353ba18695a..6343cf69e10755 100644 --- a/zjit/src/cruby_bindings.inc.rs +++ b/zjit/src/cruby_bindings.inc.rs @@ -642,6 +642,7 @@ pub const BUILTIN_ATTR_SINGLE_NOARG_LEAF: rb_builtin_attr = 2; pub const BUILTIN_ATTR_INLINE_BLOCK: rb_builtin_attr = 4; pub const BUILTIN_ATTR_C_TRACE: rb_builtin_attr = 8; pub const BUILTIN_ATTR_WITHOUT_INTERRUPTS: rb_builtin_attr = 16; +pub const BUILTIN_ATTR_CALLER_USER_BOX: rb_builtin_attr = 32; pub type rb_builtin_attr = u32; pub type rb_jit_func_t = ::std::option::Option< unsafe extern "C" fn(