Skip to content

fix: enable linux shared-library-safe v8 tls mode by default#1911

Open
mplemay wants to merge 4 commits intodenoland:mainfrom
mplemay:fix-linker-error
Open

fix: enable linux shared-library-safe v8 tls mode by default#1911
mplemay wants to merge 4 commits intodenoland:mainfrom
mplemay:fix-linker-error

Conversation

@mplemay
Copy link

@mplemay mplemay commented Feb 21, 2026

Summary

Fixes Linux shared-library (cdylib) linking failures caused by V8 TLS relocations when rusty_v8 static archives are linked into downstream shared objects.

Closes #1706.

Root cause

On Linux, V8 uses fast TLS access unless V8_TLS_USED_IN_LIBRARY is defined.
Without that define, V8 code can emit relocations such as R_X86_64_TPOFF32, which are invalid when linking into -shared and fail downstream cdylib builds (for example PyO3 extension modules).

What changed

  • build.rs:
    • Linux builds now inject:
      • extra_cflags=["-DV8_TLS_USED_IN_LIBRARY"]
    • Injection is skipped if user-provided GN_ARGS already contains V8_TLS_USED_IN_LIBRARY.
  • README.md:
    • Documented the Linux default and the exact injected GN arg.
    • Documented that Linux prebuilt release archives are built with shared-library-compatible TLS mode.

Why this approach

This fixes the issue without requiring changes to vendored V8 GN logic and avoids enabling broader build-mode switches (for example v8_monolithic=true) just to get TLS library mode behavior.

Compatibility and risk

  • Linux-only behavior change.
  • No Rust API changes.
  • No C++ binding surface changes.
  • Expected tradeoff: slight TLS access overhead in exchange for reliable downstream shared-library linking.

Override behavior

If needed, users can still fully control TLS-related flags via GN_ARGS; build.rs only injects the define when it is not already present.

@CLAassistant
Copy link

CLAassistant commented Feb 21, 2026

CLA assistant check
All committers have signed the CLA.

@mplemay
Copy link
Author

mplemay commented Mar 12, 2026

@fraidev or @bartlomieju - when you get a chance, could you take a look at this?

Copy link

@kajukitli kajukitli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like the right default to me

linux cdylib consumers hitting TLS relocation failures is a real footgun, and setting V8_TLS_USED_IN_LIBRARY by default for linux source builds is the least ugly fix.

one thing i'd still call out: the GN_ARGS parsing here is pretty flimsy.

for arg in args.split_whitespace() {
  if arg.contains("V8_TLS_USED_IN_LIBRARY") {
    has_tls_library_mode_define = true;
  }
  gn_args.push(arg.to_string());
}

GN_ARGS is not actually whitespace-safe in the general case, so this can misparse quoted args and arrays. rusty_v8 already has this problem elsewhere in build.rs, so it's not a regression unique to this PR, but it's worth keeping in mind.

for this specific change though, i think the behavior is good and the override logic is reasonable.

@bartlomieju
Copy link
Member

I'm gonna defer to @devsnek for final approval

@mplemay
Copy link
Author

mplemay commented Mar 12, 2026

looks like the right default to me

linux cdylib consumers hitting TLS relocation failures is a real footgun, and setting V8_TLS_USED_IN_LIBRARY by default for linux source builds is the least ugly fix.

one thing i'd still call out: the GN_ARGS parsing here is pretty flimsy.

for arg in args.split_whitespace() {
  if arg.contains("V8_TLS_USED_IN_LIBRARY") {
    has_tls_library_mode_define = true;
  }
  gn_args.push(arg.to_string());
}

GN_ARGS is not actually whitespace-safe in the general case, so this can misparse quoted args and arrays. rusty_v8 already has this problem elsewhere in build.rs, so it's not a regression unique to this PR, but it's worth keeping in mind.

for this specific change though, i think the behavior is good and the override logic is reasonable.

Hey @kajukitli - thanks for the quick review. I went ahead and bolstered the logics so it does a better job with the edge cases. Let me know if there is anything else I should do to improve it.

@devsnek - let me know what you think. Happy to make any additional edits to get this shipped.

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.

error: relocation R_X86_64_TPOFF32 against hidden symbol `_ZN2v88internal18g_current_isolate_E' can not be used when making a shared object

4 participants