Skip to content

Add patch: apply V8_TLS_USED_IN_LIBRARY to V8 internal_config#20

Merged
crowlKats merged 4 commits into
autorollfrom
fix-tls-internal-config-patch
May 6, 2026
Merged

Add patch: apply V8_TLS_USED_IN_LIBRARY to V8 internal_config#20
crowlKats merged 4 commits into
autorollfrom
fix-tls-internal-config-patch

Conversation

@crowlKats
Copy link
Copy Markdown
Member

@crowlKats crowlKats commented Apr 23, 2026

Summary

Adds a new floated patch (patches/0003-...) that fixes a V8 build-system bug surfacing when rusty_v8's archive is linked into a downstream cdylib/.so on Linux.

V8 already declares v8_monolithic_for_shared_library as a GN arg, but the only place it adds the V8_TLS_USED_IN_LIBRARY define is the :features config -- which is consumed only by external embedders, not by the internal_config that V8's own .cc files use. So V8 internal sources (where thread_local Isolate* g_current_isolate_ etc. live) stay in local-exec TLS model and emit R_X86_64_TPOFF32 relocations:

rust-lld: error: relocation R_X86_64_TPOFF32 against v8::internal::g_current_isolate_
  cannot be used with -shared

This breaks V8_FROM_SOURCE=1 cargo build for any downstream that links rusty_v8 into a shared library (e.g. Deno's denort_desktop cdylib).

Patch contents

  • Add defines += [ "V8_TLS_USED_IN_LIBRARY" ] to config("internal_config") under if (v8_monolithic_for_shared_library).
  • Drop the redundant v8_monolithic && part of the existing :features condition (the arg's name already implies the scope, and requiring v8_monolithic excludes us since rusty_v8 doesn't build the v8_monolith static lib target).

Companion PR

denoland/rusty_v8#1970 will start passing v8_monolithic_for_shared_library=true once this lands and gets rolled into the next 14.7-lkgr-denoland autoroll.

Test plan

  • Verify autoroll picks up the new patch and applies cleanly to 14.7-lkgr-denoland.
  • After the rusty_v8 companion PR lands, verify V8_FROM_SOURCE=1 cargo build of Deno's denort_desktop cdylib succeeds on Linux.

V8 already declares the `v8_monolithic_for_shared_library` GN arg, but
the only place it adds the `V8_TLS_USED_IN_LIBRARY` define is the
`:features` config -- which is consumed only by external embedders, not
by the `internal_config` that V8's own `.cc` files use. As a result, V8
internal sources (where the `thread_local Isolate*` etc. live) keep
their `local-exec` TLS model and emit `R_X86_64_TPOFF32` relocations
that fail to link into a downstream cdylib/.so on Linux.

Add the same define to `internal_config` under the same arg, and drop
the redundant `v8_monolithic &&` part of the existing `:features`
condition (the arg already implies its scope).
@crowlKats crowlKats merged commit 100e21c into autoroll May 6, 2026
github-actions Bot pushed a commit that referenced this pull request May 21, 2026
Original change's description:
> [maglev] Fix deopt literals IdentityMap race during GC
>
> Maglev used an IdentityMap to store deoptimization literals, assigning
> IDs based on the map's size. During concurrent GC, ThinString
> shortcutting could merge previously distinct string pointers, causing
> the map to rehash and shrink. This led to corrupted IDs and subsequent
> out-of-bounds writes in DeoptimizationLiteralArray.
>
> This CL implements an optimized hybrid approach:
> 1. Pair the IdentityMap cache with ZoneVectors to preserve original
>    insertion order.
> 2. Populate the DeoptimizationLiteralArray from the vectors.
> 3. Safely canonicalize vector handles using Maglev's persistent
>    CanonicalHandlesMap.
> 4. Optimize compiler::Ref overloads to bypass redundant lookups.
>
> We also improve V8's runtime testing infrastructure by fixing
> OptimizeMaglevOnNextCall's registration in runtime.h to support
> variable arguments, enabling native concurrent Maglev testing in JS.
>
> A robust randomized concurrent regression test is added to verify.
>
> TAG=agy
> CONV=8a5b0da0-ce00-46ee-83f5-9a341372b6f7
>
> Fixed: 508811474
> Change-Id: I69bffdbe6736552e273480c8cf23de25af112739
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/7816278
> Commit-Queue: Leszek Swirski <leszeks@chromium.org>
> Reviewed-by: Patrick Thier <pthier@chromium.org>
> Auto-Submit: Leszek Swirski <leszeks@chromium.org>
> Commit-Queue: Patrick Thier <pthier@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#107059}

(cherry picked from commit 00f6ecd8a7cca6911789a11b7a7b01aaf41f925b)

Bug: 514928956,508811474
Change-Id: I69bffdbe6736552e273480c8cf23de25af112739
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/7864503
Auto-Submit: chrome-cherry-picker@chops-service-accounts.iam.gserviceaccount.com <chrome-cherry-picker@chops-service-accounts.iam.gserviceaccount.com>
Reviewed-by: Patrick Thier <pthier@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/branch-heads/14.9@{#20}
Cr-Branched-From: 8f08364-refs/heads/14.9.207@{#1}
Cr-Branched-From: 8de67b1-refs/heads/main@{#106999}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant