Skip to content

fix: do not allocate space for uninitialized tls variables#1788

Merged
davidlattimore merged 2 commits into
wild-linker:mainfrom
ostylk:fix/tls
Jul 15, 2026
Merged

fix: do not allocate space for uninitialized tls variables#1788
davidlattimore merged 2 commits into
wild-linker:mainfrom
ostylk:fix/tls

Conversation

@ostylk

@ostylk ostylk commented Apr 1, 2026

Copy link
Copy Markdown
Contributor

I noticed the tls-common test failed on my machine depending on which environment and versions of compilers I used. It seems like the TLS program header is generated incorrectly for zero initialized tls variables.

E.g. in the tls-common we have a variable tvar that is in .tbss. However, the TLS program header is filesz=4,memsz=4 which doesn't zero-initialize it but loads bytes from the elf binary. It just so happens that all CIs and your local setup contained zeros in the binary at the exact place for this to nevertheless work. The correct TLS program header would be filesz=0,memsz=4.

So this pull request does/will do two things:

  • Implement program header assertions in testing so this bug actually errors out deterministically. Now a test can contain //#ExpectProgramHeader:PT_TLS filesz=0,memsz=4 and it will only accept if a program header with at least these properties exists. Implemented by fix: Don't count .tbss size towards TLS segment file size #2190 .
  • Fix the actual bug.

@davidlattimore

Copy link
Copy Markdown
Member

Hi @ostylk. Do you intend to return to this PR?

@ostylk

ostylk commented May 18, 2026

Copy link
Copy Markdown
Contributor Author

Hi, thank you for the reminder, this PR slipped my mind.
I do intend on returning to it, preferably in this month. Am I impeding anything by working on this slowly?

@davidlattimore

Copy link
Copy Markdown
Member

All good, I was just checking in on PRs that hadn't been touched in a while. I don't think there's any particular hurry on this though.

@ostylk

ostylk commented Jun 26, 2026

Copy link
Copy Markdown
Contributor Author

Ready for review but I have some comments/questions:

  1. The CI failure is weird, it only happens on opensuse (I could reproduce it locally with a container). Any pointers on how to approach that?
  2. There was a comment in output_section_id.rs about how uninitialized TLS data is padded with zeroes in the output file. However, I couldn't find where that happens in the code? Maybe it was forgotten and that caused this bug in the first place?
  3. Funnily enough GNU ld seems to have a bug (to see for yourself: delete the SkipLinker instruction in tls-custom.c test) where uninitialized TLS sections overlap initialized TLS sections. LLD seems to handle the case correctly though.

@ostylk
ostylk marked this pull request as ready for review June 26, 2026 14:27
@davidlattimore

Copy link
Copy Markdown
Member

Welcome back!

Regarding the CI failure - RexMovIndirectToAbsolute vs MovIndirectToLea, that sounds like something that could be caused by different defaults as to whether the binary is position independent or not. So possibly the difference might go away if you override those defaults - e.g. by passing a flag to make both linkers produce a position-independent binary.

The PR in which we started putting zero bytes in the file for .tbss was #489. It unfortunately looks like I didn't add a test for it, so it's not impossible that something around there could have regressed.

@ostylk
ostylk force-pushed the fix/tls branch 2 times, most recently from 4f37e80 to 37b1060 Compare July 11, 2026 00:27
@ostylk

ostylk commented Jul 11, 2026

Copy link
Copy Markdown
Contributor Author

Okay, so the linker failure seems to be due to lld behaving differently to GNU ld on openSUSE. While I am unsure what the exact reason is, the startup files (crtbegin.o crtend.o) on openSUSE cannot be linked with --pie. This leads to GNU ld and lld applying different relaxations. GNU ld and also wild choose to relax like this:

0000000000401020 <_start>:
  <snip>
  401038:       48 c7 c7 16 11 40 00    mov    $0x401116,%rdi
  40103f:       ff 15 93 2f 00 00       call   *0x2f93(%rip)        # 403fd8 <__libc_start_main@GLIBC_2.34>

while lld relaxes to this (effectively ignoring pie/no-pie stuff)

0000000000201570 <_start>:
  <snip>
  201588:       48 8d 3d d7 00 00 00    lea    0xd7(%rip),%rdi        # 201666 <main>
  20158f:       ff 15 03 1b 00 00       call   *0x1b03(%rip)        # 203098 <__libc_start_main@GLIBC_2.34>

So ultimately I think it is best to ignore that difference. GNU ld and wild switch to absolute addresses with no-pie files while lld ignores that fact completely.

@davidlattimore davidlattimore left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for working on this. The change looks good, so I'll merge it. Before I do though, how do you want this to appear in the release notes? Can you update the PR title to read as a line from the release notes, then prefix it with "fix:" or "feat:" depending on whether you want it in the bug fixes section or the features section.

@ostylk ostylk changed the title TLS program header is not correctly generated for zero-initialized tls variables fix: do not allocate space for uninitialized tls variables Jul 14, 2026
@ostylk

ostylk commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

Done and thank you for your patience with this PR.

@davidlattimore
davidlattimore merged commit 1532430 into wild-linker:main Jul 15, 2026
22 checks passed
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.

2 participants