[PATCH] D70475: [llvm-objcopy][MachO] Fix symbol order in the symbol table

[Fangrui Song via Phabricator via llvm-commits]

MaskRay created this revision.
MaskRay added reviewers: alexshap, jhenderson, seiya.
Herald added subscribers: llvm-commits, abrachet, jakehehrlich.
Herald added a reviewer: rupprecht.
Herald added a project: LLVM.

Only consider isUndefinedSymbol() when the symbol is not local. This
fixes an assert failure when copying the symbol table, if a n_type=0x20
symbol is followed by a n_type=0x64 symbol.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70475

Files:
  llvm/test/tools/llvm-objcopy/MachO/Inputs/strip-all.yaml
  llvm/test/tools/llvm-objcopy/MachO/strip-all.test
  llvm/tools/llvm-objcopy/MachO/MachOLayoutBuilder.cpp


Index: llvm/tools/llvm-objcopy/MachO/MachOLayoutBuilder.cpp
===================================================================
--- llvm/tools/llvm-objcopy/MachO/MachOLayoutBuilder.cpp
+++ llvm/tools/llvm-objcopy/MachO/MachOLayoutBuilder.cpp
@@ -64,9 +64,11 @@
   assert(std::is_sorted(O.SymTable.Symbols.begin(), O.SymTable.Symbols.end(),
                         [](const std::unique_ptr<SymbolEntry> &A,
                            const std::unique_ptr<SymbolEntry> &B) {
-                          return (A->isLocalSymbol() && !B->isLocalSymbol()) ||
-                                 (!A->isUndefinedSymbol() &&
-                                  B->isUndefinedSymbol());
+                          bool AL = A->isLocalSymbol(), BL = B->isLocalSymbol();
+                          if (AL != BL)
+                            return AL;
+                          return !AL && !A->isUndefinedSymbol() &&
+                                         B->isUndefinedSymbol();
                         }) &&
          "Symbols are not sorted by their types.");
 
Index: llvm/test/tools/llvm-objcopy/MachO/strip-all.test
===================================================================
--- llvm/test/tools/llvm-objcopy/MachO/strip-all.test
+++ llvm/test/tools/llvm-objcopy/MachO/strip-all.test
@@ -3,6 +3,10 @@
 # RUN: yaml2obj %p/Inputs/strip-all.yaml > %t.exec
 # RUN: yaml2obj %p/Inputs/strip-all-with-dwarf.yaml > %t.dwarf
 
+## Check that the symbol list satisfies the order: local / defined external /
+## undefined external, otherwise llvm-objcopy will fail.
+# RUN: llvm-objcopy %t.exec /dev/null
+
 # RUN: llvm-objcopy --strip-all %t.exec %t.exec.stripped
 # RUN: llvm-readobj --sections --relocations --symbols %t.exec.stripped \
 # RUN:   | FileCheck --check-prefixes=COMMON,EXEC %s
Index: llvm/test/tools/llvm-objcopy/MachO/Inputs/strip-all.yaml
===================================================================
--- llvm/test/tools/llvm-objcopy/MachO/Inputs/strip-all.yaml
+++ llvm/test/tools/llvm-objcopy/MachO/Inputs/strip-all.yaml
@@ -262,11 +262,13 @@
       n_sect:          3
       n_desc:          0
       n_value:         4294971392
+    ## A local undefined SymDebugTable entry followed by
     - n_strx:          175
       n_type:          0x20
       n_sect:          0
       n_desc:          0
       n_value:         0
+    ## a defined SymDebugTable entry.
     - n_strx:          1
       n_type:          0x64
       n_sect:          1


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D70475.230183.patch
Type: text/x-patch
Size: 2471 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20191120/6106503b/attachment.bin>