[llvm] 2420027 - [llvm-objcopy][MachO] Fix symbol order in the symbol table

[Fangrui Song via llvm-commits]

Author: Fangrui Song
Date: 2019-11-21T09:30:46-08:00
New Revision: 242002770ba00516339875a064f9ae0efd78a8dc

URL: https://github.com/llvm/llvm-project/commit/242002770ba00516339875a064f9ae0efd78a8dc
DIFF: https://github.com/llvm/llvm-project/commit/242002770ba00516339875a064f9ae0efd78a8dc.diff

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

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.

Reviewed By: alexshap, seiya

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

Added: 
    

Modified: 
    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

Removed: 
    


################################################################################
diff  --git a/llvm/test/tools/llvm-objcopy/MachO/Inputs/strip-all.yaml b/llvm/test/tools/llvm-objcopy/MachO/Inputs/strip-all.yaml
index 3ed78e0523c7..6bce937fd7a4 100644
--- a/llvm/test/tools/llvm-objcopy/MachO/Inputs/strip-all.yaml
+++ b/llvm/test/tools/llvm-objcopy/MachO/Inputs/strip-all.yaml
@@ -262,11 +262,13 @@ LinkEditData:
       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

diff  --git a/llvm/test/tools/llvm-objcopy/MachO/strip-all.test b/llvm/test/tools/llvm-objcopy/MachO/strip-all.test
index 176469465345..1a69234ac1bb 100644
--- a/llvm/test/tools/llvm-objcopy/MachO/strip-all.test
+++ b/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

diff  --git a/llvm/tools/llvm-objcopy/MachO/MachOLayoutBuilder.cpp b/llvm/tools/llvm-objcopy/MachO/MachOLayoutBuilder.cpp
index 006eb8ce7c49..380f2e989fe4 100644
--- a/llvm/tools/llvm-objcopy/MachO/MachOLayoutBuilder.cpp
+++ b/llvm/tools/llvm-objcopy/MachO/MachOLayoutBuilder.cpp
@@ -64,9 +64,11 @@ void MachOLayoutBuilder::updateDySymTab(MachO::macho_load_command &MLC) {
   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.");