[objcopy][MachO] Revert special handling of encryptable binaries #144058
Conversation
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
Code originally added in llvm#120995 and later corrected in llvm#130517 but apparently still not correct according to llvm#141494 and rust-lang/rust#141913. Revert the special handling because the test written in llvm#120995 and llvm#130517 still passes without those changes. Kept the test and improved it with a `__DATA` section to keep the current behaviour checked in case other changes modify the behaviour and break this edge case.
drodriguez
requested review from
alexander-shaposhnikov,
dianqk,
DataCorrupted and
alx32
|
@llvm/pr-subscribers-llvm-binary-utilities Author: Daniel Rodríguez Troitiño (drodriguez) ChangesCode originally added in #120995 and later corrected in #130517 but apparently still not correct according to #141494 and rust-lang/rust#141913. Revert the special handling because the test written in #120995 and #130517 still passes without those changes. Kept the test and improved it with a Full diff: https://github.com/llvm/llvm-project/pull/144058.diff 5 Files Affected:
diff --git a/llvm/lib/ObjCopy/MachO/MachOLayoutBuilder.cpp b/llvm/lib/ObjCopy/MachO/MachOLayoutBuilder.cpp
index 8ecd669e67178..93bc6631e64c8 100644
--- a/llvm/lib/ObjCopy/MachO/MachOLayoutBuilder.cpp
+++ b/llvm/lib/ObjCopy/MachO/MachOLayoutBuilder.cpp
@@ -116,10 +116,6 @@ uint64_t MachOLayoutBuilder::layoutSegments() {
const bool IsObjectFile =
O.Header.FileType == MachO::HeaderFileType::MH_OBJECT;
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 RequiresFirstSectionOutsideFirstPage =
- O.EncryptionInfoCommandIndex.has_value();
for (LoadCommand &LC : O.LoadCommands) {
auto &MLC = LC.MachOLoadCommand;
StringRef Segname;
@@ -173,10 +169,6 @@ uint64_t MachOLayoutBuilder::layoutSegments() {
if (!Sec->hasValidOffset()) {
Sec->Offset = 0;
} else {
- if (RequiresFirstSectionOutsideFirstPage) {
- SectOffset = alignToPowerOf2(SectOffset, PageSize);
- RequiresFirstSectionOutsideFirstPage = false;
- }
Sec->Offset = SegOffset + SectOffset;
Sec->Size = Sec->Content.size();
SegFileSize = std::max(SegFileSize, SectOffset + Sec->Size);
diff --git a/llvm/lib/ObjCopy/MachO/MachOObject.cpp b/llvm/lib/ObjCopy/MachO/MachOObject.cpp
index e0819d89d24ff..8d2c02dc37c99 100644
--- a/llvm/lib/ObjCopy/MachO/MachOObject.cpp
+++ b/llvm/lib/ObjCopy/MachO/MachOObject.cpp
@@ -98,10 +98,6 @@ void Object::updateLoadCommandIndexes() {
case MachO::LC_DYLD_EXPORTS_TRIE:
ExportsTrieCommandIndex = Index;
break;
- case MachO::LC_ENCRYPTION_INFO:
- case MachO::LC_ENCRYPTION_INFO_64:
- EncryptionInfoCommandIndex = Index;
- break;
}
}
}
diff --git a/llvm/lib/ObjCopy/MachO/MachOObject.h b/llvm/lib/ObjCopy/MachO/MachOObject.h
index 13ac87ed3ed06..8f9444f5fb025 100644
--- a/llvm/lib/ObjCopy/MachO/MachOObject.h
+++ b/llvm/lib/ObjCopy/MachO/MachOObject.h
@@ -341,9 +341,6 @@ struct Object {
/// The index of the LC_SEGMENT or LC_SEGMENT_64 load command
/// corresponding to the __TEXT segment.
std::optional<size_t> TextSegmentCommandIndex;
- /// The index of the LC_ENCRYPTION_INFO or LC_ENCRYPTION_INFO_64 load command
- /// if present.
- std::optional<size_t> EncryptionInfoCommandIndex;
BumpPtrAllocator Alloc;
StringSaver NewSectionsContents;
diff --git a/llvm/lib/ObjCopy/MachO/MachOReader.cpp b/llvm/lib/ObjCopy/MachO/MachOReader.cpp
index ef0e0262f9395..2b344f36d8e78 100644
--- a/llvm/lib/ObjCopy/MachO/MachOReader.cpp
+++ b/llvm/lib/ObjCopy/MachO/MachOReader.cpp
@@ -184,10 +184,6 @@ Error MachOReader::readLoadCommands(Object &O) const {
case MachO::LC_DYLD_CHAINED_FIXUPS:
O.ChainedFixupsCommandIndex = O.LoadCommands.size();
break;
- case MachO::LC_ENCRYPTION_INFO:
- case MachO::LC_ENCRYPTION_INFO_64:
- O.EncryptionInfoCommandIndex = O.LoadCommands.size();
- break;
}
#define HANDLE_LOAD_COMMAND(LCName, LCValue, LCStruct) \
case MachO::LCName: \
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 2b2bd670613de..d6f6fe10d88c2 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
@@ -16,7 +16,11 @@
# CHECK: fileoff: 0
# The YAML below is the following code
+# ```
+# static int foo = 12345;
+# int bar = 4567;
# int main(int argc, char **argv) { return 0; }
+# ```
# Compiled on macOS against the macOS SDK and passing `-Wl,-encryptable`
# Contents are removed, since they are not important for the test. We need a
# small text segment (smaller than a page).
@@ -26,8 +30,8 @@ FileHeader:
cputype: 0x100000C
cpusubtype: 0x0
filetype: 0x2
- ncmds: 15
- sizeofcmds: 696
+ ncmds: 18
+ sizeofcmds: 920
flags: 0x200085
reserved: 0x0
LoadCommands:
@@ -69,7 +73,7 @@ LoadCommands:
- sectname: __unwind_info
segname: __TEXT
addr: 0x100004020
- size: 4152
+ size: 88
offset: 0x4020
align: 2
reloff: 0x0
@@ -79,37 +83,61 @@ LoadCommands:
reserved2: 0x0
reserved3: 0x0
- cmd: LC_SEGMENT_64
- cmdsize: 72
- segname: __LINKEDIT
+ cmdsize: 152
+ segname: __DATA
vmaddr: 4295000064
- vmsize: 592
+ vmsize: 16384
fileoff: 32768
- filesize: 592
+ filesize: 16384
+ maxprot: 3
+ initprot: 3
+ nsects: 1
+ flags: 0
+ Sections:
+ - sectname: __data
+ segname: __DATA
+ addr: 0x100008000
+ size: 4
+ offset: 0x8000
+ align: 2
+ reloff: 0x0
+ nreloc: 0
+ flags: 0x0
+ reserved1: 0x0
+ reserved2: 0x0
+ reserved3: 0x0
+ - cmd: LC_SEGMENT_64
+ cmdsize: 72
+ segname: __LINKEDIT
+ vmaddr: 4295016448
+ vmsize: 16384
+ fileoff: 49152
+ filesize: 768
maxprot: 1
initprot: 1
nsects: 0
flags: 0
- cmd: LC_DYLD_CHAINED_FIXUPS
cmdsize: 16
- dataoff: 32768
- datasize: 48
+ dataoff: 49152
+ datasize: 56
- cmd: LC_DYLD_EXPORTS_TRIE
cmdsize: 16
- dataoff: 32816
- datasize: 48
+ dataoff: 49208
+ datasize: 64
- cmd: LC_SYMTAB
cmdsize: 24
- symoff: 32872
- nsyms: 2
- stroff: 32904
- strsize: 32
+ symoff: 49280
+ nsyms: 3
+ stroff: 49328
+ strsize: 40
- cmd: LC_DYSYMTAB
cmdsize: 80
ilocalsym: 0
nlocalsym: 0
iextdefsym: 0
- nextdefsym: 2
- iundefsym: 2
+ nextdefsym: 3
+ iundefsym: 3
nundefsym: 0
tocoff: 0
ntoc: 0
@@ -123,12 +151,6 @@ LoadCommands:
nextrel: 0
locreloff: 0
nlocrel: 0
- - cmd: LC_ENCRYPTION_INFO_64
- cmdsize: 24
- cryptoff: 16384
- cryptsize: 16384
- cryptid: 0
- pad: 0
- cmd: LC_LOAD_DYLINKER
cmdsize: 32
name: 12
@@ -136,32 +158,50 @@ LoadCommands:
ZeroPadBytes: 7
- cmd: LC_UUID
cmdsize: 24
- uuid: 4C4C4447-5555-3144-A18A-01E9EB7E7D92
+ uuid: ADDA943C-657A-3A49-9580-168E17A40FFB
- cmd: LC_BUILD_VERSION
cmdsize: 32
platform: 1
minos: 983040
- sdk: 983552
+ sdk: 984320
ntools: 1
Tools:
- - tool: 4
- version: 1310720
+ - tool: 3
+ version: 76481537
+ - cmd: LC_SOURCE_VERSION
+ cmdsize: 16
+ version: 0
- cmd: LC_MAIN
cmdsize: 24
entryoff: 16384
stacksize: 0
+ - cmd: LC_ENCRYPTION_INFO_64
+ cmdsize: 24
+ cryptoff: 16384
+ cryptsize: 16384
+ cryptid: 0
+ pad: 0
+ - cmd: LC_LOAD_DYLIB
+ cmdsize: 56
+ dylib:
+ name: 24
+ timestamp: 2
+ current_version: 88539136
+ compatibility_version: 65536
+ Content: '/usr/lib/libSystem.B.dylib'
+ ZeroPadBytes: 6
- cmd: LC_FUNCTION_STARTS
cmdsize: 16
- dataoff: 32864
+ dataoff: 49272
datasize: 8
- cmd: LC_DATA_IN_CODE
cmdsize: 16
- dataoff: 32872
+ dataoff: 49280
datasize: 0
- cmd: LC_CODE_SIGNATURE
cmdsize: 16
- dataoff: 32944
- datasize: 416
+ dataoff: 49376
+ datasize: 544
LinkEditData:
ExportTrie:
TerminalSize: 0
@@ -173,51 +213,67 @@ LinkEditData:
ImportName: ''
Children:
- TerminalSize: 0
- NodeOffset: 5
+ NodeOffset: 25
Name: _
Flags: 0x0
Address: 0x0
Other: 0x0
ImportName: ''
Children:
+ - TerminalSize: 2
+ NodeOffset: 9
+ Name: _mh_execute_header
+ Flags: 0x0
+ Address: 0x0
+ Other: 0x0
+ ImportName: ''
- TerminalSize: 4
- NodeOffset: 33
- Name: main
+ NodeOffset: 13
+ Name: bar
Flags: 0x0
- Address: 0x4000
+ Address: 0x8000
Other: 0x0
ImportName: ''
- - TerminalSize: 2
- NodeOffset: 39
- Name: _mh_execute_header
+ - TerminalSize: 4
+ NodeOffset: 19
+ Name: main
Flags: 0x0
- Address: 0x0
+ Address: 0x4000
Other: 0x0
ImportName: ''
NameList:
- n_strx: 2
n_type: 0xF
n_sect: 1
+ n_desc: 16
+ n_value: 4294967296
+ - n_strx: 22
+ n_type: 0xF
+ n_sect: 3
n_desc: 0
- n_value: 4294983680
- - n_strx: 8
+ n_value: 4295000064
+ - n_strx: 27
n_type: 0xF
n_sect: 1
- n_desc: 16
- n_value: 4294967296
+ n_desc: 0
+ n_value: 4294983680
StringTable:
- ' '
- - _main
- __mh_execute_header
+ - _bar
+ - _main
+ - ''
+ - ''
+ - ''
- ''
- ''
- ''
- ''
FunctionStarts: [ 0x4000 ]
- ChainedFixups: [ 0x0, 0x0, 0x0, 0x0, 0x20, 0x0, 0x0, 0x0, 0x30, 0x0,
- 0x0, 0x0, 0x30, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
- 0x1, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
- 0x0, 0x0, 0x3, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
- 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0 ]
+ ChainedFixups: [ 0x0, 0x0, 0x0, 0x0, 0x20, 0x0, 0x0, 0x0, 0x34, 0x0,
+ 0x0, 0x0, 0x34, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
+ 0x1, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
+ 0x0, 0x0, 0x4, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
+ 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
+ 0x0, 0x0, 0x0, 0x0, 0x0, 0x0 ]
...
-
|
This was referenced Jun 13, 2025
|
/cherry-pick a0662ce |
|
/pull-request #144449 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Code originally added in #120995 and later corrected in #130517 but apparently still not correct according to #141494 and rust-lang/rust#141913.
Revert the special handling because the test written in #120995 and #130517 still passes without those changes. Kept the test and improved it with a
__DATAsection to keep the current behaviour checked in case other changes modify the behaviour and break this edge case.