[llvm] [DWARFLinker] Fix placement lost-update crash in parallel marking (PR #211009)
Jonas Devlieghere via llvm-commits
llvm-commits at lists.llvm.org
Tue Jul 21 07:24:43 PDT 2026
https://github.com/JDevlieghere created https://github.com/llvm/llvm-project/pull/211009
The parallel DWARF linker computes each DIE's output placement (type table vs plain DWARF) concurrently across compile units.
A DIE's placement is the join of every mark that reaches it, over the lattice `NotSet` < {`TypeTable`, `PlainDwarf`} < `Both`, so a mark must only raise it, never lower it.
`markDIEEntryAsKeptRec` instead combined the requested placement with the current one and wrote the result back with `setPlacement`, a non-atomic read-combine-write. Under the cross-CU marking race a mark computed from a stale read could demote a DIE: a formal parameter already promoted to Both was lowered back to `TypeTable` after `markParentsAsKeepingChildren` had kept the enclosing subprogram in plain DWARF. The subprogram was then cloned into plain DWARF with no children, triggering the `cloneDIE` assertion:
```
Assertion failed: (ClonedDIE.first == nullptr ||
HasPlainChildrenToClone == ClonedDIE.first->hasChildren()),
function cloneDIE, DWARFLinkerCompileUnit.cpp
```
This PR makes placement promotion an atomic monotone join. Because the values are bit flags (`Both == TypeTable | PlainDwarf`), the least-upper-bound is a plain OR, applied by `DIEInfo::joinPlacement` without clearing a concurrently-set bit.
`getFinalPlacementForEntry` now reports whether its result is a forced placement (the ODR-unavailable and DW_TAG_variable cases, which must overwrite) or the general live/type combine (which joins). The serial path already computed this same join, so the parallel result now matches the single-threaded output on every run.
rdar://180584698
Assisted-by: Claude
>From 7496d3c38286b44d0b0056cb31a5bd7788c7183f Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jonas at devlieghere.com>
Date: Tue, 21 Jul 2026 07:09:05 -0700
Subject: [PATCH] [DWARFLinker] Fix placement lost-update crash in parallel
marking
The parallel DWARF linker computes each DIE's output placement (type
table vs plain DWARF) concurrently across compile units.
A DIE's placement is the join of every mark that reaches it, over the
lattice `NotSet` < {`TypeTable`, `PlainDwarf`} < `Both`, so a mark must
only raise it, never lower it.
markDIEEntryAsKeptRec instead combined the requested placement with the
current one and wrote the result back with setPlacement, a non-atomic
read-combine-write. Under the cross-CU marking race a mark computed from
a stale read could demote a DIE: a formal parameter already promoted to
Both was lowered back to TypeTable after markParentsAsKeepingChildren
had kept the enclosing subprogram in plain DWARF. The subprogram was
then cloned into plain DWARF with no children, triggering the cloneDIE
assertion:
```
Assertion failed: (ClonedDIE.first == nullptr ||
HasPlainChildrenToClone == ClonedDIE.first->hasChildren()),
function cloneDIE, DWARFLinkerCompileUnit.cpp
```
This PR makes placement promotion an atomic monotone join. Because the
values are bit flags (Both == TypeTable | PlainDwarf), the
least-upper-bound is a plain OR, applied by DIEInfo::joinPlacement
without clearing a concurrently-set bit.
getFinalPlacementForEntry now reports whether its result is a forced
placement (the ODR-unavailable and DW_TAG_variable cases, which must
overwrite) or the general live/type combine (which joins). The serial
path already computed this same join, so the parallel result now matches
the single-threaded output on every run.
rdar://180584698
Assisted-by: Claude
---
.../Parallel/DWARFLinkerCompileUnit.h | 11 +
.../Parallel/DependencyTracker.cpp | 62 +--
...odr-deterministic-types-in-subprogram.test | 353 ++++++++++++++++++
3 files changed, 397 insertions(+), 29 deletions(-)
create mode 100644 llvm/test/tools/dsymutil/X86/DWARFLinkerParallel/odr-deterministic-types-in-subprogram.test
diff --git a/llvm/lib/DWARFLinker/Parallel/DWARFLinkerCompileUnit.h b/llvm/lib/DWARFLinker/Parallel/DWARFLinkerCompileUnit.h
index 03e89b603eace..256fb5d0d2b0d 100644
--- a/llvm/lib/DWARFLinker/Parallel/DWARFLinkerCompileUnit.h
+++ b/llvm/lib/DWARFLinker/Parallel/DWARFLinkerCompileUnit.h
@@ -220,6 +220,17 @@ class alignas(8) CompileUnit : public DwarfUnit {
return false;
}
+ /// Atomically joins \p Placement into the current placement: the
+ /// least-upper-bound of the lattice NotSet < {TypeTable, PlainDwarf} <
+ /// Both, which is a plain OR because the values are bit flags. The join is
+ /// monotone and never clears a bit, so unlike setPlacement it composes
+ /// correctly when applied concurrently from several marks.
+ void joinPlacement(DieOutputPlacement Placement) {
+ auto InputData = Flags.load();
+ while (!Flags.compare_exchange_weak(InputData, (InputData | Placement))) {
+ }
+ }
+
#define SINGLE_FLAG_METHODS_SET(Name, Value) \
bool get##Name() const { return Flags & Value; } \
void set##Name() { \
diff --git a/llvm/lib/DWARFLinker/Parallel/DependencyTracker.cpp b/llvm/lib/DWARFLinker/Parallel/DependencyTracker.cpp
index d80ab2dbacbb2..5d6344980d59f 100644
--- a/llvm/lib/DWARFLinker/Parallel/DependencyTracker.cpp
+++ b/llvm/lib/DWARFLinker/Parallel/DependencyTracker.cpp
@@ -409,19 +409,31 @@ void DependencyTracker::markParentsAsKeepingChildren(
}
}
-// This function tries to set specified \p Placement for the \p Entry.
-// Depending on the concrete entry, the placement could be:
-// a) changed to another.
-// b) joined with current entry placement.
-// c) set as requested.
-static CompileUnit::DieOutputPlacement
+namespace {
+struct FinalPlacement {
+ CompileUnit::DieOutputPlacement Placement;
+
+ /// When true, Placement overwrites the DIE's current placement. When false it
+ /// is joined (monotone OR) with it. Only entries whose placement is fully
+ /// determined regardless of how they were reached are forced.
+ bool Forced;
+};
+} // namespace
+
+// Computes the placement to apply to \p Entry for a mark requesting \p
+// Placement (PlainDwarf for a live action, TypeTable for a type action). Most
+// entries join, so a DIE reached by both actions ends up in Both. Forced
+// entries instead pin an exact placement regardless of how they were reached:
+// ODR-unavailable entries cannot be deduplicated into the type table, and a
+// DW_TAG_variable cannot occupy the type table and plain DWARF at once.
+static FinalPlacement
getFinalPlacementForEntry(const UnitEntryPairTy &Entry,
CompileUnit::DieOutputPlacement Placement) {
assert((Placement != CompileUnit::NotSet) && "Placement is not set");
CompileUnit::DIEInfo &EntryInfo = Entry.CU->getDIEInfo(Entry.DieEntry);
if (!EntryInfo.getODRAvailable())
- return CompileUnit::PlainDwarf;
+ return {CompileUnit::PlainDwarf, /*Forced=*/true};
if (Entry.DieEntry->getTag() == dwarf::DW_TAG_variable) {
// In-class static member declarations (e.g. "static constexpr int x = 1;")
@@ -447,35 +459,20 @@ getFinalPlacementForEntry(const UnitEntryPairTy &Entry,
if (IsDeclaration && ParentIsType) {
// Pure declarations have no runtime address; they belong with the class
// type. Always place in TypeTable regardless of how they were reached.
- return CompileUnit::TypeTable;
+ return {CompileUnit::TypeTable, /*Forced=*/true};
}
// Do not put variable into the "TypeTable" and "PlainDwarf" at the same
// time.
if (EntryInfo.getPlacement() == CompileUnit::PlainDwarf ||
EntryInfo.getPlacement() == CompileUnit::Both)
- return CompileUnit::PlainDwarf;
+ return {CompileUnit::PlainDwarf, /*Forced=*/true};
if (Placement == CompileUnit::PlainDwarf || Placement == CompileUnit::Both)
- return CompileUnit::PlainDwarf;
+ return {CompileUnit::PlainDwarf, /*Forced=*/true};
}
- switch (EntryInfo.getPlacement()) {
- case CompileUnit::NotSet:
- return Placement;
-
- case CompileUnit::TypeTable:
- return Placement == CompileUnit::PlainDwarf ? CompileUnit::Both : Placement;
-
- case CompileUnit::PlainDwarf:
- return Placement == CompileUnit::TypeTable ? CompileUnit::Both : Placement;
-
- case CompileUnit::Both:
- return CompileUnit::Both;
- };
-
- llvm_unreachable("Unknown placement type.");
- return Placement;
+ return {Placement, /*Forced=*/false};
}
bool DependencyTracker::markDIEEntryAsKeptRec(
@@ -487,10 +484,11 @@ bool DependencyTracker::markDIEEntryAsKeptRec(
CompileUnit::DIEInfo &Info = Entry.CU->getDIEInfo(Entry.DieEntry);
- // Calculate final placement placement.
- CompileUnit::DieOutputPlacement Placement = getFinalPlacementForEntry(
+ // Calculate final placement.
+ FinalPlacement Final = getFinalPlacementForEntry(
Entry,
isLiveAction(Action) ? CompileUnit::PlainDwarf : CompileUnit::TypeTable);
+ CompileUnit::DieOutputPlacement Placement = Final.Placement;
assert((Info.getODRAvailable() || isLiveAction(Action) ||
Placement == CompileUnit::PlainDwarf) &&
"Wrong kind of placement for ODR unavailable entry");
@@ -515,7 +513,13 @@ bool DependencyTracker::markDIEEntryAsKeptRec(
if (!RecordDepsOnly) {
// Mark current DIE as kept.
Info.setKeep();
- Info.setPlacement(Placement);
+ // Placement is the join of every mark that reaches the DIE, so a general
+ // mark only raises it in the lattice and never demotes it. Forced
+ // placements overwrite instead.
+ if (Final.Forced)
+ Info.setPlacement(Placement);
+ else
+ Info.joinPlacement(Placement);
// Set keep children property for parents.
markParentsAsKeepingChildren(Entry);
diff --git a/llvm/test/tools/dsymutil/X86/DWARFLinkerParallel/odr-deterministic-types-in-subprogram.test b/llvm/test/tools/dsymutil/X86/DWARFLinkerParallel/odr-deterministic-types-in-subprogram.test
new file mode 100644
index 0000000000000..a992a4c8c5768
--- /dev/null
+++ b/llvm/test/tools/dsymutil/X86/DWARFLinkerParallel/odr-deterministic-types-in-subprogram.test
@@ -0,0 +1,353 @@
+# RUN: rm -rf %t && mkdir -p %t
+# RUN: split-file %s %t
+# RUN: yaml2obj %t/foo.o.yaml -o %t/foo.o
+
+## The parallel linker computes each DIE's output placement (artificial type
+## unit vs plain DWARF) concurrently across compile units. A DIE's placement is
+## the join of every mark that reaches it, so it must not depend on thread
+## interleaving.
+##
+## The input (from odr-types-in-subprogram1) defines class "clas1" inside the
+## live subprogram "foo" and also references it as a template parameter of
+## "Container" in the type table, so the same nested type is reached by both a
+## plain-DWARF (live) mark and a type-table mark while the enclosing subprogram
+## stays in plain DWARF. The debug map links the object eight times so many such
+## compile units are marked concurrently. -oso-prepend-path resolves the
+## relative object filename against the temporary directory.
+##
+## Require the threaded output to match the single-threaded output byte-for-byte
+## on every run, which is the placement invariant the parallel linker must
+## preserve. Structural correctness of this input is covered by
+## odr-types-in-subprogram1.test.
+# RUN: dsymutil --linker=parallel -oso-prepend-path=%t -y %t/link.map -f -o %t/serial.out --num-threads 1
+# RUN: dsymutil --linker=parallel -oso-prepend-path=%t -y %t/link.map -f -o %t/par1.out --num-threads 4
+# RUN: dsymutil --linker=parallel -oso-prepend-path=%t -y %t/link.map -f -o %t/par2.out --num-threads 8
+# RUN: diff %t/serial.out %t/par1.out
+# RUN: diff %t/serial.out %t/par2.out
+
+#--- link.map
+---
+triple: 'x86_64-apple-darwin'
+objects:
+ - filename: 'foo.o'
+ symbols:
+ - { sym: __Z3foov, objAddr: 0x0, binAddr: 0x10100, size: 0x10 }
+ - filename: 'foo.o'
+ symbols:
+ - { sym: __Z3foov, objAddr: 0x0, binAddr: 0x10200, size: 0x10 }
+ - filename: 'foo.o'
+ symbols:
+ - { sym: __Z3foov, objAddr: 0x0, binAddr: 0x10300, size: 0x10 }
+ - filename: 'foo.o'
+ symbols:
+ - { sym: __Z3foov, objAddr: 0x0, binAddr: 0x10400, size: 0x10 }
+ - filename: 'foo.o'
+ symbols:
+ - { sym: __Z3foov, objAddr: 0x0, binAddr: 0x10500, size: 0x10 }
+ - filename: 'foo.o'
+ symbols:
+ - { sym: __Z3foov, objAddr: 0x0, binAddr: 0x10600, size: 0x10 }
+ - filename: 'foo.o'
+ symbols:
+ - { sym: __Z3foov, objAddr: 0x0, binAddr: 0x10700, size: 0x10 }
+ - filename: 'foo.o'
+ symbols:
+ - { sym: __Z3foov, objAddr: 0x0, binAddr: 0x10800, size: 0x10 }
+...
+
+#--- foo.o.yaml
+--- !mach-o
+FileHeader:
+ magic: 0xFEEDFACF
+ cputype: 0x01000007
+ cpusubtype: 0x00000003
+ filetype: 0x00000001
+ ncmds: 2
+ sizeofcmds: 376
+ flags: 0x00002000
+ reserved: 0x00000000
+LoadCommands:
+ - cmd: LC_SEGMENT_64
+ cmdsize: 232
+ segname: ''
+ vmaddr: 0x00
+ vmsize: 0x300
+ fileoff: 0x300
+ filesize: 0x300
+ maxprot: 7
+ initprot: 7
+ nsects: 2
+ flags: 0
+ Sections:
+ - sectname: __debug_abbrev
+ segname: __DWARF
+ addr: 0x000000000000000F
+ size: 0x90
+ offset: 0x00000380
+ align: 0
+ reloff: 0x00000000
+ nreloc: 0
+ flags: 0x02000000
+ reserved1: 0x00000000
+ reserved2: 0x00000000
+ reserved3: 0x00000000
+ - sectname: __debug_info
+ segname: __DWARF
+ addr: 0x000000000000100
+ size: 0x124
+ offset: 0x00000410
+ align: 0
+ reloff: 0x00000600
+ nreloc: 1
+ flags: 0x02000000
+ reserved1: 0x00000000
+ reserved2: 0x00000000
+ reserved3: 0x00000000
+ relocations:
+ - address: 0x2C
+ symbolnum: 1
+ pcrel: true
+ length: 3
+ extern: true
+ type: 0
+ scattered: false
+ value: 0
+ - cmd: LC_SYMTAB
+ cmdsize: 24
+ symoff: 0x700
+ nsyms: 2
+ stroff: 0x720
+ strsize: 10
+LinkEditData:
+ NameList:
+ - n_strx: 1
+ n_type: 0x0F
+ n_sect: 1
+ n_desc: 0
+ n_value: 0
+ - n_strx: 1
+ n_type: 0x0F
+ n_sect: 1
+ n_desc: 0
+ n_value: 0
+ StringTable:
+ - ''
+ - '__Z3foov'
+ - ''
+DWARF:
+ debug_abbrev:
+ - Table:
+ - Tag: DW_TAG_compile_unit
+ Children: DW_CHILDREN_yes
+ Attributes:
+ - Attribute: DW_AT_producer
+ Form: DW_FORM_string
+ - Attribute: DW_AT_language
+ Form: DW_FORM_data2
+ - Attribute: DW_AT_name
+ Form: DW_FORM_string
+ - Tag: DW_TAG_subprogram
+ Children: DW_CHILDREN_yes
+ Attributes:
+ - Attribute: DW_AT_name
+ Form: DW_FORM_string
+ - Attribute: DW_AT_linkage_name
+ Form: DW_FORM_string
+ - Attribute: DW_AT_type
+ Form: DW_FORM_ref_addr
+ - Attribute: DW_AT_low_pc
+ Form: DW_FORM_addr
+ - Attribute: DW_AT_high_pc
+ Form: DW_FORM_data4
+ - Tag: DW_TAG_formal_parameter
+ Children: DW_CHILDREN_no
+ Attributes:
+ - Attribute: DW_AT_type
+ Form: DW_FORM_ref_addr
+ - Tag: DW_TAG_class_type
+ Children: DW_CHILDREN_yes
+ Attributes:
+ - Attribute: DW_AT_name
+ Form: DW_FORM_string
+ - Tag: DW_TAG_member
+ Children: DW_CHILDREN_no
+ Attributes:
+ - Attribute: DW_AT_name
+ Form: DW_FORM_string
+ - Attribute: DW_AT_type
+ Form: DW_FORM_ref_addr
+ - Tag: DW_TAG_subprogram
+ Children: DW_CHILDREN_yes
+ Attributes:
+ - Attribute: DW_AT_name
+ Form: DW_FORM_string
+ - Attribute: DW_AT_type
+ Form: DW_FORM_ref_addr
+ - Tag: DW_TAG_template_type_parameter
+ Children: DW_CHILDREN_no
+ Attributes:
+ - Attribute: DW_AT_type
+ Form: DW_FORM_ref_addr
+ - Tag: DW_TAG_base_type
+ Children: DW_CHILDREN_no
+ Attributes:
+ - Attribute: DW_AT_name
+ Form: DW_FORM_string
+ - Tag: DW_TAG_pointer_type
+ Children: DW_CHILDREN_no
+ Attributes:
+ - Attribute: DW_AT_type
+ Form: DW_FORM_ref_addr
+ - Tag: DW_TAG_variable
+ Children: DW_CHILDREN_no
+ Attributes:
+ - Attribute: DW_AT_name
+ Form: DW_FORM_string
+ - Attribute: DW_AT_const_value
+ Form: DW_FORM_data4
+ - Attribute: DW_AT_type
+ Form: DW_FORM_ref_addr
+ - Table:
+ - Tag: DW_TAG_compile_unit
+ Children: DW_CHILDREN_yes
+ Attributes:
+ - Attribute: DW_AT_producer
+ Form: DW_FORM_string
+ - Attribute: DW_AT_language
+ Form: DW_FORM_data2
+ - Attribute: DW_AT_name
+ Form: DW_FORM_string
+ - Tag: DW_TAG_class_type
+ Children: DW_CHILDREN_yes
+ Attributes:
+ - Attribute: DW_AT_name
+ Form: DW_FORM_string
+ - Tag: DW_TAG_subprogram
+ Children: DW_CHILDREN_yes
+ Attributes:
+ - Attribute: DW_AT_name
+ Form: DW_FORM_string
+ - Attribute: DW_AT_type
+ Form: DW_FORM_ref_addr
+ - Tag: DW_TAG_template_type_parameter
+ Children: DW_CHILDREN_no
+ Attributes:
+ - Attribute: DW_AT_type
+ Form: DW_FORM_ref_addr
+ - Tag: DW_TAG_base_type
+ Children: DW_CHILDREN_no
+ Attributes:
+ - Attribute: DW_AT_name
+ Form: DW_FORM_string
+ - Tag: DW_TAG_variable
+ Children: DW_CHILDREN_no
+ Attributes:
+ - Attribute: DW_AT_name
+ Form: DW_FORM_string
+ - Attribute: DW_AT_const_value
+ Form: DW_FORM_data4
+ - Attribute: DW_AT_type
+ Form: DW_FORM_ref_addr
+ debug_info:
+ - Version: 4
+ Entries:
+ - AbbrCode: 1
+ Values:
+ - CStr: by_hand
+ - Value: 0x04
+ - CStr: CU1
+ - AbbrCode: 2
+ Values:
+ - CStr: foo
+ - CStr: __Z3foov
+ - Value: 0x000000a3
+ - Value: 0x00010000
+ - Value: 0x00000010
+ - AbbrCode: 3
+ Values:
+ - Value: 0x00000098
+ - AbbrCode: 4
+ Values:
+ - CStr: clas1
+ - AbbrCode: 5
+ Values:
+ - CStr: first
+ - Value: 0x0000009d
+ - AbbrCode: 5
+ Values:
+ - CStr: second
+ - Value: 0x000000a3
+ - AbbrCode: 0
+ - AbbrCode: 0
+ - AbbrCode: 4
+ Values:
+ - CStr: clas1
+ - AbbrCode: 5
+ Values:
+ - CStr: first
+ - Value: 0x000000a3
+ - AbbrCode: 0
+ - AbbrCode: 4
+ Values:
+ - CStr: Container
+ - AbbrCode: 6
+ Values:
+ - CStr: ParametrizedFunc
+ - Value: 0x00000098
+ - AbbrCode: 7
+ Values:
+ - Value: 0x0000003d
+ - AbbrCode: 0
+ - AbbrCode: 0
+ - AbbrCode: 8
+ Values:
+ - CStr: int
+ - AbbrCode: 8
+ Values:
+ - CStr: char
+ - AbbrCode: 8
+ Values:
+ - CStr: float
+ - AbbrCode: 10
+ Values:
+ - CStr: var1
+ - Value: 0x00000000
+ - Value: 0x0000005d
+ - AbbrCode: 10
+ Values:
+ - CStr: var2
+ - Value: 0x00000000
+ - Value: 0x00000070
+ - AbbrCode: 0
+ - Version: 4
+ Entries:
+ - AbbrCode: 1
+ Values:
+ - CStr: by_hand
+ - Value: 0x04
+ - CStr: CU2
+ - AbbrCode: 2
+ Values:
+ - CStr: Container
+ - AbbrCode: 3
+ Values:
+ - CStr: ParametrizedFunc
+ - Value: 0x00000109
+ - AbbrCode: 4
+ Values:
+ - Value: 0x00000109
+ - AbbrCode: 0
+ - AbbrCode: 0
+ - AbbrCode: 5
+ Values:
+ - CStr: int
+ - AbbrCode: 5
+ Values:
+ - CStr: float
+ - AbbrCode: 6
+ Values:
+ - CStr: var1
+ - Value: 0x00000000
+ - Value: 0x000000e1
+ - AbbrCode: 0
+...
More information about the llvm-commits
mailing list