[lld] [lld-macho] Fix category merging category map non-determinism (PR #91159)
via llvm-commits
llvm-commits at lists.llvm.org
Mon May 6 15:32:11 PDT 2024
https://github.com/alx32 updated https://github.com/llvm/llvm-project/pull/91159
>From 686d003bef1c3b81536caa0bb2e2cfe2fd216508 Mon Sep 17 00:00:00 2001
From: Alex Borcan <alexborcan at fb.com>
Date: Sun, 5 May 2024 19:11:28 -0700
Subject: [PATCH 1/2] [lld-macho] Fix category merging category map
non-determinism
---
lld/MachO/ObjC.cpp | 45 +++++++++++++++++++++++++++++++++------------
1 file changed, 33 insertions(+), 12 deletions(-)
diff --git a/lld/MachO/ObjC.cpp b/lld/MachO/ObjC.cpp
index f7d7e3a7ad70dc..d8fceda4a7eedb 100644
--- a/lld/MachO/ObjC.cpp
+++ b/lld/MachO/ObjC.cpp
@@ -354,10 +354,13 @@ class ObjcCategoryMerger {
ConcatInputSection *catListIsec;
ConcatInputSection *catBodyIsec;
uint32_t offCatListIsec = 0;
+ uint32_t inputIndex = 0;
bool wasMerged = false;
};
+ typedef std::vector<InfoInputCategory> CategoryGroup;
+
// To write new (merged) categories or classes, we will try make limited
// assumptions about the alignment and the sections the various class/category
// info are stored in and . So we'll just reuse the same sections and
@@ -429,8 +432,7 @@ class ObjcCategoryMerger {
private:
void collectAndValidateCategoriesData();
- void
- mergeCategoriesIntoSingleCategory(std::vector<InfoInputCategory> &categories);
+ void mergeCategoriesIntoSingleCategory(CategoryGroup &categories);
void eraseISec(ConcatInputSection *isec);
void removeRefsToErasedIsecs();
@@ -490,8 +492,8 @@ class ObjcCategoryMerger {
InfoCategoryWriter infoCategoryWriter;
std::vector<ConcatInputSection *> &allInputSections;
- // Map of base class Symbol to list of InfoInputCategory's for it
- DenseMap<const Symbol *, std::vector<InfoInputCategory>> categoryMap;
+ // Info for all input categories, grouped by base class
+ std::vector<CategoryGroup> categoryGroups;
// Set for tracking InputSection erased via eraseISec
DenseSet<InputSection *> erasedIsecs;
@@ -1061,6 +1063,12 @@ void ObjcCategoryMerger::createSymbolReference(Defined *refFrom,
}
void ObjcCategoryMerger::collectAndValidateCategoriesData() {
+ // Make category merging deterministic by using a counter for found categories
+ uint32_t inputCategoryIndex = 0;
+ // Map of base class Symbol to list of InfoInputCategory's for it. We use this
+ // for fast lookup of categories to their base class. Later this info will be
+ // moved into 'categoryGroups' member.
+ DenseMap<const Symbol *, std::vector<InfoInputCategory>> categoryMap;
for (InputSection *sec : allInputSections) {
if (sec->getName() != section_names::objcCatList)
continue;
@@ -1090,12 +1098,25 @@ void ObjcCategoryMerger::collectAndValidateCategoriesData() {
tryGetSymbolAtIsecOffset(catBodyIsec, catLayout.klassOffset);
assert(classSym && "Category does not have a valid base class");
- InfoInputCategory catInputInfo{catListCisec, catBodyIsec, off};
+ InfoInputCategory catInputInfo{catListCisec, catBodyIsec, off,
+ inputCategoryIndex++};
categoryMap[classSym].push_back(catInputInfo);
collectCategoryWriterInfoFromCategory(catInputInfo);
}
}
+
+ // Move categoryMap into categoryGroups and sort by the first category's
+ // inputIndex. This way we can be sure that category merging will be
+ // deterministic across linker runs.
+ categoryGroups.reserve(categoryMap.size());
+ for (auto &mapEntry : categoryMap)
+ categoryGroups.push_back(mapEntry.second);
+
+ std::sort(categoryGroups.begin(), categoryGroups.end(),
+ [](const CategoryGroup &a, const CategoryGroup &b) {
+ return a[0].inputIndex < b[0].inputIndex;
+ });
}
// In the input we have multiple __objc_catlist InputSection, each of which may
@@ -1173,8 +1194,8 @@ void ObjcCategoryMerger::eraseMergedCategories() {
// Map of InputSection to a set of offsets of the categories that were merged
std::map<ConcatInputSection *, std::set<uint64_t>> catListToErasedOffsets;
- for (auto &mapEntry : categoryMap) {
- for (InfoInputCategory &catInfo : mapEntry.second) {
+ for (auto &catGroup : categoryGroups) {
+ for (InfoInputCategory &catInfo : catGroup) {
if (catInfo.wasMerged) {
eraseISec(catInfo.catListIsec);
catListToErasedOffsets[catInfo.catListIsec].insert(
@@ -1189,8 +1210,8 @@ void ObjcCategoryMerger::eraseMergedCategories() {
generateCatListForNonErasedCategories(catListToErasedOffsets);
// Erase the old method lists & names of the categories that were merged
- for (auto &mapEntry : categoryMap) {
- for (InfoInputCategory &catInfo : mapEntry.second) {
+ for (auto &catgroup : categoryGroups) {
+ for (InfoInputCategory &catInfo : catgroup) {
if (!catInfo.wasMerged)
continue;
@@ -1241,10 +1262,10 @@ void ObjcCategoryMerger::removeRefsToErasedIsecs() {
void ObjcCategoryMerger::doMerge() {
collectAndValidateCategoriesData();
- for (auto &entry : categoryMap)
- if (entry.second.size() > 1)
+ for (auto &catGroup : categoryGroups)
+ if (catGroup.size() > 1)
// Merge all categories into a new, single category
- mergeCategoriesIntoSingleCategory(entry.second);
+ mergeCategoriesIntoSingleCategory(catGroup);
// Erase all categories that were merged
eraseMergedCategories();
>From 09f6d07cafc5712359c699d34b02abf4cb322892 Mon Sep 17 00:00:00 2001
From: Alex Borcan <alexborcan at fb.com>
Date: Mon, 6 May 2024 09:30:01 -0700
Subject: [PATCH 2/2] Address Feedback nr.1
---
lld/MachO/ObjC.cpp | 51 ++++++++++++++--------------------------------
1 file changed, 15 insertions(+), 36 deletions(-)
diff --git a/lld/MachO/ObjC.cpp b/lld/MachO/ObjC.cpp
index d8fceda4a7eedb..15b89a808b05ee 100644
--- a/lld/MachO/ObjC.cpp
+++ b/lld/MachO/ObjC.cpp
@@ -354,13 +354,10 @@ class ObjcCategoryMerger {
ConcatInputSection *catListIsec;
ConcatInputSection *catBodyIsec;
uint32_t offCatListIsec = 0;
- uint32_t inputIndex = 0;
bool wasMerged = false;
};
- typedef std::vector<InfoInputCategory> CategoryGroup;
-
// To write new (merged) categories or classes, we will try make limited
// assumptions about the alignment and the sections the various class/category
// info are stored in and . So we'll just reuse the same sections and
@@ -432,14 +429,15 @@ class ObjcCategoryMerger {
private:
void collectAndValidateCategoriesData();
- void mergeCategoriesIntoSingleCategory(CategoryGroup &categories);
+ void
+ mergeCategoriesIntoSingleCategory(std::vector<InfoInputCategory> &categories);
void eraseISec(ConcatInputSection *isec);
void removeRefsToErasedIsecs();
void eraseMergedCategories();
void generateCatListForNonErasedCategories(
- std::map<ConcatInputSection *, std::set<uint64_t>>
+ MapVector<ConcatInputSection *, std::set<uint64_t>>
catListToErasedOffsets);
void collectSectionWriteInfoFromIsec(const InputSection *isec,
InfoWriteSection &catWriteInfo);
@@ -492,8 +490,8 @@ class ObjcCategoryMerger {
InfoCategoryWriter infoCategoryWriter;
std::vector<ConcatInputSection *> &allInputSections;
- // Info for all input categories, grouped by base class
- std::vector<CategoryGroup> categoryGroups;
+ // Map of base class Symbol to list of InfoInputCategory's for it
+ MapVector<const Symbol *, std::vector<InfoInputCategory>> categoryMap;
// Set for tracking InputSection erased via eraseISec
DenseSet<InputSection *> erasedIsecs;
@@ -1063,12 +1061,6 @@ void ObjcCategoryMerger::createSymbolReference(Defined *refFrom,
}
void ObjcCategoryMerger::collectAndValidateCategoriesData() {
- // Make category merging deterministic by using a counter for found categories
- uint32_t inputCategoryIndex = 0;
- // Map of base class Symbol to list of InfoInputCategory's for it. We use this
- // for fast lookup of categories to their base class. Later this info will be
- // moved into 'categoryGroups' member.
- DenseMap<const Symbol *, std::vector<InfoInputCategory>> categoryMap;
for (InputSection *sec : allInputSections) {
if (sec->getName() != section_names::objcCatList)
continue;
@@ -1098,25 +1090,12 @@ void ObjcCategoryMerger::collectAndValidateCategoriesData() {
tryGetSymbolAtIsecOffset(catBodyIsec, catLayout.klassOffset);
assert(classSym && "Category does not have a valid base class");
- InfoInputCategory catInputInfo{catListCisec, catBodyIsec, off,
- inputCategoryIndex++};
+ InfoInputCategory catInputInfo{catListCisec, catBodyIsec, off};
categoryMap[classSym].push_back(catInputInfo);
collectCategoryWriterInfoFromCategory(catInputInfo);
}
}
-
- // Move categoryMap into categoryGroups and sort by the first category's
- // inputIndex. This way we can be sure that category merging will be
- // deterministic across linker runs.
- categoryGroups.reserve(categoryMap.size());
- for (auto &mapEntry : categoryMap)
- categoryGroups.push_back(mapEntry.second);
-
- std::sort(categoryGroups.begin(), categoryGroups.end(),
- [](const CategoryGroup &a, const CategoryGroup &b) {
- return a[0].inputIndex < b[0].inputIndex;
- });
}
// In the input we have multiple __objc_catlist InputSection, each of which may
@@ -1125,7 +1104,7 @@ void ObjcCategoryMerger::collectAndValidateCategoriesData() {
// (not erased). For these not erased categories, we generate new __objc_catlist
// entries since the parent __objc_catlist entry will be erased
void ObjcCategoryMerger::generateCatListForNonErasedCategories(
- const std::map<ConcatInputSection *, std::set<uint64_t>>
+ const MapVector<ConcatInputSection *, std::set<uint64_t>>
catListToErasedOffsets) {
// Go through all offsets of all __objc_catlist's that we process and if there
@@ -1192,10 +1171,10 @@ void ObjcCategoryMerger::eraseISec(ConcatInputSection *isec) {
// them.
void ObjcCategoryMerger::eraseMergedCategories() {
// Map of InputSection to a set of offsets of the categories that were merged
- std::map<ConcatInputSection *, std::set<uint64_t>> catListToErasedOffsets;
+ MapVector<ConcatInputSection *, std::set<uint64_t>> catListToErasedOffsets;
- for (auto &catGroup : categoryGroups) {
- for (InfoInputCategory &catInfo : catGroup) {
+ for (auto &mapEntry : categoryMap) {
+ for (InfoInputCategory &catInfo : mapEntry.second) {
if (catInfo.wasMerged) {
eraseISec(catInfo.catListIsec);
catListToErasedOffsets[catInfo.catListIsec].insert(
@@ -1210,8 +1189,8 @@ void ObjcCategoryMerger::eraseMergedCategories() {
generateCatListForNonErasedCategories(catListToErasedOffsets);
// Erase the old method lists & names of the categories that were merged
- for (auto &catgroup : categoryGroups) {
- for (InfoInputCategory &catInfo : catgroup) {
+ for (auto &mapEntry : categoryMap) {
+ for (InfoInputCategory &catInfo : mapEntry.second) {
if (!catInfo.wasMerged)
continue;
@@ -1262,10 +1241,10 @@ void ObjcCategoryMerger::removeRefsToErasedIsecs() {
void ObjcCategoryMerger::doMerge() {
collectAndValidateCategoriesData();
- for (auto &catGroup : categoryGroups)
- if (catGroup.size() > 1)
+ for (auto &entry : categoryMap)
+ if (entry.second.size() > 1)
// Merge all categories into a new, single category
- mergeCategoriesIntoSingleCategory(catGroup);
+ mergeCategoriesIntoSingleCategory(entry.second);
// Erase all categories that were merged
eraseMergedCategories();
More information about the llvm-commits
mailing list