diff --git a/lld/MachO/ICF.cpp b/lld/MachO/ICF.cpp --- a/lld/MachO/ICF.cpp +++ b/lld/MachO/ICF.cpp @@ -176,8 +176,31 @@ } return isecA->icfEqClass[icfPass % 2] == isecB->icfEqClass[icfPass % 2]; }; - return std::equal(ia->relocs.begin(), ia->relocs.end(), ib->relocs.begin(), - f); + if (!std::equal(ia->relocs.begin(), ia->relocs.end(), ib->relocs.begin(), 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.) + auto hasCU = [](Defined *d) { return d->compactUnwind; }; + auto itA = std::find_if(ia->symbols.begin(), ia->symbols.end(), hasCU); + auto itB = std::find_if(ib->symbols.begin(), ib->symbols.end(), hasCU); + 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->compactUnwind->icfEqClass[icfPass % 2] != + db->compactUnwind->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(); } // Find the first InputSection after BEGIN whose equivalence class differs @@ -334,18 +357,14 @@ bool isHashable = (isCodeSection(isec) || isCfStringSection(isec)) && !isec->shouldOmitFromOutput() && isec->isHashableForICF(); - // ICF can't fold functions with unwind info - if (isHashable) - for (Defined *d : isec->symbols) - if (d->compactUnwind) { - isHashable = false; - break; - } - - if (isHashable) + if (isHashable) { hashable.push_back(isec); - else + for (Defined *d : isec->symbols) + if (d->compactUnwind) + hashable.push_back(d->compactUnwind); + } else { isec->icfEqClass[0] = ++icfUniqueID; + } } parallelForEach(hashable, [](ConcatInputSection *isec) { isec->hashForICF(); }); diff --git a/lld/MachO/InputFiles.cpp b/lld/MachO/InputFiles.cpp --- a/lld/MachO/InputFiles.cpp +++ b/lld/MachO/InputFiles.cpp @@ -892,10 +892,23 @@ for (SubsectionEntry &entry : *cuSubsecMap) { ConcatInputSection *isec = cast(entry.isec); + // Hack!! Since each CUE contains a different function address, if ICF + // operated naively and compared the entire contents of each CUE, entries + // with identical unwind info but belonging to different functions would + // never be considered equivalent. To work around this problem, we slice + // away the function address here. (Note that we do not adjust the offsets + // of the corresponding relocations.) We rely on `relocateCompactUnwind()` + // to correctly handle these truncated input sections. + isec->data = isec->data.slice(target->wordSize); + ConcatInputSection *referentIsec; - for (const Reloc &r : isec->relocs) { - if (r.offset != 0) + for (auto it = isec->relocs.begin(); it != isec->relocs.end();) { + Reloc &r = *it; + // We only wish to handle the relocation for CUE::functionAddress. + if (r.offset != 0) { + ++it; continue; + } uint64_t add = r.addend; if (auto *sym = cast_or_null(r.referent.dyn_cast())) { add += sym->value; @@ -904,19 +917,29 @@ referentIsec = cast(r.referent.dyn_cast()); } + if (referentIsec->getSegName() != segment_names::text) + error("compact unwind references address in " + toString(referentIsec) + + " which is not in segment __TEXT"); // The functionAddress relocations are typically section relocations. // However, unwind info operates on a per-symbol basis, so we search for // the function symbol here. - auto it = llvm::lower_bound( + auto symIt = llvm::lower_bound( referentIsec->symbols, add, [](Defined *d, uint64_t add) { return d->value < add; }); // The relocation should point at the exact address of a symbol (with no // addend). - if (it == referentIsec->symbols.end() || (*it)->value != add) { + if (symIt == referentIsec->symbols.end() || (*symIt)->value != add) { assert(referentIsec->wasCoalesced); + ++it; continue; } - (*it)->compactUnwind = isec; + (*symIt)->compactUnwind = isec; + // Since we've sliced away the functionAddress, we should remove the + // corresponding relocation too. Given that clang emits relocations in + // reverse order of address, this relocation should be at the end of the + // vector for most of our input object files, so this is typically an O(1) + // operation. + it = isec->relocs.erase(it); } } } diff --git a/lld/MachO/UnwindInfoSection.cpp b/lld/MachO/UnwindInfoSection.cpp --- a/lld/MachO/UnwindInfoSection.cpp +++ b/lld/MachO/UnwindInfoSection.cpp @@ -274,7 +274,8 @@ return; // Write the rest of the CUE. - memcpy(buf, d->compactUnwind->data.data(), d->compactUnwind->data.size()); + memcpy(buf + sizeof(Ptr), d->compactUnwind->data.data(), + d->compactUnwind->data.size()); for (const Reloc &r : d->compactUnwind->relocs) { uint64_t referentVA = 0; if (auto *referentSym = r.referent.dyn_cast()) { diff --git a/lld/test/MachO/icf.s b/lld/test/MachO/icf.s --- a/lld/test/MachO/icf.s +++ b/lld/test/MachO/icf.s @@ -32,8 +32,9 @@ # CHECK: [[#%x,CALL_RECURSIVE_2:]] l F __TEXT,__text _call_recursive_2 # CHECK: [[#%x,CHECK_LENGTH_1:]] l F __TEXT,__text _check_length_1 # CHECK: [[#%x,CHECK_LENGTH_2:]] l F __TEXT,__text _check_length_2 -# CHECK: [[#%x,HAS_UNWIND_1:]] l F __TEXT,__text _has_unwind_1 +# CHECK: [[#%x,HAS_UNWIND_2:]] l F __TEXT,__text _has_unwind_1 # CHECK: [[#%x,HAS_UNWIND_2:]] l F __TEXT,__text _has_unwind_2 +# CHECK: [[#%x,HAS_UNWIND_3:]] l F __TEXT,__text _has_unwind_3 # CHECK: [[#%x,MUTALLY_RECURSIVE_2:]] l F __TEXT,__text _mutually_recursive_1 # CHECK: [[#%x,MUTALLY_RECURSIVE_2:]] l F __TEXT,__text _mutually_recursive_2 # CHECK: [[#%x,INIT_2:]] l F __TEXT,__text _init_1 @@ -64,8 +65,9 @@ # CHECK: callq 0x[[#%x,CALL_RECURSIVE_2:]] <_call_recursive_2> # CHECK: callq 0x[[#%x,CHECK_LENGTH_1:]] <_check_length_1> # CHECK: callq 0x[[#%x,CHECK_LENGTH_2:]] <_check_length_2> -# CHECK: callq 0x[[#%x,HAS_UNWIND_1:]] <_has_unwind_1> # CHECK: callq 0x[[#%x,HAS_UNWIND_2:]] <_has_unwind_2> +# CHECK: callq 0x[[#%x,HAS_UNWIND_2:]] <_has_unwind_2> +# CHECK: callq 0x[[#%x,HAS_UNWIND_3:]] <_has_unwind_3> # CHECK: callq 0x[[#%x,MUTALLY_RECURSIVE_2:]] <_mutually_recursive_2> # CHECK: callq 0x[[#%x,MUTALLY_RECURSIVE_2:]] <_mutually_recursive_2> ## FIXME: Mutually-recursive functions with identical bodies (see below) @@ -170,8 +172,7 @@ _my_personality: mov $1345, %rax -## No fold: functions have unwind info. -## FIXME: Fold functions with identical unwind info. +## Functions with identical unwind info should be folded. _has_unwind_1: .cfi_startproc .cfi_personality 155, _my_personality @@ -186,6 +187,15 @@ ret .cfi_endproc +## This function has different unwind info from the preceding two, and therefore +## should not be folded. +_has_unwind_3: + .cfi_startproc + .cfi_personality 155, _my_personality + .cfi_def_cfa_offset 8 + ret + .cfi_endproc + ## Fold: Mutually-recursive functions with symmetric bodies _mutually_recursive_1: callq _mutually_recursive_1 # call myself @@ -253,6 +263,7 @@ callq _check_length_2 callq _has_unwind_1 callq _has_unwind_2 + callq _has_unwind_3 callq _mutually_recursive_1 callq _mutually_recursive_2 callq _asymmetric_recursive_1 diff --git a/lld/test/MachO/invalid/compact-unwind-bad-reloc.s b/lld/test/MachO/invalid/compact-unwind-bad-reloc.s --- a/lld/test/MachO/invalid/compact-unwind-bad-reloc.s +++ b/lld/test/MachO/invalid/compact-unwind-bad-reloc.s @@ -1,13 +1,12 @@ # REQUIRES: x86 -# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin19.0.0 %s -o %t.o -# RUN: not %lld -pie -lSystem -lc++ %t.o -o %t 2>&1 | FileCheck %s -DFILE=%t.o +# RUN: rm -rf %t; split-file %s %t +# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin19.0.0 %t/bad-function.s -o %t/bad-function.o +# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin19.0.0 %t/bad-personality.s -o %t/bad-personality.o +# RUN: not %lld -pie -lSystem -lc++ %t/bad-function.o -o %t 2>&1 | FileCheck %s -DFILE=%t/bad-function.o +# RUN: not %lld -pie -lSystem -lc++ %t/bad-personality.o -o %t 2>&1 | FileCheck %s -DFILE=%t/bad-personality.o # CHECK: error: compact unwind references address in [[FILE]]:(__data) which is not in segment __TEXT -.globl _main, _not_a_function -.text -_main: - retq - +#--- bad-function.s .data _not_a_function: .cfi_startproc @@ -15,3 +14,17 @@ .cfi_def_cfa_offset 16 retq .cfi_endproc + +#--- bad-personality.s +.globl _main, _not_a_function +.text +_main: + .cfi_startproc + .cfi_personality 155, _my_personality + .cfi_def_cfa_offset 16 + retq + .cfi_endproc + +.data +.globl _my_personality +_my_personality: