Validate invalid base type declarations in ILVerify by pkuyo · Pull Request #129118 · dotnet/runtime

pkuyo · 2026-06-08T06:04:57Z

Fixes #119536

This adds ILVerify validation for invalid class base type declarations.

The original issue reports a case where ILAsm accepts a type declared with
extends object, but the resulting type cannot be loaded by the runtime.
ILVerify previously accepted that assembly without reporting an error.

This change reports InvalidBaseType for invalid base type declarations rather
than special-casing only the exact extends object spelling.

Tests added in BaseTypeTests.il:

ObjectTypeSpecBase_InvalidType_InvalidBaseType
- Covers the original repro: a class declared with extends object.
- Expected result: InvalidBaseType.
NilBaseInterface_ValidType_Valid
- Ensures interfaces with no class base continue to verify successfully.
GenericClassTypeSpecBase_ValidType_Valid
- Ensures valid generic class TypeSpec bases are still accepted.
ValueTypeBase_InvalidType_InvalidBaseType
- Ensures a non-generic value type cannot be used as a class base.
GenericValueTypeSpecBase_InvalidType_InvalidBaseType
- Ensures a generic value type instantiation cannot be used as a class base.

dotnet-policy-service · 2026-06-08T06:06:06Z

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

pkuyo · 2026-06-08T06:06:17Z

@dotnet-policy-service agree

jkotas · 2026-06-08T06:22:08Z

+                TypeSpecification typeSpecification = _module.MetadataReader.GetTypeSpecification(typeSpecificationHandle);
+                BlobReader signatureReader = _module.MetadataReader.GetBlobReader(typeSpecification.Signature);
+
+                if (signatureReader.ReadSignatureTypeCode() != SignatureTypeCode.GenericTypeInstance)
+                {
+                    return false;
+                }
+
+                int genericTypeKind = signatureReader.ReadCompressedInteger();
+                if (genericTypeKind != (int)SignatureTypeKind.Class)
+                {
+                    return false;
+                }
+
+                EntityHandle genericTypeHandle = signatureReader.ReadTypeHandle();
+                if (genericTypeHandle.Kind != HandleKind.TypeDefinition &&
+                    genericTypeHandle.Kind != HandleKind.TypeReference)
+                {
+                    return false;
+                }


Is this any better than just calling _module.GetType(typeHandle)?

I tried that locally again, using only _module.GetType(typeHandle) does not catch the #119536 repro. (ObjectTypeSpecBase_InvalidType_InvalidBaseType)

In the extends object case, _module.GetType(typeHandle) resolves the TypeSpec to System.Object, which loses the issue that the base type was encoded as an invalid TypeSpec in the extends clause.

If you would like to check for invalid type specs, it should be done in the central location where typespecs are decoded so that it applies to all of them. The problem is not specific to base type.

Thanks, I tried moving this into EcmaModule.GetType, but I don't think this check can be global.

EcmaModule.GetType is context-independent. A top-level built-in TypeSpec like ELEMENT_TYPE_OBJECT is invalid in a TypeDef extends clause, but it is not invalid in every context. For example, CoreCLR accepts stobj with a TypeSpec object token, and ILVerification already has a similar existing test case using stobj string ( [LoadStoreIndirectTests] StoreObject.ValidTypeToken).

Rejecting these TypeSpecs in EcmaModule.GetType would therefore make ILVerify reject IL that is accepted outside of base type declarations. I think this validation should stay in the base type verification path, where we know the token is being used as a class base type.

TypeSpec with object token is invalid in every context according to the spec. Look for TypeSpecBlob grammar in ECMA-335.

ILVerify is meant to check for speced behavior. It is not meant to check for what CoreCLR happens to accept. The runtime often accepts more than what's allowed by the spec.

Thanks for clarifying. I understand now.

I updated the change so malformed TypeSpecs are validated in EcmaModule.GetType, and added related test coverage.

After making that change, I found a few existing ILVerification tests that were also using invalid TypeSpecs, so I updated them as well:

In LoadStoreIndirectTests.il, changed stobj string to stobj [System.Runtime]System.String.

In CastingTests.il, replaced BoxByRefInt_Invalid_BoxByRef with BoxIntPointer_Invalid_UnmanagedPointer, so the test no longer depends on an invalid TypeSpec.

For .override method instance string class A.T1::Func1()-style cases, removed the unnecessary class modifier. When class is present, ILAsm emits a TypeSpec as the MemberRef.Parent for a non-generic class, instead of using a TypeDef or TypeRef. This is invalid according to the TypeSpecBlob grammar.

Please let me know if you would prefer any of these test updates to be split out or adjusted differently.

pkuyo · 2026-06-17T15:46:02Z

Hi @jkotas, just checking in on this PR. I’ve addressed the feedback and made the requested updates. Please let me know if there’s anything else I should adjust. Thanks!

jkotas · 2026-06-29T21:47:59Z

+#if ILVERIFICATION
+        private static void ValidateTypeSpecificationSignature(BlobReader signatureReader)
+        {
+            switch (signatureReader.ReadSignatureTypeCode())


This check may be too restrictive after looking into this in more detail.

We have ECMA-335 spec augment that suggests that the only TypeSpec should only disallow CLASS and VALUETYPE since the other forms may show up in different contexts: https://github.com/dotnet/runtime/blob/main/docs/design/specs/Ecma-335-Augments.md#5-typespecs-can-encode-more-than-specified

I think we should implement the behavior from ECMA spec augment here, and also link to it so that future readers understand where it came from.

jkotas · 2026-06-29T21:49:04Z

    {
        ldarg.0
-        box     int32&
+        box     int32*


These changes should be unnecessary with the other feedback

jkotas · 2026-06-29T22:14:36Z

+                if (resolvedBaseType.IsValueType ||
+                    (baseType.Kind == HandleKind.TypeSpecification && resolvedBaseType is not InstantiatedType))


Suggested change

if (resolvedBaseType.IsValueType ||

(baseType.Kind == HandleKind.TypeSpecification && resolvedBaseType is not InstantiatedType))

if (!resolvedBaseType.IsDefType || resolvedBaseType.IsInterface || resolvedBaseType.IsPrimitive)

I think this would be more precise.

Thanks, I updated this to use a more precise check.

I used resolvedBaseType.IsValueType || resolvedBaseType.IsInterface || !resolvedBaseType.IsDefType because base types should reject value types in general, not just primitive types. IsValueType also covers the primitive case.

jkotas · 2026-06-29T22:27:49Z

+            // Once the base type metadata is invalid, later checks can force the type system to
+            // resolve the same bad base token again and produce a duplicate token resolution error.
+            if (!VerifyBaseType())
+            {
+                return;
+            }


Suggested change

// Once the base type metadata is invalid, later checks can force the type system to

// resolve the same bad base token again and produce a duplicate token resolution error.

if (!VerifyBaseType())

{

return;

}

VerifyBaseType();

We may want to stop on errors thrown by the type system only (throw an exception that will be caught by the caller of Verify method) ,but keep going for non-fatal errors to produce as many of them as possible. It is how the existing VerifyInterfaces method seems to work.

pkuyo · 2026-06-30T08:28:06Z

Thanks for the review. I updated the PR to address the latest feedback, The TypeSpec validation now follows the ECMA-335 augment behavior.

Please let me know if there is anything else I should adjust.

MichalStrehovsky · 2026-07-01T06:01:51Z

(Edited to fix issue number)

I wonder if we should approach this from an angle that also allows to address #4945

Add ParseTypeSpec to EcmaSignatureParser.
Do the validation in EcmaSignatureParser (this allows us getting rid of the ILVERIFICATION ifdef, because this could be a ReportInvalidTypeSpec partial method on EcmaSignatureParser: this matches more closely how invalid things are reported (e.g. in ILImporter.cs).

jkotas · 2026-07-01T11:09:32Z

Add ParseTypeSpec to EcmaSignatureParser.

Do the validation in EcmaSignatureParser (this allows us getting rid of the ILVERIFICATION ifdef, because this could be a ReportInvalidTypeSpec partial method on EcmaSignatureParser: this matches more closely how invalid things are reported (e.g. in ILImporter.cs).

Yes, I think it would look better. @pkuyo Could you please implement it?

jkotas · 2026-07-01T11:20:14Z

+            }
+            else
+            {
+                if (baseType.Kind == HandleKind.TypeSpecification &&


This should not be needed. It is covered by the resolvedBaseType.IsValueType || resolvedBaseType.IsInterface || !resolvedBaseType.IsDefType check below.

Thanks. I simplified this to a smaller TypeSpec-specific check.

The resolvedBaseType.IsValueType || resolvedBaseType.IsInterface || !resolvedBaseType.IsDefType check alone does not catch the #119536 case, because a TypeSpec that resolves to System.Object passes those resolved-type checks. The extra TypeSpec check is only for that context-specific extends validation.

jkotas · 2026-07-01T11:25:13Z

+                {
+                    resolvedBaseType = _module.GetType(baseType);
+                }
+                catch (BadImageFormatException)


I think it would be better to let the errors from the core type system to propagate. Catching the exception and reporting it as something else loses information.

pkuyo · 2026-07-01T14:28:56Z

Add ParseTypeSpec to EcmaSignatureParser.

Do the validation in EcmaSignatureParser (this allows us getting rid of the ILVERIFICATION ifdef, because this could be a ReportInvalidTypeSpec partial method on EcmaSignatureParser: this matches more closely how invalid things are reported (e.g. in ILImporter.cs).

Yes, I think it would look better. @pkuyo Could you please implement it?

Sure, thanks. I’ll implement it this way and update the PR.

pkuyo · 2026-07-01T17:31:46Z

(Edited to fix issue number)

I wonder if we should approach this from an angle that also allows to address #4945

Add ParseTypeSpec to EcmaSignatureParser.

Do the validation in EcmaSignatureParser (this allows us getting rid of the ILVERIFICATION ifdef, because this could be a ReportInvalidTypeSpec partial method on EcmaSignatureParser: this matches more closely how invalid things are reported (e.g. in ILImporter.cs).

Thanks, I reworked this to use the EcmaSignatureParser approach.

The TypeSpec validation is now done through ParseTypeSpec / ReportInvalidTypeSpec, and I addedCoverage-relatedto #4945.

For CMOD_OPT/CMOD_REQ, I kept the check simple for now: it reports a TypeSpec reference while parsing another TypeSpec, but does not try to distinguish whether it actually forms a cycle. Full cycle detection would need extra tracking for the active TypeSpec resolution stack, since the current TypeSpec is not in the cache yet.

I wasn't sure that extra state was necessary for this PR, so I wanted to check before adding it.

MichalStrehovsky · 2026-07-01T21:13:08Z

For CMOD_OPT/CMOD_REQ, I kept the check simple for now: it reports a TypeSpec reference while parsing another TypeSpec, but does not try to distinguish whether it actually forms a cycle. Full cycle detection would need extra tracking for the active TypeSpec resolution stack, since the current TypeSpec is not in the cache yet.

We shouldn't allow TypeSpec as a modifier. It only causes problems. If the augment says we allow it, we should update the augment. We can capture the reason so that somebody doesn't try to relax it without thinking about the consequences (@jkotas do you agree?). I think this recently came up in #123819 and we decided not to handle TypeSpec either, but GitHub makes it really hard to find the relevant comment.

Implement base type verification and add related tests

87dc394

github-actions Bot added the area-Tools-ILVerification Issues related to ilverify tool and IL verification in general label Jun 8, 2026

dotnet-policy-service Bot added the community-contribution Indicates that the PR has been added by a community member label Jun 8, 2026

jkotas reviewed Jun 8, 2026

View reviewed changes

build-analysis Bot mentioned this pull request Jun 8, 2026

CI build failure: error CS8032: An instance of analyzer Microsoft.NetCore.CSharp.Analyzers.... cannot be created #129031

Closed

pkuyo requested a review from MichalStrehovsky as a code owner June 9, 2026 11:52

pkuyo force-pushed the fix-ilverify-119536 branch from a03f2df to 87dc394 Compare June 9, 2026 12:51

build-analysis Bot mentioned this pull request Jun 9, 2026

"We stopped hearing from agent Azure Pipelines 32. Verify the agent machine is running and has a healthy network connection" dotnet/dnceng#1886

Open

3 tasks

Add type specification validation and related tests

0657062

This was referenced Jun 9, 2026

maccatalyst/iossimulator Debug CoreCLR work items fail with PACKAGE_NOT_FOUND (missing Info.plist) #126459

Open

[wasm] WasmBuildTests Blazor apps fail to boot: 404 on _framework Blazor bootstrapper (blazor.webassembly.js / blazor.web.js) #129182

Open

jkotas reviewed Jun 10, 2026

View reviewed changes

Comment thread src/coreclr/tools/Common/TypeSystem/Ecma/EcmaModule.cs Outdated

jkotas reviewed Jun 10, 2026

View reviewed changes

Comment thread src/coreclr/tools/ILVerification/TypeVerifier.cs Outdated

Gate TypeSpec validation under ILVERIFICATION

a7ece37

build-analysis Bot mentioned this pull request Jun 10, 2026

Unable to pull image from mcr.microsoft.com #117164

Open

pkuyo requested a review from jkotas June 13, 2026 05:49

jkotas reviewed Jun 29, 2026

View reviewed changes

pkuyo added 3 commits June 30, 2026 15:33

Refine base type validation and update ILVerify expectations

828661d

Fix formatting in IL test files

74f03d2

Avoid base type resolution in interface checks

60febc6

This was referenced Jun 30, 2026

slow macOS - "##[error]The job running on agent Azure Pipelines 9 ran longer than the maximum time of 60 minutes." dotnet/dnceng#1883

Open

The Operation will be canceled. The next steps may not contain expected logs. dotnet/dnceng#3008

Open

jkotas reviewed Jul 1, 2026

View reviewed changes

pkuyo added 2 commits July 2, 2026 00:23

Validate TypeSpec parsing in EcmaSignatureParser

6265b35

Add ILVerify test for cyclic TypeSpec

e1ea8a3

Simplify cyclic TypeSpec ILVerify test

7ddbf4b

		if (resolvedBaseType.IsValueType \|\|
		(baseType.Kind == HandleKind.TypeSpecification && resolvedBaseType is not InstantiatedType))

	if (resolvedBaseType.IsValueType \|\|
	(baseType.Kind == HandleKind.TypeSpecification && resolvedBaseType is not InstantiatedType))
	if (!resolvedBaseType.IsDefType \|\| resolvedBaseType.IsInterface \|\| resolvedBaseType.IsPrimitive)

Uh oh!

Conversation

pkuyo commented Jun 8, 2026

Uh oh!

dotnet-policy-service Bot commented Jun 8, 2026

Uh oh!

pkuyo commented Jun 8, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pkuyo Jun 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

pkuyo commented Jun 17, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jkotas Jun 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pkuyo commented Jun 30, 2026

Uh oh!

MichalStrehovsky commented Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jkotas commented Jul 1, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pkuyo commented Jul 1, 2026

Uh oh!

pkuyo commented Jul 1, 2026

Uh oh!

MichalStrehovsky commented Jul 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

pkuyo Jun 8, 2026 •

edited

Loading

jkotas Jun 29, 2026 •

edited

Loading

MichalStrehovsky commented Jul 1, 2026 •

edited

Loading