Skip to content

Commit

Permalink
Handle corrupted default values in metadata
Browse files Browse the repository at this point in the history
The native compiler handled corrupted default parameter values by
substituting in default(T).  It's unclear if this was intentional
behavior in the compiler or not.

Either way though obfuscators took advantage of this behavior and at
least one prominent one will corrupt default parameter values when the
value is null.  Enough prominent libraries have shipped using such
obfuscators that it is a blocker to upgrading.  Hence we need to emulate
the native compiler behavior here.

This change is a bit large because I needed to update the test resources
with a corrupted DLL in order to test out the changes.

closes dotnet#4196
  • Loading branch information
jaredpar committed Aug 28, 2015
1 parent 26f4f25 commit 9ac3e8e
Show file tree
Hide file tree
Showing 43 changed files with 222 additions and 98 deletions.
6 changes: 3 additions & 3 deletions cibuild.sh
Expand Up @@ -58,9 +58,9 @@ done

restore_nuget()
{
curl -O https://dotnetci.blob.core.windows.net/roslyn/nuget.3.zip
unzip nuget.3.zip
rm nuget.3.zip
curl -O https://dotnetci.blob.core.windows.net/roslyn/nuget.4.zip
unzip nuget.4.zip
rm nuget.4.zip
}

run_msbuild()
Expand Down
Expand Up @@ -874,9 +874,15 @@ private BoundExpression GetDefaultParameterValue(CSharpSyntaxNode syntax, Parame
Debug.Assert(parameter.IsOptional);
ConstantValue defaultConstantValue = parameter.ExplicitDefaultConstantValue;
BoundExpression defaultValue;

SourceLocation callerSourceLocation;

// For compatibility with the native compiler we treat all bad imported constant
// values as default(T).
if (defaultConstantValue != null && defaultConstantValue.IsBad)
{
defaultConstantValue = ConstantValue.Null;
}

if (parameter.IsCallerLineNumber && ((callerSourceLocation = GetCallerLocation(syntax, enableCallerInfo)) != null))
{
int line = callerSourceLocation.SourceTree.GetDisplayLineNumber(callerSourceLocation.SourceSpan);
Expand Down
Expand Up @@ -85,7 +85,7 @@
<ItemGroup>
<Reference Include="Microsoft.CodeAnalysis.Test.Resources.Proprietary, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
<SpecificVersion>False</SpecificVersion>
<HintPath>..\..\..\..\..\packages\Microsoft.CodeAnalysis.Test.Resources.Proprietary.1.1.0-beta1-20150716-08\lib\net45\Microsoft.CodeAnalysis.Test.Resources.Proprietary.dll</HintPath>
<HintPath>..\..\..\..\..\packages\Microsoft.CodeAnalysis.Test.Resources.Proprietary.1.1.0-beta1-20150824-02\lib\net45\Microsoft.CodeAnalysis.Test.Resources.Proprietary.dll</HintPath>
</Reference>
<Reference Include="Microsoft.CSharp" />
<Reference Include="System" />
Expand Down Expand Up @@ -124,4 +124,4 @@
<Import Project="..\..\..\..\..\build\Targets\Roslyn.Toolsets.Xunit.targets" />
<Import Project="$(SolutionDir)\.nuget\NuGet.targets" Condition="Exists('$(SolutionDir)\.nuget\NuGet.targets')" />
</ImportGroup>
</Project>
</Project>
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Test/CommandLine/packages.config
@@ -1,5 +1,5 @@
<?xml version="1.0" encoding="utf-8"?>
<packages>
<package id="Microsoft.CodeAnalysis.Test.Resources.Proprietary" version="1.1.0-beta1-20150716-08" targetFramework="net45" />
<package id="Microsoft.CodeAnalysis.Test.Resources.Proprietary" version="1.1.0-beta1-20150824-02" targetFramework="net45" />
<package id="xunit" version="1.9.2" targetFramework="net45" />
</packages>
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Test/Emit/CSharpCompilerEmitTest.csproj
Expand Up @@ -184,7 +184,7 @@
<ItemGroup>
<Reference Include="Microsoft.CodeAnalysis.Test.Resources.Proprietary, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
<SpecificVersion>False</SpecificVersion>
<HintPath>..\..\..\..\..\packages\Microsoft.CodeAnalysis.Test.Resources.Proprietary.1.1.0-beta1-20150716-08\lib\net45\Microsoft.CodeAnalysis.Test.Resources.Proprietary.dll</HintPath>
<HintPath>..\..\..\..\..\packages\Microsoft.CodeAnalysis.Test.Resources.Proprietary.1.1.0-beta1-20150824-02\lib\net45\Microsoft.CodeAnalysis.Test.Resources.Proprietary.dll</HintPath>
</Reference>
<Reference Include="Microsoft.CSharp" />
<Reference Include="System" />
Expand Down Expand Up @@ -212,4 +212,4 @@
<Import Project="..\..\..\..\..\build\Targets\Roslyn.Toolsets.Xunit.targets" />
<Import Project="$(SolutionDir)\.nuget\NuGet.targets" Condition="Exists('$(SolutionDir)\.nuget\NuGet.targets')" />
</ImportGroup>
</Project>
</Project>
32 changes: 32 additions & 0 deletions src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenTests.cs
Expand Up @@ -14157,5 +14157,37 @@ .locals init (int V_0)

}

[Fact]
[WorkItem(4196, "https://github.com/dotnet/roslyn/issues/4196")]
public void BadDefaultParameterValue()
{
// In this DLL there is an optional parameter which has a corrupted metadata value
// as the default argument. This can happen in legitamite code when run through an
// obfuscator program. For compatibility with the native compiler we need to treat
// the value as default(T)
string source = @"
using System;
using BadDefaultParameterValue;
class Program
{
static void Main()
{
Util.M(""test"");
}
}";

var testReference = AssemblyMetadata.CreateFromImage(TestResources.Repros.BadDefaultParameterValue).GetReference();
var compilation = CompileAndVerify(source, additionalRefs: new[] { testReference });
compilation.VerifyIL("Program.Main", @"
{
// Code size 12 (0xc)
.maxstack 2
IL_0000: ldstr ""test""
IL_0005: ldnull
IL_0006: call ""void BadDefaultParameterValue.Util.M(string, string)""
IL_000b: ret
}");
}
}
}
60 changes: 58 additions & 2 deletions src/Compilers/CSharp/Test/Emit/Emit/EmitErrorTests.cs
Expand Up @@ -106,7 +106,10 @@ public static void Main()
}
}
";
VerifyEmitDiagnostics(source2, compilation1);
var compilation2 = CompileAndVerify(
source2,
new[] { new CSharpCompilationReference(compilation1) },
verify: false);
}

[WorkItem(543039, "DevDiv")]
Expand Down Expand Up @@ -135,7 +138,60 @@ public static void Main()
}
}
";
VerifyEmitDiagnostics(source2, compilation1);
var compilation2 = CompileAndVerify(
source2,
new[] { new CSharpCompilationReference(compilation1) },
verify: false);
}

[WorkItem(543039, "DevDiv")]
[Fact]
public void BadDefaultArgumentInOtherAssembly_UserDefinedType()
{
string source1 = @"
public struct S
{
public override string ToString() { return ""S::ToString""; }
}
public class A
{
public static S Foo(S p = 42) { return p; }
}
";
var compilation1 = CreateCompilationWithMscorlib(source1);
compilation1.VerifyDiagnostics(
// (9,27): error CS1750: A value of type 'int' cannot be used as a default parameter because there are no standard conversions to type 'S'
// public static S Foo(S p = 42) { return p; }
Diagnostic(ErrorCode.ERR_NoConversionForDefaultParam, "p").WithArguments("int", "S").WithLocation(9, 27));

string source2 = @"
public class B
{
public static void Main()
{
System.Console.WriteLine(A.Foo());
}
}
";

var compilation2 = CompileAndVerify(
source2,
new[] { new CSharpCompilationReference(compilation1) },
verify: false);
compilation2.VerifyIL("B.Main()", @"
{
// Code size 25 (0x19)
.maxstack 1
.locals init (S V_0)
IL_0000: ldloca.s V_0
IL_0002: initobj ""S""
IL_0008: ldloc.0
IL_0009: call ""S A.Foo(S)""
IL_000e: box ""S""
IL_0013: call ""void System.Console.WriteLine(object)""
IL_0018: ret
}");
}

[WorkItem(543039, "DevDiv")]
Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Test/Emit/packages.config
@@ -1,5 +1,5 @@
<?xml version="1.0" encoding="utf-8"?>
<packages>
<package id="Microsoft.CodeAnalysis.Test.Resources.Proprietary" version="1.1.0-beta1-20150716-08" targetFramework="net45" />
<package id="Microsoft.CodeAnalysis.Test.Resources.Proprietary" version="1.1.0-beta1-20150824-02" targetFramework="net45" />
<package id="Microsoft.DiaSymReader" version="1.0.5" targetFramework="net45" />
</packages>
</packages>
Expand Up @@ -138,7 +138,7 @@
<ItemGroup>
<Reference Include="Microsoft.CodeAnalysis.Test.Resources.Proprietary, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
<SpecificVersion>False</SpecificVersion>
<HintPath>..\..\..\..\..\packages\Microsoft.CodeAnalysis.Test.Resources.Proprietary.1.1.0-beta1-20150716-08\lib\net45\Microsoft.CodeAnalysis.Test.Resources.Proprietary.dll</HintPath>
<HintPath>..\..\..\..\..\packages\Microsoft.CodeAnalysis.Test.Resources.Proprietary.1.1.0-beta1-20150824-02\lib\net45\Microsoft.CodeAnalysis.Test.Resources.Proprietary.dll</HintPath>
</Reference>
<Reference Include="Microsoft.CSharp" />
<Reference Include="System" />
Expand All @@ -156,4 +156,4 @@
<Import Project="..\..\..\..\..\build\Targets\Roslyn.Toolsets.Xunit.targets" />
<Import Project="$(SolutionDir)\.nuget\NuGet.targets" Condition="Exists('$(SolutionDir)\.nuget\NuGet.targets')" />
</ImportGroup>
</Project>
</Project>
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Test/Semantic/packages.config
@@ -1,5 +1,5 @@
<?xml version="1.0" encoding="utf-8"?>
<packages>
<package id="Microsoft.CodeAnalysis.Test.Resources.Proprietary" version="1.1.0-beta1-20150716-08" targetFramework="net45" />
<package id="Microsoft.CodeAnalysis.Test.Resources.Proprietary" version="1.1.0-beta1-20150824-02" targetFramework="net45" />
<package id="xunit" version="1.9.2" targetFramework="net45" />
</packages>
Expand Up @@ -50,7 +50,7 @@
<ItemGroup Label="File References">
<Reference Include="Microsoft.CodeAnalysis.Test.Resources.Proprietary, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
<SpecificVersion>False</SpecificVersion>
<HintPath>..\..\..\..\..\packages\Microsoft.CodeAnalysis.Test.Resources.Proprietary.1.1.0-beta1-20150716-08\lib\net45\Microsoft.CodeAnalysis.Test.Resources.Proprietary.dll</HintPath>
<HintPath>..\..\..\..\..\packages\Microsoft.CodeAnalysis.Test.Resources.Proprietary.1.1.0-beta1-20150824-02\lib\net45\Microsoft.CodeAnalysis.Test.Resources.Proprietary.dll</HintPath>
</Reference>
<Reference Include="System.Reflection.Metadata.dll">
<HintPath>..\..\..\..\..\packages\System.Reflection.Metadata.$(SystemReflectionMetadataVersion)\lib\portable-net45+win8\System.Reflection.Metadata.dll</HintPath>
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Test/Symbol/packages.config
@@ -1,6 +1,6 @@
<?xml version="1.0" encoding="utf-8"?>
<packages>
<package id="Microsoft.CodeAnalysis.Test.Resources.Proprietary" version="1.1.0-beta1-20150716-08" targetFramework="net45" />
<package id="Microsoft.CodeAnalysis.Test.Resources.Proprietary" version="1.1.0-beta1-20150824-02" targetFramework="net45" />
<package id="Microsoft.DiaSymReader" version="1.0.5" targetFramework="net45" />
<package id="xunit" version="1.9.2" targetFramework="net45" />
</packages>
Expand Up @@ -164,7 +164,7 @@
<ItemGroup>
<Reference Include="Microsoft.CodeAnalysis.Test.Resources.Proprietary, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
<SpecificVersion>False</SpecificVersion>
<HintPath>..\..\..\..\..\packages\Microsoft.CodeAnalysis.Test.Resources.Proprietary.1.1.0-beta1-20150716-08\lib\net45\Microsoft.CodeAnalysis.Test.Resources.Proprietary.dll</HintPath>
<HintPath>..\..\..\..\..\packages\Microsoft.CodeAnalysis.Test.Resources.Proprietary.1.1.0-beta1-20150824-02\lib\net45\Microsoft.CodeAnalysis.Test.Resources.Proprietary.dll</HintPath>
</Reference>
<Reference Include="Microsoft.CSharp" />
<Reference Include="System" />
Expand All @@ -185,4 +185,4 @@
<Import Project="..\..\..\..\..\build\Targets\Roslyn.Toolsets.Xunit.targets" />
<Import Project="$(SolutionDir)\.nuget\NuGet.targets" Condition="Exists('$(SolutionDir)\.nuget\NuGet.targets')" />
</ImportGroup>
</Project>
</Project>
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Test/Syntax/packages.config
@@ -1,5 +1,5 @@
<?xml version="1.0" encoding="utf-8"?>
<packages>
<package id="Microsoft.CodeAnalysis.Test.Resources.Proprietary" version="1.1.0-beta1-20150716-08" targetFramework="net45" />
<package id="Microsoft.CodeAnalysis.Test.Resources.Proprietary" version="1.1.0-beta1-20150824-02" targetFramework="net45" />
<package id="xunit" version="1.9.2" targetFramework="net45" />
</packages>
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Test/WinRT/CSharpWinRTTest.csproj
Expand Up @@ -53,7 +53,7 @@
</ItemGroup>
<ItemGroup>
<Reference Include="Microsoft.CodeAnalysis.Test.Resources.Proprietary">
<HintPath>..\..\..\..\..\packages\Microsoft.CodeAnalysis.Test.Resources.Proprietary.1.1.0-beta1-20150716-08\lib\net45\Microsoft.CodeAnalysis.Test.Resources.Proprietary.dll</HintPath>
<HintPath>..\..\..\..\..\packages\Microsoft.CodeAnalysis.Test.Resources.Proprietary.1.1.0-beta1-20150824-02\lib\net45\Microsoft.CodeAnalysis.Test.Resources.Proprietary.dll</HintPath>
</Reference>
<Reference Include="System.Collections.Immutable, Version=$(SystemCollectionsImmutableAssemblyVersion), Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a, processorArchitecture=MSIL">
<SpecificVersion>False</SpecificVersion>
Expand All @@ -79,4 +79,4 @@
<Import Project="..\..\..\..\..\build\Targets\VSL.Imports.targets" />
<Import Project="..\..\..\..\..\build\Targets\Roslyn.Toolsets.Xunit.targets" />
</ImportGroup>
</Project>
</Project>
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Test/WinRT/packages.config
@@ -1,5 +1,5 @@
<?xml version="1.0" encoding="utf-8"?>
<packages>
<package id="Microsoft.CodeAnalysis.Test.Resources.Proprietary" version="1.1.0-beta1-20150716-08" targetFramework="net45" />
<package id="Microsoft.CodeAnalysis.Test.Resources.Proprietary" version="1.1.0-beta1-20150824-02" targetFramework="net45" />
<package id="xunit" version="1.9.2" targetFramework="net45" />
</packages>
</packages>
4 changes: 2 additions & 2 deletions src/Compilers/Core/CodeAnalysisTest/CodeAnalysisTest.csproj
Expand Up @@ -94,7 +94,7 @@
<ItemGroup>
<Reference Include="Microsoft.CodeAnalysis.Test.Resources.Proprietary, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
<SpecificVersion>False</SpecificVersion>
<HintPath>..\..\..\..\packages\Microsoft.CodeAnalysis.Test.Resources.Proprietary.1.1.0-beta1-20150716-08\lib\net45\Microsoft.CodeAnalysis.Test.Resources.Proprietary.dll</HintPath>
<HintPath>..\..\..\..\packages\Microsoft.CodeAnalysis.Test.Resources.Proprietary.1.1.0-beta1-20150824-02\lib\net45\Microsoft.CodeAnalysis.Test.Resources.Proprietary.dll</HintPath>
</Reference>
<Reference Include="System" />
<Reference Include="System.Xml" />
Expand Down Expand Up @@ -192,4 +192,4 @@
<Import Project="..\..\..\..\build\Targets\Roslyn.Toolsets.Xunit.targets" />
<Import Project="$(SolutionDir)\.nuget\NuGet.targets" Condition="Exists('$(SolutionDir)\.nuget\NuGet.targets')" />
</ImportGroup>
</Project>
</Project>
2 changes: 1 addition & 1 deletion src/Compilers/Core/CodeAnalysisTest/packages.config
@@ -1,5 +1,5 @@
<?xml version="1.0" encoding="utf-8"?>
<packages>
<package id="Microsoft.CodeAnalysis.Test.Resources.Proprietary" version="1.1.0-beta1-20150716-08" targetFramework="net45" />
<package id="Microsoft.CodeAnalysis.Test.Resources.Proprietary" version="1.1.0-beta1-20150824-02" targetFramework="net45" />
<package id="xunit" version="1.9.2" targetFramework="net45" />
</packages>
4 changes: 2 additions & 2 deletions src/Compilers/Core/MSBuildTaskTests/MSBuildTaskTests.csproj
Expand Up @@ -93,7 +93,7 @@
<ItemGroup>
<Reference Include="Microsoft.CodeAnalysis.Test.Resources.Proprietary, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
<SpecificVersion>False</SpecificVersion>
<HintPath>..\..\..\..\packages\Microsoft.CodeAnalysis.Test.Resources.Proprietary.1.1.0-beta1-20150716-08\lib\net45\Microsoft.CodeAnalysis.Test.Resources.Proprietary.dll</HintPath>
<HintPath>..\..\..\..\packages\Microsoft.CodeAnalysis.Test.Resources.Proprietary.1.1.0-beta1-20150824-02\lib\net45\Microsoft.CodeAnalysis.Test.Resources.Proprietary.dll</HintPath>
</Reference>
<Reference Include="System" />
<Reference Include="System.Xml" />
Expand All @@ -114,4 +114,4 @@
<ImportGroup Label="Targets">
<Import Project="..\..\..\..\build\Targets\VSL.Imports.targets" />
</ImportGroup>
</Project>
</Project>
2 changes: 1 addition & 1 deletion src/Compilers/Core/MSBuildTaskTests/packages.config
@@ -1,6 +1,6 @@
<?xml version="1.0" encoding="utf-8"?>
<packages>
<package id="Microsoft.CodeAnalysis.Test.Resources.Proprietary" version="1.1.0-beta1-20150716-08" targetFramework="net45" />
<package id="Microsoft.CodeAnalysis.Test.Resources.Proprietary" version="1.1.0-beta1-20150824-02" targetFramework="net45" />
<package id="Microsoft.DiaSymReader" version="1.0.5" targetFramework="net45" />
<package id="Moq" version="4.2.1402.2112" targetFramework="net45" />
<package id="xunit" version="1.9.2" targetFramework="net45" />
Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/Core/VBCSCompilerTests/VBCSCompilerTests.csproj
Expand Up @@ -113,7 +113,7 @@
<ItemGroup>
<Reference Include="Microsoft.CodeAnalysis.Test.Resources.Proprietary, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
<SpecificVersion>False</SpecificVersion>
<HintPath>..\..\..\..\packages\Microsoft.CodeAnalysis.Test.Resources.Proprietary.1.1.0-beta1-20150716-08\lib\net45\Microsoft.CodeAnalysis.Test.Resources.Proprietary.dll</HintPath>
<HintPath>..\..\..\..\packages\Microsoft.CodeAnalysis.Test.Resources.Proprietary.1.1.0-beta1-20150824-02\lib\net45\Microsoft.CodeAnalysis.Test.Resources.Proprietary.dll</HintPath>
</Reference>
<Reference Include="System" />
<Reference Include="System.Xml" />
Expand All @@ -139,4 +139,4 @@
<Import Project="..\..\..\..\build\Targets\Roslyn.Toolsets.Xunit.targets" />
<Import Project="$(SolutionDir)\.nuget\NuGet.targets" Condition="Exists('$(SolutionDir)\.nuget\NuGet.targets')" />
</ImportGroup>
</Project>
</Project>
2 changes: 1 addition & 1 deletion src/Compilers/Core/VBCSCompilerTests/packages.config
@@ -1,6 +1,6 @@
<?xml version="1.0" encoding="utf-8"?>
<packages>
<package id="Microsoft.CodeAnalysis.Test.Resources.Proprietary" version="1.1.0-beta1-20150716-08" targetFramework="net45" />
<package id="Microsoft.CodeAnalysis.Test.Resources.Proprietary" version="1.1.0-beta1-20150824-02" targetFramework="net45" />
<package id="Microsoft.DiaSymReader" version="1.0.5" targetFramework="net45" />
<package id="Moq" version="4.2.1402.2112" targetFramework="net45" />
<package id="xunit" version="1.9.2" targetFramework="net45" />
Expand Down

0 comments on commit 9ac3e8e

Please sign in to comment.