From 8a8a0d64eb6efa397ea61177ee5cc8e5e3d74284 Mon Sep 17 00:00:00 2001 From: Ofek Lev Date: Tue, 14 Jul 2026 18:18:57 -0400 Subject: [PATCH] Reduce the size of the Windows shim --- .github/workflows/verify-windows-shim.yml | 25 +-- windows_shim/Cargo.toml | 1 + windows_shim/README.md | 12 +- windows_shim/dotslash_windows_linker_stub.exe | Bin 0 -> 69 bytes .../dotslash_windows_shim-aarch64.exe | Bin 4096 -> 2560 bytes windows_shim/dotslash_windows_shim-x86_64.exe | Bin 4096 -> 2560 bytes windows_shim/dotslash_windows_shim.rs | 179 +++++++++--------- windows_shim/release.py | 41 +++- windows_shim/tests/test.py | 139 +++++++++++--- 9 files changed, 255 insertions(+), 142 deletions(-) create mode 100644 windows_shim/dotslash_windows_linker_stub.exe diff --git a/.github/workflows/verify-windows-shim.yml b/.github/workflows/verify-windows-shim.yml index 47600c1..288e2dd 100644 --- a/.github/workflows/verify-windows-shim.yml +++ b/.github/workflows/verify-windows-shim.yml @@ -1,15 +1,14 @@ name: verify windows shim # Rebuilds the Windows shim from dotslash_windows_shim.rs and fails if the -# checked-in .exe does not match, so the source cannot change without the -# regenerated binary. The build is byte-for-byte reproducible (rust-lld with +# checked-in artifacts do not match, so the source cannot change without the +# regenerated files. The build is byte-for-byte reproducible (rust-lld with # /Brepro, plus the pinned toolchain in windows_shim/rust-toolchain.toml), so a # plain `git diff` is a reliable check. Each architecture is built on its own # native runner to avoid cross-linking. # -# The freshly built binary is uploaded as an artifact before the diff check, so -# when the check fails (e.g. the committed binaries are out of date) you can -# download the correct binary from the run's "Artifacts" section and commit it +# The freshly built shim and linker stub are uploaded before the diff check, so +# when the check fails you can download the correct artifacts and commit them # without needing a local Windows machine. on: @@ -55,17 +54,19 @@ jobs: rustup target add "${{ matrix.target }}" - name: Rebuild the shim run: python release.py "${{ matrix.target }}" - - name: Upload the rebuilt shim - # Runs before the diff check so the binary is downloadable even when the - # committed copy is out of date and the job ultimately fails. + - name: Upload the rebuilt artifacts + # This runs before the diff check so corrected files remain downloadable + # when the committed copies are out of date and the job ultimately fails. uses: actions/upload-artifact@v4 with: name: dotslash_windows_shim-${{ matrix.arch }} - path: windows_shim/dotslash_windows_shim-${{ matrix.arch }}.exe + path: | + windows_shim/dotslash_windows_linker_stub.exe + windows_shim/dotslash_windows_shim-${{ matrix.arch }}.exe if-no-files-found: error - - name: Verify the checked-in binary is up to date + - name: Verify the checked-in artifacts are up to date run: | - if ! git diff --exit-code -- 'dotslash_windows_shim-*.exe'; then - echo "::error::Checked-in shim binary is out of date. Download the 'dotslash_windows_shim-${{ matrix.arch }}' artifact from this run and commit it (or run 'py release.py' locally)." + if ! git diff --exit-code -- 'dotslash_windows_linker_stub.exe' 'dotslash_windows_shim-*.exe'; then + echo "::error::Checked-in Windows shim artifacts are out of date. Download 'dotslash_windows_shim-${{ matrix.arch }}' from this run and commit its files, or run 'py release.py' locally." exit 1 fi diff --git a/windows_shim/Cargo.toml b/windows_shim/Cargo.toml index 5b5f92a..ae914ba 100644 --- a/windows_shim/Cargo.toml +++ b/windows_shim/Cargo.toml @@ -27,6 +27,7 @@ opt-level = "z" lto = true codegen-units = 1 panic = "abort" +strip = "symbols" [profile.dev] opt-level = 1 diff --git a/windows_shim/README.md b/windows_shim/README.md index 8f9bd13..8843989 100644 --- a/windows_shim/README.md +++ b/windows_shim/README.md @@ -31,9 +31,7 @@ few kilobytes. ## Release -The checked-in `dotslash_windows_shim-x86_64.exe` and -`dotslash_windows_shim-aarch64.exe` are built from `dotslash_windows_shim.rs`. -Regenerate them on Windows with: +The checked-in `dotslash_windows_shim-x86_64.exe` and `dotslash_windows_shim-aarch64.exe` are built from `dotslash_windows_shim.rs`, and `dotslash_windows_linker_stub.exe` is generated as their linker input. Regenerate all three on Windows with: ```shell py release.py @@ -56,13 +54,7 @@ rebuilds the shim whenever anything under `windows_shim/` changes and fails if the committed binaries are stale, so regenerate and commit them in the same change as any edit to the source or a bump of the pinned toolchain. -If you do not have a Windows machine, let CI build the binaries for you: push -your change (or trigger the workflow manually), then download the -`dotslash_windows_shim-x86_64` and `dotslash_windows_shim-aarch64` artifacts -from the workflow run — each contains the freshly built `.exe`. Because the -build is reproducible, those artifacts are exactly what a local `py release.py` -would produce; commit them into `windows_shim/` and re-run the workflow to -confirm it passes. +If you do not have a Windows machine, let CI build the artifacts for you: push your change (or trigger the workflow manually), then download `dotslash_windows_shim-x86_64` and `dotslash_windows_shim-aarch64` from the workflow run. Each contains the freshly built architecture-specific shim and the shared linker stub. Because the build is reproducible, those files are exactly what a local `py release.py` would produce; commit them into `windows_shim/` and re-run the workflow to confirm it passes. ## Testing diff --git a/windows_shim/dotslash_windows_linker_stub.exe b/windows_shim/dotslash_windows_linker_stub.exe new file mode 100644 index 0000000000000000000000000000000000000000..5b1bb15991d1920a070bae60113d180828de1129 GIT binary patch literal 69 gcmeZ`a%Es-U|?VY;{X4F1Qa;H7zF4Y3_fQS0m3*1A^-pY literal 0 HcmV?d00001 diff --git a/windows_shim/dotslash_windows_shim-aarch64.exe b/windows_shim/dotslash_windows_shim-aarch64.exe index 09d294662f43838a1118574c25e8f5537af04dfe..5dbba60802279261b2691e71e70d574af797fa46 100644 GIT binary patch literal 2560 zcmds3T}&KR6h1QpbzEAjuvVK$b|`2l+KsX3gV8X6AZ@6U-A!n;#+~lm?XC_xGCRmt zOjy#y)bv3o*7Q+)>k>)&Waxv^!b{aOaT6sPW85EFmtYes#jp>X@!UHzg=*9IqA}hu z_dEaR+;c87d+4P;-~gZwAKS*wU9v~q0S8bw2ZNVe5Qq8z)EORr*P1`{Tz-W;(7@m0 z1g8mb9d5$yUPj6^FQa*YZA`R@@Hh%kPL}}DDbLCEMX3|S18V0=C<%Z?wL28szut2z z@2Qq0fQLNhZ{YhFH*p8Mph+?&>JtKPv>_Z(hN_qfKqs|B0kiX(fGk}y z`(xxkwylLf&($WlT&))@yBG5IPB}xJQ^&FpU^?`*Y7^bLweZ5(3&N{QTjNrH8V?U~4j-$?XG101K2yLrMWJctC#0F~ z2v{wU4gG*T%_Si%UY4sgw*=BRYvIpw>n~%i6MU|A0_$Dm^SjV)sU%k~@J@Y2A<(g4 ze-si|9KXUOm*dc;B??vnYkUXbbS2!KD`4GRz5moD`@K*T=l_c46MuBhMDtB=ht7%s z9h1DcDsP9#NnSj~;>>|1-UoRatFjZS2F7E)c_-f*p0k2ABzgIG*Sjz}4G=jXK<8P2 zr`qv6cLDfC4np;aT~jdHw)?TB*2sNi_LSMPW)D`e*6LCY2O2NTl^MZncF&lc1M2Ut!^P3 zO5h!#cVg0q_8df*uEJRtQCGztGX8DOQjlNw_$Y5Fb=lB$kN*zuTYWZk4f)!{e6A+o zY}@4Amv|>0ZId&r`0v@p`BnkrHyq!a*;^0R-E~l<@dL=$CN8qO&T>TrtH@3!)$a^z{rr-BY9q(K7(_XJ4zE(i8Vh%tRkQ>Bz{ zO2>#Pc`>AzJ~E=DREcDgCNe4s4l7BsUpK{`c=R&$a7nIhBxCDf!B2fJxTrGrY^>@y4WeFPPLki{J! SLvd=kjGuCmB4+vDtUmx&7#Jh~ literal 4096 zcmeHKZ-^9S7=LDGJw~roJnyccx;ec<4|dU1`a{@`>)93Ozuk+o6x?y=z1ykTopWZk z+wg;HLP!cRAQTaJQiQuC9AS`A-Dw{ZLbxOGOCQ?1z0AX)VAULA+n)EG8Fy2wPkpF- zVfOd@d!P6BKJ&cu-fey{2OIzd@o5~9M&(cB9NHIle7X=$%|GAy3DWHX|g5jSp)i)mdU;Vyt}Jpf9Pha;0;4fyGQhGozkSm=@wT$CH_UBx{oVVqhQyfFjZRwr?IWZuJTY6B*{r|`6U&eD+2fQ zMQSgv0ehNXS`@ZR&J5quJ6>|507S|ZBA!;(YS7D2pFiGNnOwv#LT(gIHP*Fd5A9&$by zm}(Edpmg=36OC zj)<`xLD-vG0R1!v^$Tg23(L8(K~{Y^@@g*!YiXSr*D;stPUK2CQ%*Z5@8*aDv!Dz2 z=3`iMXMF!tg#~Plj^a4~;EfsFOBtW<`Ht>Oh(F*{;<=M^2WgHLlmjQbu&yPZKY4jM zoIb&GQ_pou&fIb+M36VG-|g3N8gt+~E(*`O9FH}@UODLQX_Vh}gM<7MVYBNS4Z^zd zcDHw^PY6lQ@i^GDZnPH1aUOok$EeSI2>S}()#$-pHz?AYjJ8KkH@<7u4}H8X@%b2j z@|H6FLcl(Qx{Ccl`+?PRwYz?Y0DCzO`dQp0XJ4sJ(BkesXDIY>1{oYphq;S;}Y{pJt{AJe!* zAmXrd@x03iFRN48CYnpaoDruI|B{^x#z_p?)7Yao_jQVA9M^dJs_q*vRC%yBdOFVe z$A+R?rY4)I(1@B*^bs>;rquM4;sOP9u)qWjWc*FBD@Q~YNjn69Y65b|Tsc144BWe8=(v!1D##Hr;pZ*zQZPt~nMmFJUTjVqu@I9twARXkFNy<1$ zw(7GsWCsU{A!0sbP}4_Pz5TyU=(XH94t3VG?k)g&$1ldmACIAJ0(G&$Hh{Lt2HlVK z@v`T;?xC>U^|X1o!o&3*Zt+m@Fz4YL9=_w@2OfUp;W-a4cv$k#=ken$dK2=dqs>EC z1NgV`1&C_8NqVtKHG-Rn8U$Nlo1rI(X%5g`^p( zwf4!T)ngdCL9^W6e1EXI>$CMR(#z9JD^V75&8-5vi>1@Pmh;Mv}RXd!-xm`A(^FY6VpDZ*+S4vuI@FyCH##bY&u)-rXI0lcW$)BZlu|3Mo++Zo>s^XDIdQX7GU|k$EgaTVFr?S>5FdwNq>4 zu3B(T0J^jm)9OG>aNa6Md}{NcWi~Tt=jgf9Y!-Tt~@?4hgSGxS3`s+8|;H@8`Yr<&YqyVEvNcJ?NIol_E1N^z}Q)txrCdZ)8K?k_YZUinQC?rdb?D|M`DLhH3N%=kOuGiE{BtQQ93HblZ^-HeVa# zwN<6K6Y8L^Jp)y3MSl-sm(%xfe=q&%`)aGZ6e(^3zLxR(fG_+2?Z;Kn+F1Jr+B&vv zVeJ!WZ?UbDwfBQtHL0W~yPJE}sg_J~V%}-%KT_oeJNIV#yp;*zo{RMRP8r0NNPxtszf@d?4hR zkdKG_c*tK1`I{ksFXXkTME_PEqV0KUIC3Cw56eO!&H0mPTc|_B@oum6GnLIrUI2ub zY)504mob_$3QjVgxAPq3=C*`r=p&>t79zVGQ{cW>l6A(rEz2Gb<-WWO@LOCVtVrjx zPSAKdDDydKJ@jDAv@96V;BJhScvG13Q@#;0^EdF{BMO;0{||W>GV{eyR#r69if0?T hB4f&s($qjrJuDF^Eq73hqEpQRy-}gY9_0ED&|kGJu}uH~ literal 4096 zcmeHKU2GIp6h70frP9(ZngN18Okp1=ao1fGK|{>;4==hvrGzF*aHl)B-No6N&Fn1g zgGR~nH`ev%2}5E+jKK$eFop*xK9rxNl8B0$0^tE3eAomYK#i#q7{5C^OP3$@$p->A zy>rg_&d)vf+;eBPYhD>4iHPbkYe&3R_y>Ljc*}yZ7CP|Y$+m-1;$+)~bSAIlEOW?G zvr0-e4AWKyxMCFyC1WVv>-vpt?K0a`i|2C+_Vb(({0E}fYlKtT3O|5=segxS|k4# zyJ8{Hm;YiEvH6G%eRBX0u}`swTMk1ZN=7VAwN;{hLI=PrfL|j>-iQ;$Be{y5mpk@A zs1>24wIdF@TKPVI?^a-Ae;+GeiZW-CIp4F=rqC0`MRe$Q7eh`yM8t|(NFnANVa{J2 z&gIq4SyxU2|9};bM_+wy^RbQn@Lyc@pY=tMXlyR(S^fkenDZxdel712Y-!eKLMI8` zP5_4l=gP-s69t<(Hi|^HU7GT+^T0X|UU#v)b7V|(xhtOo#!6p@-iE}S8LT+e2wC2M zm{_HJ3eni;^2F$HfB9Brclnk~@ ztf;nP!9<8Lr#lo6&h8FHaV(2zqC2D@F2zou%00319Dalm^3?idGF+7Q}QT!;jrbLF_OostTcC$rIIK>+TgkkalXo|{~` z8BnkZhx#7&JT@OmTx^CkAf(43m4CVBy1^z_zKR1XPSq9WLK}eg;sr40XTj_d;dC0( zOyRUhNGBle6}omI9j-K|!kp90`HnfKT=|+f?$j9;*xbmrU3GD|!;i7i7F5`k&kEg+ zaeF>-dvIu`o!8ZTIy{^)G;=r~&Zjfk z=aiWmnPgL*bW-t5lY;t*nKBEyrikzQ5Vw`8w~{imS=G=KK9aE^;2YnsX6#j_)t51b zbiQt23s2d~pqkOS7P;ZYM|i4Ws{=asA`Y4bLyJtWP*L;SR*CegcDgH-UeB}UR^BsW zb0eQIjcWK8xV^^I3OZkf)z+$6-e1+^Z42ojKO?2WNwufFItzn?+)@zF8r02U?38Z& zHbJZ3G8=wB{(+fVWtVVR{7P$d=i!sNi6$$tIF3XpwOE>eJp-FX) zO>|itEzoP3DW1>wi(U1gb$uBUny9PN(}|41g#i#@(uL-pHWO$<&D%YeWm+Q2?ag-+ zT3O#)50-u$#?_o})n{urQY3+Q=(?GLQt+!RE>fpETrGiG8QW`l@H#pjE$*!&c%2&_ zf9H!MVhRm1>ad0?)e!Q_1=HqzITa;>_v_p;xc=1Ah^FJEkjsR#dGXQ+r%a27b1)C< zv2ZLL_4N6)J`5X=Cuv4K%~&6Z*5#z6)DWPCPQejL%?;EnEuaUY^|?T@?t*8x)VHgb zsJ>HVc7Ob@6~N1_15?BO7IYV84!ocr<9FU0-~|=`hrJ74&}Ph!z~3Kte+B*q6Yv8k diff --git a/windows_shim/dotslash_windows_shim.rs b/windows_shim/dotslash_windows_shim.rs index 78ac060..60ed35c 100644 --- a/windows_shim/dotslash_windows_shim.rs +++ b/windows_shim/dotslash_windows_shim.rs @@ -22,7 +22,8 @@ #![cfg_attr(feature = "no_std", feature(lang_items))] #![cfg_attr(feature = "no_std", no_std)] #![cfg_attr(feature = "no_std", no_main)] -#![cfg_attr(feature = "no_std", windows_subsystem = "console")] // Set Entrypoint to "mainCRTStartup" +// Select the console subsystem; the no_std entry point is `mainCRTStartup` below. +#![cfg_attr(feature = "no_std", windows_subsystem = "console")] #[allow(clippy::upper_case_acronyms)] type DWORD = u32; @@ -31,25 +32,17 @@ use core::mem; use core::ptr; use core::str; -use windows_sys::Win32::Foundation::CloseHandle; use windows_sys::Win32::Foundation::ERROR_FILE_NOT_FOUND; use windows_sys::Win32::Foundation::GetLastError; use windows_sys::Win32::Foundation::HANDLE; -use windows_sys::Win32::Foundation::HMODULE; -use windows_sys::Win32::Foundation::S_FALSE; -use windows_sys::Win32::Foundation::S_OK; use windows_sys::Win32::Foundation::TRUE; use windows_sys::Win32::Foundation::WAIT_OBJECT_0; -use windows_sys::Win32::Globalization::lstrcatW; -use windows_sys::Win32::Globalization::lstrlenW; use windows_sys::Win32::Storage::FileSystem::WriteFile; use windows_sys::Win32::System::Console::GetStdHandle; use windows_sys::Win32::System::Console::STD_ERROR_HANDLE; use windows_sys::Win32::System::Environment::GetCommandLineW; use windows_sys::Win32::System::LibraryLoader::GetModuleFileNameW; -use windows_sys::Win32::System::LibraryLoader::GetModuleHandleW; use windows_sys::Win32::System::Memory::GetProcessHeap; -use windows_sys::Win32::System::Memory::HEAP_ZERO_MEMORY; use windows_sys::Win32::System::Memory::HeapAlloc; use windows_sys::Win32::System::Memory::HeapFree; use windows_sys::Win32::System::Threading::CreateProcessW; @@ -59,12 +52,8 @@ use windows_sys::Win32::System::Threading::INFINITE; use windows_sys::Win32::System::Threading::PROCESS_INFORMATION; use windows_sys::Win32::System::Threading::STARTUPINFOW; use windows_sys::Win32::System::Threading::WaitForSingleObject; -use windows_sys::Win32::UI::Shell::PathCchRemoveExtension; -use windows_sys::Win32::UI::Shell::PathGetArgsW; -use windows_sys::Win32::UI::Shell::PathQuoteSpacesW; use windows_sys::core::BOOL; use windows_sys::core::PCWSTR; -use windows_sys::core::PWSTR; use windows_sys::w; fn write_stderr(text: &str) -> BOOL { @@ -72,9 +61,9 @@ fn write_stderr(text: &str) -> BOOL { // parameter is not NULL. let mut bytes_written: u32 = 0; unsafe { - let stdout: HANDLE = GetStdHandle(STD_ERROR_HANDLE); + let stderr: HANDLE = GetStdHandle(STD_ERROR_HANDLE); let ok: BOOL = WriteFile( - stdout, /* hFile */ + stderr, /* hFile */ text.as_ptr(), /* lpBuffer */ text.len() as u32, /* nNumberOfBytesToWrite */ &mut bytes_written as *mut u32, /* lpNumberOfBytesWritten */ @@ -85,14 +74,48 @@ fn write_stderr(text: &str) -> BOOL { } fn fatal(text: &str) -> ! { + // Diagnostics are best-effort because there is no useful recovery if + // stderr itself cannot be written. write_stderr("dotslash-windows-shim: "); write_stderr(text); write_stderr("\n"); unsafe { ExitProcess(1) } } -// CreateProcessW's lpCommandLine has a maximum length of 32,767 -// characters. +// Find the raw argument tail without parsing and reconstructing it, so the +// caller's quoting and backslashes reach dotslash unchanged. Only argv[0] is +// scanned: quotes may delimit spans anywhere within it, while spaces and tabs +// terminate it only when outside quotes. +// +// SAFETY: `p` must point to a readable, null-terminated UTF-16 string. +unsafe fn command_line_args(mut p: *const u16) -> *const u16 { + let mut in_quotes = false; + + loop { + let ch = unsafe { *p }; + + if ch == 0 { + return p; + } + + if ch == b'"' as u16 { + in_quotes = !in_quotes; + } else if !in_quotes && (ch == b' ' as u16 || ch == b'\t' as u16) { + break; + } + + p = unsafe { p.add(1) }; + } + + while unsafe { *p } == b' ' as u16 || unsafe { *p } == b'\t' as u16 { + p = unsafe { p.add(1) }; + } + + p +} + +// CreateProcessW's lpCommandLine has a maximum length of 32,767 UTF-16 code +// units, including its terminating null. const BUF_MAX_SIZE: usize = 32767; struct PoorMansString { @@ -101,51 +124,41 @@ struct PoorMansString { } impl PoorMansString { - fn append(&mut self, other: *const u16) { - let other_len = unsafe { lstrlenW(other) as usize }; - if self.len + other_len > BUF_MAX_SIZE { - fatal("Buffer overflow"); - } - unsafe { - // Concatenate other string to self.buf - if lstrcatW(self.buf, other).is_null() { - fatal("string concatenation failed"); + fn append(&mut self, mut other: *const u16) { + while unsafe { *other } != 0 { + if self.len == self.capacity() { + fatal("Buffer overflow"); + } + unsafe { + *self.buf.add(self.len) = *other; + other = other.add(1); } + self.len += 1; } - - self.len = self.len + other_len; + unsafe { *self.buf.add(self.len) = 0 }; } fn capacity(&self) -> usize { + // append() writes a null after every copy, so one code unit is never + // available for content. BUF_MAX_SIZE - 1 } fn new() -> Self { - // Allocate BUF_MAX_SIZE upfront to avoid growing buffers. + // Allocate once at the API's maximum size. Zero-initialization is + // unnecessary because append() writes both content and its terminator. let buf = unsafe { - HeapAlloc( - GetProcessHeap(), - HEAP_ZERO_MEMORY, - BUF_MAX_SIZE * mem::size_of::(), - ) as *mut u16 + HeapAlloc(GetProcessHeap(), 0, BUF_MAX_SIZE * mem::size_of::()) as *mut u16 }; Self { buf, len: 0 } } } -impl Drop for PoorMansString { - fn drop(&mut self) { - unsafe { - HeapFree(GetProcessHeap(), 0, self.buf as *mut _); - } - } -} - fn main_impl() -> ! { let mut ds_cmd = PoorMansString::new(); - // Append "dotslash " to the command string. - ds_cmd.append(w!("dotslash ")); + // Append `dotslash "` to the command string. + ds_cmd.append(w!("dotslash \"")); // Append the DotSlash file path to the command string. // @@ -153,24 +166,18 @@ fn main_impl() -> ! { // command string will happen in-place in the command string. unsafe { let ds_file_ptr = ds_cmd.buf.add(ds_cmd.len); - // Get a handle to this executable. - // - // When passed NULL, GetModuleHandle returns a handle to the file - // used to create the calling process (.exe file). - let handle: HMODULE = GetModuleHandleW(ptr::null_mut()); - // Append the fully qualified path for this executable to the // command string. // // For an executable named `foo.exe` we should now have a command - // string that looks like `dotslash C:\path\to\foo.exe`. + // string that looks like `dotslash "C:\path\to\foo.exe`. // - // GetModuleFileName requires you to keep growing a buffer until it - // fits the path. No need to do this because the command buffer - // is already as large as can be. + // GetModuleFileNameW reports truncation when its buffer is too small. + // Retrying with a larger allocation cannot help because the completed + // CreateProcessW command must fit in this same maximum-sized buffer. let remaining_capacity = ds_cmd.capacity() - ds_cmd.len; let new_len = GetModuleFileNameW( - handle, /* hModule */ + ptr::null_mut(), /* hModule */ ds_file_ptr, /* lpFilename */ remaining_capacity as _, /* nSize */ ) as usize; @@ -179,42 +186,29 @@ fn main_impl() -> ! { } ds_cmd.len += new_len; - // Remove the extension from this executable's full path. - // - // We assume that the DotSlash file is named just like this executable but - // without the `.exe`. + // Remove the final `.exe` extension from this executable's full path. // - // For an executable named `foo.exe` we should now have a command string that - // looks like `dotslash C:\path\to\foo`. + // The shim contract requires it to be named `.exe`, so + // the suffix is unconditionally removed as four UTF-16 code units. No + // case check is needed, which also handles the conventional `.EXE`. // - // PathCchRemoveExtension returns `S_OK` when an extension was found and - // removed. It returns `S_FALSE` when there is no extension. In this case, we'll - // pass this executable as the DotSlash file path. `dotslash` will fail and - // complain that it's not a valid DotSlash file. + // The command now looks like `dotslash "C:\path\to\foo`. // - let found_extension = PathCchRemoveExtension(ds_file_ptr, new_len + 1); - if found_extension != S_OK && found_extension != S_FALSE { - fatal("PathCchRemoveExtension failed."); - } + ds_cmd.len -= 4; - // Quote the entire DotSlash file path if there are spaces. - // - // No need to worry about escaping quotes because those aren't - // allowed in Windows paths. - // - // For an executable named `foo.exe` this is a noop. - // - // For an executable named `foo bar.exe` we should now have a command - // string that looks like `dotslash "C:\path\to\foo bar"`. - PathQuoteSpacesW(ds_file_ptr); + // Always close the quote around the DotSlash file path. Quoting paths + // without spaces is valid, and no escaping is needed because quotes + // are not allowed in Windows paths. + ds_cmd.append(w!("\"")); // Get the arguments that were passed to us. let line_ptr: PCWSTR = GetCommandLineW(); // Skip `argv[0]` and focus on the remaining arguments. - let args_ptr: PWSTR = PathGetArgsW(line_ptr); + let args_ptr = command_line_args(line_ptr); // Append the arguments to the command string if there are any. if *args_ptr != 0 { - // Append a separator for the arguments. + // Normalize the discarded argv[0] separator whitespace to one + // space. The argument tail itself remains untouched. ds_cmd.append(w!(" ")); ds_cmd.append(args_ptr); } @@ -226,6 +220,10 @@ fn main_impl() -> ! { si.cb = mem::size_of::() as DWORD; let mut pi: PROCESS_INFORMATION = unsafe { mem::zeroed() }; + // A null application name makes CreateProcessW resolve the first command + // token (`dotslash`) normally, including through PATH. Handles, the + // environment, and the working directory are inherited so the shim is + // transparent to the child process. let status = unsafe { CreateProcessW( ptr::null_mut(), // lpApplicationName @@ -241,10 +239,12 @@ fn main_impl() -> ! { ) }; - // Once CreateProcessW is called, there is no need to hold onto - // lpCommandLine. - // https://stackoverflow.com/a/31031165 - drop(ds_cmd); + // CreateProcessW has finished reading the command line when it returns, so + // release the maximum-sized buffer before a potentially long child wait. + // Capture a failure code first because cleanup may change the thread's + // last-error value. + let err = unsafe { GetLastError() }; + unsafe { HeapFree(GetProcessHeap(), 0, ds_cmd.buf.cast()) }; if status == TRUE { let res = unsafe { WaitForSingleObject(pi.hProcess, INFINITE) }; @@ -258,14 +258,11 @@ fn main_impl() -> ! { fatal("could not get dotslash command exit code."); } - unsafe { - CloseHandle(pi.hProcess); - CloseHandle(pi.hThread); - ExitProcess(status) - }; + // Process teardown closes both handles in pi. Closing the thread handle + // earlier would add code and an import without materially reducing RSS. + unsafe { ExitProcess(status) }; } - let err = unsafe { GetLastError() }; if err == ERROR_FILE_NOT_FOUND { fatal("dotslash executable not found."); } diff --git a/windows_shim/release.py b/windows_shim/release.py index dc5973c..82218b0 100644 --- a/windows_shim/release.py +++ b/windows_shim/release.py @@ -10,6 +10,7 @@ import os import shutil +import struct import subprocess import sys from pathlib import Path @@ -19,6 +20,36 @@ target_triplets: list[str] = ["x86_64-pc-windows-msvc", "aarch64-pc-windows-msvc"] +def write_linker_stub(path: Path) -> None: + # A PE image starts with an MZ-compatible prefix whose field at offset + # 0x3c points Windows to the PE signature. lld-link's default prefix makes + # the headers spill into a second 512-byte file-alignment block. Supplying + # this minimal valid prefix via /STUB keeps SizeOfHeaders, and therefore the + # complete shim, 512 bytes smaller without changing how Windows starts it. + # + # References: + # - Microsoft documents the PE/COFF format at + # https://learn.microsoft.com/en-us/windows/win32/debug/pe-format. + # - Microsoft documents the /STUB linker option at + # https://learn.microsoft.com/en-us/cpp/build/reference/stub-ms-dos-stub-file-name + # + # The header describes one 69-byte image with a 64-byte header and no + # relocations. Its five-byte payload makes the input a complete executable, + # as required by /STUB; Windows does not execute it when loading the PE. + stub = bytearray(64) + # These fields describe the complete one-page image and its 64-byte header. + struct.pack_into(" None: if not IS_WINDOWS: raise Exception("Only Windows is supported.") @@ -55,17 +86,23 @@ def main(targets: list[str] | None = None) -> None: if not rust_lld.is_file(): raise FileNotFoundError(f"Rust's bundled linker was not found: {rust_lld}") + # Regenerate the checked-in linker input before any selected release binary. + linker_stub = dotslash_windows_shim_root / "dotslash_windows_linker_stub.exe" + write_linker_stub(linker_stub) + rustflags = [ f"-Clinker={rust_lld}", "-Clinker-flavor=lld-link", "-Clink-arg=/DEBUG:NONE", # Avoid an embedded PDB path. "-Clink-arg=/NODEFAULTLIB:msvcrt", # The shim does not use the CRT. "-Clink-arg=/Brepro", # Hash-based timestamps instead of wall-clock time. + "-Clink-arg=/MERGE:.pdata=.rdata", # Both sections are read-only. + f"-Clink-arg=/STUB:{linker_stub}", ] # Ambient RUSTFLAGS could change the measured release layout and break - # reproducibility. Encoded flags also preserve the linker path as a single - # argument when the workspace path contains spaces. + # reproducibility. Encoded flags also preserve the linker and stub paths as + # single arguments when the workspace path contains spaces. build_env = {**os.environ} build_env.pop("RUSTFLAGS", None) build_env["RUSTC_BOOTSTRAP"] = "1" # Required by no_std language items. diff --git a/windows_shim/tests/test.py b/windows_shim/tests/test.py index ce1d671..6193369 100755 --- a/windows_shim/tests/test.py +++ b/windows_shim/tests/test.py @@ -22,6 +22,20 @@ from typing import Final EMPTY_STR_LIST: Final[list[str]] = [] +WINDOWS_SHIM_ROOT: Final[Path] = Path(__file__).resolve().parent.parent + +# CreateProcessW limits lpCommandLine to 32,767 UTF-16 code units, including +# the terminating null character. +# https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessw#parameters +MAX_COMMAND_LINE_LENGTH: Final[int] = 32767 + +# Keep size changes visible in review: both increases and improvements should +# update these expectations in the same PR as the regenerated artifacts. +RELEASE_ARTIFACT_SIZES: Final[dict[str, int]] = { + "dotslash_windows_linker_stub.exe": 69, + "dotslash_windows_shim-aarch64.exe": 2560, + "dotslash_windows_shim-x86_64.exe": 2560, +} try: from .fb.ci import set_ci_envs @@ -53,6 +67,20 @@ def test_require_dotslash_windows_shim_env(self) -> None: self.assertTrue(os.path.exists(os.environ["DOTSLASH_WINDOWS_SHIM"])) +class ReleaseArtifactSizeTest(unittest.TestCase): + def test_release_artifact_sizes(self) -> None: + for filename, expected_bytes in RELEASE_ARTIFACT_SIZES.items(): + with self.subTest(filename=filename): + artifact = WINDOWS_SHIM_ROOT / filename + actual_bytes = artifact.stat().st_size + self.assertEqual( + actual_bytes, + expected_bytes, + f"{filename} changed from {expected_bytes} bytes " + f"to {actual_bytes} bytes", + ) + + def generate_dotslash_file(name: str) -> str: spec = { "name": name, @@ -190,29 +218,6 @@ def test_args_none_with_period_in_name(self) -> None: self.assertRegex(ret.stdout, PRINT_ARGS_ARG0) self.assertEqual(ret.returncode, 0) - def test_args_none_with_no_extension(self) -> None: - with move_cwd(self._fixtures): - shutil.move("print_args.exe", "print_args") - - # This executes because CreateProcessW adds an implicit `.exe`. - # PathCchRemoveExtension won't have an extension to remove, - # so we'll pass the exetuable to `dotslash`, which will then - # fail because it's not an actual DotSlash file. - print_args_path = self._fixtures / "print_args" - ret = subprocess.run( - [print_args_path], - capture_output=True, - encoding="utf8", - ) - self.assertEqual( - ret.stderr, - f"dotslash error: problem with `{print_args_path}`\n" - "caused by: failed to read DotSlash file\n" - "caused by: stream did not contain valid UTF-8\n", - ) - self.assertEqual(ret.stdout, "") - self.assertEqual(ret.returncode, 1) - def test_args_none_with_unc_path(self) -> None: ret = subprocess.run( ["\\\\?\\" + str(self._fixtures / "print_args.exe")], @@ -275,6 +280,52 @@ def test_args_simple(self) -> None: self.assertRegex(ret.stdout, PRINT_ARGS_ARG0) self.assertEqual(ret.returncode, 0) + def test_raw_command_lines(self) -> None: + actual_shim = self._fixtures / "print_args.exe" + + # A copy of the shim whose own path contains a space, to check that a + # quoted argv[0] is still skipped correctly. + spaced_manifest = self._fixtures / "print args" + spaced_shim = self._fixtures / "print args.exe" + shutil.copy(self._fixtures / "print_args", spaced_manifest) + shutil.copy(actual_shim, spaced_shim) + + cases = [ + (actual_shim, "print_args.exe", ""), + (actual_shim, "print_args.exe one two", "1:one\n2:two\n"), + (actual_shim, 'print_args.exe "one two"', "1:one two\n"), + (actual_shim, 'print_args.exe "" "a\\\"b" trailing\\', '1:\n2:a"b\n3:trailing\\\n'), + (actual_shim, "print_args.exe\tone\ttwo", "1:one\n2:two\n"), + (actual_shim, 'print" args".exe one', "1:one\n"), + (actual_shim, '"print_args.exe"suffix one', "1:one\n"), + (actual_shim, '"print_args.exe"one', ""), + (spaced_shim, f'"{spaced_shim}" one', "1:one\n"), + (spaced_shim, f'"{spaced_shim}" "one two"', "1:one two\n"), + ] + for shim, raw_command_line, stderr in cases: + with self.subTest(raw_command_line=raw_command_line): + ret = subprocess.run( + raw_command_line, + executable=str(shim), + capture_output=True, + encoding="utf8", + ) + self.assertEqual(ret.stderr, stderr) + self.assertRegex(ret.stdout, PRINT_ARGS_ARG0) + self.assertEqual(ret.returncode, 0) + + def test_args_none_with_uppercase_exe_extension(self) -> None: + uppercase_shim = self._fixtures / "print_args.EXE" + shutil.move(self._fixtures / "print_args.exe", uppercase_shim) + ret = subprocess.run( + [str(uppercase_shim)], + capture_output=True, + encoding="utf8", + ) + self.assertEqual(ret.stderr, "") + self.assertRegex(ret.stdout, PRINT_ARGS_ARG0) + self.assertEqual(ret.returncode, 0) + def test_args_simple_with_unicode_in_name(self) -> None: shutil.move( self._fixtures / "print_args", @@ -367,15 +418,12 @@ def test_args_unicode(self) -> None: self.assertEqual(ret.returncode, 0) def test_args_long_args(self) -> None: - # Windows CreateProcess API has a length limit of 32,768. # The shim will actually use a bit more than the original call: # Original: foo.exe a b c # Shim: dotslash foo a b c # Actually more than the above because "foo" is resolved to an # absolute path. So here we test staying a bit below this limit. - # https://docs.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessa#parameters - MAX_COMMAND_LINE = 32768 - long_arg = "x" * (MAX_COMMAND_LINE - 512) + long_arg = "x" * (MAX_COMMAND_LINE_LENGTH - 512) ret = subprocess.run( [str(self._fixtures / "print_args.exe"), long_arg], capture_output=True, @@ -385,6 +433,26 @@ def test_args_long_args(self) -> None: self.assertRegex(ret.stdout, PRINT_ARGS_ARG0) self.assertEqual(ret.returncode, 0) + def test_args_over_command_buffer_boundary(self) -> None: + actual_shim = self._fixtures / "print_args.exe" + manifest = self._fixtures / "print_args" + capacity = MAX_COMMAND_LINE_LENGTH - 1 + fixed_command_length = len('dotslash "') + len(str(manifest)) + len('" ') + overflowing_tail = "x" * (capacity - fixed_command_length + 1) + + ret = subprocess.run( + f"print_args.exe {overflowing_tail}", + executable=str(actual_shim), + capture_output=True, + encoding="utf8", + ) + self.assertEqual( + ret.stderr, + "dotslash-windows-shim: Buffer overflow\n", + ) + self.assertEqual(ret.stdout, "") + self.assertEqual(ret.returncode, 1) + def test_stdin_to_stdout(self) -> None: ret = subprocess.run( [str(self._fixtures / "stdin_to_stdout.exe")], @@ -431,6 +499,23 @@ def test_missing_dotslash(self) -> None: self.assertEqual(ret.stdout, "") self.assertEqual(ret.returncode, 1) + def test_invalid_dotslash_executable(self) -> None: + invalid_bin = self._fixtures / "invalid_bin" + invalid_bin.mkdir() + (invalid_bin / "dotslash.exe").write_bytes(b"not a Windows executable") + with prepend_path(invalid_bin): + ret = subprocess.run( + [str(self._fixtures / "exit_code.exe"), "0"], + capture_output=True, + encoding="utf8", + ) + self.assertEqual( + ret.stderr, + "dotslash-windows-shim: could not execute dotslash command.\n", + ) + self.assertEqual(ret.stdout, "") + self.assertEqual(ret.returncode, 1) + if __name__ == "__main__": unittest.main(verbosity=2)