[llvm] 57bd11f - [ObjectYAML][MachO] Encode export trie address as ULEB128, not as SLEB128

[Daniel Rodríguez Troitiño via llvm-commits]

Author: Daniel Rodríguez Troitiño
Date: 2022-10-04T09:17:58-07:00
New Revision: 57bd11f047d040fb52520a0ae2bc74542099a162

URL: https://github.com/llvm/llvm-project/commit/57bd11f047d040fb52520a0ae2bc74542099a162
DIFF: https://github.com/llvm/llvm-project/commit/57bd11f047d040fb52520a0ae2bc74542099a162.diff

LOG: [ObjectYAML][MachO] Encode export trie address as ULEB128, not as SLEB128

The `dumpExportEntry` was dumping everything using signed LEB128, but
the format seems to use unsigned LEB128. This can be cross-checked with
the implementation in MachOObjectFile.cpp, the implementation in LLD's
ExportTrie.cpp, and the implementation in macho2yaml.cpp, which all use
ULEB128 functions..

The difference is only apparent when encoding some values with specific
bit patterns (bit active in the 7th, 14th, ... bits of the binary). The
encoding was not always creating problems in the resulting binaries
because if the extra byte was part of the padding, the result of
decoding it as ULEB128 is the same as decoding as SLEB128, however, the
code of MachOObjectFile.cpp (used by llvm-objdump) checks the buffer
decoding position against the reported length, which triggered an error.

Modified a test that used an address with this pattern (0x3FA0, the 14th
bit is active), to show that a round trip still produces the same
results, and added a check using llvm-objdump to use their extra checks
to verify this implementation.

Reviewed By: pete

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

Added: 
    

Modified: 
    llvm/lib/ObjectYAML/MachOEmitter.cpp
    llvm/test/ObjectYAML/MachO/export_trie.yaml

Removed: 
    


################################################################################
diff  --git a/llvm/lib/ObjectYAML/MachOEmitter.cpp b/llvm/lib/ObjectYAML/MachOEmitter.cpp
index 3d06f3d0bf862..53563c422b15c 100644
--- a/llvm/lib/ObjectYAML/MachOEmitter.cpp
+++ b/llvm/lib/ObjectYAML/MachOEmitter.cpp
@@ -435,24 +435,24 @@ void MachOWriter::writeBindOpcodes(
 
 void MachOWriter::dumpExportEntry(raw_ostream &OS,
                                   MachOYAML::ExportEntry &Entry) {
-  encodeSLEB128(Entry.TerminalSize, OS);
+  encodeULEB128(Entry.TerminalSize, OS);
   if (Entry.TerminalSize > 0) {
-    encodeSLEB128(Entry.Flags, OS);
+    encodeULEB128(Entry.Flags, OS);
     if (Entry.Flags & MachO::EXPORT_SYMBOL_FLAGS_REEXPORT) {
-      encodeSLEB128(Entry.Other, OS);
+      encodeULEB128(Entry.Other, OS);
       OS << Entry.ImportName;
       OS.write('\0');
     } else {
-      encodeSLEB128(Entry.Address, OS);
+      encodeULEB128(Entry.Address, OS);
       if (Entry.Flags & MachO::EXPORT_SYMBOL_FLAGS_STUB_AND_RESOLVER)
-        encodeSLEB128(Entry.Other, OS);
+        encodeULEB128(Entry.Other, OS);
     }
   }
   OS.write(static_cast<uint8_t>(Entry.Children.size()));
   for (auto EE : Entry.Children) {
     OS << EE.Name;
     OS.write('\0');
-    encodeSLEB128(EE.NodeOffset, OS);
+    encodeULEB128(EE.NodeOffset, OS);
   }
   for (auto EE : Entry.Children)
     dumpExportEntry(OS, EE);

diff  --git a/llvm/test/ObjectYAML/MachO/export_trie.yaml b/llvm/test/ObjectYAML/MachO/export_trie.yaml
index 635e507f5f646..9c359db6b90d6 100644
--- a/llvm/test/ObjectYAML/MachO/export_trie.yaml
+++ b/llvm/test/ObjectYAML/MachO/export_trie.yaml
@@ -1,4 +1,6 @@
-# RUN: yaml2obj %s | obj2yaml | FileCheck %s
+# RUN: yaml2obj %s -o=%t
+# RUN: obj2yaml %t | FileCheck %s
+# RUN: llvm-objdump --macho --exports-trie %t | FileCheck %s --check-prefix=OBJDUMP-VERIFY
 
 --- !mach-o
 FileHeader:      
@@ -167,7 +169,7 @@ LinkEditData:
             NodeOffset:      37
             Name:            main
             Flags:           0x0000000000000000
-            Address:         0x0000000000001160
+            Address:         0x0000000000003FA0
             Other:           0x0000000000000000
             ImportName:      ''
 ...
@@ -188,4 +190,8 @@ LinkEditData:
 #CHECK:           - TerminalSize:    3
 #CHECK:             NodeOffset:      37
 #CHECK:             Name:            main
-#CHECK:             Address:         0x1160
+#CHECK:             Address:         0x3FA0
+
+# OBJDUMP-VERIFY: Exports trie:
+# OBJDUMP-VERIFY: 0x100000000  __mh_execute_header
+# OBJDUMP-VERIFY: 0x100003FA0  _main