Skip to content

[lld][macho] Move unwind logic from equalsVariable to equalsConstant #165325

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+37 −26
Open

[lld][macho] Move unwind logic from equalsVariable to equalsConstant #165325

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 37 additions & 26 deletions lld/MachO/ICF.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,14 +173,37 @@ bool ICF::equalsConstant(const ConcatInputSection *ia,
// a valid offset in the literal section.
return isecA->getOffset(valueA) == isecB->getOffset(valueB) &&
ra.addend == rb.addend;
else {
assert(valueA == 0 && valueB == 0);
// For section relocs, we compare the content at the section offset.
return isecA->getOffset(ra.addend) == isecB->getOffset(rb.addend);
}
assert(valueA == 0 && valueB == 0);
// For section relocs, we compare the content at the section offset.
return isecA->getOffset(ra.addend) == isecB->getOffset(rb.addend);
};
return std::equal(ia->relocs.begin(), ia->relocs.end(), ib->relocs.begin(),
f);
if (!llvm::equal(ia->relocs, ib->relocs, f))
return false;

// Check unwind info structural compatibility: if there are symbols with
// associated unwind info, check that both sections have compatible symbol
// layouts. For simplicity, we only attempt folding when all symbols are at
// offset zero within the section (which is typically the case with
// .subsections_via_symbols.)
auto hasUnwind = [](Defined *d) { return d->unwindEntry() != nullptr; };
const auto *itA = llvm::find_if(ia->symbols, hasUnwind);
const auto *itB = llvm::find_if(ib->symbols, hasUnwind);
if (itA == ia->symbols.end())
return itB == ib->symbols.end();
if (itB == ib->symbols.end())
return false;
const Defined *da = *itA;
const Defined *db = *itB;
if (da->value != 0 || db->value != 0)
return false;
auto isZero = [](Defined *d) { return d->value == 0; };
// Since symbols are stored in order of value, and since we have already
// checked that da/db have value zero, we just need to do the isZero check on
// the subsequent symbols.
return std::find_if_not(std::next(itA), ia->symbols.end(), isZero) ==
ia->symbols.end() &&
std::find_if_not(std::next(itB), ib->symbols.end(), isZero) ==
ib->symbols.end();
Comment on lines +203 to +206
Copy link
Contributor

@ellishg ellishg Oct 27, 2025

Choose a reason for hiding this comment

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

This doesn't check if all symbols have offset zero, it only checks if all symbols after the first one with unwind info has offset zero. Is that what we want?

Copy link
Contributor Author

@int3 int3 Oct 27, 2025

Choose a reason for hiding this comment

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

my memory is a bit hazy here, but this is preserving the logic of the existing code :)

but I think it works because symbols is sorted in ascending order of offset, and since we're checking that the first one with unwind info has offset zero, this subsequent check ensures all symbols in the section have offset zero too.

Copy link
Contributor

@ellishg ellishg Nov 5, 2025

Choose a reason for hiding this comment

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

Maybe we should add a comment here to let future folks know this isn't a bug

}

// Compare the "moving" parts of two ConcatInputSections -- i.e. everything not
Expand Down Expand Up @@ -217,31 +240,19 @@ bool ICF::equalsVariable(const ConcatInputSection *ia,
}
return isecA->icfEqClass[icfPass % 2] == isecB->icfEqClass[icfPass % 2];
};
if (!std::equal(ia->relocs.begin(), ia->relocs.end(), ib->relocs.begin(), f))
if (!llvm::equal(ia->relocs, ib->relocs, f))
return false;

// If there are symbols with associated unwind info, check that the unwind
// info matches. For simplicity, we only handle the case where there are only
// symbols at offset zero within the section (which is typically the case with
// .subsections_via_symbols.)
// Compare unwind info equivalence classes.
auto hasUnwind = [](Defined *d) { return d->unwindEntry() != nullptr; };
const auto *itA = llvm::find_if(ia->symbols, hasUnwind);
const auto *itB = llvm::find_if(ib->symbols, hasUnwind);
if (itA == ia->symbols.end())
return itB == ib->symbols.end();
if (itB == ib->symbols.end())
return false;
return true;
const Defined *da = *itA;
const Defined *db = *itB;
if (da->unwindEntry()->icfEqClass[icfPass % 2] !=
db->unwindEntry()->icfEqClass[icfPass % 2] ||
da->value != 0 || db->value != 0)
return false;
auto isZero = [](Defined *d) { return d->value == 0; };
return std::find_if_not(std::next(itA), ia->symbols.end(), isZero) ==
ia->symbols.end() &&
std::find_if_not(std::next(itB), ib->symbols.end(), isZero) ==
ib->symbols.end();
// equalsConstant() guarantees that both sections have unwind info.
const Defined *db = *llvm::find_if(ib->symbols, hasUnwind);
return da->unwindEntry()->icfEqClass[icfPass % 2] ==
db->unwindEntry()->icfEqClass[icfPass % 2];
}

// Find the first InputSection after BEGIN whose equivalence class differs
Expand Down