Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[llvm-objcopy] Add support of symbol modification flags for MachO #120895

Merged
merged 4 commits into from
Dec 24, 2024

Conversation

RIscRIpt
Copy link
Member

@RIscRIpt RIscRIpt commented Dec 22, 2024

This patch adds support of the following llvm-objcopy flags for MachO:

  • --globalize-symbol, --globalize-symbols,
  • --keep-global-symbol, -G, --keep-global-symbols,
  • --localize-symbol, -L, --localize-symbols,
  • --skip-symbol, --skip-symbols.

Code in updateAndRemoveSymbols for MachO
is kept similar to its version for ELF.

Fixes #120894

@llvmbot
Copy link
Member

llvmbot commented Dec 22, 2024

@llvm/pr-subscribers-llvm-binary-utilities

Author: Richard Dzenis (RIscRIpt)

Changes

This patch adds support of the following llvm-objcopy flags for MachO:

- --skip-symbol
- --localize-symbol
- --globalize-symbol
- --keep-global-symbol

Code in updateAndRemoveSymbols for MachO
is kept similar to its version for ELF.

Fixes #120894


Patch is 22.93 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/120895.diff

8 Files Affected:

  • (modified) llvm/lib/ObjCopy/ConfigManager.cpp (+2-4)
  • (modified) llvm/lib/ObjCopy/MachO/MachOObjcopy.cpp (+26-7)
  • (modified) llvm/lib/ObjCopy/MachO/MachOObject.cpp (+13)
  • (modified) llvm/lib/ObjCopy/MachO/MachOObject.h (+1)
  • (added) llvm/test/tools/llvm-objcopy/MachO/globalize-symbol.test (+150)
  • (added) llvm/test/tools/llvm-objcopy/MachO/keep-global-symbol.test (+163)
  • (added) llvm/test/tools/llvm-objcopy/MachO/localize-symbol.test (+147)
  • (added) llvm/test/tools/llvm-objcopy/MachO/skip-symbol.test (+163)
diff --git a/llvm/lib/ObjCopy/ConfigManager.cpp b/llvm/lib/ObjCopy/ConfigManager.cpp
index 78fc0c451e1a3b..79bbb289d16233 100644
--- a/llvm/lib/ObjCopy/ConfigManager.cpp
+++ b/llvm/lib/ObjCopy/ConfigManager.cpp
@@ -36,11 +36,9 @@ Expected<const COFFConfig &> ConfigManager::getCOFFConfig() const {
 
 Expected<const MachOConfig &> ConfigManager::getMachOConfig() const {
   if (!Common.SplitDWO.empty() || !Common.SymbolsPrefix.empty() ||
-      !Common.SymbolsPrefixRemove.empty() || !Common.SymbolsToSkip.empty() ||
+      !Common.SymbolsPrefixRemove.empty() ||
       !Common.AllocSectionsPrefix.empty() || !Common.KeepSection.empty() ||
-      !Common.SymbolsToGlobalize.empty() || !Common.SymbolsToKeep.empty() ||
-      !Common.SymbolsToLocalize.empty() ||
-      !Common.SymbolsToKeepGlobal.empty() || !Common.SectionsToRename.empty() ||
+      !Common.SymbolsToKeep.empty() || !Common.SectionsToRename.empty() ||
       !Common.UnneededSymbolsToRemove.empty() ||
       !Common.SetSectionAlignment.empty() || !Common.SetSectionFlags.empty() ||
       !Common.SetSectionType.empty() || Common.ExtractDWO ||
diff --git a/llvm/lib/ObjCopy/MachO/MachOObjcopy.cpp b/llvm/lib/ObjCopy/MachO/MachOObjcopy.cpp
index 91500c2d9dd47d..ecd9225230cd63 100644
--- a/llvm/lib/ObjCopy/MachO/MachOObjcopy.cpp
+++ b/llvm/lib/ObjCopy/MachO/MachOObjcopy.cpp
@@ -93,19 +93,38 @@ static void markSymbols(const CommonConfig &, Object &Obj) {
 static void updateAndRemoveSymbols(const CommonConfig &Config,
                                    const MachOConfig &MachOConfig,
                                    Object &Obj) {
-  for (SymbolEntry &Sym : Obj.SymTable) {
-    // Weaken symbols first to match ELFObjcopy behavior.
-    bool IsExportedAndDefined =
-        (Sym.n_type & llvm::MachO::N_EXT) &&
-        (Sym.n_type & llvm::MachO::N_TYPE) != llvm::MachO::N_UNDF;
-    if (IsExportedAndDefined &&
+  Obj.SymTable.updateSymbols([&](SymbolEntry &Sym) {
+    if (Config.SymbolsToSkip.matches(Sym.Name))
+      return;
+
+    if (!Sym.isUndefinedSymbol() && Config.SymbolsToLocalize.matches(Sym.Name))
+      Sym.n_type &= ~llvm::MachO::N_EXT;
+
+    // Note: these two globalize flags have very similar names but different
+    // meanings:
+    //
+    // --globalize-symbol: promote a symbol to global
+    // --keep-global-symbol: all symbols except for these should be made local
+    //
+    // If --globalize-symbol is specified for a given symbol, it will be
+    // global in the output file even if it is not included via
+    // --keep-global-symbol. Because of that, make sure to check
+    // --globalize-symbol second.
+    if (!Sym.isUndefinedSymbol() && !Config.SymbolsToKeepGlobal.empty() &&
+        !Config.SymbolsToKeepGlobal.matches(Sym.Name))
+      Sym.n_type &= ~llvm::MachO::N_EXT;
+
+    if (!Sym.isUndefinedSymbol() && Config.SymbolsToGlobalize.matches(Sym.Name))
+      Sym.n_type |= llvm::MachO::N_EXT;
+
+    if (Sym.isExternalSymbol() && !Sym.isUndefinedSymbol() &&
         (Config.Weaken || Config.SymbolsToWeaken.matches(Sym.Name)))
       Sym.n_desc |= llvm::MachO::N_WEAK_DEF;
 
     auto I = Config.SymbolsToRename.find(Sym.Name);
     if (I != Config.SymbolsToRename.end())
       Sym.Name = std::string(I->getValue());
-  }
+  });
 
   auto RemovePred = [&Config, &MachOConfig,
                      &Obj](const std::unique_ptr<SymbolEntry> &N) {
diff --git a/llvm/lib/ObjCopy/MachO/MachOObject.cpp b/llvm/lib/ObjCopy/MachO/MachOObject.cpp
index d593d6788e112f..9166b45f532de7 100644
--- a/llvm/lib/ObjCopy/MachO/MachOObject.cpp
+++ b/llvm/lib/ObjCopy/MachO/MachOObject.cpp
@@ -33,6 +33,19 @@ SymbolEntry *SymbolTable::getSymbolByIndex(uint32_t Index) {
       static_cast<const SymbolTable *>(this)->getSymbolByIndex(Index));
 }
 
+void SymbolTable::updateSymbols(function_ref<void(SymbolEntry &)> Callable) {
+  for (auto &Sym : Symbols)
+    Callable(*Sym);
+
+  // Partition symbols: local < defined external < undefined external.
+  auto it_ext = std::stable_partition(
+      std::begin(Symbols), std::end(Symbols),
+      [](const auto &Sym) { return Sym->isLocalSymbol(); });
+  std::stable_partition(
+      it_ext, std::end(Symbols),
+      [](const auto &Sym) { return !Sym->isUndefinedSymbol(); });
+}
+
 void SymbolTable::removeSymbols(
     function_ref<bool(const std::unique_ptr<SymbolEntry> &)> ToRemove) {
   llvm::erase_if(Symbols, ToRemove);
diff --git a/llvm/lib/ObjCopy/MachO/MachOObject.h b/llvm/lib/ObjCopy/MachO/MachOObject.h
index b3303fd291c82c..a454c4f502fd6f 100644
--- a/llvm/lib/ObjCopy/MachO/MachOObject.h
+++ b/llvm/lib/ObjCopy/MachO/MachOObject.h
@@ -142,6 +142,7 @@ struct SymbolTable {
 
   const SymbolEntry *getSymbolByIndex(uint32_t Index) const;
   SymbolEntry *getSymbolByIndex(uint32_t Index);
+  void updateSymbols(function_ref<void(SymbolEntry &)> Callable);
   void removeSymbols(
       function_ref<bool(const std::unique_ptr<SymbolEntry> &)> ToRemove);
 };
diff --git a/llvm/test/tools/llvm-objcopy/MachO/globalize-symbol.test b/llvm/test/tools/llvm-objcopy/MachO/globalize-symbol.test
new file mode 100644
index 00000000000000..a434386052482c
--- /dev/null
+++ b/llvm/test/tools/llvm-objcopy/MachO/globalize-symbol.test
@@ -0,0 +1,150 @@
+# RUN: yaml2obj %s -o %t
+# RUN: llvm-objcopy --wildcard --globalize-symbol "*" %t %t.copy
+# RUN: llvm-readobj --symbols %t.copy | FileCheck %s
+
+# CHECK: Symbols [
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name: _PrivateSymbol
+# CHECK-NEXT:     Extern
+# CHECK-NEXT:     Type: Section (0xE)
+# CHECK-NEXT:     Section: __text (0x1)
+# CHECK-NEXT:     RefType: UndefinedNonLazy (0x0)
+# CHECK-NEXT:     Flags [ (0x0)
+# CHECK-NEXT:     ]
+# CHECK-NEXT:     Value: 0x1
+# CHECK-NEXT:   }
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name: _PrivateExternalSymbol
+# CHECK-NEXT:     PrivateExtern
+# CHECK-NEXT:     Extern
+# CHECK-NEXT:     Type: Section (0xE)
+# CHECK-NEXT:     Section: __text (0x1)
+# CHECK-NEXT:     RefType: UndefinedNonLazy (0x0)
+# CHECK-NEXT:     Flags [ (0x0)
+# CHECK-NEXT:     ]
+# CHECK-NEXT:     Value: 0x2
+# CHECK-NEXT:   }
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name: _CommonSymbol
+# CHECK-NEXT:     Extern
+# CHECK-NEXT:     Type: Section (0xE)
+# CHECK-NEXT:     Section: __text (0x1)
+# CHECK-NEXT:     RefType: UndefinedNonLazy (0x0)
+# CHECK-NEXT:     Flags [ (0x0)
+# CHECK-NEXT:     ]
+# CHECK-NEXT:     Value: 0x3
+# CHECK-NEXT:   }
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name: _UndefinedExternalSymbol
+# CHECK-NEXT:     Extern
+# CHECK-NEXT:     Type: Undef (0x0)
+# CHECK-NEXT:     Section:  (0x0)
+# CHECK-NEXT:     RefType: UndefinedNonLazy (0x0)
+# CHECK-NEXT:     Flags [ (0x0)
+# CHECK-NEXT:     ]
+# CHECK-NEXT:     Value: 0x0
+# CHECK-NEXT:   }
+# CHECK-NEXT: ]
+
+--- !mach-o
+FileHeader:
+  magic:           0xFEEDFACF
+  cputype:         0x100000C
+  cpusubtype:      0x0
+  filetype:        0x2
+  ncmds:           4
+  sizeofcmds:      328
+  flags:           0x200085
+  reserved:        0x0
+LoadCommands:
+  - cmd:             LC_SEGMENT_64
+    cmdsize:         152
+    segname:         __TEXT
+    vmaddr:          4294967296
+    vmsize:          4096
+    fileoff:         0
+    filesize:        4096
+    maxprot:         5
+    initprot:        5
+    nsects:          1
+    flags:           0
+    Sections:
+      - sectname:        __text
+        segname:         __TEXT
+        addr:            0x100000FF8
+        size:            8
+        offset:          0xFF8
+        align:           2
+        reloff:          0x0
+        nreloc:          0
+        flags:           0x80000400
+        reserved1:       0x0
+        reserved2:       0x0
+        reserved3:       0x0
+        content:         00008052C0035FD6
+  - cmd:             LC_SEGMENT_64
+    cmdsize:         72
+    segname:         __LINKEDIT
+    vmaddr:          4294971392
+    vmsize:          4096
+    fileoff:         4096
+    filesize:        147
+    maxprot:         1
+    initprot:        1
+    nsects:          0
+    flags:           0
+  - cmd:             LC_SYMTAB
+    cmdsize:         24
+    symoff:          4096
+    nsyms:           4
+    stroff:          4164
+    strsize:         79
+  - cmd:             LC_DYSYMTAB
+    cmdsize:         80
+    ilocalsym:       0
+    nlocalsym:       1
+    iextdefsym:      1
+    nextdefsym:      1
+    iundefsym:       1
+    nundefsym:       1
+    tocoff:          0
+    ntoc:            0
+    modtaboff:       0
+    nmodtab:         0
+    extrefsymoff:    0
+    nextrefsyms:     0
+    indirectsymoff:  0
+    nindirectsyms:   0
+    extreloff:       0
+    nextrel:         0
+    locreloff:       0
+    nlocrel:         0
+LinkEditData:
+  NameList:
+    - n_strx:          2
+      n_type:          0x0E
+      n_sect:          1
+      n_desc:          0
+      n_value:         1
+    - n_strx:          17
+      n_type:          0x1E
+      n_sect:          1
+      n_desc:          0
+      n_value:         2
+    - n_strx:          40
+      n_type:          0x0F
+      n_sect:          1
+      n_desc:          0
+      n_value:         3
+    - n_strx:          54
+      n_type:          0x01
+      n_sect:          0
+      n_desc:          0
+      n_value:         0
+  StringTable:
+    - ' '
+    - _PrivateSymbol
+    - _PrivateExternalSymbol
+    - _CommonSymbol
+    - _UndefinedExternalSymbol
+...
diff --git a/llvm/test/tools/llvm-objcopy/MachO/keep-global-symbol.test b/llvm/test/tools/llvm-objcopy/MachO/keep-global-symbol.test
new file mode 100644
index 00000000000000..22234f7c0cfff7
--- /dev/null
+++ b/llvm/test/tools/llvm-objcopy/MachO/keep-global-symbol.test
@@ -0,0 +1,163 @@
+# RUN: yaml2obj %s -o %t
+# RUN: llvm-objcopy --keep-global-symbol _CommonSymbol %t %t.copy
+# RUN: llvm-readobj --symbols %t.copy | FileCheck %s
+
+# CHECK: Symbols [
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name: _PrivateSymbol
+# CHECK-NEXT:     Type: Section (0xE)
+# CHECK-NEXT:     Section: __text (0x1)
+# CHECK-NEXT:     RefType: UndefinedNonLazy (0x0)
+# CHECK-NEXT:     Flags [ (0x0)
+# CHECK-NEXT:     ]
+# CHECK-NEXT:     Value: 0x1
+# CHECK-NEXT:   }
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name: _PrivateExternalSymbol
+# CHECK-NEXT:     PrivateExtern
+# CHECK-NEXT:     Type: Section (0xE)
+# CHECK-NEXT:     Section: __text (0x1)
+# CHECK-NEXT:     RefType: UndefinedNonLazy (0x0)
+# CHECK-NEXT:     Flags [ (0x0)
+# CHECK-NEXT:     ]
+# CHECK-NEXT:     Value: 0x2
+# CHECK-NEXT:   }
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name: _CommonSymbol2
+# CHECK-NEXT:     Type: Section (0xE)
+# CHECK-NEXT:     Section: __text (0x1)
+# CHECK-NEXT:     RefType: UndefinedNonLazy (0x0)
+# CHECK-NEXT:     Flags [ (0x0)
+# CHECK-NEXT:     ]
+# CHECK-NEXT:     Value: 0x4
+# CHECK-NEXT:   }
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name: _CommonSymbol
+# CHECK-NEXT:     Extern
+# CHECK-NEXT:     Type: Section (0xE)
+# CHECK-NEXT:     Section: __text (0x1)
+# CHECK-NEXT:     RefType: UndefinedNonLazy (0x0)
+# CHECK-NEXT:     Flags [ (0x0)
+# CHECK-NEXT:     ]
+# CHECK-NEXT:     Value: 0x3
+# CHECK-NEXT:   }
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name: _UndefinedExternalSymbol
+# CHECK-NEXT:     Extern
+# CHECK-NEXT:     Type: Undef (0x0)
+# CHECK-NEXT:     Section:  (0x0)
+# CHECK-NEXT:     RefType: UndefinedNonLazy (0x0)
+# CHECK-NEXT:     Flags [ (0x0)
+# CHECK-NEXT:     ]
+# CHECK-NEXT:     Value: 0x0
+# CHECK-NEXT:   }
+# CHECK-NEXT: ]
+
+--- !mach-o
+FileHeader:
+  magic:           0xFEEDFACF
+  cputype:         0x100000C
+  cpusubtype:      0x0
+  filetype:        0x2
+  ncmds:           4
+  sizeofcmds:      328
+  flags:           0x200085
+  reserved:        0x0
+LoadCommands:
+  - cmd:             LC_SEGMENT_64
+    cmdsize:         152
+    segname:         __TEXT
+    vmaddr:          4294967296
+    vmsize:          4096
+    fileoff:         0
+    filesize:        4096
+    maxprot:         5
+    initprot:        5
+    nsects:          1
+    flags:           0
+    Sections:
+      - sectname:        __text
+        segname:         __TEXT
+        addr:            0x100000FF8
+        size:            8
+        offset:          0xFF8
+        align:           2
+        reloff:          0x0
+        nreloc:          0
+        flags:           0x80000400
+        reserved1:       0x0
+        reserved2:       0x0
+        reserved3:       0x0
+        content:         00008052C0035FD6
+  - cmd:             LC_SEGMENT_64
+    cmdsize:         72
+    segname:         __LINKEDIT
+    vmaddr:          4294971392
+    vmsize:          4096
+    fileoff:         4096
+    filesize:        174
+    maxprot:         1
+    initprot:        1
+    nsects:          0
+    flags:           0
+  - cmd:             LC_SYMTAB
+    cmdsize:         24
+    symoff:          4096
+    nsyms:           5
+    stroff:          4176
+    strsize:         94
+  - cmd:             LC_DYSYMTAB
+    cmdsize:         80
+    ilocalsym:       0
+    nlocalsym:       1
+    iextdefsym:      1
+    nextdefsym:      1
+    iundefsym:       1
+    nundefsym:       1
+    tocoff:          0
+    ntoc:            0
+    modtaboff:       0
+    nmodtab:         0
+    extrefsymoff:    0
+    nextrefsyms:     0
+    indirectsymoff:  0
+    nindirectsyms:   0
+    extreloff:       0
+    nextrel:         0
+    locreloff:       0
+    nlocrel:         0
+LinkEditData:
+  NameList:
+    - n_strx:          2
+      n_type:          0x0E
+      n_sect:          1
+      n_desc:          0
+      n_value:         1
+    - n_strx:          17
+      n_type:          0x1E
+      n_sect:          1
+      n_desc:          0
+      n_value:         2
+    - n_strx:          40
+      n_type:          0x0F
+      n_sect:          1
+      n_desc:          0
+      n_value:         3
+    - n_strx:          54
+      n_type:          0x0F
+      n_sect:          1
+      n_desc:          0
+      n_value:         4
+    - n_strx:          69
+      n_type:          0x01
+      n_sect:          0
+      n_desc:          0
+      n_value:         0
+  StringTable:
+    - ' '
+    - _PrivateSymbol
+    - _PrivateExternalSymbol
+    - _CommonSymbol
+    - _CommonSymbol2
+    - _UndefinedExternalSymbol
+...
diff --git a/llvm/test/tools/llvm-objcopy/MachO/localize-symbol.test b/llvm/test/tools/llvm-objcopy/MachO/localize-symbol.test
new file mode 100644
index 00000000000000..20ef72f4c32951
--- /dev/null
+++ b/llvm/test/tools/llvm-objcopy/MachO/localize-symbol.test
@@ -0,0 +1,147 @@
+# RUN: yaml2obj %s -o %t
+# RUN: llvm-objcopy --wildcard --localize-symbol "*" %t %t.copy
+# RUN: llvm-readobj --symbols %t.copy | FileCheck %s
+
+# CHECK: Symbols [
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name: _PrivateSymbol
+# CHECK-NEXT:     Type: Section (0xE)
+# CHECK-NEXT:     Section: __text (0x1)
+# CHECK-NEXT:     RefType: UndefinedNonLazy (0x0)
+# CHECK-NEXT:     Flags [ (0x0)
+# CHECK-NEXT:     ]
+# CHECK-NEXT:     Value: 0x1
+# CHECK-NEXT:   }
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name: _PrivateExternalSymbol
+# CHECK-NEXT:     PrivateExtern
+# CHECK-NEXT:     Type: Section (0xE)
+# CHECK-NEXT:     Section: __text (0x1)
+# CHECK-NEXT:     RefType: UndefinedNonLazy (0x0)
+# CHECK-NEXT:     Flags [ (0x0)
+# CHECK-NEXT:     ]
+# CHECK-NEXT:     Value: 0x2
+# CHECK-NEXT:   }
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name: _CommonSymbol
+# CHECK-NEXT:     Type: Section (0xE)
+# CHECK-NEXT:     Section: __text (0x1)
+# CHECK-NEXT:     RefType: UndefinedNonLazy (0x0)
+# CHECK-NEXT:     Flags [ (0x0)
+# CHECK-NEXT:     ]
+# CHECK-NEXT:     Value: 0x3
+# CHECK-NEXT:   }
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name: _UndefinedExternalSymbol
+# CHECK-NEXT:     Extern
+# CHECK-NEXT:     Type: Undef (0x0)
+# CHECK-NEXT:     Section:  (0x0)
+# CHECK-NEXT:     RefType: UndefinedNonLazy (0x0)
+# CHECK-NEXT:     Flags [ (0x0)
+# CHECK-NEXT:     ]
+# CHECK-NEXT:     Value: 0x0
+# CHECK-NEXT:   }
+# CHECK-NEXT: ]
+
+--- !mach-o
+FileHeader:
+  magic:           0xFEEDFACF
+  cputype:         0x100000C
+  cpusubtype:      0x0
+  filetype:        0x2
+  ncmds:           4
+  sizeofcmds:      328
+  flags:           0x200085
+  reserved:        0x0
+LoadCommands:
+  - cmd:             LC_SEGMENT_64
+    cmdsize:         152
+    segname:         __TEXT
+    vmaddr:          4294967296
+    vmsize:          4096
+    fileoff:         0
+    filesize:        4096
+    maxprot:         5
+    initprot:        5
+    nsects:          1
+    flags:           0
+    Sections:
+      - sectname:        __text
+        segname:         __TEXT
+        addr:            0x100000FF8
+        size:            8
+        offset:          0xFF8
+        align:           2
+        reloff:          0x0
+        nreloc:          0
+        flags:           0x80000400
+        reserved1:       0x0
+        reserved2:       0x0
+        reserved3:       0x0
+        content:         00008052C0035FD6
+  - cmd:             LC_SEGMENT_64
+    cmdsize:         72
+    segname:         __LINKEDIT
+    vmaddr:          4294971392
+    vmsize:          4096
+    fileoff:         4096
+    filesize:        147
+    maxprot:         1
+    initprot:        1
+    nsects:          0
+    flags:           0
+  - cmd:             LC_SYMTAB
+    cmdsize:         24
+    symoff:          4096
+    nsyms:           4
+    stroff:          4164
+    strsize:         79
+  - cmd:             LC_DYSYMTAB
+    cmdsize:         80
+    ilocalsym:       0
+    nlocalsym:       1
+    iextdefsym:      1
+    nextdefsym:      1
+    iundefsym:       1
+    nundefsym:       1
+    tocoff:          0
+    ntoc:            0
+    modtaboff:       0
+    nmodtab:         0
+    extrefsymoff:    0
+    nextrefsyms:     0
+    indirectsymoff:  0
+    nindirectsyms:   0
+    extreloff:       0
+    nextrel:         0
+    locreloff:       0
+    nlocrel:         0
+LinkEditData:
+  NameList:
+    - n_strx:          2
+      n_type:          0x0E
+      n_sect:          1
+      n_desc:          0
+      n_value:         1
+    - n_strx:          17
+      n_type:          0x1E
+      n_sect:          1
+      n_desc:          0
+      n_value:         2
+    - n_strx:          40
+      n_type:          0x0F
+      n_sect:          1
+      n_desc:          0
+      n_value:         3
+    - n_strx:          54
+      n_type:          0x01
+      n_sect:          0
+      n_desc:          0
+      n_value:         0
+  StringTable:
+    - ' '
+    - _PrivateSymbol
+    - _PrivateExternalSymbol
+    - _CommonSymbol
+    - _UndefinedExternalSymbol
+...
diff --git a/llvm/test/tools/llvm-objcopy/MachO/skip-symbol.test b/llvm/test/tools/llvm-objcopy/MachO/skip-symbol.test
new file mode 100644
index 00000000000000..3b8e4157f660b4
--- /dev/null
+++ b/llvm/test/tools/llvm-objcopy/MachO/skip-symbol.test
@@ -0,0 +1,163 @@
+# RUN: yaml2obj %s -o %t
+# RUN: llvm-objcopy --wildcard --localize-symbol "*" --skip-symbol _CommonSymbol %t %t.copy
+# RUN: llvm-readobj --symbols %t.copy | FileCheck %s
+
+# CHECK: Symbols [
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name: _PrivateSymbol
+# CHECK-NEXT:     Type: Section (0xE)
+# CHECK-NEXT:     Section: __text (0x1)
+# CHECK-NEXT:     RefType: UndefinedNonLazy (0x0)
+# CHECK-NEXT:     Flags [ (0x0)
+# CHECK-NEXT:     ]
+# CHECK-NEXT:     Value: 0x1
+# CHECK-NEXT:   }
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name: _PrivateExternalSymbol
+# CHECK-NEXT:     PrivateExtern
+# CHECK-NEXT:     Type: Section (0xE)
+# CHECK-NEXT:     Section: __text (0x1)
+# CHECK-NEXT:     RefType: UndefinedNonLazy (0x0)
+# CHECK-NEXT:     Flags [ (0x0)
+# CHECK-NEXT:     ]
+# CHECK-NEXT:     Value: 0x2
+# CHECK-NEXT:   }
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name: _CommonSymbol2
+# CHECK-NEXT:     Type: Section (0xE)
+# CHECK-NEXT:     Section: __text (0x1)
+# CHECK-NEXT:     RefType: UndefinedNonLazy (0x0)
+# CHECK-NEXT:     Flags [ (0x0)
+# CHECK-NEXT:     ]
+# CHECK-NEXT:     Value: 0x4
+# CHECK-NEXT:   }
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name: _CommonSymbol
+# CHECK-NEXT:     Extern
+# CHECK-NEXT:     Type: Section (0xE)
+# CHECK-NEXT:     Section: __text (0x1)
+# CHECK-NEXT:     RefType: UndefinedNonLazy (0x0)
+# CHECK-NEXT:     Flags [ (0x0)
+# CHECK-NEXT:     ]
+# CHECK-NEXT:     Value: 0x3
+# CHECK-NEXT:   }
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name: _UndefinedExternalSymbol
+# CHECK-NEXT:     Extern
+# CHECK-NEXT:     Type: Undef (0x...
[truncated]

Copy link

github-actions bot commented Dec 22, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@RIscRIpt
Copy link
Member Author

I was not sure if I have to adjust N_PEXT flag in case we are applying either "localization" or "globalization". So I just keep it unchanged. Please let me know if this is not correct.

@RIscRIpt RIscRIpt requested a review from MaskRay December 22, 2024 13:05
This patch adds support of the following llvm-objcopy flags for MachO:

    - --skip-symbol
    - --localize-symbol
    - --globalize-symbol
    - --keep-global-symbol

Code in `updateAndRemoveSymbols` for MachO
is kept similar to its version for ELF.
@RIscRIpt RIscRIpt force-pushed the macho-objcopy-sym-mod branch from f10c87e to 41cbb64 Compare December 22, 2024 18:20
@RIscRIpt
Copy link
Member Author

Fixed tests on Windows, and rebased onto main. I did a force-push, because none has submitted review comments yet.

Copy link
Collaborator

@jh7370 jh7370 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update the docs (llvm/docs/CommandGuide/llvm-objcopy.rst) to move the relevant options from the ELF-specific section to the generic options section.

Am I right in thinking that this automatically supports the file-based inputs like --globalize-symbols too, because this bit is in the generic code? If so, please update the tests to cover them and the PR description to mention this.

llvm/lib/ObjCopy/MachO/MachOObjcopy.cpp Outdated Show resolved Hide resolved
llvm/test/tools/llvm-objcopy/MachO/globalize-symbol.test Outdated Show resolved Hide resolved
@RIscRIpt
Copy link
Member Author

Please update the docs (llvm/docs/CommandGuide/llvm-objcopy.rst) to move the relevant options from the ELF-specific section to the generic options section.

Done. I've also added a short notes to each option that it is supported only for ELF and MachO. Please check it.

Additionally, I've updated llvm/ReleaseNotes.md

Am I right in thinking that this automatically supports the file-based inputs like --globalize-symbols too, because this bit is in the generic code? If so, please update the tests to cover them and the PR description to mention this.

That's right. I've updated PR description, and tests.

Copy link
Collaborator

@jh7370 jh7370 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM from my point of view, but please wait for @alexander-shaposhnikov, as I don't know Mach-O really, so can't comment on the format-specific details.

llvm/docs/CommandGuide/llvm-objcopy.rst Outdated Show resolved Hide resolved
llvm/docs/ReleaseNotes.md Outdated Show resolved Hide resolved
llvm/docs/ReleaseNotes.md Outdated Show resolved Hide resolved
llvm/docs/ReleaseNotes.md Outdated Show resolved Hide resolved
@RIscRIpt
Copy link
Member Author

Thank you for the review @jh7370

@alexander-shaposhnikov
Copy link
Collaborator

LG

@RIscRIpt
Copy link
Member Author

Thank you for reviews! I took another look at my tests and realized that I can remove LC_DYSYMTAB load command - that's what I did in the another fixup commit.

I'll merge this PR once all pre-merge checks pass.

@RIscRIpt RIscRIpt merged commit 334a576 into llvm:main Dec 24, 2024
9 checks passed
@RIscRIpt RIscRIpt deleted the macho-objcopy-sym-mod branch December 24, 2024 14:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

llvm-objcopy MachO: does not support localizing and globalizing symbols
4 participants