Skip to content

ILVerify error cleanup - #135798

Merged
jkotas merged 2 commits into
mainfrom
jkotas-verifier-error-cleanup
Oct 16, 2026
Merged

jkotas merged 2 commits into
mainfrom
jkotas-verifier-error-cleanup

Conversation

@jkotas

@jkotas jkotas commented Oct 14, 2026

Copy link
Copy Markdown
Member

The existing verifier already covers most of the legacy diagnostics under newer error codes. I found one concrete gap: unaligned. currently ignores its alignment operand, so I’m adding that check and a regression test, while removing the obsolete commented-out entries.

Closes #37393

The existing verifier already covers most of the legacy diagnostics under newer error codes. I found one concrete gap: `unaligned.` currently ignores its alignment operand, so I’m adding that check and a regression test, while removing the obsolete commented-out entries.

Closes #37393
@github-actions github-actions Bot added the area-Tools-ILVerification Issues related to ilverify tool and IL verification in general label Oct 14, 2026
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @JulieLeeMSFT, @dotnet/jit-contrib
See info in area-owners.md if you want to be subscribed.

@jkotas
jkotas requested review from JulieLeeMSFT and a balanced review from Copilot October 14, 2026 21:34

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The targeted validation and regression cases are straightforward; the remaining diagnostic-message feedback is non-blocking.

1 open finding
What changed in this PR

This PR closes an ILVerify validation gap by checking unaligned. operands and removing obsolete diagnostic comments.

Changes:

  • Accept only alignment values 1, 2, and 4.
  • Add valid-alignment coverage and an invalid-alignment regression case.
  • Add the diagnostic code without renumbering existing codes.

Builds and tests were not run.

File Description
src/​coreclr/​tools/​ILVerification/​VerifierError.cs Removes obsolete comments and adds UnalignedAlignment.
src/​coreclr/​tools/​ILVerification/​ILImporter.Verify.cs Validates the alignment operand.
src/​coreclr/​tools/​ILVerification.Tests/​ILTests/​PrefixTests.il Covers valid values and rejects alignment 3.

🧠 Review effort: Balanced

@jkotas

jkotas commented Oct 15, 2026

Copy link
Copy Markdown
Member Author

/ba-g opened #135843 on the test failure

@JulieLeeMSFT

Copy link
Copy Markdown
Member

LGTM. It correctly restricts unaligned. operands to 1, 2, or 4, with valid/invalid regression coverage and a diagnostic message. The new error is appended, preserving existing numeric codes; the cleanup removes comments, not validation logic.

@jkotas
jkotas merged commit 22255fb into main Oct 16, 2026
120 of 122 checks passed
@jkotas
jkotas deleted the jkotas-verifier-error-cleanup branch October 16, 2026 16:16
pcshrosbree added a commit to pcshrosbree/runtime that referenced this pull request Oct 17, 2026
Resolve the conflict in VerifierError.cs with dotnet#135798, which removed
the commented-out native verifier codes from the enum: keep main's
enum and put this branch's doc comments back on each of its values.
The other files merge without conflicts.
pcshrosbree added a commit to pcshrosbree/runtime that referenced this pull request Oct 17, 2026
dotnet#135924 made IsByRefLike(StackValue) return true for ByRef-like types
as well as ByRefs, so ArrayByRef, TailByRef and BoxByRef now cover
ByRef-like types too; say so in their summaries. Document
UnalignedAlignment, which dotnet#135798 added.
@dotnet-milestone-bot dotnet-milestone-bot Bot added this to the 12.0-preview1 milestone Oct 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-Tools-ILVerification Issues related to ilverify tool and IL verification in general

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Ordering unimplemented ILVerify errors by their impact in compiler output validation

3 participants