Skip to content

Fix linker failure when building opcache statically #18939

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

arnaud-lb
Copy link
Member

@arnaud-lb arnaud-lb commented Jun 25, 2025

Spun of #18660

RFC: https://wiki.php.net/rfc/make_opcache_required

We use linker relocations to fetch the TLS index and offset of _tsrm_ls_cache. When building Opcache statically, linkers may attempt to optimize that into a more efficient code sequence (relaxing from "General Dynamic" to "Local Exec" model [1]). Unfortunately, linkers will fail, rather than ignore our relocations, when they don't recognize the exact code sequence they are expecting.

This results in errors as reported by #15074:

TLS transition from R_X86_64_TLSGD to R_X86_64_GOTTPOFF against
`_tsrm_ls_cache' at 0x12fc3 in section `.text' failed"

Here I take a different approach:

  • Emit the exact full code sequence expected by linkers
  • Extract the TLS index/offset by inspecting the linked ASM code, rather than executing it (execution would give us the thread-local address).
  • We detect when the code was relaxed, in which case we can extract the TCB offset instead.
  • This is done in a conservative way so that if the linker did something we didn't expect, we fallback to a safer (but slower) mechanism.

One benefit of that is we are now able to use the Local Exec model in more cases, in JIT'ed code. This makes non-glibc builds faster in these cases.

This is tested on Linux (glibc, musl), FreeBSD, MacOS, Windows; lld, gold, bdf; clang; gcc; VS; x86, x86_64, aarch64 (not MacOS/Apple Silicon, as JIT+ZTS is not supported on this combo yet, and not Windows/aarch64 for the same reason), with various combinations of static/shared/dl(). The PR includes these tests. Other OSes fallback to the slower mechanism.

[1] https://www.akkadia.org/drepper/tls.pdf

This fixes the following linker error:

    TLS transition from R_X86_64_TLSGD to R_X86_64_GOTTPOFF against
    `_tsrm_ls_cache' at 0x12fc3 in section `.text' failed"

The error arises from how we obtain information about the _tsrm_ls_cache TLS
variable for use in JIT'ed code:

Normally, TLS variables are resolved via linker relocations [1], which of course
can not be used in JIT'ed code. Therefore we emit the relocation in AOT code and
use the result in JIT.

Specifically we use a fragment of the "General Dynamic" code sequence described
in [1]. Using the full code sequence would give us the address of the variable
in the current thread. Therefore we only use a fragment that gives us the
variable's TLS index and offset.

When Opcache is statically linked into the binary, linkers attempt to relax
(rewrite) this code sequence into a more efficient one. However, this fails
because they will not recognize the code sequence.

We now take a different approach:

 * Emit the exact full code sequence expected by linkers
 * Extract the TLS index/offset or TCB offset by inspecting the ASM code, rather
   than executing it (execution would give us the thread-local address).
 * This is done in a conservative way so that if the linker did
   something we didn't expect, we fallback to a safer (but slower) mechanism.

[1] https://www.akkadia.org/drepper/tls.pdf
Copy link
Member

@TimWolla TimWolla left a comment

Choose a reason for hiding this comment

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

I cannot comment meaningfully on the code itself, but it certainly looks more maintainable now and I like that it's independently tested in CI.

Copy link
Member

@TimWolla TimWolla left a comment

Choose a reason for hiding this comment

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

Apart from the small typo I have no further remarks. I leave the actual review to someone who knows how this is supposed to work.

@arnaud-lb arnaud-lb requested a review from nielsdos June 25, 2025 11:44
@arnaud-lb arnaud-lb marked this pull request as ready for review June 25, 2025 11:45
@arnaud-lb arnaud-lb requested a review from dstogov as a code owner June 25, 2025 11:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants