diff --git a/lld/MachO/ICF.cpp b/lld/MachO/ICF.cpp --- a/lld/MachO/ICF.cpp +++ b/lld/MachO/ICF.cpp @@ -101,8 +101,6 @@ return false; if (ra.offset != rb.offset) return false; - if (ra.addend != rb.addend) - return false; if (ra.referent.is() != rb.referent.is()) return false; @@ -115,16 +113,16 @@ const auto *sb = rb.referent.get(); if (sa->kind() != sb->kind()) return false; - if (!isa(sa)) { - // ICF runs before Undefineds are reported. - assert(isa(sa) || isa(sa)); - return sa == sb; - } + if (isa(sa)) + return sa == sb && ra.addend == rb.addend; + else if (isa(sa)) // ICF runs before Undefineds are reported. + return false; + assert(isa(sa)); const auto *da = cast(sa); const auto *db = cast(sb); if (!da->isec || !db->isec) { assert(da->isAbsolute() && db->isAbsolute()); - return da->value == db->value; + return da->value + ra.addend == db->value + rb.addend; } isecA = da->isec; valueA = da->value; @@ -141,7 +139,7 @@ assert(isecA->kind() == isecB->kind()); // We will compare ConcatInputSection contents in equalsVariable. if (isa(isecA)) - return true; + return ra.addend == rb.addend; // Else we have two literal sections. References to them are equal iff their // offsets in the output section are equal. return isecA->getOffset(valueA + ra.addend) == @@ -374,6 +372,18 @@ } } parallelForEach(hashable, [](ConcatInputSection *isec) { + // __cfstring has embedded addends that foil ICF's hashing / equality + // checks. (We can ignore embedded addends when doing ICF because the same + // information gets recorded in our Reloc structs.) We therefore create a + // mutable copy of the CFString and zero out the embedded addends before + // performing any hashing / equality checks. + if (isCfStringSection(isec)) { + uint8_t *copy = bAlloc().Allocate(isec->data.size()); + std::uninitialized_copy(isec->data.begin(), isec->data.end(), copy); + for (const Reloc &r : isec->relocs) + target->relocateOne(copy + r.offset, r, /*va=*/0, /*relocVA=*/0); + isec->data = ArrayRef(copy, isec->data.size()); + } assert(isec->icfEqClass[0] == 0); // don't overwrite a unique ID! // Turn-on the top bit to guarantee that valid hashes have no collisions // with the small-integer unique IDs for ICF-ineligible sections diff --git a/lld/test/MachO/cfstring-dedup.s b/lld/test/MachO/cfstring-dedup.s --- a/lld/test/MachO/cfstring-dedup.s +++ b/lld/test/MachO/cfstring-dedup.s @@ -37,6 +37,10 @@ #--- foo1.s .cstring +L_.str.0: + .asciz "bar" +## This string is at a different offset than the corresponding "foo" string in +## foo2.s. Make sure that we treat references to either string as equivalent. L_.str: .asciz "foo" 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 @@ -22,11 +22,13 @@ # CHECK: [[#%x,DYLIB_REF_2:]] l F __TEXT,__text _dylib_ref_1 # CHECK: [[#%x,DYLIB_REF_2:]] l F __TEXT,__text _dylib_ref_2 # CHECK: [[#%x,DYLIB_REF_3:]] l F __TEXT,__text _dylib_ref_3 +# CHECK: [[#%x,DYLIB_REF_4:]] l F __TEXT,__text _dylib_ref_4 # CHECK: [[#%x,ALT:]] l F __TEXT,__text _alt # CHECK: [[#%x,WITH_ALT_ENTRY:]] l F __TEXT,__text _with_alt_entry # CHECK: [[#%x,WITH_ALT_ENTRY:]] l F __TEXT,__text _no_alt_entry # CHECK: [[#%x,DEFINED_REF_WITH_ADDEND_2:]] l F __TEXT,__text _defined_ref_with_addend_1 # CHECK: [[#%x,DEFINED_REF_WITH_ADDEND_2:]] l F __TEXT,__text _defined_ref_with_addend_2 +# CHECK: [[#%x,DEFINED_REF_WITH_ADDEND_3:]] l F __TEXT,__text _defined_ref_with_addend_3 # CHECK: [[#%x,RECURSIVE:]] l F __TEXT,__text _recursive # CHECK: [[#%x,CALL_RECURSIVE_2:]] l F __TEXT,__text _call_recursive_1 # CHECK: [[#%x,CALL_RECURSIVE_2:]] l F __TEXT,__text _call_recursive_2 @@ -55,11 +57,13 @@ # CHECK: callq 0x[[#%x,DYLIB_REF_2:]] <_dylib_ref_2> # CHECK: callq 0x[[#%x,DYLIB_REF_2:]] <_dylib_ref_2> # CHECK: callq 0x[[#%x,DYLIB_REF_3:]] <_dylib_ref_3> +# CHECK: callq 0x[[#%x,DYLIB_REF_4:]] <_dylib_ref_4> # CHECK: callq 0x[[#%x,ALT:]] <_alt> # CHECK: callq 0x[[#%x,WITH_ALT_ENTRY:]] <_with_alt_entry> # CHECK: callq 0x[[#%x,WITH_ALT_ENTRY:]] <_with_alt_entry> # CHECK: callq 0x[[#%x,DEFINED_REF_WITH_ADDEND_2:]] <_defined_ref_with_addend_2> # CHECK: callq 0x[[#%x,DEFINED_REF_WITH_ADDEND_2:]] <_defined_ref_with_addend_2> +# CHECK: callq 0x[[#%x,DEFINED_REF_WITH_ADDEND_3:]] <_defined_ref_with_addend_3> # CHECK: callq 0x[[#%x,RECURSIVE:]] <_recursive> # CHECK: callq 0x[[#%x,CALL_RECURSIVE_2:]] <_call_recursive_2> # CHECK: callq 0x[[#%x,CALL_RECURSIVE_2:]] <_call_recursive_2> @@ -132,6 +136,11 @@ mov ___inf@GOTPCREL(%rip), %rax callq ___inf +## No fold: referent dylib addend differs +_dylib_ref_4: + mov ___nan + 1@GOTPCREL(%rip), %rax + callq ___inf + 1 + ## We can merge two sections even if one of them has an alt entry. Just make ## sure we don't merge the alt entry symbol with a regular symbol. .alt_entry _alt @@ -150,6 +159,10 @@ _defined_ref_with_addend_2: callq _with_alt_entry + 4 +# No fold: addend differs +_defined_ref_with_addend_3: + callq _with_alt_entry + 8 + ## _recursive has the same body as its next two callers, but cannot be folded ## with them. _recursive: @@ -251,11 +264,13 @@ callq _dylib_ref_1 callq _dylib_ref_2 callq _dylib_ref_3 + callq _dylib_ref_4 callq _alt callq _with_alt_entry callq _no_alt_entry callq _defined_ref_with_addend_1 callq _defined_ref_with_addend_2 + callq _defined_ref_with_addend_3 callq _recursive callq _call_recursive_1 callq _call_recursive_2