[yaml2obj][MachO] Fix byte order of the indirect symbol table#205044
Conversation
writeDynamicSymbolTable emitted the indirect symbol table in host byte order rather than the object's byte order. Adds the missing swap and a regression test that round-trips all endian-sensitive fields with both endianness values.
|
@llvm/pr-subscribers-objectyaml Author: Sungbin Jo (goranmoomin) ChangesThis is a follow-up of PR #203678 that added the test case The failure seems to be on This PR adds the missing swap and a regression test that round-trips all endian-sensitive fields with both endianness values. Full diff: https://github.com/llvm/llvm-project/pull/205044.diff 2 Files Affected:
diff --git a/llvm/lib/ObjectYAML/MachOEmitter.cpp b/llvm/lib/ObjectYAML/MachOEmitter.cpp
index a4d17dfe1e320..cf7202c7da949 100644
--- a/llvm/lib/ObjectYAML/MachOEmitter.cpp
+++ b/llvm/lib/ObjectYAML/MachOEmitter.cpp
@@ -625,9 +625,12 @@ void MachOWriter::writeStringTable(raw_ostream &OS) {
}
void MachOWriter::writeDynamicSymbolTable(raw_ostream &OS) {
- for (auto Data : Obj.LinkEdit.IndirectSymbols)
- OS.write(reinterpret_cast<const char *>(&Data),
- sizeof(yaml::Hex32::BaseType));
+ for (auto Data : Obj.LinkEdit.IndirectSymbols) {
+ uint32_t Value = Data;
+ if (Obj.IsLittleEndian != sys::IsLittleEndianHost)
+ MachO::swapStruct(Value);
+ OS.write(reinterpret_cast<const char *>(&Value), sizeof(uint32_t));
+ }
}
void MachOWriter::writeFunctionStarts(raw_ostream &OS) {
diff --git a/llvm/test/ObjectYAML/MachO/endianness.yaml b/llvm/test/ObjectYAML/MachO/endianness.yaml
new file mode 100644
index 0000000000000..a0880e4642274
--- /dev/null
+++ b/llvm/test/ObjectYAML/MachO/endianness.yaml
@@ -0,0 +1,207 @@
+# RUN: yaml2obj -DENDIAN=false %s | obj2yaml | FileCheck %s
+# RUN: yaml2obj -DENDIAN=true %s | obj2yaml | FileCheck %s
+
+## Check that yaml2obj writes all endian-sensitive Mach-O fields in object
+## endianness rather than host endianness.
+
+# CHECK: FileHeader:
+# CHECK: magic: 0xFEEDFACF
+# CHECK: cputype: 0x1000012
+# CHECK: cpusubtype: 0x34
+# CHECK: filetype: 0x1
+# CHECK: ncmds: 5
+# CHECK: sizeofcmds: 304
+# CHECK: flags: 0x1020304
+
+# CHECK: LoadCommands:
+# CHECK: - cmd: LC_SEGMENT_64
+# CHECK: cmdsize: 152
+# CHECK: segname: __TEXT
+# CHECK: vmaddr: 2387509390608836384
+# CHECK: vmsize: 256
+# CHECK: fileoff: 336
+# CHECK: filesize: 4
+# CHECK: maxprot: 16909060
+# CHECK: initprot: 84281096
+# CHECK: nsects: 1
+# CHECK: flags: 151653132
+# CHECK: Sections:
+# CHECK: - sectname: __text
+# CHECK: segname: __TEXT
+# CHECK: addr: 0x2122232425262728
+# CHECK: size: 4
+# CHECK: offset: 0x150
+# CHECK: align: 2
+# CHECK: reloff: 0x154
+# CHECK: nreloc: 1
+# CHECK: flags: 0x80000400
+# CHECK: reserved1: 0x1020304
+# CHECK: reserved2: 0x5060708
+# CHECK: reserved3: 0x90A0B0C
+# CHECK: content: DEADBEEF
+# CHECK: relocations:
+# CHECK: - address: 0x1020304
+# CHECK: symbolnum: 1
+# CHECK: pcrel: false
+# CHECK: length: 3
+# CHECK: extern: true
+# CHECK: type: 2
+# CHECK: scattered: false
+# CHECK: value: 0
+# CHECK: - cmd: LC_SYMTAB
+# CHECK: cmdsize: 24
+# CHECK: symoff: 356
+# CHECK: nsyms: 2
+# CHECK: stroff: 396
+# CHECK: strsize: 8
+# CHECK: - cmd: LC_DYSYMTAB
+# CHECK: cmdsize: 80
+# CHECK: ilocalsym: 0
+# CHECK: nlocalsym: 1
+# CHECK: iextdefsym: 1
+# CHECK: nextdefsym: 0
+# CHECK: iundefsym: 1
+# CHECK: nundefsym: 1
+# CHECK: indirectsymoff: 388
+# CHECK: nindirectsyms: 2
+# CHECK: - cmd: LC_DATA_IN_CODE
+# CHECK: cmdsize: 16
+# CHECK: dataoff: 348
+# CHECK: datasize: 8
+# CHECK: - cmd: LC_BUILD_VERSION
+# CHECK: cmdsize: 32
+# CHECK: platform: 16909060
+# CHECK: minos: 84281096
+# CHECK: sdk: 151653132
+# CHECK: ntools: 1
+# CHECK: Tools:
+# CHECK: - tool: 219025168
+# CHECK: version: 286397204
+
+# CHECK: LinkEditData:
+# CHECK: NameList:
+# CHECK: - n_strx: 1
+# CHECK: n_type: 0x1
+# CHECK: n_sect: 1
+# CHECK: n_desc: 4660
+# CHECK: n_value: 72623859790382856
+# CHECK: - n_strx: 4
+# CHECK: n_type: 0x1
+# CHECK: n_sect: 0
+# CHECK: n_desc: 22136
+# CHECK: n_value: 1230066625199609624
+# CHECK: StringTable:
+# CHECK: - ''
+# CHECK: - _a
+# CHECK: - _b
+# CHECK: - ''
+# CHECK: IndirectSymbols: [ 0x1, 0x40000000 ]
+# CHECK: DataInCode:
+# CHECK: - Offset: 0x1020304
+# CHECK: Length: 1286
+# CHECK: Kind: 0x708
+
+--- !mach-o
+IsLittleEndian: [[ENDIAN]]
+FileHeader:
+ magic: 0xFEEDFACF
+ cputype: 0x01000012
+ cpusubtype: 0x00000034
+ filetype: 0x00000001
+ ncmds: 5
+ sizeofcmds: 304
+ flags: 0x01020304
+ reserved: 0x05060708
+LoadCommands:
+ - cmd: LC_SEGMENT_64
+ cmdsize: 152
+ segname: __TEXT
+ vmaddr: 0x2122232425262720
+ vmsize: 0x0000000000000100
+ fileoff: 336
+ filesize: 4
+ maxprot: 0x01020304
+ initprot: 0x05060708
+ nsects: 1
+ flags: 0x090A0B0C
+ Sections:
+ - sectname: __text
+ segname: __TEXT
+ addr: 0x2122232425262728
+ size: 4
+ offset: 336
+ align: 2
+ reloff: 340
+ nreloc: 1
+ flags: 0x80000400
+ reserved1: 0x01020304
+ reserved2: 0x05060708
+ reserved3: 0x090A0B0C
+ content: DEADBEEF
+ relocations:
+ - address: 0x01020304
+ symbolnum: 1
+ pcrel: false
+ length: 3
+ extern: true
+ type: 2
+ scattered: false
+ value: 0
+ - cmd: LC_SYMTAB
+ cmdsize: 24
+ symoff: 356
+ nsyms: 2
+ stroff: 396
+ strsize: 8
+ - cmd: LC_DYSYMTAB
+ cmdsize: 80
+ ilocalsym: 0
+ nlocalsym: 1
+ iextdefsym: 1
+ nextdefsym: 0
+ iundefsym: 1
+ nundefsym: 1
+ tocoff: 0
+ ntoc: 0
+ modtaboff: 0
+ nmodtab: 0
+ extrefsymoff: 0
+ nextrefsyms: 0
+ indirectsymoff: 388
+ nindirectsyms: 2
+ extreloff: 0
+ nextrel: 0
+ locreloff: 0
+ nlocrel: 0
+ - cmd: LC_DATA_IN_CODE
+ cmdsize: 16
+ dataoff: 348
+ datasize: 8
+ - cmd: LC_BUILD_VERSION
+ cmdsize: 32
+ platform: 0x01020304
+ minos: 0x05060708
+ sdk: 0x090A0B0C
+ ntools: 1
+ Tools:
+ - tool: 0x0D0E0F10
+ version: 0x11121314
+LinkEditData:
+ NameList:
+ - n_strx: 1
+ n_type: 0x01
+ n_sect: 1
+ n_desc: 0x1234
+ n_value: 0x0102030405060708
+ - n_strx: 4
+ n_type: 0x01
+ n_sect: 0
+ n_desc: 0x5678
+ n_value: 0x1112131415161718
+ StringTable: [ '', _a, _b, '' ]
+ IndirectSymbols: [ 0x1, 0x40000000 ]
+ DataInCode:
+ - Offset: 0x01020304
+ Length: 0x0506
+ Kind: 0x0708
+...
|
|
Don't forget to ping appropriate people to review the change. I've added @drodriguez (as the reviewer of the original PR) and @rorth (for the build-bot issue). (I did a cursory look and this change looks reasonable, but I don't have sufficient Mach-O knowledge to be happy to approve the change). |
Thanks, will do next time! Sorry for that. |
|
I can confirm that the patch fixes the Solaris/sparcv9 failure. Thanks. |
|
Also, (if/after this gets merged) would it be possible if we can backport this change and #203680 to the LLVM 22 branch? The context of this is: Rust Zulip: Backport LLVM change to Rust for macOS 27 support?. It would be very helpful if this change could be synced into rust's llvm tree. I was made aware that we're (unfortunately) probably at the edge of the LLVM 22 release cycle. Sorry if the change is not eligible for backporting, as I am not much aware on the general criteria/eligibility on LLVM backports. |
|
Thanks for the follow up, @goranmoomin . According to https://discourse.llvm.org/t/llvm-22-1-8-released/91084 22.1.8 is the last release expected in the 22.x branch. The criteria for accepting patches is https://llvm.org/docs/HowToReleaseLLVM.html#release-patch-rules, but I am not sure if this classifies as bug fix. It might be interesting to go through the motions in case a big problem happens in the branch and these patches might get released early. I am not a release maintainer, so everything I have written means nothing and their word is the only one valid. In any case, macOS 27 is not expected until end of September, so LLVM 23 should be in their last release candidates or even the first final version by then. Not sure how quick Rust adopts new LLVM versions, but the timing might work out. For the time being being, you might want to investigate how Rust invokes |
Thanks for the info! No problem, I'll be waiting for LLVM 23's release. Thanks for merging! |
…05044) This is a follow-up of PR llvm#203680 that added the test case `linkedit-alignment.test`, which currently fails on big-endian buildbots (see: https://lab.llvm.org/buildbot/#/builders/98/builds/3084 and https://lab.llvm.org/buildbot/#/builders/114/builds/906). The failure seems to be on `yaml2obj`, where `writeDynamicSymbolTable` emits an indirect symbol table in host byte order rather than the specified object's byte order (i.e. the `IsLittleEndian` field value). This PR adds the missing swap and a regression test that round-trips all endian-sensitive fields with both endianness values. (cherry picked from commit 6e56216)
This is a follow-up of PR #203680 that added the test case
linkedit-alignment.test, which currently fails on big-endian buildbots (see: https://lab.llvm.org/buildbot/#/builders/98/builds/3084 and https://lab.llvm.org/buildbot/#/builders/114/builds/906).The failure seems to be on
yaml2obj, wherewriteDynamicSymbolTableemits an indirect symbol table in host byte order rather than the specified object's byte order (i.e. theIsLittleEndianfield value).This PR adds the missing swap and a regression test that round-trips all endian-sensitive fields with both endianness values.