From cc61c283640a43dbbb8d22c6eab9d35380802296 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Rodri=CC=81guez=20Troitin=CC=83o?= Date: Sun, 9 Mar 2025 14:05:12 -0700 Subject: [PATCH 1/3] [llvm-objcopy] Apply encryptable offset to first segment, not section Bug introduced #120995. The LLD code calculates the "size" of the Mach-O headers, and then uses that size to place the segments, but the `__TEXT` section stays at `fileoff` zero. When I wrote the code into llvm-objcopy I calculated the extra space into the initial offset, which moved all the sections back 1 page. Besides the modified test checking for the right `fileoff` values of the sections and the segments, I also checked the generated binaries after `llvm-objcopy` using `dyld_info`, as the bug report suggested. Fixes #130472 --- llvm/lib/ObjCopy/MachO/MachOLayoutBuilder.cpp | 12 +++++++----- .../MachO/strip-with-encryption-info.test | 12 +++++++++--- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/llvm/lib/ObjCopy/MachO/MachOLayoutBuilder.cpp b/llvm/lib/ObjCopy/MachO/MachOLayoutBuilder.cpp index d4eb6a9b9fc0b..91f73b59db1bd 100644 --- a/llvm/lib/ObjCopy/MachO/MachOLayoutBuilder.cpp +++ b/llvm/lib/ObjCopy/MachO/MachOLayoutBuilder.cpp @@ -116,11 +116,9 @@ uint64_t MachOLayoutBuilder::layoutSegments() { const bool IsObjectFile = O.Header.FileType == MachO::HeaderFileType::MH_OBJECT; uint64_t Offset = IsObjectFile ? (HeaderSize + O.Header.SizeOfCmds) : 0; - if (O.EncryptionInfoCommandIndex) { - // If we are emitting an encryptable binary, our load commands must have a - // separate (non-encrypted) page to themselves. - Offset = alignToPowerOf2(HeaderSize + O.Header.SizeOfCmds, PageSize); - } + // If we are emitting an encryptable binary, our load commands must have a + // separate (non-encrypted) page to themselves. + bool FirstSectExtraEncryptableOffset = O.EncryptionInfoCommandIndex.has_value(); for (LoadCommand &LC : O.LoadCommands) { auto &MLC = LC.MachOLoadCommand; StringRef Segname; @@ -174,6 +172,10 @@ uint64_t MachOLayoutBuilder::layoutSegments() { if (!Sec->hasValidOffset()) { Sec->Offset = 0; } else { + if (FirstSectExtraEncryptableOffset) { + SectOffset = alignToPowerOf2(SectOffset, PageSize); + FirstSectExtraEncryptableOffset = false; + } Sec->Offset = SegOffset + SectOffset; Sec->Size = Sec->Content.size(); SegFileSize = std::max(SegFileSize, SectOffset + Sec->Size); diff --git a/llvm/test/tools/llvm-objcopy/MachO/strip-with-encryption-info.test b/llvm/test/tools/llvm-objcopy/MachO/strip-with-encryption-info.test index 19b06b1ec02c8..2b2bd670613de 100644 --- a/llvm/test/tools/llvm-objcopy/MachO/strip-with-encryption-info.test +++ b/llvm/test/tools/llvm-objcopy/MachO/strip-with-encryption-info.test @@ -1,13 +1,19 @@ # RUN: rm -rf %t && mkdir %t # RUN: yaml2obj %s -o %t/original # RUN: llvm-strip --strip-all %t/original -o %t/stripped -# RUN: llvm-readobj --macho-segment %t/stripped | FileCheck %s +# RUN: llvm-readobj --macho-segment --section-headers %t/stripped | FileCheck %s + +# CHECK-LABEL: Sections [ +# CHECK: Index: 0 +# CHECK-NEXT: Name: __text +# CHECK-NEXT: Segment: __TEXT +# CHECK: Offset: 16384 # CHECK-LABEL: Name: __PAGEZERO -# CHECK: fileoff: 16384 +# CHECK: fileoff: 0 # CHECK-LABEL: Name: __TEXT -# CHECK: fileoff: 16384 +# CHECK: fileoff: 0 # The YAML below is the following code # int main(int argc, char **argv) { return 0; } From fe56bd4e9514d56f96b0373fd75d3e53eef9713b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Rodri=CC=81guez=20Troitin=CC=83o?= Date: Sun, 9 Mar 2025 14:31:25 -0700 Subject: [PATCH 2/3] Lint --- llvm/lib/ObjCopy/MachO/MachOLayoutBuilder.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/llvm/lib/ObjCopy/MachO/MachOLayoutBuilder.cpp b/llvm/lib/ObjCopy/MachO/MachOLayoutBuilder.cpp index 91f73b59db1bd..d2c47ca94ed1e 100644 --- a/llvm/lib/ObjCopy/MachO/MachOLayoutBuilder.cpp +++ b/llvm/lib/ObjCopy/MachO/MachOLayoutBuilder.cpp @@ -118,7 +118,8 @@ uint64_t MachOLayoutBuilder::layoutSegments() { uint64_t Offset = IsObjectFile ? (HeaderSize + O.Header.SizeOfCmds) : 0; // If we are emitting an encryptable binary, our load commands must have a // separate (non-encrypted) page to themselves. - bool FirstSectExtraEncryptableOffset = O.EncryptionInfoCommandIndex.has_value(); + bool FirstSectExtraEncryptableOffset = + O.EncryptionInfoCommandIndex.has_value(); for (LoadCommand &LC : O.LoadCommands) { auto &MLC = LC.MachOLoadCommand; StringRef Segname; @@ -173,7 +174,7 @@ uint64_t MachOLayoutBuilder::layoutSegments() { Sec->Offset = 0; } else { if (FirstSectExtraEncryptableOffset) { - SectOffset = alignToPowerOf2(SectOffset, PageSize); + SectOffset = alignToPowerOf2(SectOffset, PageSize); FirstSectExtraEncryptableOffset = false; } Sec->Offset = SegOffset + SectOffset; From 515d5718eb1b3128b4e3013c8321a507ac0fd89b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Rodri=CC=81guez=20Troitin=CC=83o?= Date: Thu, 13 Mar 2025 18:21:06 -0700 Subject: [PATCH 3/3] Change variable spelling --- llvm/lib/ObjCopy/MachO/MachOLayoutBuilder.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/llvm/lib/ObjCopy/MachO/MachOLayoutBuilder.cpp b/llvm/lib/ObjCopy/MachO/MachOLayoutBuilder.cpp index d2c47ca94ed1e..8ecd669e67178 100644 --- a/llvm/lib/ObjCopy/MachO/MachOLayoutBuilder.cpp +++ b/llvm/lib/ObjCopy/MachO/MachOLayoutBuilder.cpp @@ -118,7 +118,7 @@ uint64_t MachOLayoutBuilder::layoutSegments() { uint64_t Offset = IsObjectFile ? (HeaderSize + O.Header.SizeOfCmds) : 0; // If we are emitting an encryptable binary, our load commands must have a // separate (non-encrypted) page to themselves. - bool FirstSectExtraEncryptableOffset = + bool RequiresFirstSectionOutsideFirstPage = O.EncryptionInfoCommandIndex.has_value(); for (LoadCommand &LC : O.LoadCommands) { auto &MLC = LC.MachOLoadCommand; @@ -173,9 +173,9 @@ uint64_t MachOLayoutBuilder::layoutSegments() { if (!Sec->hasValidOffset()) { Sec->Offset = 0; } else { - if (FirstSectExtraEncryptableOffset) { + if (RequiresFirstSectionOutsideFirstPage) { SectOffset = alignToPowerOf2(SectOffset, PageSize); - FirstSectExtraEncryptableOffset = false; + RequiresFirstSectionOutsideFirstPage = false; } Sec->Offset = SegOffset + SectOffset; Sec->Size = Sec->Content.size();