diff --git a/lld/MachO/ICF.cpp b/lld/MachO/ICF.cpp --- a/lld/MachO/ICF.cpp +++ b/lld/MachO/ICF.cpp @@ -111,8 +111,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; @@ -125,16 +123,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; - } + // ICF runs before Undefineds are treated (and potentially converted into + // DylibSymbols). + if (isa(sa) || isa(sa)) + return sa == sb && ra.addend == rb.addend; + 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; @@ -151,7 +149,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) == @@ -389,6 +387,17 @@ } } 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)) { + MutableArrayRef copy = isec->data.copy(bAlloc()); + for (const Reloc &r : isec->relocs) + target->relocateOne(copy.data() + r.offset, r, /*va=*/0, /*relocVA=*/0); + isec->data = copy; + } 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" @@ -57,7 +61,7 @@ .quad 3 ## strlen .section __TEXT,__ustring -l_.str.2: +l_.ustr: .short 102 ## f .short 111 ## o .short 0 ## \0 @@ -77,7 +81,7 @@ .quad ___CFConstantStringClassReference .long 2000 ## utf-16 .space 4 - .quad l_.str.2 + .quad l_.ustr .quad 4 ## strlen .text @@ -116,7 +120,7 @@ .section __TEXT,__ustring .p2align 1 -l_.str.2: +l_.ustr: .short 102 ## f .short 111 ## o .short 0 ## \0 @@ -129,7 +133,7 @@ .quad ___CFConstantStringClassReference .long 2000 ## utf-16 .space 4 - .quad l_.str.2 + .quad l_.ustr .quad 4 ## strlen .text diff --git a/lld/test/MachO/icf-undef.s b/lld/test/MachO/icf-undef.s new file mode 100644 --- /dev/null +++ b/lld/test/MachO/icf-undef.s @@ -0,0 +1,28 @@ +# REQUIRES: x86 +# RUN: rm -rf %t; split-file %s %t + +# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin19.0.0 %t/test.s -o %t/test.o + +## Check that we correctly dedup sections that reference dynamic-lookup symbols. +# RUN: %lld -lSystem -dylib --icf=all -undefined dynamic_lookup -o %t/test %t/test.o +# RUN: llvm-objdump --macho --syms %t/test | FileCheck %s + +## Check that we still raise an error when using regular undefined symbol +## treatment. +# RUN: not %lld -lSystem -dylib --icf=all -o /dev/null %t/test.o 2>&1 | \ +# RUN: FileCheck %s --check-prefix=ERR + +# CHECK: [[#%x,ADDR:]] l F __TEXT,__text _foo +# CHECK: [[#ADDR]] l F __TEXT,__text _bar + +# ERR: error: undefined symbol: _undef + +#--- test.s + +.subsections_via_symbols + +_foo: + callq _undef + 1 + +_bar: + callq _undef + 1 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