Reduce the size of the Windows shim#148
Open
ofek wants to merge 1 commit into
Open
Conversation
Contributor
|
This pull request has been imported. If you are a Meta employee, you can view this in D112049927. (Because this pull request was imported automatically, there will not be any future comments.) |
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
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.
Summary
Reduce both checked-in Windows shim binaries from 4,096 to 2,560 bytes.
This also adds a checked-in, generated 69-byte PE stub and exact artifact-size regression tests.
Size impact
Changes are cumulative; each delta is relative to the preceding row.
HeapFree/CloseHandleremovalHeapFreebefore waitingstrip = "symbols"rust-lld.pdatainto read-only.rdataOverall reduction: 1,536 bytes per binary, or 37.5%.
Changes that reduced file size
Bounded local UTF-16 copying: -512 bytes
Replace
lstrlenWandlstrcatWwith a single bounded copy loop.This removes two imports and, importantly, brings
.rdatabelow a 512-byte file-alignment boundary. It also fixes the previous terminator-boundary overflow: the command buffer now explicitly reserves one UTF-16 code unit for the terminating null.Merge
.pdatainto.rdata: -512 bytesUse
/MERGE:.pdata=.rdatawith LLD.LLD's smaller read-only data layout leaves enough room for the unwind records in the existing
.rdataallocation. The PE exception directory still points to all five runtime-function records on both architectures.Unlike the rejected
.pdata=.textexperiment, the resulting metadata remains read-only and non-executable.Compact PE stub: -512 bytes
Generate and check in a minimal valid 69-byte stub and pass it through
/STUB.This moves the PE header to offset 72 and allows all PE headers to fit in one 512-byte file-alignment block, reducing
SizeOfHeadersfrom 1,024 to 512 bytes.Changes retained despite no immediate file-size reduction
These changes reduced complexity and imports but did not individually cross a file-alignment boundary:
Pass
NULLdirectly toGetModuleFileNameW, removingGetModuleHandleW.Truncate the final four
.execode units under the documented shim naming contract, removingPathCchRemoveExtension.Extensionless shims were already nonfunctional: the previous implementation passed the PE executable itself to DotSlash as though it were the manifest, and the removed test asserted that this failed. Unconditional truncation therefore changes only how an incorrectly named shim fails, not any working behavior under the documented
<DotSlash-file>.execontract.Quote the manifest path directly, removing
PathQuoteSpacesW.Locate the raw argument tail with a local quote-state parser, removing
PathGetArgsW.Free the command buffer immediately after
CreateProcessW. On failure, captureGetLastErrorbefore freeing it so the existing diagnostic remains unchanged. RestoringHeapFreeadds one import but does not change the aligned artifact size.Let process teardown close the child handles, removing
CloseHandle.Retain
strip = "symbols"; it tied without changing the aligned artifact size.Use Rust's bundled
rust-lld; switching linkers alone tied at 3,584 bytes, but its smaller.rdatalayout enabled the subsequent safe section merge.Ignore the unused default
msvcrtrequest with/NODEFAULTLIB:msvcrt, allowing the CRT-free LLD cross-build.Together, the source cleanups reduce imports from 19 functions across three DLLs to 12 functions from
kernel32.dll.Resource-cleanup tradeoff
All three options produced 2,560-byte binaries:
HeapFreeHeapFreeandCloseHandleAlthough restoring both cleanup APIs currently fits within the same aligned file size, restoring only
HeapFreeleaves more section headroom for future changes.The section impact is measurable even though every option retains 1,024-byte raw
.textand.rdatasections:.text/.rdatavirtual bytes.text/.rdatavirtual bytesHeapFree+28/+32)+40/+28)HeapFreeandCloseHandle+50/+64)+64/+60)The selected option therefore recovers 22 bytes of x86-64
.text, 24 bytes of ARM64.text, and 32 bytes of.rdataon both architectures compared with restoring both APIs.You can run the following command to confirm:
Experiments not retained
WriteFilecall did not improve the final size.HeapFreeimport but retained a 65,534-byte heap allocation for the lifetime of every waiting shim. Restoring explicit cleanup did not change the 2,560-byte artifact size, so the removal was reverted.opt-level = "s"and the other strip/profile combinations were not smaller than the selected"z"profile.__chkstkdependency..rdataor.pdatainto executable.textworsened section permissions..pdata=.textreached 3,072 bytes, but executable unwind metadata was rejected./FIXED /DYNAMICBASE:NOwas measured on x86-64 and reverted because disabling ASLR was outside the acceptable security contract and it had no effect on size.Considered but deliberately not attempted
I didn't pursue changes likely to violate user-visible or platform contracts:
CreateProcessWmaximum or truncate arguments.Testing