Skip to content

Implement label validation and typedef resolution in C# ilasm #123002

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

Open
am11 wants to merge 1 commit into
base: main
Choose a base branch
from
+885 −74
Open

Implement label validation and typedef resolution in C# ilasm #123002

Show file tree
Hide file tree
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
33 changes: 27 additions & 6 deletions src/tools/ilasm/src/ILAssembler/CIL.g4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ The .NET Foundation licenses this file to you under the MIT license.

grammar CIL;

import Instructions;

tokens { IncludedFileEof, SyntheticIncludedFileEof }

INT32: '-'? ('0x' [0-9A-Fa-f]+ | [0-9]+);
Expand Down Expand Up @@ -122,6 +120,22 @@ PP_ENDIF: '#endif';
PP_INCLUDE: '#include';
MRESOURCE: '.mresource';

// Instruction tokens MUST be defined before DOTTEDNAME and ID to ensure they take precedence
// For example, "ldc.r8" must be recognized as INSTR_R token, not as DOTTEDNAME
INSTR_NONE: ('nop'|'break'|'ldarg.0'|'ldarg.1'|'ldarg.2'|'ldarg.3'|'ldloc.0'|'ldloc.1'|'ldloc.2'|'ldloc.3'|'stloc.0'|'stloc.1'|'stloc.2'|'stloc.3'|'ldnull'|'ldc.i4.m1'|'ldc.i4.0'|'ldc.i4.1'|'ldc.i4.2'|'ldc.i4.3'|'ldc.i4.4'|'ldc.i4.5'|'ldc.i4.6'|'ldc.i4.7'|'ldc.i4.8'|'dup'|'pop'|'ret'|'ldind.i1'|'ldind.u1'|'ldind.i2'|'ldind.u2'|'ldind.i4'|'ldind.u4'|'ldind.i8'|'ldind.i'|'ldind.r4'|'ldind.r8'|'ldind.ref'|'stind.ref'|'stind.i1'|'stind.i2'|'stind.i4'|'stind.i8'|'stind.r4'|'stind.r8'|'add'|'sub'|'mul'|'div'|'div.un'|'rem'|'rem.un'|'and'|'or'|'xor'|'shl'|'shr'|'shr.un'|'neg'|'not'|'conv.i1'|'conv.i2'|'conv.i4'|'conv.i8'|'conv.r4'|'conv.r8'|'conv.u4'|'conv.u8'|'conv.r.un'|'throw'|'conv.ovf.i1.un'|'conv.ovf.i2.un'|'conv.ovf.i4.un'|'conv.ovf.i8.un'|'conv.ovf.u1.un'|'conv.ovf.u2.un'|'conv.ovf.u4.un'|'conv.ovf.u8.un'|'conv.ovf.i.un'|'conv.ovf.u.un'|'ldlen'|'ldelem.i1'|'ldelem.u1'|'ldelem.i2'|'ldelem.u2'|'ldelem.i4'|'ldelem.u4'|'ldelem.i8'|'ldelem.i'|'ldelem.r4'|'ldelem.r8'|'ldelem.ref'|'stelem.i'|'stelem.i1'|'stelem.i2'|'stelem.i4'|'stelem.i8'|'stelem.r4'|'stelem.r8'|'stelem.ref'|'conv.ovf.i1'|'conv.ovf.u1'|'conv.ovf.i2'|'conv.ovf.u2'|'conv.ovf.i4'|'conv.ovf.u4'|'conv.ovf.i8'|'conv.ovf.u8'|'ckfinite'|'conv.u2'|'conv.u1'|'conv.i'|'conv.ovf.i'|'conv.ovf.u'|'add.ovf'|'add.ovf.un'|'mul.ovf'|'mul.ovf.un'|'sub.ovf'|'sub.ovf.un'|'endfinally'|'stind.i'|'conv.u'|'prefix7'|'prefix6'|'prefix5'|'prefix4'|'prefix3'|'prefix2'|'prefix1'|'prefixref'|'arglist'|'ceq'|'cgt'|'cgt.un'|'clt'|'clt.un'|'localloc'|'endfilter'|'volatile.'|'tail.'|'cpblk'|'initblk'|'rethrow'|'refanytype'|'readonly.'|'illegal'|'endmac');
INSTR_VAR: ('ldarg.s'|'ldarga.s'|'starg.s'|'ldloc.s'|'ldloca.s'|'stloc.s'|'ldarg'|'ldarga'|'starg'|'ldloc'|'ldloca'|'stloc');
INSTR_I: ('ldc.i4.s'|'ldc.i4'|'unaligned.'|'no.');
INSTR_I8: ('ldc.i8');
INSTR_R: ('ldc.r4'|'ldc.r8');
INSTR_METHOD: ('jmp'|'call'|'callvirt'|'newobj'|'ldftn'|'ldvirtftn');
INSTR_SIG: ('calli');
INSTR_BRTARGET: ('br.s'|'brfalse.s'|'brtrue.s'|'beq.s'|'bge.s'|'bgt.s'|'ble.s'|'blt.s'|'bne.un.s'|'bge.un.s'|'bgt.un.s'|'ble.un.s'|'blt.un.s'|'br'|'brfalse'|'brtrue'|'beq'|'bge'|'bgt'|'ble'|'blt'|'bne.un'|'bge.un'|'bgt.un'|'ble.un'|'blt.un'|'leave'|'leave.s');
INSTR_SWITCH: ('switch');
INSTR_TYPE: ('cpobj'|'ldobj'|'castclass'|'isinst'|'unbox'|'stobj'|'box'|'newarr'|'ldelema'|'ldelem'|'stelem'|'unbox.any'|'refanyval'|'mkrefany'|'initobj'|'constrained.'|'sizeof');
INSTR_STRING: ('ldstr');
INSTR_FIELD: ('ldfld'|'ldflda'|'stfld'|'ldsfld'|'ldsflda'|'stsfld');
INSTR_TOK: ('ldtoken');
Comment on lines +125 to +137
Copy link
Member

@jkoritzinsky jkoritzinsky Jan 8, 2026

Choose a reason for hiding this comment

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

Can we format these like the rest of the rules in this file?


// ID needs to be last to ensure it doesn't take priority over other token types
fragment IDSTART: [A-Za-z_#$@];
fragment IDCONT: [A-Za-z0-9_#?$@`];
Expand Down Expand Up @@ -414,6 +428,7 @@ instr:
| instr_r float64
| instr_r int64
| instr_r '(' bytes ')'
| instr_r 'bytearray' '(' bytes ')' // Support bytearray syntax for floating point instructions
| instr_brtarget int32
| instr_brtarget id
| instr_method methodRef
Expand Down Expand Up @@ -922,20 +937,20 @@ VTENTRY: '.vtentry';
methodDecls: methodDecl*;

methodDecl:
EMITBYTE int32
instr // MOVED TO TOP - instructions must be matched first!
| EMITBYTE int32
| sehBlock
| MAXSTACK int32
| LOCALS sigArgs
| LOCALS 'init' sigArgs
| ENTRYPOINT
| ZEROINIT
| dataDecl
| instr
| id ':'
| labelDecl
| secDecl
| extSourceSpec // Leave for later when I get to generating symbols.
| languageDecl // Leave for later when I get to generating symbols.
| customAttrDecl
| customDescrInMethodBody // Only customDescr and customDescrWithOwner, NOT bare typedefs
| compControl
| EXPORT '[' int32 ']'
| EXPORT '[' int32 ']' 'as' id
Expand All @@ -949,6 +964,12 @@ methodDecl:
| PARAM CONSTRAINT dottedName ',' typeSpec customAttrDecl*
| PARAM '[' int32 ']' initOpt customAttrDecl*;

labelDecl: id ':';

customDescrInMethodBody:
customDescr
| customDescrWithOwner;

scopeBlock: '{' methodDecls '}';

/* Structured exception handling directives */
Expand Down
20 changes: 20 additions & 0 deletions src/tools/ilasm/src/ILAssembler/Diagnostic.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,16 @@ public static class DiagnosticIds
public const string ArgumentNotFound = "ILA0018";
public const string LocalNotFound = "ILA0019";
public const string TypedefNotFound = "ILA0020";
public const string AbstractMethodNotInAbstractType = "ILA0021";
public const string InvalidPInvokeSignature = "ILA0022";
public const string MissingInstanceCallConv = "ILA0023";
public const string DeprecatedNativeType = "ILA0024";
public const string DeprecatedCustomMarshaller = "ILA0025";
public const string UnsupportedSecurityDeclaration = "ILA0026";
public const string GenericParameterIndexOutOfRange = "ILA0027";
public const string UnknownGenericParameter = "ILA0028";
public const string ParameterIndexOutOfRange = "ILA0029";
public const string DuplicateMethod = "ILA0030";
}

internal static class DiagnosticMessageTemplates
Expand All @@ -64,4 +74,14 @@ internal static class DiagnosticMessageTemplates
public const string ArgumentNotFound = "Argument '{0}' not found";
public const string LocalNotFound = "Local variable '{0}' not found";
public const string TypedefNotFound = "Typedef '{0}' not found";
public const string AbstractMethodNotInAbstractType = "Abstract method '{0}' cannot be declared in a non-abstract type";
public const string InvalidPInvokeSignature = "Invalid P/Invoke signature: module name is required";
public const string MissingInstanceCallConv = "Instance call convention required for method reference";
public const string DeprecatedNativeType = "Native type '{0}' is deprecated";
public const string DeprecatedCustomMarshaller = "The 4-string form of custom marshaller is deprecated";
public const string UnsupportedSecurityDeclaration = "Individual SecurityAttribute permissions are not supported; use PermissionSet instead";
public const string GenericParameterIndexOutOfRange = "Generic parameter index {0} is out of range";
public const string UnknownGenericParameter = "Unknown generic parameter '{0}'";
public const string ParameterIndexOutOfRange = "Parameter index {0} is out of range";
public const string DuplicateMethod = "Duplicate method definition";
}
43 changes: 37 additions & 6 deletions src/tools/ilasm/src/ILAssembler/EntityRegistry.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,15 +177,15 @@ public void WriteContentTo(MetadataBuilder builder, BlobBuilder ilStream, IReadO
builder.GetOrAddString(type.Namespace),
builder.GetOrAddString(type.Name),
type.BaseType is null ? default : type.BaseType.Handle,
(FieldDefinitionHandle)GetHandleForList(type.Fields, GetSeenEntities(TableIndex.TypeDef), type => ((TypeDefinitionEntity)type).Fields, i, TableIndex.Field),
(MethodDefinitionHandle)GetHandleForList(type.Methods, GetSeenEntities(TableIndex.TypeDef), type => ((TypeDefinitionEntity)type).Methods, i, TableIndex.MethodDef));
GetFieldHandleForList(type.Fields, GetSeenEntities(TableIndex.TypeDef), type => ((TypeDefinitionEntity)type).Fields, i),
GetMethodHandleForList(type.Methods, GetSeenEntities(TableIndex.TypeDef), type => ((TypeDefinitionEntity)type).Methods, i));

builder.AddEventMap(
(TypeDefinitionHandle)type.Handle,
(EventDefinitionHandle)GetHandleForList(type.Events, GetSeenEntities(TableIndex.TypeDef), type => ((TypeDefinitionEntity)type).Events, i, TableIndex.Event));
GetEventHandleForList(type.Events, GetSeenEntities(TableIndex.TypeDef), type => ((TypeDefinitionEntity)type).Events, i));
builder.AddPropertyMap(
(TypeDefinitionHandle)type.Handle,
(PropertyDefinitionHandle)GetHandleForList(type.Properties, GetSeenEntities(TableIndex.TypeDef), type => ((TypeDefinitionEntity)type).Properties, i, TableIndex.Property));
GetPropertyHandleForList(type.Properties, GetSeenEntities(TableIndex.TypeDef), type => ((TypeDefinitionEntity)type).Properties, i));

if (type.PackingSize is not null || type.ClassSize is not null)
{
Expand Down Expand Up @@ -241,7 +241,7 @@ public void WriteContentTo(MetadataBuilder builder, BlobBuilder ilStream, IReadO
builder.GetOrAddString(methodDef.Name),
builder.GetOrAddBlob(methodDef.MethodSignature!),
rva,
(ParameterHandle)GetHandleForList(methodDef.Parameters, GetSeenEntities(TableIndex.MethodDef), method => ((MethodDefinitionEntity)method).Parameters, i, TableIndex.Param));
GetParameterHandleForList(methodDef.Parameters, GetSeenEntities(TableIndex.MethodDef), method => ((MethodDefinitionEntity)method).Parameters, i));

if (methodDef.MethodImportInformation is not null)
{
Expand Down Expand Up @@ -385,6 +385,36 @@ public void WriteContentTo(MetadataBuilder builder, BlobBuilder ilStream, IReadO
builder.AddMethodSpecification(methodSpec.Parent.Handle, builder.GetOrAddBlob(methodSpec.Signature));
}

static FieldDefinitionHandle GetFieldHandleForList(IReadOnlyList<EntityBase> list, IReadOnlyList<EntityBase> listOwner, Func<EntityBase, IReadOnlyList<EntityBase>> getList, int ownerIndex)
{
var handle = GetHandleForList(list, listOwner, getList, ownerIndex, TableIndex.Field);
return handle.IsNil ? default : (FieldDefinitionHandle)handle;
Copy link
Member

@jkoritzinsky jkoritzinsky Jan 8, 2026

Choose a reason for hiding this comment

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

If we take the suggestion in GetHandleForList, we can convert all of these returns to just be a cast to the right handle type.

}

static MethodDefinitionHandle GetMethodHandleForList(IReadOnlyList<EntityBase> list, IReadOnlyList<EntityBase> listOwner, Func<EntityBase, IReadOnlyList<EntityBase>> getList, int ownerIndex)
{
var handle = GetHandleForList(list, listOwner, getList, ownerIndex, TableIndex.MethodDef);
return handle.IsNil ? default : (MethodDefinitionHandle)handle;
}

static PropertyDefinitionHandle GetPropertyHandleForList(IReadOnlyList<EntityBase> list, IReadOnlyList<EntityBase> listOwner, Func<EntityBase, IReadOnlyList<EntityBase>> getList, int ownerIndex)
{
var handle = GetHandleForList(list, listOwner, getList, ownerIndex, TableIndex.Property);
return handle.IsNil ? default : (PropertyDefinitionHandle)handle;
}

static EventDefinitionHandle GetEventHandleForList(IReadOnlyList<EntityBase> list, IReadOnlyList<EntityBase> listOwner, Func<EntityBase, IReadOnlyList<EntityBase>> getList, int ownerIndex)
{
var handle = GetHandleForList(list, listOwner, getList, ownerIndex, TableIndex.Event);
return handle.IsNil ? default : (EventDefinitionHandle)handle;
}

static ParameterHandle GetParameterHandleForList(IReadOnlyList<EntityBase> list, IReadOnlyList<EntityBase> listOwner, Func<EntityBase, IReadOnlyList<EntityBase>> getList, int ownerIndex)
{
var handle = GetHandleForList(list, listOwner, getList, ownerIndex, TableIndex.Param);
return handle.IsNil ? default : (ParameterHandle)handle;
}

static EntityHandle GetHandleForList(IReadOnlyList<EntityBase> list, IReadOnlyList<EntityBase> listOwner, Func<EntityBase, IReadOnlyList<EntityBase>> getList, int ownerIndex, TableIndex tokenType)
{
// Return the first entry in the list.
Expand Down Expand Up @@ -412,7 +442,8 @@ static EntityHandle GetHandleForList(IReadOnlyList<EntityBase> list, IReadOnlyLi
return MetadataTokens.EntityHandle(tokenType, MetadataTokens.GetRowNumber(otherList[otherList.Count - 1].Handle) + 1);
}
}
return MetadataTokens.EntityHandle(tokenType, 0);
// If all lists are empty, return a nil handle
return default(EntityHandle);
Copy link
Member

@jkoritzinsky jkoritzinsky Jan 8, 2026

Choose a reason for hiding this comment

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

We should return a handle for the right table at least (what we were doing before this change). That way we won't have a possible bug for callers that don't explicitly handle the Nil handle case.

}
}

Expand Down
Loading