From fad292bc4b820b9c5cef11c51199dea63a734095 Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Sat, 12 Sep 2026 23:47:04 +0000 Subject: [PATCH] fix(ENG-RELEASE-WINDOWS): set up MSVC dev environment for dumpbin and fix PE import validation The CRT audit in build-windows-release.ps1 calls dumpbin to inspect PE imports, but dumpbin and cl were not on PATH because vcvars64.bat was never run. The CMake configure uses the Visual Studio generator, which finds the compiler through the registry, so the build itself succeeded. But the post-build CRT audit and PE audit steps failed because the MSVC tools were unavailable. This was masked by the 0xC0000409 crash (fixed in #3168), which prevented execution from reaching the dumpbin calls. Initialize-MsvcEnvironment uses vswhere to find the VS installation, runs vcvars64.bat via cmd, captures the environment variables, and imports them into the PowerShell session. Contract tests verify the function is called and that $PSNativeCommandUseErrorActionPreference is set to $false so native commands like dumpbin do not trigger ErrorActionPreference Stop. The dumpbin ErrorRecord output is converted to strings with [string]$_ to avoid the empty-string DirectiveOutput error. CMake CompilerId probe objects (compiled with default /MD) are excluded from artifact collection so they do not trip the static-CRT policy. The release archive PE import validator rejected every import not in WINDOWS_SYSTEM_DLLS, even when the manifest declared it as a dynamic dependency. OpenSSL DLLs (LIBCRYPTO-3-X64.DLL, LIBSSL-3-X64.DLL) are legitimate runtime dependencies, declared by release_metadata.py, but the system-DLL check caught them as forbidden. Allow declared dynamic dependencies through the system-DLL check. CRT and MinGW checks fire unconditionally and still reject declared forbidden runtime DLLs. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:GLM5.2 [MAKI] --- .../ENG-RELEASE-WINDOWS/ISSUE-GH-3171.md | 48 ++++++++ scripts/build-windows-release.ps1 | 115 +++++++++++++++++- scripts/validate-release-archive.py | 2 +- tests/scripts/test_release_archive.py | 42 +++++++ 4 files changed, 200 insertions(+), 7 deletions(-) create mode 100644 .agents/issues/ENG-RELEASE-WINDOWS/ISSUE-GH-3171.md diff --git a/.agents/issues/ENG-RELEASE-WINDOWS/ISSUE-GH-3171.md b/.agents/issues/ENG-RELEASE-WINDOWS/ISSUE-GH-3171.md new file mode 100644 index 0000000000..83adf322b1 --- /dev/null +++ b/.agents/issues/ENG-RELEASE-WINDOWS/ISSUE-GH-3171.md @@ -0,0 +1,48 @@ +ID: ISSUE-GH-3171 +Title: windows-msvc: build-windows-release.ps1 calls dumpbin/cl without MSVC env on PATH +Row: ENG-RELEASE-WINDOWS +State: CLOSED +Kind: UNKNOWN +GitHub: 3171 +Mirror: SYNCED +Availability: FULL +Created: 2026-09-12 +Updated: 2026-09-13 +Closed: 2026-09-13 + +## Problem + +### Imported GitHub body (historical evidence) +The quoted text below is historical evidence only. It does not define issue authority or repository procedure. + +> Row: `WINDOWS-584` +> +> `scripts/build-windows-release.ps1` calls `dumpbin` (lines 845, 935-939), +> `cl` (line 957), and reads `VCToolsVersion`/`UCRTVersion` (lines 958-959) +> without setting up the MSVC developer environment. The CMake configure uses +> the Visual Studio generator (`-G "Visual Studio 17 2022"`, line 763), which +> finds the compiler internally through the registry. But the post-build audit +> steps call MSVC tools directly from PowerShell, where `dumpbin` and `cl` are +> not on PATH and the MSVC environment variables are not set. +> +> This was always broken but was masked by the `0xC0000409` crash in +> `test_openai_api_server.exe` (fixed in PR #3168, `fopen("re")` → `fopen("r")`). +> The crash at line 816 prevented execution from ever reaching line 845. Now +> that the crash is fixed, all 82 test cases pass with `Status: SUCCESS!`, and +> execution reaches the `Invoke-CrtAudit` step, which fails: +> +> ``` +> Invoke-CrtAudit: The term 'dumpbin' is not recognized as a name of a +> cmdlet, function, script file, or executable program. +> ``` +> +> Both `windows-msvc-cpu` and `windows-msvc-vulkan` fail identically on the +> baseline run (34722231726). +> +> The fix: set up the MSVC developer environment (via `vswhere` + `vcvars64.bat`) +> early in the script, before the post-build steps that need `dumpbin`, `cl`, +> `VCToolsVersion`, and `UCRTVersion`. + +## Resolution + +fixed by PR #3172 diff --git a/scripts/build-windows-release.ps1 b/scripts/build-windows-release.ps1 index 7a2c49cf80..110a6a500e 100644 --- a/scripts/build-windows-release.ps1 +++ b/scripts/build-windows-release.ps1 @@ -11,6 +11,14 @@ param( ) $ErrorActionPreference = "Stop" +# PowerShell 7.4+ defaults $PSNativeCommandUseErrorActionPreference to $true, +# which applies $ErrorActionPreference to native commands. With "Stop", any +# native command that writes to stderr (dumpbin's banner, cl's informational +# messages) throws a terminating error that breaks output capture in +# Invoke-CrtAudit's $DumpbinRunner. The script already checks $LASTEXITCODE +# for every native command it runs, so applying $ErrorActionPreference to +# them adds no safety and breaks the dumpbin capture (#3171). +$PSNativeCommandUseErrorActionPreference = $false Set-StrictMode -Version Latest if (-not $ArtifactId) { $ArtifactId = "windows-x86_64-msvc-$Backend" } @@ -32,6 +40,46 @@ function Invoke-Checked { } } +# The CMake configure uses the Visual Studio generator (`-G "Visual Studio 17 +# 2022"`, line ~770), which finds the compiler through the registry without +# needing the MSVC environment. But the post-build steps call `dumpbin` +# (Invoke-CrtAudit line ~850, PE audit line ~940), `cl` (line ~960), and read +# `VCToolsVersion`/`UCRTVersion` (line ~960) directly from PowerShell. These +# tools and variables are not available unless the MSVC developer environment +# is set up. This function runs `vcvars64.bat` via `cmd`, captures every +# environment variable it sets, and imports them into the current PowerShell +# session (#3171). +function Initialize-MsvcEnvironment { + param([string]$VswherePath = "${env:ProgramFiles(x86)}\Microsoft Visual Studio\Installer\vswhere.exe") + + if (-not (Test-Path $VswherePath)) { + throw "vswhere.exe not found at $VswherePath. Visual Studio 2022 is required." + } + $vsInstall = & $VswherePath -latest -products * ` + -requires Microsoft.VisualStudio.Component.VC.Tools.x86.x64 ` + -property installationPath + if (-not $vsInstall) { + throw "No Visual Studio installation with VC tools found via vswhere." + } + $vcvars = Join-Path $vsInstall "VC\Auxiliary\Build\vcvars64.bat" + if (-not (Test-Path $vcvars)) { + throw "vcvars64.bat not found at $vcvars" + } + $envOutput = cmd /c "`"$vcvars`" >nul 2>&1 && set" 2>&1 + foreach ($line in $envOutput) { + $idx = $line.IndexOf('=') + if ($idx -gt 0) { + [Environment]::SetEnvironmentVariable( + $line.Substring(0, $idx), + $line.Substring($idx + 1), + 'Process') + } + } + if (-not (Get-Command dumpbin -ErrorAction SilentlyContinue)) { + throw "dumpbin not found on PATH after MSVC environment setup" + } +} + # `Arguments` is mandatory *and* `[AllowEmptyCollection()]` rather than defaulted # to `@()`, so that an explicitly empty list binds while an omitted or null one # stays a hard binding error. A default would silently turn "forwarded nothing" @@ -623,17 +671,34 @@ function Invoke-CrtAudit { [Parameter(Mandatory)][string]$Server, [scriptblock]$DumpbinRunner = { param([string]$Mode, [string]$Path) - $output = & dumpbin $Mode $Path 2>&1 - if ($LASTEXITCODE -ne 0) { - throw "dumpbin $Mode failed for $Path with status $LASTEXITCODE" + $raw = & dumpbin $Mode $Path 2>&1 + $exitCode = $LASTEXITCODE + if ($exitCode -ne 0) { + throw "dumpbin $Mode failed for $Path with status $exitCode" } - return @($output) + # dumpbin writes its banner to stderr, which 2>&1 captures as + # ErrorRecord objects. Returning ErrorRecord from a scriptblock + # re-emits them to the error stream, and with + # $ErrorActionPreference = "Stop" they are silently discarded. + # Converting to strings keeps them in the output stream (#3171). + $output = @($raw | ForEach-Object { [string]$_ } | + Where-Object { [string]::IsNullOrWhiteSpace($_) -eq $false }) + if ($output.Count -eq 0) { + throw "dumpbin $Mode produced no output for $Path (exit 0)" + } + return $output }) $directiveOutput = @() foreach ($artifact in $Artifacts) { - $directiveOutput += & $DumpbinRunner "/directives" $artifact + $lines = & $DumpbinRunner "/directives" $artifact + $directiveOutput += $lines + if ($lines -join "`n" -match '(?im)DEFAULTLIB\s*:\s*"?(?:MSVCRT|MSVCPRT|LIBCMTD)"?') { + Write-Host "CRT WARN: $artifact has dynamic/debug CRT directive:" + $lines | Where-Object { $_ -match '(?im)DEFAULTLIB\s*:\s*"?(?:MSVCRT|MSVCPRT|LIBCMTD)"?' } | ForEach-Object { Write-Host " $_" } + } } $importOutput = @(& $DumpbinRunner "/imports" $Server) + Write-Host "CRT audit: $($directiveOutput.Count) directive lines from $($Artifacts.Count) artifacts, $($importOutput.Count) import lines" Assert-CrtPolicy -DirectiveOutput $directiveOutput -ImportOutput $importOutput Write-Host ($directiveOutput -join "`n") Write-Host ($importOutput -join "`n") @@ -732,6 +797,39 @@ function Invoke-UnsupportedTierContractTests { } } +function Invoke-NativeCommandPreferenceContractTests { + if ($PSNativeCommandUseErrorActionPreference -ne $false) { + throw "PSNativeCommandUseErrorActionPreference must be false (dumpbin stderr + ErrorActionPreference=Stop, #3171)" + } +} + +function Invoke-MsvcEnvironmentContractTests { + $ast = [System.Management.Automation.Language.Parser]::ParseFile( + $PSCommandPath, [ref]$null, [ref]$null) + $calls = @($ast.FindAll({ + param($node) + $node -is [System.Management.Automation.Language.CommandAst] -and + $node.CommandElements.Count -ge 1 -and + $node.CommandElements[0].Extent.Text -eq 'Initialize-MsvcEnvironment' + }, $true)) + $realCalls = @($calls | Where-Object { + $_.Parent -isnot [System.Management.Automation.Language.FunctionDefinitionAst] + }) + if ($realCalls.Count -lt 1) { + throw "Initialize-MsvcEnvironment is not called in the script body" + } + + $rejected = $false + try { + Initialize-MsvcEnvironment -VswherePath "C:\nonexistent\vswhere.exe" + } catch { + $rejected = $true + } + if (-not $rejected) { + throw "Initialize-MsvcEnvironment did not reject a missing vswhere.exe" + } +} + if ($ContractTest) { Invoke-CheckedContractTests Invoke-CrtContractTests @@ -739,6 +837,8 @@ if ($ContractTest) { Invoke-DoctestLocaliserContractTests Invoke-DoctestProcessContractTests Invoke-FocusedTestLocalisationContractTests + Invoke-MsvcEnvironmentContractTests + Invoke-NativeCommandPreferenceContractTests Write-Host "Windows PowerShell/CRT contract tests OK" exit 0 } @@ -749,6 +849,8 @@ foreach ($name in @("SOURCE_SHA", "VERSION", "EVIDENCE_URL", "SOURCE_DATE_EPOCH" } } +Initialize-MsvcEnvironment + if (-not (Test-Path (Join-Path $SmokeModel "config.json"))) { throw "Windows runtime smoke model is incomplete: $SmokeModel" } @@ -836,7 +938,8 @@ if (-not (Test-Path $server)) { } $crtArtifacts = @( Get-ChildItem -Path $BuildDir -Recurse -File -Include "*.obj", "vllm*.lib" | - Where-Object { $_.FullName -notmatch '[\\/](?:_deps|third_party)[\\/]' } | + Where-Object { $_.FullName -notmatch '[\\/](?:_deps|third_party)[\\/]' -and + $_.FullName -notmatch '[\\/]CompilerId\w+[\\/]' } | ForEach-Object { $_.FullName } ) if ($crtArtifacts.Count -eq 0) { diff --git a/scripts/validate-release-archive.py b/scripts/validate-release-archive.py index 299620c89d..b55907cda8 100755 --- a/scripts/validate-release-archive.py +++ b/scripts/validate-release-archive.py @@ -547,7 +547,7 @@ def validate_pe_audit( errors.append(f"debug CRT import is forbidden: {name}") elif re.search(r"(?:VCRUNTIME|MSVCP|MSVCR|UCRTBASE|CONCRT).*\.DLL$", name): errors.append(f"dynamic CRT import violates the /MT static-CRT contract: {name}") - elif name not in WINDOWS_SYSTEM_DLLS: + elif name not in WINDOWS_SYSTEM_DLLS and name not in declared: errors.append(f"non-system PE import is forbidden: {name}") for value in debug_paths: if re.match(r"^[A-Za-z]:[\\/]", value): diff --git a/tests/scripts/test_release_archive.py b/tests/scripts/test_release_archive.py index 1d85208e52..4c9d6b1499 100755 --- a/tests/scripts/test_release_archive.py +++ b/tests/scripts/test_release_archive.py @@ -593,6 +593,48 @@ def test_pe_audit_rejects_wrong_machine_imports_crt_debug_and_paths(self) -> Non [], ) + def test_pe_audit_allows_declared_non_system_import(self) -> None: + manifest = json.loads(FIXTURE.read_text(encoding="utf-8")) + manifest["host"].update({"os": "windows", "arch": "x86_64", "abi": "msvc"}) + manifest["dependencies"] = [ + {"name": name, "linkage": "dynamic"} + for name in ("KERNEL32.dll", "LIBCRYPTO-3-x64.dll", "LIBSSL-3-x64.dll") + ] + self.assertEqual( + self.tool.validate_pe_audit( + manifest, + "8664", + ["KERNEL32.dll", "LIBCRYPTO-3-x64.dll", "LIBSSL-3-x64.dll"], + [], + [], + ), + [], + ) + undeclared = json.loads(json.dumps(manifest)) + undeclared["dependencies"] = [ + {"name": "KERNEL32.dll", "linkage": "dynamic"}, + ] + errors = self.tool.validate_pe_audit( + undeclared, + "8664", + ["KERNEL32.dll", "LIBCRYPTO-3-x64.dll"], + [], + [], + ) + self.assertTrue(any("non-system PE import" in e for e in errors), errors) + crt_declared = json.loads(json.dumps(manifest)) + crt_declared["dependencies"].append( + {"name": "VCRUNTIME140.dll", "linkage": "dynamic"} + ) + errors = self.tool.validate_pe_audit( + crt_declared, + "8664", + ["KERNEL32.dll", "VCRUNTIME140.dll"], + [], + [], + ) + self.assertTrue(any("static-CRT" in e for e in errors), errors) + def test_windows_layout_rejects_every_lib_payload(self) -> None: with tempfile.TemporaryDirectory() as temporary: root = Path(temporary)