Skip to content

[mono][interp] Preserve nullable boxing for GetType - #132732

Merged
steveisok merged 3 commits into
mainfrom
steveisok-fix-mono-nullable-gettype
Sep 3, 2026
Merged

[mono][interp] Preserve nullable boxing for GetType#132732
steveisok merged 3 commits into
mainfrom
steveisok-fix-mono-nullable-gettype

Conversation

@steveisok

Copy link
Copy Markdown
Member

Fixes #132461.

The interpreter optimized box T followed by Object.GetType() into a constant type lookup. For Nullable<T>, this bypassed nullable boxing semantics and returned typeof(Nullable<T>) instead of the underlying type.

Exclude Nullable<T> from this optimization so it follows the normal boxing path:

  • A populated nullable returns the underlying type.
  • A null nullable throws NullReferenceException.

Adds regression coverage for the direct box Nullable<T>; call Object.GetType() IL pattern under the Mono interpreter.

Note

This pull request description was generated by GitHub Copilot.

Skip the box/GetType constant fold for Nullable<T> so normal nullable boxing determines the runtime type or throws for null values. Add direct box/call regression coverage to the normal merged test run.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI lite review requested due to automatic review settings August 25, 2026 03:13
@steveisok
steveisok requested a review from lewing August 25, 2026 03:13
@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: @vitek-karas, @BrzVlad, @kotlarmilos
See info in area-owners.md if you want to be subscribed.

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.

Pull request overview

Fixes Mono interpreter nullable boxing semantics for Object.GetType() and adds regression coverage.

Changes:

  • Excludes Nullable<T> from the constant GetType() optimization.
  • Tests populated and null nullable behavior.
Show a summary per file
File Description
src/tests/JIT/Directed/nullabletypes/gettype.cs Adds nullable GetType() regression tests.
src/mono/mono/mini/interp/transform.c Preserves normal nullable boxing behavior.

Review details

Suppressed comments (2)

src/mono/mono/mini/interp/transform.c:5070

  • This condition evaluates mono_class_is_nullable for every box pattern, including boxes whose next instruction is not a call. Since mono_class_is_nullable walks generic-class metadata, this adds transform-time work to unrelated boxing; keep the cheap opcode/method-shape checks first and perform the nullable check only after identifying Object.GetType().
	if (!mono_class_is_nullable (box_class) &&
			*next_ip == CEE_CALL &&
			(cmethod = interp_get_method (method, read32 (next_ip + 1), image, generic_context, error)) &&
			(cmethod->klass == mono_defaults.object_class) &&
			(strcmp (cmethod->name, "GetType") == 0)) {

src/mono/mono/mini/interp/transform.c:5067

  • ❌ This optimization only recognizes CEE_CALL, but the affected C# lowering uses CEE_CALLVIRT for box Nullable<T> followed by Object.GetType() (the linked repro verifies this). Consequently the new test will take the normal path and this guard does not change the failing shape; include CEE_CALLVIRT here as well so nullable boxes cannot be folded for either legal call opcode.
	if (!mono_class_is_nullable (box_class) &&
			*next_ip == CEE_CALL &&
  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Lite

Copilot AI review requested due to automatic review settings August 27, 2026 18:02

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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Copilot AI review requested due to automatic review settings September 1, 2026 22:11

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.

Copilot review overview

🟢 Approval recommended

The change is narrowly scoped to a correctness fix (nullable boxing semantics) and includes targeted regression coverage for the problematic IL shape.

Review tier: Lite
Findings: None

@steveisok

Copy link
Copy Markdown
Member Author

/ba-g Helix monitor known issue

@steveisok
steveisok merged commit bbe4da3 into main Sep 3, 2026
121 of 123 checks passed
@steveisok
steveisok deleted the steveisok-fix-mono-nullable-gettype branch September 3, 2026 16:39
@steveisok

Copy link
Copy Markdown
Member Author

/backport to release/11.0

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Started backporting to release/11.0 (link to workflow run)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[mono][interp] GetType() on a Nullable<T> returns Nullable`1 on WebAssembly in .NET 9/10 (regression of #61007)

4 participants