From b3f863ad85e2303df1dfd863dc70a52aa3f0aa30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Storsj=C3=B6?= Date: Wed, 30 Apr 2025 14:57:04 +0300 Subject: [PATCH] [DWARFLinkerParallel] Fix incorrect uses of compare_exchange_weak The documentation for compare_exchange_weak says that it is allowed to spuriously fail. If compare_exchange_weak is called in a loop, spurious failures usually are benign - but in these cases, a spurious failure would give incorrect behaviour. E.g. in TypePool::getOrCreateTypeEntryBody, we assume that if the compare_exchange call returned false, we had been preempted by another thread and that DIE is non-null. This fixes running the dsymutil tests on Windows on aarch64 (built with a mingw toolchain with libc++). --- .../DWARFLinker/Parallel/DWARFLinkerCompileUnit.cpp | 12 ++++++------ .../DWARFLinker/Parallel/DWARFLinkerCompileUnit.h | 2 +- llvm/lib/DWARFLinker/Parallel/TypePool.h | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/llvm/lib/DWARFLinker/Parallel/DWARFLinkerCompileUnit.cpp b/llvm/lib/DWARFLinker/Parallel/DWARFLinkerCompileUnit.cpp index fa426d41854bb..38373b2c54919 100644 --- a/llvm/lib/DWARFLinker/Parallel/DWARFLinkerCompileUnit.cpp +++ b/llvm/lib/DWARFLinker/Parallel/DWARFLinkerCompileUnit.cpp @@ -1433,13 +1433,13 @@ DIE *CompileUnit::allocateTypeDie(TypeEntryBody *TypeDescriptor, if (IsDeclaration && !DeclarationDie) { // Alocate declaration DIE. DIE *NewDie = TypeDIEGenerator.createDIE(DieTag, 0); - if (TypeDescriptor->DeclarationDie.compare_exchange_weak(DeclarationDie, - NewDie)) + if (TypeDescriptor->DeclarationDie.compare_exchange_strong(DeclarationDie, + NewDie)) return NewDie; } else if (IsDeclaration && !IsParentDeclaration && OldParentIsDeclaration) { // Overwrite existing declaration DIE if it's parent is also an declaration // while parent of current declaration DIE is a definition. - if (TypeDescriptor->ParentIsDeclaration.compare_exchange_weak( + if (TypeDescriptor->ParentIsDeclaration.compare_exchange_strong( OldParentIsDeclaration, false)) { DIE *NewDie = TypeDIEGenerator.createDIE(DieTag, 0); TypeDescriptor->DeclarationDie = NewDie; @@ -1449,13 +1449,13 @@ DIE *CompileUnit::allocateTypeDie(TypeEntryBody *TypeDescriptor, // Alocate declaration DIE since parent of current DIE is marked as // declaration. DIE *NewDie = TypeDIEGenerator.createDIE(DieTag, 0); - if (TypeDescriptor->DeclarationDie.compare_exchange_weak(DeclarationDie, - NewDie)) + if (TypeDescriptor->DeclarationDie.compare_exchange_strong(DeclarationDie, + NewDie)) return NewDie; } else if (!IsDeclaration && !IsParentDeclaration) { // Allocate definition DIE. DIE *NewDie = TypeDIEGenerator.createDIE(DieTag, 0); - if (TypeDescriptor->Die.compare_exchange_weak(DefinitionDie, NewDie)) { + if (TypeDescriptor->Die.compare_exchange_strong(DefinitionDie, NewDie)) { TypeDescriptor->ParentIsDeclaration = false; return NewDie; } diff --git a/llvm/lib/DWARFLinker/Parallel/DWARFLinkerCompileUnit.h b/llvm/lib/DWARFLinker/Parallel/DWARFLinkerCompileUnit.h index 20e20222a4ac0..90c007ebe5f6c 100644 --- a/llvm/lib/DWARFLinker/Parallel/DWARFLinkerCompileUnit.h +++ b/llvm/lib/DWARFLinker/Parallel/DWARFLinkerCompileUnit.h @@ -201,7 +201,7 @@ class alignas(8) CompileUnit : public DwarfUnit { bool setPlacementIfUnset(DieOutputPlacement Placement) { auto InputData = Flags.load(); if ((InputData & 0x7) == NotSet) - if (Flags.compare_exchange_weak(InputData, (InputData | Placement))) + if (Flags.compare_exchange_strong(InputData, (InputData | Placement))) return true; return false; diff --git a/llvm/lib/DWARFLinker/Parallel/TypePool.h b/llvm/lib/DWARFLinker/Parallel/TypePool.h index 547532977262b..15a6428e1b8d6 100644 --- a/llvm/lib/DWARFLinker/Parallel/TypePool.h +++ b/llvm/lib/DWARFLinker/Parallel/TypePool.h @@ -135,7 +135,7 @@ class TypePool return DIE; TypeEntryBody *NewDIE = TypeEntryBody::create(Allocator); - if (Entry->getValue().compare_exchange_weak(DIE, NewDIE)) { + if (Entry->getValue().compare_exchange_strong(DIE, NewDIE)) { ParentEntry->getValue().load()->Children.add(Entry); return NewDIE; }