diff --git a/napi/oxlint2/src/lib.rs b/napi/oxlint2/src/lib.rs index 8b5302b680649..b357fa2000b9a 100644 --- a/napi/oxlint2/src/lib.rs +++ b/napi/oxlint2/src/lib.rs @@ -75,70 +75,9 @@ fn wrap_lint_file(cb: JsLintFileCb) -> ExternalLinterLintFileCb { let (tx, rx) = channel(); - // Each buffer is sent over to JS only once. - // JS side stores them in an array, and holds them until process ends. - // A flag in `FixedSizeAllocatorMetadata` records whether the buffer has already been transferred - // to JS or not. If it hasn't, send it. Otherwise, just send the ID of the buffer which is the - // index of that buffer in the array on JS side, and JS side will get the buffer from the array. - // This means there's only ever 1 instance of a buffer on Rust side, and 1 on JS side, - // which makes it simpler to avoid use-after-free or double-free problems. - - // SAFETY: This crate enables the `fixed_size` feature on `oxc_allocator`, so all AST `Allocator`s - // are created via `FixedSizeAllocator`. We only create an immutable ref from this pointer. - let metadata_ptr = unsafe { allocator.fixed_size_metadata_ptr() }; - let (buffer_id, already_sent_to_js) = { - // SAFETY: Fixed-size allocators always have a valid `FixedSizeAllocatorMetadata` - // stored at the pointer returned by `Allocator::fixed_size_metadata_ptr`. - let metadata = unsafe { metadata_ptr.as_ref() }; - // TODO: Is `Ordering::SeqCst` excessive here? - let already_sent_to_js = metadata.is_double_owned.swap(true, Ordering::SeqCst); - - (metadata.id, already_sent_to_js) - }; - - let buffer = if already_sent_to_js { - // Buffer has already been sent to JS. Don't send it again. - None - } else { - // Buffer has not already been sent to JS. Send it. - - // Get pointer to start of allocator chunk. - // Note: `Allocator::data_ptr` would not provide the right pointer, because source text - // gets written to start of the allocator chunk, and `data_ptr` gets moved to after it. - // SAFETY: Fixed-size allocators have their chunk aligned on `BLOCK_ALIGN`, - // and size less than `BLOCK_ALIGN`. So we can get pointer to start of `Allocator` chunk - // by rounding down to next multiple of `BLOCK_ALIGN`. That can't go out of bounds of - // the backing allocation. - let chunk_ptr = unsafe { - let ptr = metadata_ptr.cast::(); - let offset = ptr.as_ptr() as usize % BLOCK_ALIGN; - ptr.sub(offset) - }; - - // SAFETY: - // Range of memory starting at `chunk_ptr` and encompassing `BUFFER_SIZE` is all within - // the allocation backing the `Allocator`. - // - // We can't prove that no mutable references to data in the buffer exist, - // but there shouldn't be any, because linter doesn't mutate the AST. - // Anyway, I (@overlookmotel) am not sure if the aliasing rules apply to code in another - // language. Probably not, as JS code is outside the domain of the "Rust abstract machine". - // As long as we don't mutate data in the buffer on JS side, it should be fine. - // - // On the other side, while many immutable references to data in the buffer exist - // (`AstKind`s for every AST node), JS side does not mutate the data in the buffer, - // so that shouldn't break the guarantees of `&` references. - // - // This is all a bit wavy, but such is the way with sharing memory outside of Rust. - let buffer = unsafe { - Uint8Array::with_external_data( - chunk_ptr.as_ptr(), - BUFFER_SIZE, - move |_ptr, _len| free_fixed_size_allocator(metadata_ptr), - ) - }; - Some(buffer) - }; + // SAFETY: This crate enables the `fixed_size` feature on `oxc_allocator`, + // so all AST `Allocator`s are created via `FixedSizeAllocator` + let (buffer_id, buffer) = unsafe { get_buffer(allocator) }; // Send data to JS let status = cb.call_with_return_value( @@ -169,6 +108,84 @@ fn wrap_lint_file(cb: JsLintFileCb) -> ExternalLinterLintFileCb { }) } +/// Get buffer ID of the `Allocator` and, if it hasn't already been sent to JS, +/// create a `Uint8Array` referencing the `Allocator`'s memory. +/// +/// Each buffer is sent over to JS only once. +/// JS side stores them in an array (indexed by buffer ID), and holds them until process ends. +/// This means there's only ever 1 instance of a buffer on Rust side, and 1 on JS side, +/// which makes it simpler to avoid use-after-free or double-free problems. +/// +/// So only create a `Uint8Array` if it's not already sent to JS. +/// +/// Whether the buffer has already been sent to JS is tracked by a flag in `FixedSizeAllocatorMetadata`, +/// which is stored in memory backing the `Allocator`. +/// +/// # SAFETY +/// `allocator` must have been created via `FixedSizeAllocator` +unsafe fn get_buffer( + allocator: &Allocator, +) -> ( + u32, // Buffer ID + Option, // Buffer, if not already sent to JS +) { + // SAFETY: Caller guarantees `Allocator` was created by a `FixedSizeAllocator`. + // We only create an immutable ref from this pointer. + let metadata_ptr = unsafe { allocator.fixed_size_metadata_ptr() }; + // SAFETY: Fixed-size allocators always have a valid `FixedSizeAllocatorMetadata` + // stored at the pointer returned by `Allocator::fixed_size_metadata_ptr`. + let metadata = unsafe { metadata_ptr.as_ref() }; + + let buffer_id = metadata.id; + + // Get whether this buffer has already been sent to JS + // TODO: Is `SeqCst` excessive here? + let already_sent_to_js = metadata.is_double_owned.swap(true, Ordering::SeqCst); + + // If buffer has already been sent to JS, don't send it again + if already_sent_to_js { + return (buffer_id, None); + } + + // Buffer has not already been sent to JS. Send it. + + // Get pointer to start of allocator chunk. + // Note: `Allocator::data_ptr` would not provide the right pointer, because source text + // gets written to start of the allocator chunk, and `data_ptr` gets moved to after it. + // SAFETY: Fixed-size allocators have their chunk aligned on `BLOCK_ALIGN`, + // and size less than `BLOCK_ALIGN`. So we can get pointer to start of `Allocator` chunk + // by rounding down to next multiple of `BLOCK_ALIGN`. That can't go out of bounds of + // the backing allocation. + let chunk_ptr = unsafe { + let ptr = metadata_ptr.cast::(); + let offset = ptr.as_ptr() as usize % BLOCK_ALIGN; + ptr.sub(offset) + }; + + // SAFETY: + // Range of memory starting at `chunk_ptr` and encompassing `BUFFER_SIZE` is all within + // the allocation backing the `Allocator`. + // + // We can't prove that no mutable references to data in the buffer exist, + // but there shouldn't be any, because linter doesn't mutate the AST. + // Anyway, I (@overlookmotel) am not sure if the aliasing rules apply to code in another + // language. Probably not, as JS code is outside the domain of the "Rust abstract machine". + // As long as we don't mutate data in the buffer on JS side, it should be fine. + // + // On the other side, while many immutable references to data in the buffer exist + // (`AstKind`s for every AST node), JS side does not mutate the data in the buffer, + // so that shouldn't break the guarantees of `&` references. + // + // This is all a bit wavy, but such is the way with sharing memory outside of Rust. + let buffer = unsafe { + Uint8Array::with_external_data(chunk_ptr.as_ptr(), BUFFER_SIZE, move |_ptr, _len| { + free_fixed_size_allocator(metadata_ptr); + }) + }; + + (buffer_id, Some(buffer)) +} + #[expect(clippy::allow_attributes)] #[allow(clippy::trailing_empty_array, clippy::unused_async)] // https://github.com/napi-rs/napi-rs/issues/2758 #[napi]