[lld-macho] Fix bugs around EH_Frame symbols

[int3]

Summary

While extending the map file to cover unwind info, I realized we had two
issues with our EH_Frame symbols:

1. Their size was not set
2. We would create two EH_Frame symbols per frame when we only needed one. This was because the Defined constructor would add the symbol itself to InputSection::symbols, but we were also manually appending the symbol to that same vector.

Note that ld64 prints "CIE" and "FDE for: <function>" instead of just
"EH_Frame", but I'm punting on that for now unless we discover that
users really depend upon it.

[int3]

update

[Roger]

Nice catch :)

lld/MachO/InputFiles.cpp

void ObjFile::registerEhFrames(Section &ehFrameSection) {
   for (const Subsection &subsec : ehFrameSection.subsections) {
     auto *isec = cast<ConcatInputSection>(subsec.isec);
     uint64_t isecOff = subsec.offset;
 
     // Subtractor relocs require the subtrahend to be a symbol reloc. Ensure
     // that all EH frames have an associated symbol so that we can generate
     // subtractor relocs that reference them.
     if (isec->symbols.size() == 0)
-      isec->symbols.push_back(make<Defined>(
-          "EH_Frame", isec->getFile(), isec, /*value=*/0, /*size=*/0,
-          /*isWeakDef=*/false, /*isExternal=*/false, /*isPrivateExtern=*/false,
-          /*includeInSymtab=*/false, /*isThumb=*/false,
-          /*isReferencedDynamically=*/false, /*noDeadStrip=*/false));
+      make<Defined>("EH_Frame", isec->getFile(), isec, /*value=*/0,
+                    isec->getSize(), /*isWeakDef=*/false, /*isExternal=*/false,
+                    /*isPrivateExtern=*/false, /*includeInSymtab=*/false,
+                    /*isThumb=*/false, /*isReferencedDynamically=*/false,
+                    /*noDeadStrip=*/false);
     else if (isec->symbols[0]->value != 0)
       fatal("found symbol at unexpected offset in __eh_frame");
 
     EhReader reader(this, isec->data, subsec.offset);
     size_t dataOff = 0; // Offset from the start of the EH frame.
     reader.skipValidLength(&dataOff); // readLength() already validated this.
     // cieOffOff is the offset from the start of the EH frame to the cieOff
     // value, which is itself an offset from the current PC to a CIE.

lld/test/MachO/map-file.s

 # REQUIRES: x86
 # RUN: rm -rf %t; split-file %s %t
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/foo.s -o %t/foo.o
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/test.s -o %t/test.o
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/c-string-literal.s -o %t/c-string-literal.o
 
 # RUN: %lld -demangle -map %t/map %t/test.o %t/foo.o %t/c-string-literal.o \
 # RUN:   --time-trace -o %t/test
 # RUN: llvm-objdump --syms --section-headers %t/test > %t/objdump
 ## Check that symbols in cstring sections aren't emitted
-# RUN: cat %t/objdump %t/map | FileCheck %s --implicit-check-not _hello_world
+## Also check that we don't have redundant EH_Frame symbols (regression test)
+# RUN: cat %t/objdump %t/map | FileCheck %s --implicit-check-not _hello_world \
+# RUN:   --implicit-check-not EH_Frame
 # RUN: FileCheck %s --check-prefix=MAPFILE < %t/test.time-trace
 
 # CHECK:      Sections:
 # CHECK-NEXT: Idx  Name          Size           VMA               Type
 # CHECK-NEXT: 0    __text        {{[0-9a-f]+}}  [[#%x,TEXT:]]     TEXT
 # CHECK-NEXT: 1    __cstring     {{[0-9a-f]+}}  [[#%x,CSTR:]]     DATA
 # CHECK-NEXT: 2    __unwind_info {{[0-9a-f]+}}  [[#%x,UNWIND:]]   DATA
 # CHECK-NEXT: 3    __eh_frame    {{[0-9a-f]+}}  [[#%x,EH_FRAME:]] DATA

 # CHECK-NEXT: # Address           Size              Segment  Section
 # CHECK-NEXT: 0x[[#%X,TEXT]]      0x{{[0-9A-F]+}}   __TEXT   __text
 # CHECK-NEXT: 0x[[#%X,CSTR]]      0x{{[0-9A-F]+}}   __TEXT   __cstring
 # CHECK-NEXT: 0x[[#%X,UNWIND]]    0x{{[0-9A-F]+}}   __TEXT   __unwind_info
 # CHECK-NEXT: 0x[[#%X,EH_FRAME]]  0x{{[0-9A-F]+}}   __TEXT   __eh_frame
 # CHECK-NEXT: 0x[[#%X,BSS]]       0x{{[0-9A-F]+}}   __DATA   __common
 
 # CHECK-NEXT: # Symbols:
-# CHECK-NEXT: # Address                Size        File   Name
-# CHECK-DAG:  0x[[#%X,MAIN]]           0x00000001  [  1]  _main
-# CHECK-DAG:  0x[[#%X,BAR]]            0x00000001  [  1]  _bar
-# CHECK-DAG:  0x[[#%X,FOO]]            0x00000001  [  2]  __ZTIN3foo3bar4MethE
-# CHECK-DAG:  0x[[#%X,HIWORLD]]        0x0000000E  [  3]  literal string: Hello world!\n
-# CHECK-DAG:  0x[[#%X,HIITSME]]        0x0000000F  [  3]  literal string: Hello, it's me
-# CHECK-DAG:  0x[[#%X,HIITSME + 0xf]]  0x0000000E  [  3]  literal string: Hello world!\n
-# CHECK-DAG:  0x[[#%X,NUMBER]]         0x00000001  [  1]  _number
-# CHECK-DAG:  0x[[#%X,UNWIND]]         0x0000103C  [  0]  compact unwind info
+# CHECK-NEXT: # Address                  Size        File   Name
+# CHECK-DAG:  0x[[#%X,MAIN]]             0x00000001  [  1]  _main
+# CHECK-DAG:  0x[[#%X,BAR]]              0x00000001  [  1]  _bar
+# CHECK-DAG:  0x[[#%X,FOO]]              0x00000001  [  2]  __ZTIN3foo3bar4MethE
+# CHECK-DAG:  0x[[#%X,HIWORLD]]          0x0000000E  [  3]  literal string: Hello world!\n
+# CHECK-DAG:  0x[[#%X,HIITSME]]          0x0000000F  [  3]  literal string: Hello, it's me
+# CHECK-DAG:  0x[[#%X,HIITSME + 0xf]]    0x0000000E  [  3]  literal string: Hello world!\n
+# CHECK-DAG:  0x[[#%X,NUMBER]]           0x00000001  [  1]  _number
+# CHECK-DAG:  0x[[#%X,UNWIND]]           0x0000103C  [  0]  compact unwind info
+## Note: ld64 prints "CIE" and "FDE for: <function>" instead of "EH_Frame".
+# CHECK-DAG:  0x[[#%X,EH_FRAME]]         0x00000018  [  1]  EH_Frame
+# CHECK-DAG:  0x[[#%X,EH_FRAME + 0x18]]  0x00000020  [  1]  EH_Frame
 
 # MAPFILE: "name":"Total Write map file"
 
 # RUN: %lld -demangle -dead_strip -map %t/stripped-map %t/test.o %t/foo.o %t/c-string-literal.o -o %t/stripped
 # RUN: FileCheck --check-prefix=STRIPPED %s < %t/stripped-map
 
 ## C-string literals should be printed as "literal string: <C string literal>"
 # STRIPPED-LABEL: Dead Stripped Symbols: