This is an archive of the discontinued LLVM Phabricator instance.

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

Authored by drodriguez on Sep 23 2022, 1:30 PM.

Details

Summary

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.

Diff Detail

Event Timeline

drodriguez created this revision.Sep 23 2022, 1:30 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 23 2022, 1:30 PM
Herald added a subscriber: hiraditya. · View Herald Transcript
drodriguez requested review of this revision.Sep 23 2022, 1:30 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 23 2022, 1:30 PM
pete accepted this revision.Oct 3 2022, 5:44 PM

Thanks for doing this! From a quick look at ld64, there are no sleb's anywhere in the Export Trie logic so LGTM.

This revision is now accepted and ready to land.Oct 3 2022, 5:44 PM