[lld] 4c49f9c - [lld-macho] Handle non-extern symbols marked as private extern

[Jez Ng via llvm-commits]

Author: Jez Ng
Date: 2021-06-18T16:36:14-04:00
New Revision: 4c49f9ceafd6bdc261e0ee157a2ea0fa943d6921

URL: https://github.com/llvm/llvm-project/commit/4c49f9ceafd6bdc261e0ee157a2ea0fa943d6921
DIFF: https://github.com/llvm/llvm-project/commit/4c49f9ceafd6bdc261e0ee157a2ea0fa943d6921.diff

LOG: [lld-macho] Handle non-extern symbols marked as private extern

Previously, we asserted that such a case was invalid, but in fact
`ld -r` can emit such symbols if the input contained a (true) private
extern, or if it contained a symbol started with "L".

Non-extern symbols marked as private extern are essentially equivalent
to regular TU-scoped symbols, so no new functionality is needed.

Reviewed By: #lld-macho, thakis

Differential Revision: https://reviews.llvm.org/D104502

Added: 
    lld/test/MachO/local-private-extern.yaml

Modified: 
    lld/MachO/InputFiles.cpp

Removed: 
    


################################################################################
diff  --git a/lld/MachO/InputFiles.cpp b/lld/MachO/InputFiles.cpp
index d4ab9a4c1e62..32527c7f4c7b 100644
--- a/lld/MachO/InputFiles.cpp
+++ b/lld/MachO/InputFiles.cpp
@@ -471,18 +471,16 @@ static macho::Symbol *createDefined(const NList &sym, StringRef name,
   //                 either reported (for non-weak symbols) or merged
   //                 (for weak symbols), but they do not go in the export
   //                 table of the output.
-  // N_PEXT: Does not occur in input files in practice,
-  //         a private extern must be external.
+  // N_PEXT: llvm-mc does not emit these, but `ld -r` (wherein ld64 emits
+  //         object files) may produce them. LLD does not yet support -r.
+  //         These are translation-unit scoped, identical to the `0` case.
   // 0: Translation-unit scoped. These are not in the symbol table during
   //    link, and not in the export table of the output either.
-
   bool isWeakDefCanBeHidden =
       (sym.n_desc & (N_WEAK_DEF | N_WEAK_REF)) == (N_WEAK_DEF | N_WEAK_REF);
 
-  if (sym.n_type & (N_EXT | N_PEXT)) {
-    assert((sym.n_type & N_EXT) && "invalid input");
+  if (sym.n_type & N_EXT) {
     bool isPrivateExtern = sym.n_type & N_PEXT;
-
     // lld's behavior for merging symbols is slightly 
diff erent from ld64:
     // ld64 picks the winning symbol based on several criteria (see
     // pickBetweenRegularAtoms() in ld64's SymbolTable.cpp), while lld
@@ -533,8 +531,7 @@ static macho::Symbol *createDefined(const NList &sym, StringRef name,
 template <class NList>
 static macho::Symbol *createAbsolute(const NList &sym, InputFile *file,
                                      StringRef name) {
-  if (sym.n_type & (N_EXT | N_PEXT)) {
-    assert((sym.n_type & N_EXT) && "invalid input");
+  if (sym.n_type & N_EXT) {
     return symtab->addDefined(name, file, nullptr, sym.n_value, /*size=*/0,
                               /*isWeakDef=*/false, sym.n_type & N_PEXT,
                               sym.n_desc & N_ARM_THUMB_DEF,

diff  --git a/lld/test/MachO/local-private-extern.yaml b/lld/test/MachO/local-private-extern.yaml
new file mode 100644
index 000000000000..a1a916fd13e5
--- /dev/null
+++ b/lld/test/MachO/local-private-extern.yaml
@@ -0,0 +1,81 @@
+## Check that local private externs -- symbols without N_EXT but with N_PEXT set
+## -- are translation-unit-scoped. These symbols may be generated by `ld -r`,
+## which emits an object file. Since LLD does not yet support `-r`, we use
+## yaml2obj to construct the input.
+
+# RUN: rm -rf %t; mkdir %t
+# RUN: yaml2obj %s > %t/foo.o
+## No duplicate symbol conflict since _foo is not extern
+# RUN: %lld -dylib %t/foo.o %t/foo.o -o %t/foo
+# RUN: llvm-nm -m %t/foo | FileCheck %s
+
+## Note that the symbols in the output are no longer marked as "was a private
+## external".
+# CHECK: (absolute) non-external _bar
+# CHECK: (absolute) non-external _bar
+# CHECK: (__DATA,__data) non-external _foo
+# CHECK: (__DATA,__data) non-external _foo
+
+--- !mach-o
+FileHeader:
+  magic:           0xFEEDFACF
+  cputype:         0x1000007
+  cpusubtype:      0x3
+  filetype:        0x1
+  ncmds:           2
+  sizeofcmds:      208
+  flags:           0x0
+  reserved:        0x0
+LoadCommands:
+  - cmd:             LC_SEGMENT_64
+    cmdsize:         152
+    segname:         ''
+    vmaddr:          0
+    vmsize:          0
+    fileoff:         272
+    filesize:        0
+    maxprot:         7
+    initprot:        7
+    nsects:          1
+    flags:           0
+    Sections:
+      - sectname:        __data
+        segname:         __DATA
+        addr:            0x0
+        size:            0
+        offset:          0x110
+        align:           0
+        reloff:          0x0
+        nreloc:          0
+        flags:           0x0
+        reserved1:       0x0
+        reserved2:       0x0
+        reserved3:       0x0
+        content:         ''
+  - cmd:             LC_SYMTAB
+    cmdsize:         24
+    symoff:          272
+    nsyms:           2
+    stroff:          304
+    strsize:         16
+LinkEditData:
+  NameList:
+    - n_strx:          2 ## _foo
+      n_type:          0x1E ## N_PEXT | N_SECT
+      n_sect:          1
+      n_desc:          32
+      n_value:         0
+    - n_strx:          7 ## _bar
+      n_type:          0x12 ## N_PEXT | N_ABS
+      n_sect:          0
+      n_desc:          0
+      n_value:         291
+  StringTable:
+    - ' '
+    - _foo
+    - _bar
+    - ''
+    - ''
+    - ''
+    - ''
+...