Merge pull request #8027 from tmat/TypeDefRefSpec

Disallow TypeSpec after CLASS and VALUETYPE in signatures

src/Compilers/CSharp/Test/Symbol/Symbols/Metadata/PE/LoadingMethods.cs

@@ -1255,7 +1255,7 @@ public static void Main()
     }
 }
 ";
-            var longFormRef = MetadataReference.CreateFromImage(TestResources.MetadataTests.Invalid.LongTypeFormInSignature.AsImmutableOrNull());
+            var longFormRef = MetadataReference.CreateFromImage(TestResources.MetadataTests.Invalid.LongTypeFormInSignature);
 
             var c = CreateCompilationWithMscorlib(source, new[] { longFormRef });
 
@@ -1265,6 +1265,50 @@ public static void Main()
                 // (7,20): error CS0570: 'C.VT()' is not supported by the language
                 Diagnostic(ErrorCode.ERR_BindToBogus, "VT").WithArguments("C.VT()"));
         }
+
+        [WorkItem(7971, "https://github.com/dotnet/roslyn/issues/7971")]
+        [Fact(Skip = "7971")]
+        public void MemberSignature_CycleTrhuTypeSpecInCustomModifiers()
+        {
+            string source = @"
+class P
+{
+    static void Main()
+    {
+        User.X(new Extender());
+    }
+}
+";
+            var lib = MetadataReference.CreateFromImage(TestResources.MetadataTests.Invalid.Signatures.SignatureCycle2);
+
+            var c = CreateCompilationWithMscorlib(source, new[] { lib });
+
+            c.VerifyDiagnostics();
+        }
+
+        [WorkItem(7970, "https://github.com/dotnet/roslyn/issues/7970")]
+        [Fact]
+        public void MemberSignature_TypeSpecInWrongPlace()
+        {
+            string source = @"
+class P
+{
+    static void Main()
+    {
+        User.X(new System.Collections.Generic.List<int>());
+    }
+}
+";
+            var lib = MetadataReference.CreateFromImage(TestResources.MetadataTests.Invalid.Signatures.TypeSpecInWrongPlace);
+
+            var c = CreateCompilationWithMscorlib(source, new[] { lib });
+
+            c.VerifyDiagnostics(
+                // (6,14): error CS0570: 'User.X(?)' is not supported by the language
+                //         User.X(new System.Collections.Generic.List<int>());
+                Diagnostic(ErrorCode.ERR_BindToBogus, "X").WithArguments("User.X(?)"));
+        }
+
         [WorkItem(666162, "DevDiv")]
         [ClrOnlyFact(ClrOnlyReason.Ilasm)]
         public void Repro666162()

src/Compilers/Core/Portable/MetadataReader/MetadataDecoder.cs

@@ -188,7 +188,10 @@ private TypeSymbol DecodeTypeOrThrow(ref BlobReader ppSig, SignatureTypeCode typ
                     break;
 
                 case SignatureTypeCode.TypeHandle:
-                    typeSymbol = ResolveSignatureTypeHandleOrThrow(ref ppSig, out refersToNoPiaLocalType);
+                    // Spec (6th edition): In II.23.2.12 and II.23.2.14, it is implied that the token in (CLASS | VALUETYPE) TypeDefOrRefOrSpecEncoded 
+                    // can be a TypeSpec, when in fact it must be a TypeDef or TypeRef.
+                    // See https://github.com/dotnet/roslyn/issues/7970
+                    typeSymbol = ResolveSignatureTypeHandleOrThrow(ref ppSig, out refersToNoPiaLocalType, allowTypeSpec: false);
                     break;
 
                 case SignatureTypeCode.Array:
@@ -370,11 +373,16 @@ private TypeSymbol DecodeGenericTypeInstanceOrThrow(ref BlobReader ppSig, out bo
 
         /// <exception cref="UnsupportedSignatureContent">If the encoded type is invalid.</exception>
         /// <exception cref="BadImageFormatException">An exception from metadata reader.</exception>
-        private TypeSymbol ResolveSignatureTypeHandleOrThrow(ref BlobReader ppSig, out bool isNoPiaLocalType)
+        private TypeSymbol ResolveSignatureTypeHandleOrThrow(ref BlobReader ppSig, out bool isNoPiaLocalType, bool allowTypeSpec)
         {
             TypeSymbol typeSymbol;
 
             EntityHandle token = ppSig.ReadTypeHandle();
+            if (token.IsNil)
+            {
+                throw new UnsupportedSignatureContent();
+            }
+
             HandleKind tokenType = token.Kind;
 
             if (tokenType == HandleKind.TypeDefinition)
@@ -385,8 +393,15 @@ private TypeSymbol ResolveSignatureTypeHandleOrThrow(ref BlobReader ppSig, out b
             {
                 typeSymbol = GetTypeOfTypeRef((TypeReferenceHandle)token, out isNoPiaLocalType);
             }
-            else
+            else 
             {
+                Debug.Assert(tokenType == HandleKind.TypeSpecification);
+
+                if (!allowTypeSpec)
+                {
+                    throw new UnsupportedSignatureContent();
+                }
+
                 isNoPiaLocalType = false;
                 typeSymbol = GetTypeOfTypeSpec((TypeSpecificationHandle)token);
             }
@@ -1088,7 +1103,7 @@ private void DecodeCustomAttributeParameterTypeOrThrow(ref BlobReader sigReader,
                 case SignatureTypeCode.TypeHandle:
                     // The type of the parameter can either be an enum type or System.Type.
                     bool isNoPiaLocalType;
-                    type = ResolveSignatureTypeHandleOrThrow(ref sigReader, out isNoPiaLocalType);
+                    type = ResolveSignatureTypeHandleOrThrow(ref sigReader, out isNoPiaLocalType, allowTypeSpec: true);
 
                     var underlyingEnumType = GetEnumUnderlyingType(type);
 

src/Compilers/Test/Resources/Core/CompilerTestResources.csproj

@@ -117,6 +117,12 @@
     <EmbeddedResource Include="MetadataTests\Invalid\ManyMethodSpecs.vb" />
     <EmbeddedResource Include="MetadataTests\Invalid\Obfuscated.dll" />
     <EmbeddedResource Include="MetadataTests\Invalid\Obfuscated2.dll" />
+    <Content Include="MetadataTests\Invalid\Signatures\build.bat" />
+    <Content Include="MetadataTests\Invalid\Signatures\munge.csx" />
+    <Content Include="MetadataTests\Invalid\Signatures\SignatureCycle2.il" />
+    <Content Include="MetadataTests\Invalid\Signatures\TypeSpecInWrongPlace.il" />
+    <EmbeddedResource Include="MetadataTests\Invalid\Signatures\SignatureCycle2.exe" />
+    <EmbeddedResource Include="MetadataTests\Invalid\Signatures\TypeSpecInWrongPlace.exe" />
     <Content Include="MetadataTests\Members.cs" />
     <EmbeddedResource Include="MetadataTests\Members.dll" />
     <EmbeddedResource Include="MetadataTests\NativeApp.exe" />

src/Compilers/Test/Resources/Core/MetadataTests/Invalid/Signatures/SignatureCycle2.il

@@ -0,0 +1,55 @@
+//
+// Example #2 -- call within same assembly to cyclical signature
+//
+// NOTE: build.bat creates the cycle by modifying the ilasm output
+//
+// From:
+//   List`1<int32 modopt([TypeSpec #1])>
+//                             |
+//                             v
+//                           int32
+//
+// To:
+//   List`1<int32 modopt([TypeSpec #2])
+//       ^---------------------|
+//
+// Symptoms: 
+//   * peverify.exe succeeds
+//   * program crashes (stack overflow)
+//   * ildasm crashes (likely stack overflow)
+//
+
+.assembly extern mscorlib { .ver 4:0:0:0 .publickeytoken = (B7 7A 5C 56 19 34 E0 89 ) }
+.assembly SignatureCycle2 {}
+.module SignatureCycle2.exe
+
+.class public Extender extends class [mscorlib]System.Collections.Generic.List`1<int32 modopt(int32)>
+{
+    .method public specialname rtspecialname instance void .ctor()
+    {
+        ldarg.0
+        call instance void class [mscorlib]System.Collections.Generic.List`1<int32 modopt(int32)>::.ctor()
+        ldstr "hello world"
+        call void [mscorlib]System.Console::WriteLine(object)
+        ret
+    }
+
+    .method public static void Main()
+    {
+        .entrypoint
+        newobj instance void Extender::.ctor()
+        call void User::X(class [mscorlib]System.Collections.Generic.List`1<int32 modopt(int32)>)
+        ret
+    }
+}
+
+.class public User extends [mscorlib]System.Object
+{
+    .method public static void X(class [mscorlib]System.Collections.Generic.List`1<int32 modopt(int32)>)
+    {
+        ldstr "goodbye world"
+        call void [mscorlib]System.Console::WriteLine(object)
+        ret
+    }
+}
+

src/Compilers/Test/Resources/Core/MetadataTests/Invalid/Signatures/TypeSpecInWrongPlace.il

@@ -0,0 +1,34 @@
+//
+// Example #5 -- using (CLASS | VALUETYPE) TypeSpec instead of embedding the type in to the signature.
+//
+// build.bat will change the siganture of User::X to use a TypeSpec for List<int> instead of embedding GENERICINST in the signature.
+//
+// * peverify correctly identifies error:
+//    [MD]: Error: Signature has token following ELEMENT_TYPE_CLASS (_VALUETYPE) that is not a TypeDef or TypeRef (token: 0x1b000001; offset: 0x00000005). [token:0x06000002]
+//
+// This just demonstrates that the rule is not called out in the CLI spec
+
+.assembly extern mscorlib { .ver 4:0:0:0 .publickeytoken = (B7 7A 5C 56 19 34 E0 89 ) }
+.assembly TypeSpecInWrongPlace {}
+.module TypeSpecInWrongPlace.exe
+
+.class public Extender extends class [mscorlib]System.Collections.Generic.List`1<int32>
+{
+    .method public static void Main()
+    {
+        .entrypoint
+        newobj instance void class [mscorlib]System.Collections.Generic.List`1<int32>::.ctor()
+        call void User::X(class [mscorlib]System.Collections.Generic.List`1<int32>)
+        ret
+    }
+}
+
+.class public User extends [mscorlib]System.Object
+{
+    .method public static void X(class [mscorlib]System.Collections.Generic.List`1<int32>)
+    {
+        ldstr "goodbye world"
+        call void [mscorlib]System.Console::WriteLine(object)
+        ret
+    }
+}

src/Compilers/Test/Resources/Core/MetadataTests/Invalid/Signatures/build.bat

@@ -0,0 +1,6 @@
+ilasm SignatureCycle2.il
+csi munge.csx SignatureCycle2.exe 15-12-05-01-20-06-08 15-12-05-01-20-0A-08
+
+ilasm TypeSpecInWrongPlace.il
+csi munge.csx TypeSpecInWrongPlace.exe 00-01-01-15-12-05-01-08 00-01-01-12-06-00-00-00 
+

src/Compilers/Test/Resources/Core/MetadataTests/Invalid/Signatures/munge.csx

@@ -0,0 +1,14 @@
+string file = Args[0];
+string hexSearch = Args[1];
+string hexReplace = Args[2];
+
+File.WriteAllBytes(
+    Args[0],
+    BitConverter.ToString(File.ReadAllBytes(Args[0]))
+                .Replace(hexSearch, hexReplace)
+                .Split('-')
+                .Select(s => Convert.ToByte(s, 16))
+                .ToArray());
+
+
+

src/Compilers/Test/Resources/Core/TestResources.cs

@@ -123,6 +123,15 @@ public static class Invalid
 
         private static byte[] _Obfuscated2;
         public static byte[] Obfuscated2 => ResourceLoader.GetOrCreateResource(ref _Obfuscated2, "MetadataTests.Invalid.Obfuscated2.dll");
+
+        public static class Signatures
+        {
+            private static byte[] _SignatureCycle2;
+            public static byte[] SignatureCycle2 => ResourceLoader.GetOrCreateResource(ref _SignatureCycle2, "MetadataTests.Invalid.Signatures.SignatureCycle2.exe");
+
+            private static byte[] _TypeSpecInWrongPlace;
+            public static byte[] TypeSpecInWrongPlace => ResourceLoader.GetOrCreateResource(ref _TypeSpecInWrongPlace, "MetadataTests.Invalid.Signatures.TypeSpecInWrongPlace.exe");
+        }
     }
 
     public static class NetModule01