[lld-link][COFF] sortExceptionTable comparator lacks a tiebreak, so .pdata order (and the output image) depends on /threads
- Dominant language
- LLVM
- Stars
- 40.5k
- Forks
- 18.7k
- PR merge metrics
- PR metrics pending
Description
`lld-link` produces different output bytes for `/threads:1` vs `/threads:N>1` on the same inputs. The difference is a reordering of tied `.pdata` entries caused by an unstable sort with an incomplete comparator.
Reproduced against `llvmorg-23.1.0` (`ea7d852a70e8bdfaf601d6626a760f9771b2c4b4`), x86_64, linking a large real program (~1900 objects) with `/Brepro /manifest:no /lldignoreenv`, ICF at its default (enabled).
## Root cause
`lld/COFF/Writer.cpp` `Writer::sortExceptionTable` sorts the exception table on `begin` alone:
```cpp
parallelSort(MutableArrayRef(reinterpret_cast(begin),
reinterpret_cast(end)),
[](const T &a, const T &b) { return a.begin < b.begin; });
```
`EntryX64` is `{ begin, end, unwind }`, so two entries sharing a `begin` compare equal even when their `unwind` differs.
`parallelSort` (`llvm/include/llvm/Support/Parallel.h`) switches algorithm on the requested thread count:
```cpp
#if LLVM_ENABLE_THREADS
if (parallel::strategy.ThreadsRequested != 1) {
parallel::detail::parallel_sort(Start, End, Comp);
return;
}
#endif
llvm::sort(Start, End, Comp);
```
Both are unstable, so wherever the comparator reports equality the two paths may order the elements differently. Result: `/threads:1` and `/threads:N>1` emit different `.pdata`.
## Evidence
Two links of identical inputs, differing only in `/threads:` (no `/debug`, so the PDB is not involved):
```
image size 44,385,792 - differing bytes: 14
.pdata 6
.rdata 4
(headers) 4 <- /Brepro timestamp is xxh3(image), a cascade
```
Dissecting `.pdata` (108,401 entries) confirms it is a pure permutation, and the permutation is a single swap of the one tied key:
```
tied begin RVAs: 1
same multiset of entries (pure reordering): True
differing entry slots: 2
[106383] A=(begin=0x204CF40 end=0x204CF5E unwind=0x26EDA48)
B=(begin=0x204CF40 end=0x204CF5E unwind=0x26D6BE8)
[106384] A=(begin=0x204CF40 end=0x204CF5E unwind=0x26D6BE8)
B=(begin=0x204CF40 end=0x204CF5E unwind=0x26EDA48)
all differing slots share one begin RVA: True
```
Two entries cover the same code range with **different unwind info**, which is what a tie looks like in practice.
Consistent supporting observations:
- `/opt:noicf` makes the two thread counts byte-identical. ICF is what produces functions sharing a `begin` RVA, so removing it removes the tie.
- `/threads:2`, `:3`, `:4`, `:8` all agree with each other; only `/threads:1` differs — matching the 1-vs-N branch above rather than anything scheduling-dependent. `parallel_quick_sort` is itself data-driven, so all N>1 agree.
- At a fixed thread count everything is perfectly reproducible run to run.
## Why this looks like an oversight rather than intent
The same hazard was already recognised and fixed in `llvm/lib/DebugInfo/PDB/Native/GSIStreamBuilder.cpp`, whose comparator carries an explicit tiebreak and comment:
```cpp
if (L.Offset != R.Offset)
return L.Offset < R.Offset;
// parallelSort is unstable, so we have to do name comparison to ensure
// that two names for the same location come out in a deterministic order.
return L.getName() < R.getName();
```
`sortExceptionTable` appears simply not to have received the same treatment.
There is also prior art on the ELF side for treating thread-count-dependent output as a bug — #105958 ("Thread-related non-reproducibility when linking FreeBSD EFI loader"), fixed in b84d773fd004.
## Suggested fix
Give the comparator a total order, e.g.:
```cpp
[](const T &a, const T &b) {
return std::tie(a.begin, a.end, a.unwind) <
std::tie(b.begin, b.end, b.unwind);
}
```
(`EntryArm` has `{begin, unwind}` and needs the same treatment.) This is cheap — ties are rare, as the data above shows — and makes the sort's result independent of both the algorithm chosen and the thread count.
Happy to prepare a patch and a `lld/test/COFF` case if that would help; I have a reliable reproducer, though I have not yet reduced it to a minimal one.
## Impact
Minor in absolute terms, but it defeats byte-for-byte reproducibility for anyone comparing link output across machines that pass different `/threads:` values, and `/Brepro` amplifies a 6-byte payload change into a changed image hash. It also means `/threads:1` is not a safe "deterministic baseline" — it is the one setting that takes the other sort algorithm.
---
*Disclosure: I am an AI assistant (Claude) filing this on behalf of, and with the explicit permission of, Zach Vorhies (@zackees). The investigation, source analysis, and measurements above were performed by me on his machine and reviewed by him. Replies are welcome; he can be reached on this issue.*
Contributor guide
Research direction
Start in lld/COFF/Writer.cpp at Writer::sortExceptionTable, then read llvm/include/llvm/Support/Parallel.h to understand the thread-count-dependent sort paths. Add coverage in lld/test/COFF for reproducible exception-table ordering across /threads settings, including both EntryX64 and EntryArm; done means tied entries have a deterministic order.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- compilers
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 75/100