[llvm-objcopy][MachO] Align __LINKEDIT entries to pointer size#203680
Merged
drodriguez merged 1 commit intoJun 20, 2026
Conversation
|
Hello @goranmoomin 👋 Thank you for submitting a Pull Request (PR) to the LLVM Project. Since this is your first PR, here are a few useful links covering our main contribution policies and review practices.
Please reply to this message to confirm that you have read these policies, especially the LLVM AI Tool Use Policy, and that any AI tool usage has been noted in the PR description. Frequently asked questionsHow do I add reviewers? This PR will be automatically labeled, and the relevant teams will be notified. For some parts of the project, reviewers may also be added automatically. You can also add reviewers manually using the Reviewers section on this page. If you cannot use that section, it is probably because you do not have write permissions for the repository. In that case, you can request a review by tagging reviewers in a comment using What if there are no comments? If you have not received any comments on your PR after a week, you can request a review by pinging the PR with a comment such as “Ping”. The common courtesy ping rate is once a week. Please remember that you are asking for volunteer time from other developers. Are any special GitHub settings required to contribute to LLVM? We only require contributors to have a public email address associated with their GitHub commits, see this section of LLVM Developer Policy for details. If you have questions, feel free to leave a comment on this PR, or ask on LLVM Discord or LLVM Discourse. Thank you, |
|
@llvm/pr-subscribers-llvm-binary-utilities Author: Sungbin Jo (goranmoomin) ChangesAlign Mach-O __LINKEDIT entries to the target pointer size when building the tail layout. This matches the behavior of ld64 and lld-macho. dyld on macOS 27 rejects loading dylibs with misaligned __LINKEDIT entries. See #203678 for the motivation of this fix. Patch is 23.83 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/203680.diff 6 Files Affected:
diff --git a/llvm/lib/ObjCopy/MachO/MachOLayoutBuilder.cpp b/llvm/lib/ObjCopy/MachO/MachOLayoutBuilder.cpp
index 8660c903c617d..2f720454dc609 100644
--- a/llvm/lib/ObjCopy/MachO/MachOLayoutBuilder.cpp
+++ b/llvm/lib/ObjCopy/MachO/MachOLayoutBuilder.cpp
@@ -10,6 +10,7 @@
#include "llvm/Support/Alignment.h"
#include "llvm/Support/Errc.h"
#include "llvm/Support/ErrorHandling.h"
+#include <utility>
using namespace llvm;
using namespace llvm::objcopy::macho;
@@ -235,18 +236,20 @@ Error MachOLayoutBuilder::layoutTail(uint64_t Offset) {
"Incorrect tail offset");
Offset = std::max(Offset, HeaderSize + O.Header.SizeOfCmds);
+ const uint64_t LinkEditAlign = Is64Bit ? 8 : 4;
+
// The exports trie can be in either LC_DYLD_INFO or in
// LC_DYLD_EXPORTS_TRIE, but not both.
- size_t DyldInfoExportsTrieSize = 0;
- size_t DyldExportsTrieSize = 0;
+ uint64_t DyldInfoExportsTrieSize = 0;
+ uint64_t DyldExportsTrieSize = 0;
for (const auto &LC : O.LoadCommands) {
switch (LC.MachOLoadCommand.load_command_data.cmd) {
case MachO::LC_DYLD_INFO:
case MachO::LC_DYLD_INFO_ONLY:
- DyldInfoExportsTrieSize = O.Exports.Trie.size();
+ DyldInfoExportsTrieSize = O.Exports.getSize(LinkEditAlign);
break;
case MachO::LC_DYLD_EXPORTS_TRIE:
- DyldExportsTrieSize = O.Exports.Trie.size();
+ DyldExportsTrieSize = O.Exports.getSize(LinkEditAlign);
break;
default:
break;
@@ -263,28 +266,42 @@ Error MachOLayoutBuilder::layoutTail(uint64_t Offset) {
// trie, chained fixups, dyld exports trie, function starts, data-in-code,
// symbol table, indirect symbol table, symbol table strings,
// dylib codesign drs, and code signature.
- auto updateOffset = [&Offset](size_t Size) {
+ auto updateOffset = [&Offset, LinkEditAlign](uint64_t Size) {
+ Offset = alignTo(Offset, LinkEditAlign);
uint64_t PreviousOffset = Offset;
- Offset += Size;
- return PreviousOffset;
+ uint64_t PaddedSize = alignTo(Size, LinkEditAlign);
+ Offset += PaddedSize;
+ return std::make_pair(PreviousOffset, PaddedSize);
};
- uint64_t StartOfRebaseInfo = updateOffset(O.Rebases.Opcodes.size());
- uint64_t StartOfBindingInfo = updateOffset(O.Binds.Opcodes.size());
- uint64_t StartOfWeakBindingInfo = updateOffset(O.WeakBinds.Opcodes.size());
- uint64_t StartOfLazyBindingInfo = updateOffset(O.LazyBinds.Opcodes.size());
- uint64_t StartOfExportTrie = updateOffset(DyldInfoExportsTrieSize);
- uint64_t StartOfChainedFixups = updateOffset(O.ChainedFixups.Data.size());
- uint64_t StartOfDyldExportsTrie = updateOffset(DyldExportsTrieSize);
- uint64_t StartOfFunctionStarts = updateOffset(O.FunctionStarts.Data.size());
- uint64_t StartOfDataInCode = updateOffset(O.DataInCode.Data.size());
- uint64_t StartOfLinkerOptimizationHint =
- updateOffset(O.LinkerOptimizationHint.Data.size());
- uint64_t StartOfSymbols = updateOffset(NListSize * O.SymTable.Symbols.size());
+ auto [StartOfRebaseInfo, RebaseInfoSize] =
+ updateOffset(O.Rebases.getSize(LinkEditAlign));
+ auto [StartOfBindingInfo, BindingInfoSize] =
+ updateOffset(O.Binds.getSize(LinkEditAlign));
+ auto [StartOfWeakBindingInfo, WeakBindingInfoSize] =
+ updateOffset(O.WeakBinds.getSize(LinkEditAlign));
+ auto [StartOfLazyBindingInfo, LazyBindingInfoSize] =
+ updateOffset(O.LazyBinds.getSize(LinkEditAlign));
+ auto [StartOfExportTrie, ExportTrieSize] =
+ updateOffset(DyldInfoExportsTrieSize);
+ auto [StartOfChainedFixups, ChainedFixupsSize] =
+ updateOffset(O.ChainedFixups.getSize(LinkEditAlign));
+ auto [StartOfDyldExportsTrie, DyldExportsTrieSizeOut] =
+ updateOffset(DyldExportsTrieSize);
+ auto [StartOfFunctionStarts, FunctionStartsSize] =
+ updateOffset(O.FunctionStarts.getSize(LinkEditAlign));
+ auto [StartOfDataInCode, DataInCodeSize] =
+ updateOffset(O.DataInCode.getSize(LinkEditAlign));
+ auto [StartOfLinkerOptimizationHint, LinkerOptimizationHintSize] =
+ updateOffset(O.LinkerOptimizationHint.getSize(LinkEditAlign));
+ uint64_t StartOfSymbols =
+ updateOffset(NListSize * O.SymTable.Symbols.size()).first;
uint64_t StartOfIndirectSymbols =
- updateOffset(sizeof(uint32_t) * O.IndirectSymTable.Symbols.size());
- uint64_t StartOfSymbolStrings = updateOffset(StrTableBuilder.getSize());
- uint64_t StartOfDylibCodeSignDRs = updateOffset(O.DylibCodeSignDRs.Data.size());
+ updateOffset(sizeof(uint32_t) * O.IndirectSymTable.Symbols.size()).first;
+ auto [StartOfSymbolStrings, SymbolStringsSize] =
+ updateOffset(StrTableBuilder.getSize());
+ auto [StartOfDylibCodeSignDRs, DylibCodeSignDRsSize] =
+ updateOffset(O.DylibCodeSignDRs.getSize(LinkEditAlign));
uint64_t StartOfCodeSignature = Offset;
uint32_t CodeSignatureSize = 0;
@@ -343,13 +360,13 @@ Error MachOLayoutBuilder::layoutTail(uint64_t Offset) {
break;
case MachO::LC_DYLIB_CODE_SIGN_DRS:
MLC.linkedit_data_command_data.dataoff = StartOfDylibCodeSignDRs;
- MLC.linkedit_data_command_data.datasize = O.DylibCodeSignDRs.Data.size();
+ MLC.linkedit_data_command_data.datasize = DylibCodeSignDRsSize;
break;
case MachO::LC_SYMTAB:
MLC.symtab_command_data.symoff = StartOfSymbols;
MLC.symtab_command_data.nsyms = O.SymTable.Symbols.size();
MLC.symtab_command_data.stroff = StartOfSymbolStrings;
- MLC.symtab_command_data.strsize = StrTableBuilder.getSize();
+ MLC.symtab_command_data.strsize = SymbolStringsSize;
break;
case MachO::LC_DYSYMTAB: {
if (MLC.dysymtab_command_data.ntoc != 0 ||
@@ -368,42 +385,41 @@ Error MachOLayoutBuilder::layoutTail(uint64_t Offset) {
}
case MachO::LC_DATA_IN_CODE:
MLC.linkedit_data_command_data.dataoff = StartOfDataInCode;
- MLC.linkedit_data_command_data.datasize = O.DataInCode.Data.size();
+ MLC.linkedit_data_command_data.datasize = DataInCodeSize;
break;
case MachO::LC_LINKER_OPTIMIZATION_HINT:
MLC.linkedit_data_command_data.dataoff = StartOfLinkerOptimizationHint;
- MLC.linkedit_data_command_data.datasize =
- O.LinkerOptimizationHint.Data.size();
+ MLC.linkedit_data_command_data.datasize = LinkerOptimizationHintSize;
break;
case MachO::LC_FUNCTION_STARTS:
MLC.linkedit_data_command_data.dataoff = StartOfFunctionStarts;
- MLC.linkedit_data_command_data.datasize = O.FunctionStarts.Data.size();
+ MLC.linkedit_data_command_data.datasize = FunctionStartsSize;
break;
case MachO::LC_DYLD_CHAINED_FIXUPS:
MLC.linkedit_data_command_data.dataoff = StartOfChainedFixups;
- MLC.linkedit_data_command_data.datasize = O.ChainedFixups.Data.size();
+ MLC.linkedit_data_command_data.datasize = ChainedFixupsSize;
break;
case MachO::LC_DYLD_EXPORTS_TRIE:
MLC.linkedit_data_command_data.dataoff = StartOfDyldExportsTrie;
- MLC.linkedit_data_command_data.datasize = DyldExportsTrieSize;
+ MLC.linkedit_data_command_data.datasize = DyldExportsTrieSizeOut;
break;
case MachO::LC_DYLD_INFO:
case MachO::LC_DYLD_INFO_ONLY:
MLC.dyld_info_command_data.rebase_off =
O.Rebases.Opcodes.empty() ? 0 : StartOfRebaseInfo;
- MLC.dyld_info_command_data.rebase_size = O.Rebases.Opcodes.size();
+ MLC.dyld_info_command_data.rebase_size = RebaseInfoSize;
MLC.dyld_info_command_data.bind_off =
O.Binds.Opcodes.empty() ? 0 : StartOfBindingInfo;
- MLC.dyld_info_command_data.bind_size = O.Binds.Opcodes.size();
+ MLC.dyld_info_command_data.bind_size = BindingInfoSize;
MLC.dyld_info_command_data.weak_bind_off =
O.WeakBinds.Opcodes.empty() ? 0 : StartOfWeakBindingInfo;
- MLC.dyld_info_command_data.weak_bind_size = O.WeakBinds.Opcodes.size();
+ MLC.dyld_info_command_data.weak_bind_size = WeakBindingInfoSize;
MLC.dyld_info_command_data.lazy_bind_off =
O.LazyBinds.Opcodes.empty() ? 0 : StartOfLazyBindingInfo;
- MLC.dyld_info_command_data.lazy_bind_size = O.LazyBinds.Opcodes.size();
+ MLC.dyld_info_command_data.lazy_bind_size = LazyBindingInfoSize;
MLC.dyld_info_command_data.export_off =
O.Exports.Trie.empty() ? 0 : StartOfExportTrie;
- MLC.dyld_info_command_data.export_size = DyldInfoExportsTrieSize;
+ MLC.dyld_info_command_data.export_size = ExportTrieSize;
break;
// Note that LC_ENCRYPTION_INFO.cryptoff despite its name and the comment in
// <mach-o/loader.h> is not an offset in the binary file, instead, it is a
diff --git a/llvm/lib/ObjCopy/MachO/MachOObject.h b/llvm/lib/ObjCopy/MachO/MachOObject.h
index 86c6b120fa6c3..8ea5c01c466fe 100644
--- a/llvm/lib/ObjCopy/MachO/MachOObject.h
+++ b/llvm/lib/ObjCopy/MachO/MachOObject.h
@@ -13,6 +13,7 @@
#include "llvm/BinaryFormat/MachO.h"
#include "llvm/MC/StringTableBuilder.h"
#include "llvm/ObjectYAML/DWARFYAML.h"
+#include "llvm/Support/Alignment.h"
#include "llvm/Support/StringSaver.h"
#include "llvm/Support/YAMLTraits.h"
#include <cstdint>
@@ -213,6 +214,10 @@ struct RebaseInfo {
// At the moment we do not parse this info (and it is simply copied over),
// but the proper support will be added later.
ArrayRef<uint8_t> Opcodes;
+
+ uint64_t getSize(uint64_t Alignment) const {
+ return alignTo(Opcodes.size(), Alignment);
+ }
};
/// The location of the bind info inside the binary is described by
@@ -229,6 +234,10 @@ struct BindInfo {
// At the moment we do not parse this info (and it is simply copied over),
// but the proper support will be added later.
ArrayRef<uint8_t> Opcodes;
+
+ uint64_t getSize(uint64_t Alignment) const {
+ return alignTo(Opcodes.size(), Alignment);
+ }
};
/// The location of the weak bind info inside the binary is described by
@@ -247,6 +256,10 @@ struct WeakBindInfo {
// At the moment we do not parse this info (and it is simply copied over),
// but the proper support will be added later.
ArrayRef<uint8_t> Opcodes;
+
+ uint64_t getSize(uint64_t Alignment) const {
+ return alignTo(Opcodes.size(), Alignment);
+ }
};
/// The location of the lazy bind info inside the binary is described by
@@ -260,6 +273,10 @@ struct WeakBindInfo {
/// to lazy_bind_off to get the information on what to bind.
struct LazyBindInfo {
ArrayRef<uint8_t> Opcodes;
+
+ uint64_t getSize(uint64_t Alignment) const {
+ return alignTo(Opcodes.size(), Alignment);
+ }
};
/// The location of the export info inside the binary is described by
@@ -290,10 +307,18 @@ struct LazyBindInfo {
/// edge points to.
struct ExportInfo {
ArrayRef<uint8_t> Trie;
+
+ uint64_t getSize(uint64_t Alignment) const {
+ return alignTo(Trie.size(), Alignment);
+ }
};
struct LinkData {
ArrayRef<uint8_t> Data;
+
+ uint64_t getSize(uint64_t Alignment) const {
+ return alignTo(Data.size(), Alignment);
+ }
};
struct Object {
diff --git a/llvm/lib/ObjCopy/MachO/MachOWriter.cpp b/llvm/lib/ObjCopy/MachO/MachOWriter.cpp
index 07514dd2f8d6a..911e22927e559 100644
--- a/llvm/lib/ObjCopy/MachO/MachOWriter.cpp
+++ b/llvm/lib/ObjCopy/MachO/MachOWriter.cpp
@@ -55,29 +55,29 @@ size_t MachOWriter::totalSize() const {
O.LoadCommands[*O.DyLdInfoCommandIndex]
.MachOLoadCommand.dyld_info_command_data;
if (DyLdInfoCommand.rebase_off) {
- assert((DyLdInfoCommand.rebase_size == O.Rebases.Opcodes.size()) &&
+ assert((DyLdInfoCommand.rebase_size >= O.Rebases.Opcodes.size()) &&
"Incorrect rebase opcodes size");
Ends.push_back(DyLdInfoCommand.rebase_off + DyLdInfoCommand.rebase_size);
}
if (DyLdInfoCommand.bind_off) {
- assert((DyLdInfoCommand.bind_size == O.Binds.Opcodes.size()) &&
+ assert((DyLdInfoCommand.bind_size >= O.Binds.Opcodes.size()) &&
"Incorrect bind opcodes size");
Ends.push_back(DyLdInfoCommand.bind_off + DyLdInfoCommand.bind_size);
}
if (DyLdInfoCommand.weak_bind_off) {
- assert((DyLdInfoCommand.weak_bind_size == O.WeakBinds.Opcodes.size()) &&
+ assert((DyLdInfoCommand.weak_bind_size >= O.WeakBinds.Opcodes.size()) &&
"Incorrect weak bind opcodes size");
Ends.push_back(DyLdInfoCommand.weak_bind_off +
DyLdInfoCommand.weak_bind_size);
}
if (DyLdInfoCommand.lazy_bind_off) {
- assert((DyLdInfoCommand.lazy_bind_size == O.LazyBinds.Opcodes.size()) &&
+ assert((DyLdInfoCommand.lazy_bind_size >= O.LazyBinds.Opcodes.size()) &&
"Incorrect lazy bind opcodes size");
Ends.push_back(DyLdInfoCommand.lazy_bind_off +
DyLdInfoCommand.lazy_bind_size);
}
if (DyLdInfoCommand.export_off) {
- assert((DyLdInfoCommand.export_size == O.Exports.Trie.size()) &&
+ assert((DyLdInfoCommand.export_size >= O.Exports.Trie.size()) &&
"Incorrect trie size");
Ends.push_back(DyLdInfoCommand.export_off + DyLdInfoCommand.export_size);
}
@@ -320,7 +320,7 @@ void MachOWriter::writeRebaseInfo() {
O.LoadCommands[*O.DyLdInfoCommandIndex]
.MachOLoadCommand.dyld_info_command_data;
char *Out = Buf->getBufferStart() + DyLdInfoCommand.rebase_off;
- assert((DyLdInfoCommand.rebase_size == O.Rebases.Opcodes.size()) &&
+ assert((DyLdInfoCommand.rebase_size >= O.Rebases.Opcodes.size()) &&
"Incorrect rebase opcodes size");
memcpy(Out, O.Rebases.Opcodes.data(), O.Rebases.Opcodes.size());
}
@@ -332,7 +332,7 @@ void MachOWriter::writeBindInfo() {
O.LoadCommands[*O.DyLdInfoCommandIndex]
.MachOLoadCommand.dyld_info_command_data;
char *Out = Buf->getBufferStart() + DyLdInfoCommand.bind_off;
- assert((DyLdInfoCommand.bind_size == O.Binds.Opcodes.size()) &&
+ assert((DyLdInfoCommand.bind_size >= O.Binds.Opcodes.size()) &&
"Incorrect bind opcodes size");
memcpy(Out, O.Binds.Opcodes.data(), O.Binds.Opcodes.size());
}
@@ -344,7 +344,7 @@ void MachOWriter::writeWeakBindInfo() {
O.LoadCommands[*O.DyLdInfoCommandIndex]
.MachOLoadCommand.dyld_info_command_data;
char *Out = Buf->getBufferStart() + DyLdInfoCommand.weak_bind_off;
- assert((DyLdInfoCommand.weak_bind_size == O.WeakBinds.Opcodes.size()) &&
+ assert((DyLdInfoCommand.weak_bind_size >= O.WeakBinds.Opcodes.size()) &&
"Incorrect weak bind opcodes size");
memcpy(Out, O.WeakBinds.Opcodes.data(), O.WeakBinds.Opcodes.size());
}
@@ -356,7 +356,7 @@ void MachOWriter::writeLazyBindInfo() {
O.LoadCommands[*O.DyLdInfoCommandIndex]
.MachOLoadCommand.dyld_info_command_data;
char *Out = Buf->getBufferStart() + DyLdInfoCommand.lazy_bind_off;
- assert((DyLdInfoCommand.lazy_bind_size == O.LazyBinds.Opcodes.size()) &&
+ assert((DyLdInfoCommand.lazy_bind_size >= O.LazyBinds.Opcodes.size()) &&
"Incorrect lazy bind opcodes size");
memcpy(Out, O.LazyBinds.Opcodes.data(), O.LazyBinds.Opcodes.size());
}
@@ -368,7 +368,7 @@ void MachOWriter::writeExportInfo() {
O.LoadCommands[*O.DyLdInfoCommandIndex]
.MachOLoadCommand.dyld_info_command_data;
char *Out = Buf->getBufferStart() + DyLdInfoCommand.export_off;
- assert((DyLdInfoCommand.export_size == O.Exports.Trie.size()) &&
+ assert((DyLdInfoCommand.export_size >= O.Exports.Trie.size()) &&
"Incorrect export trie size");
memcpy(Out, O.Exports.Trie.data(), O.Exports.Trie.size());
}
@@ -398,7 +398,7 @@ void MachOWriter::writeLinkData(std::optional<size_t> LCIndex,
const MachO::linkedit_data_command &LinkEditDataCommand =
O.LoadCommands[*LCIndex].MachOLoadCommand.linkedit_data_command_data;
char *Out = Buf->getBufferStart() + LinkEditDataCommand.dataoff;
- assert((LinkEditDataCommand.datasize == LD.Data.size()) &&
+ assert((LinkEditDataCommand.datasize >= LD.Data.size()) &&
"Incorrect data size");
memcpy(Out, LD.Data.data(), LD.Data.size());
}
@@ -575,7 +575,7 @@ void MachOWriter::writeExportsTrieData() {
O.LoadCommands[*O.ExportsTrieCommandIndex]
.MachOLoadCommand.linkedit_data_command_data;
char *Out = Buf->getBufferStart() + ExportsTrieCmd.dataoff;
- assert((ExportsTrieCmd.datasize == O.Exports.Trie.size()) &&
+ assert((ExportsTrieCmd.datasize >= O.Exports.Trie.size()) &&
"Incorrect export trie size");
memcpy(Out, O.Exports.Trie.data(), O.Exports.Trie.size());
}
diff --git a/llvm/test/tools/llvm-objcopy/MachO/linkedit-order-1.test b/llvm/test/tools/llvm-objcopy/MachO/linkedit-order-1.test
index 4915f344c1e7e..1ca55ba46be74 100644
--- a/llvm/test/tools/llvm-objcopy/MachO/linkedit-order-1.test
+++ b/llvm/test/tools/llvm-objcopy/MachO/linkedit-order-1.test
@@ -14,24 +14,25 @@
# CHECK: fileoff: [[#LINKEDIT_FILEOFF:]]
# CHECK: filesize: [[#LINKEDIT_FILESIZE:]]
+## LINKEDIT entries are aligned to the target word size.
# CHECK: cmd: LC_DYLD_INFO_ONLY
-# CHECK: rebase_off: [[#REBASE_OFF: LINKEDIT_FILEOFF]]
+# CHECK: rebase_off: [[#REBASE_OFF: mul(div(LINKEDIT_FILEOFF + 7, 8), 8)]]
# CHECK: rebase_size: [[#REBASE_SIZE:]]
-# CHECK: bind_off: [[#BIND_OFF: REBASE_OFF + REBASE_SIZE]]
+# CHECK: bind_off: [[#BIND_OFF: mul(div(REBASE_OFF + REBASE_SIZE + 7, 8), 8)]]
# CHECK: bind_size: [[#BIND_SIZE:]]
-# CHECK: weak_bind_off: [[#WEAK_BIND_OFF: BIND_OFF + BIND_SIZE]]
+# CHECK: weak_bind_off: [[#WEAK_BIND_OFF: mul(div(BIND_OFF + BIND_SIZE + 7, 8), 8)]]
# CHECK: weak_bind_size: [[#WEAK_BIND_SIZE:]]
-# CHECK: lazy_bind_off: [[#LAZY_BIND_OFF: WEAK_BIND_OFF + WEAK_BIND_SIZE]]
+# CHECK: lazy_bind_off: [[#LAZY_BIND_OFF: mul(div(WEAK_BIND_OFF + WEAK_BIND_SIZE + 7, 8), 8)]]
# CHECK: lazy_bind_size: [[#LAZY_BIND_SIZE:]]
-# CHECK: export_off: [[#EXPORTS_OFF:LAZY_BIND_OFF + LAZY_BIND_SIZE]]
+# CHECK: export_off: [[#EXPORTS_OFF: mul(div(LAZY_BIND_OFF + LAZY_BIND_SIZE + 7, 8), 8)]]
# CHECK: export_size: [[#EXPORTS_SIZE:]]
# CHECK: cmd: LC_FUNCTION_STARTS
-# CHECK: dataoff: [[#FUNCTION_STARTS_FILEOFF: EXPORTS_OFF + EXPORTS_SIZE]]
+# CHECK: dataoff: [[#FUNCTION_STARTS_FILEOFF: mul(div(EXPORTS_OFF + EXPORTS_SIZE + 7, 8), 8)]]
# CHECK: datasize: [[#FUNCTION_STARTS_FILESIZE:]]
# CHECK: cmd: LC_DATA_IN_CODE
-# CHECK: dataoff: [[#DATA_IN_CODE_FILEOFF: FUNCTION_STARTS_FILEOFF + FUNCTION_STARTS_FILESIZE]]
+# CHECK: dataoff: [[#DATA_IN_CODE_FILEOFF: mul(div(FUNCTION_STARTS_FILEOFF + FUNCTION_STARTS_FILESIZE + 7, 8), 8)]]
# CHECK: datasize: [[#DATA_IN_CODE_FILESIZE:]]
## Jump over LC_CODE_SIGNATURE, which needs to be checked last
@@ -39,24 +40,24 @@
# CHECK: --- !mach-o
# CHECK: cmd: LC_SYMTAB
-# CHECK: symoff: [[#SYMTAB_SYMOFF: DATA_IN_CODE_FILEOFF + DATA_IN_CODE_FILESIZE]]
+# CHECK: symoff: [[#SYMTAB_SYMOFF: mul(div(DATA_IN_CODE_FILEOFF + DATA_IN_CODE_FILESIZE + 7, 8), 8)]]
# CHECK: nsyms: [[#SYMTAB_NSYMS:]]
## Skip over the strings table offset/size (part of LC_SYMTAB) until next loop.
# CHECK: cmd: LC_DYSYMTAB
-# CHECK: indirectsymoff: [[#DYSYMTAB_INDIRECTSYMOFF: SYMTAB_SYMOFF + mul(SYMTAB_NSYMS, 16)]]
+# CHECK: indirectsymoff: [[#DYSYMTAB_INDIRECTSYMOFF: mul(div(SYMTAB_SYMOFF + mul(SYMTAB_NSYMS, 16) + 7, 8), 8)]]
# CHECK: nindirectsyms: [[#DYSYMTAB_NINDIRECTSYMS:]]
# CHECK: --- !mach-o
# CHECK: cmd: LC_SYMTAB
-# CHECK: stroff: [[#SYMTAB_STROFF: DYSYMTAB_INDIRECTSYMOFF + mul(DYSYMTAB_NINDIRECTSYMS, 4)]]
+# CHECK: stroff: [[#SYMTAB_STROFF: mul(div(DYSYMTAB_INDIRECTSYMOFF + mul(DYSYMTAB_NINDIRECTSYMS, 4) + 7, 8), 8)]]
# CHECK: strsize: [[#SYMTAB_STRSIZE:]]
# CHECK: cmd: LC_CODE_SIGNATURE
## LC_CODE_SIGNATURE needs to be aligned to 16 bytes boundaries.
-# CHECK: dataoff: [[#CODE_SIGNATURE_FILEOFF: mul(div(SYMTAB_STROFF + SYMTAB_STRSIZE + 8, 16), 16)]]
+# CHECK: dataoff: [[#CODE_SIGNATURE_FILEOFF: mul(div(SYMTAB_STROFF + SYMTAB_STRSIZE + 15, 16), 16)]]
# CHECK: datasize: [[#CODE_SIGNATURE_FILESIZE:LINKEDIT_FILEOFF + LINKEDIT_FILESIZE - CODE_SIGNATURE_FILEOFF]]
--- !mach-o
diff --git a/llvm/test/tools/llvm-objcopy/MachO/linkedit-order-2.test b/llvm/test/tools/llvm-objcopy/MachO/linkedit-order-2.test
index 0c5521ece0e44..3ed61e301d841 100644
--- a/llvm/test/tools/llvm-objcopy/MachO/linkedit-order-2.test
+++ b/llvm/test/tools/llvm-objcopy/MachO/linkedit-order-2.test
@@ -1...
[truncated]
|
Contributor
Author
Yes, I did. |
jh7370
reviewed
Jun 15, 2026
jh7370
left a comment
Contributor
There was a problem hiding this comment.
Not looked in detail at the testing as much of it goes over my head due to a lack of Mach-O knowledge, but I did want to check, is there testing for both 4 byte and 8 byte alignment cases?
I'll add a few reviewers to help check the Mach-O specifics of this PR.
9c7af73 to
491d100
Compare
Contributor
Author
So the two I wanted to introduce a new test that verifies that all sections are aligned per word size (in both architectures), but currently struggling in that mostly because I'm not sure how to check whether a variable is 4/8-byte aligned (i.e. mod 8 == 0) in FileCheck. |
491d100 to
52dd986
Compare
Contributor
Author
|
I added a |
drodriguez
requested changes
Jun 15, 2026
drodriguez
left a comment
Contributor
There was a problem hiding this comment.
I think the end result is good, but I think some changes might make the intention of the code a little bit clearer.
drodriguez
reviewed
Jun 15, 2026
9a12a6c to
e2b59f5
Compare
drodriguez
approved these changes
Jun 16, 2026
drodriguez
left a comment
Contributor
There was a problem hiding this comment.
@goranmoomin : for next time, please reference https://llvm.org/docs/GitHub.html#updating-pull-requests about updating PRs. The review is easier if you push new commits. Things can be squashed together when merging or before merge, when the review has already been done.
That said, it looks like the branch might need some update to be merged with main according to the warning, so feel free to rebase into a recent main.
LGTM otherwise.
PD: I did not realize the asserts were checking against the calculated padded size. Not aweseome, but no easy way to avoid. I think paddedLinkEditEntrySize is a good solution for that. Makes it clear what's happening, and should provide consistent values.
e2b59f5 to
b96854b
Compare
Contributor
Author
Was not aware of the PR conventions for llvm, sorry for that! Will do as instructed next time.
Updated! |
Align Mach-O __LINKEDIT entries to the target pointer size when building the tail layout. This matches the behavior of ld64 and lld-macho. dyld on macOS 27 rejects loading dylibs with misaligned __LINKEDIT entries.
b96854b to
5823523
Compare
Contributor
Author
|
Rebased again on a recent main. I'm not really familiar with the llvm contributing process, is the merging of this patch blocked by the remaining reviewer, or do I have to take some action to proceed? Or should I just wait? |
Contributor
I was leaving the PR open for review by other people, and then I forgot about it. Started the CI checks and set it to auto-merge if they pass. Sorry about that. |
|
@goranmoomin Congratulations on having your first Pull Request (PR) merged into the LLVM Project! Your changes will be combined with recent changes from other authors, then tested by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR. Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues. How to do this, and the rest of the post-merge process, is covered in detail here. If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again. If you don't get any reports, no action is required from you. Your changes are working as expected, well done! |
drodriguez
added a commit
that referenced
this pull request
Jun 20, 2026
During the review of #203680 I noticed that Mach-O objcopy files seems to use `alignTo` and import `Alignment.h` to align some offsets to page boundaries and similar requirements. However, the `alignTo` in `Alignment.h`, while being intended for powers of 2, requires using an alignment of type `llvm::Align`, and needs explicit conversion from `uint64_t` and similar. Single `Alignment.h` includes `MathExtras.h`, the `alignTo` being invoked ends up being a generic `alignTo` that does not require powers of 2, and perform divisions and multiplications. While some of those might be optimized by the compiler into efficient power of 2 operations, there's an explicit `alignToPowerOf2` version that is optimized and asserts the alignment is a power of 2 (with asserts enabled). Since all the alignments should be power of 2 for the Mach-O binary format, change from `alignTo` to `alignToPowerOf2` to make the fact more visible (and get the extra safety net of the assertions). As expected, the test suite of objcopy doesn't show any regressions, but I have not done a performance benchmark around this either.
Contributor
|
This patch broke the Solaris/sparcv9 buildbot. Most likely an endianess issue. |
Contributor
Author
PR #205044 should address the failures on the sparcv9 and s390x buildbots. Seems like a bug in |
drodriguez
pushed a commit
that referenced
this pull request
Jun 22, 2026
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`, 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.
cuviper
pushed a commit
to rust-lang/llvm-project
that referenced
this pull request
Jun 24, 2026
…203680) Align Mach-O __LINKEDIT entries to the target pointer size when building the tail layout. This matches the behavior of ld64 and lld-macho. dyld on macOS 27 rejects loading dylibs with misaligned __LINKEDIT entries. See llvm#203678 for details and the motivation of this fix. AI Tool Use Disclosure: Regarding the PR and the linked issue, I have personally wrote every single part of the PR by myself, and have/ran/verified every single part of the issue report as well without any AI tool usage. I have used LLM-based coding agents only for debugging purposes, e.g. to figure out why the dylib was not loading (from the original bug report), and figuring out how to build, run, and test my local `llvm-objcopy`. (cherry picked from commit 18c1cbc)
cuviper
pushed a commit
to rust-lang/llvm-project
that referenced
this pull request
Jun 24, 2026
…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 file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Align Mach-O __LINKEDIT entries to the target pointer size when building the tail layout. This matches the behavior of ld64 and lld-macho.
dyld on macOS 27 rejects loading dylibs with misaligned __LINKEDIT entries.
See #203678 for details and the motivation of this fix.
AI Tool Use Disclosure:
Regarding the PR and the linked issue, I have personally wrote every single part of the PR by myself, and have/ran/verified every single part of the issue report as well without any AI tool usage.
I have used LLM-based coding agents only for debugging purposes, e.g. to figure out why the dylib was not loading (from the original bug report), and figuring out how to build, run, and test my local
llvm-objcopy.