Skip to content

Avoid considering types boxed in the scanner #118529

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
MichalStrehovsky wants to merge 1 commit into from
Closed

Avoid considering types boxed in the scanner #118529

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions src/coreclr/tools/aot/ILCompiler.Compiler/IL/ILImporter.Scanner.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1143,10 +1143,8 @@ private void ImportBox(int token)
{
var type = (TypeDesc)_methodIL.GetObject(token);

// There are some sequences of box with ByRefLike types that are allowed
// per the extension to the ECMA-335 specification.
// Everything else is invalid.
if (!type.IsRuntimeDeterminedType && type.IsByRefLike)
// Some box operations will trivially be eliminated during import
if (!type.IsRuntimeDeterminedType)
{
ILReader reader = new ILReader(_ilBytes, _currentOffset);
ILOpcode nextOpcode = reader.ReadILOpcode();
Expand Down Expand Up @@ -1180,7 +1178,9 @@ private void ImportBox(int token)
}
}

ThrowHelper.ThrowInvalidProgramException();
// If this is a byref-like type, only the above operations are permitted.

@jkotas jkotas Aug 8, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This works only if we recognize all patterns that are recognized by the JIT. I see RyuJIT recognizing more patterns than what we do here - e.g. something with LDNULL:

runtime/src/coreclr/jit/importer.cpp

Line 3216 in f23d779

// for ldnull case, we'll replace the whole "box + isinst + ldnull + cgt_un" sequence

if (type.IsByRefLike)
ThrowHelper.ThrowInvalidProgramException();
}

Copilot AI Aug 8, 2025

Copy link

Choose a reason for hiding this comment

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

The ThrowInvalidProgramException call is now only reachable when type.IsByRefLike is true, but it's placed outside the if block. This will cause the exception to be thrown for all byref-like types, even those that should be valid according to the preceding validation logic. The exception should be moved inside the if block or the logic should be restructured.

Suggested change
}
if (type.IsByRefLike)
ThrowHelper.ThrowInvalidProgramException();

Copilot uses AI. Check for mistakes.
AddBoxingDependencies(type, "Box");
Expand Down
Loading