From 1df6b06b6686cae1ca2873c9d30ebb9983205a93 Mon Sep 17 00:00:00 2001 From: Vlad Dyshakov Date: Mon, 6 Apr 2026 10:15:38 +0700 Subject: [PATCH] ILVerify: soft comparison ignoring IL byte offset drift (#18090) Add offset-tolerant fallback to ILVerify baseline comparison so that changes which only shift IL byte offsets no longer fail CI. --- .github/skills/ilverify-failure/SKILL.md | 4 + DEVGUIDE.md | 3 + tests/ILVerify/ilverify.Tests.ps1 | 138 +++++++++++++++++++++++ tests/ILVerify/ilverify.ps1 | 48 ++++++-- 4 files changed, 181 insertions(+), 12 deletions(-) create mode 100644 tests/ILVerify/ilverify.Tests.ps1 diff --git a/.github/skills/ilverify-failure/SKILL.md b/.github/skills/ilverify-failure/SKILL.md index dfc30ceda33..85eaf582ac3 100644 --- a/.github/skills/ilverify-failure/SKILL.md +++ b/.github/skills/ilverify-failure/SKILL.md @@ -8,10 +8,14 @@ description: Fix ILVerify baseline failures when IL shape changes (codegen, new ## When to Use IL shape changed (codegen, new types, method signatures) and ILVerify CI job fails. +## Offset-Only Differences +Changes that only shift IL byte offsets (`[offset 0x...]`) — for example adding code above an existing error site — are detected automatically and **do not fail CI**. A warning is printed suggesting a baseline update. No action is required, but refreshing baselines keeps them accurate. + ## Update Baselines ```bash TEST_UPDATE_BSL=1 pwsh tests/ILVerify/ilverify.ps1 ``` +Or use the `/run ilverify` PR comment command to update baselines via CI. ## Baselines Location `tests/ILVerify/*.bsl` diff --git a/DEVGUIDE.md b/DEVGUIDE.md index 568b64bfe0d..83e8292aa20 100644 --- a/DEVGUIDE.md +++ b/DEVGUIDE.md @@ -335,10 +335,13 @@ ilverify_FSharp.Core_Release_netstandard2.0.bsl ilverify_FSharp.Core_Release_netstandard2.1.bsl ``` +The comparison uses a two-level approach: an exact match is tried first, then a **soft comparison** that ignores IL byte offsets (`[offset 0x...]`) and trailing whitespace. Offsets shift whenever code above the error site changes, even though the verification error itself is semantically identical. When only offsets differ the check passes with a warning suggesting a baseline update. + If you want to update them, either 1. Run the [ilverify.ps1]([url](https://github.com/dotnet/fsharp/blob/main/tests/ILVerify/ilverify.ps1)) script in PowerShell. The script will create `.actual` files. If the differences make sense, replace the original baselines with the actual files. 2. Set the `TEST_UPDATE_BSL` to `1` (please refer to "Updating baselines in tests" section in this file) **and** run `ilverify.ps1` - this will automatically replace baselines. After that, please carefully review the change and push it to your branch if it makes sense. +3. Use the `/run ilverify` PR comment command to update baselines automatically via CI. ## Automated Source Code Formatting diff --git a/tests/ILVerify/ilverify.Tests.ps1 b/tests/ILVerify/ilverify.Tests.ps1 new file mode 100644 index 00000000000..20167776cfb --- /dev/null +++ b/tests/ILVerify/ilverify.Tests.ps1 @@ -0,0 +1,138 @@ +# Pester tests for ILVerify helper functions defined in ilverify.ps1. +# Compatible with Pester 3.x+ (ships with Windows PowerShell). +# Run with: Invoke-Pester ./tests/ILVerify/ilverify.Tests.ps1 + +# Extract and load only the function definitions from ilverify.ps1 +# without executing the build/verification logic. +$scriptContent = Get-Content "$PSScriptRoot/ilverify.ps1" -Raw +$ast = [System.Management.Automation.Language.Parser]::ParseInput($scriptContent, [ref]$null, [ref]$null) +$functions = $ast.FindAll({ $args[0] -is [System.Management.Automation.Language.FunctionDefinitionAst] }, $false) +foreach ($fn in $functions) { + Invoke-Expression $fn.Extent.Text +} + +Describe "Normalize-IlverifyOutputLine" { + It "removes closure suffixes" { + Normalize-IlverifyOutputLine 'Foo+clo@924-516::Invoke()' | Should Be 'Foo+clo::Invoke()' + } + + It "removes function suffixes with line numbers" { + Normalize-IlverifyOutputLine 'parseOption@269::Bar()' | Should Be 'parseOption::Bar()' + } + + It "removes 'at line NNNN'" { + Normalize-IlverifyOutputLine 'something at line 1234 rest' | Should Be 'something rest' + } + + It "removes pipe stage patterns" { + Normalize-IlverifyOutputLine 'Foo+Pipe #1 stage #1 at line 1782@1782::Invoke()' | Should Be 'Foo+::Invoke()' + } + + It "collapses multiple spaces" { + Normalize-IlverifyOutputLine 'a b c' | Should Be 'a b c' + } + + It "trims leading and trailing whitespace" { + Normalize-IlverifyOutputLine ' hello world ' | Should Be 'hello world' + } +} + +Describe "Remove-IlverifyOffsets" { + It "strips a single offset from an error line" { + $line = "[IL]: Error [StackByRef]: : Foo::Bar(int32)][offset 0x0000001E][found Native Int] Expected ByRef." + $result = Remove-IlverifyOffsets @($line) + $result | Should Be "[IL]: Error [StackByRef]: : Foo::Bar(int32)][found Native Int] Expected ByRef." + } + + It "handles uppercase and lowercase hex digits" { + $result = Remove-IlverifyOffsets @("prefix [offset 0xABcd0012] suffix") + $result | Should Be "prefix suffix" + } + + It "trims trailing whitespace" { + $result = Remove-IlverifyOffsets @("some text ") + $result | Should Be "some text" + } + + It "returns empty array for empty input" { + $result = @(Remove-IlverifyOffsets @()) + $result.Count | Should Be 0 + } + + It "processes multiple lines" { + $lines = @( + "[IL]: Error [X]: : A::M()][offset 0x00000011][found Y] Msg1.", + "[IL]: Error [X]: : B::N()][offset 0x00000022][found Z] Msg2." + ) + $result = Remove-IlverifyOffsets $lines + $result.Count | Should Be 2 + $result[0] | Should Be "[IL]: Error [X]: : A::M()][found Y] Msg1." + $result[1] | Should Be "[IL]: Error [X]: : B::N()][found Z] Msg2." + } + + It "passes through lines without offsets unchanged" { + $result = Remove-IlverifyOffsets @("no offsets here") + $result | Should Be "no offsets here" + } +} + +Describe "Soft comparison (offset-tolerant)" { + It "matches when only IL offsets differ" { + $output = @( + "[IL]: Error [StackByRef]: : Foo::Bar()][offset 0x0000001E][found Native Int] Expected ByRef.", + "[IL]: Error [ReturnPtrToStack]: : Baz::Qux()][offset 0x00000070] Return type is ByRef." + ) + $baseline = @( + "[IL]: Error [StackByRef]: : Foo::Bar()][offset 0x0000001A][found Native Int] Expected ByRef.", + "[IL]: Error [ReturnPtrToStack]: : Baz::Qux()][offset 0x00000064] Return type is ByRef." + ) + $cmp = Compare-Object (Remove-IlverifyOffsets $output) (Remove-IlverifyOffsets $baseline) + $cmp | Should BeNullOrEmpty + } + + It "detects real differences even when offsets also differ" { + $output = @( + "[IL]: Error [StackByRef]: : Foo::Bar()][offset 0x0000001E][found Native Int] Expected ByRef.", + "[IL]: Error [NEW_ERROR]: : New::Method()][offset 0x00000099][found X] New error." + ) + $baseline = @( + "[IL]: Error [StackByRef]: : Foo::Bar()][offset 0x0000001A][found Native Int] Expected ByRef.", + "[IL]: Error [ReturnPtrToStack]: : Baz::Qux()][offset 0x00000064] Return type is ByRef." + ) + $cmp = Compare-Object (Remove-IlverifyOffsets $output) (Remove-IlverifyOffsets $baseline) + $cmp | Should Not BeNullOrEmpty + } + + It "detects added errors" { + $output = @( + "[IL]: Error [X]: : Foo::A()][offset 0x00000011] Msg.", + "[IL]: Error [X]: : Foo::B()][offset 0x00000022] Msg.", + "[IL]: Error [X]: : Foo::C()][offset 0x00000033] New." + ) + $baseline = @( + "[IL]: Error [X]: : Foo::A()][offset 0x00000011] Msg.", + "[IL]: Error [X]: : Foo::B()][offset 0x00000022] Msg." + ) + $cmp = Compare-Object (Remove-IlverifyOffsets $output) (Remove-IlverifyOffsets $baseline) + $cmp | Should Not BeNullOrEmpty + } + + It "detects removed errors" { + $output = @( + "[IL]: Error [X]: : Foo::A()][offset 0x00000011] Msg." + ) + $baseline = @( + "[IL]: Error [X]: : Foo::A()][offset 0x00000011] Msg.", + "[IL]: Error [X]: : Foo::B()][offset 0x00000022] Msg." + ) + $cmp = Compare-Object (Remove-IlverifyOffsets $output) (Remove-IlverifyOffsets $baseline) + $cmp | Should Not BeNullOrEmpty + } + + It "handles trailing whitespace differences between output and baseline" { + $output = @("[IL]: Error [X]: : Foo::A()][offset 0x00000011] Msg. ") + $baseline = @("[IL]: Error [X]: : Foo::A()][offset 0x000000FF] Msg.") + $cmp = Compare-Object (Remove-IlverifyOffsets $output) (Remove-IlverifyOffsets $baseline) + $cmp | Should BeNullOrEmpty + } +} diff --git a/tests/ILVerify/ilverify.ps1 b/tests/ILVerify/ilverify.ps1 index 41733ff2abf..d59e325b30f 100644 --- a/tests/ILVerify/ilverify.ps1 +++ b/tests/ILVerify/ilverify.ps1 @@ -18,6 +18,18 @@ function Normalize-IlverifyOutputLine { return $line } +function Remove-IlverifyOffsets { + param( + [string[]]$lines + ) + # Strip IL byte offsets and trailing whitespace for soft comparison. + # Offsets like [offset 0x0000001E] change when code above is modified, + # even though the verification error itself is the same. + return @($lines | ForEach-Object { + ($_ -replace '\[offset 0x[0-9A-Fa-f]+\]', '').Trim() + }) +} + # Set build script based on which OS we're running on - Windows (build.cmd), Linux or macOS (build.sh) Write-Host "Checking whether running on Windows: $IsWindows" @@ -37,8 +49,8 @@ $env:PublishWindowsPdb = "false" # Set configurations to build [string[]] $configurations = @("Debug", "Release") -# The following are not passing ilverify checks, so we ignore them for now -[string[]] $ignore_errors = @() # @("StackUnexpected", "UnmanagedPointer", "StackByRef", "ReturnPtrToStack", "ExpectedNumericType", "StackUnderflow") +# Error types that should be excluded from verification via the -g flag (currently none). +[string[]] $ignore_errors = @() [string] $default_tfm = "netstandard2.0" # Read product TFM from centralized source of truth via MSBuild @@ -197,19 +209,31 @@ foreach ($project in $projects.Keys) { if (-not $cmp) { Write-Host "ILverify output matches baseline." } else { - Write-Host "ILverify output does not match baseline, differences:" + # Exact match failed — try soft comparison ignoring IL byte offsets and whitespace. + # IL offsets drift when code above the error site changes, even though the + # verification error itself is semantically identical. + $outputSoft = Remove-IlverifyOffsets $ilverify_output + $baselineSoft = Remove-IlverifyOffsets $baseline + $cmpSoft = Compare-Object $outputSoft $baselineSoft + + if (-not $cmpSoft) { + Write-Host "ILverify output matches baseline (IL offsets differ, errors are the same)." + Write-Host " Consider updating baselines: run with TEST_UPDATE_BSL=1 or use '/run ilverify' PR comment." + } else { + Write-Host "ILverify output does not match baseline, differences:" - $cmp | Format-Table -AutoSize -Wrap | Out-String | Write-Host + $cmp | Format-Table -AutoSize -Wrap | Out-String | Write-Host - # Update baselines if TEST_UPDATE_BSL is set to 1 - if ($env:TEST_UPDATE_BSL -eq "1") { - Write-Host "Updating baseline file: $baseline_file" - $ilverify_output | Set-Content $baseline_file - } else { - $ilverify_output | Set-Content $baseline_actual_file + # Update baselines if TEST_UPDATE_BSL is set to 1 + if ($env:TEST_UPDATE_BSL -eq "1") { + Write-Host "Updating baseline file: $baseline_file" + $ilverify_output | Set-Content $baseline_file + } else { + $ilverify_output | Set-Content $baseline_actual_file + } + $failed = $true + continue } - $failed = $true - continue }