Skip to content
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.

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

Bug 217740: don't crash when emitting or loading a DateTimeConstant(-1) attribute #11536

Merged
merged 13 commits into from
May 26, 2016
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using System.Runtime.InteropServices;
using System.Text;
using Microsoft.CodeAnalysis.CSharp.Symbols;
using Microsoft.CodeAnalysis.CSharp.Symbols.Metadata.PE;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.CSharp.Test.Utilities;
using Microsoft.CodeAnalysis.Test.Utilities;
Expand Down Expand Up @@ -416,6 +417,204 @@ class C
CompileAndVerify(text, additionalRefs: new[] { SystemRef }, sourceSymbolValidator: attributeValidator);
}

[Fact]
[WorkItem(217740, "https://devdiv.visualstudio.com/DefaultCollection/DevDiv/_workitems?id=217740")]
public void DateTimeConstantAttribute()
{
#region "Source"
var source = @"
using System;
using System.Runtime.CompilerServices;

public class Bar
{
public void Method([DateTimeConstant(-1)]DateTime p1) { }
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we need to have a test that tries to consume this method without providing the argument

  • in the same compilation
  • in another compilation, referencing metadata produced by this one.

Is behavior for these scenarios consistent with native compiler?

Copy link
Contributor

@AlekseyTs AlekseyTs May 25, 2016

Choose a reason for hiding this comment

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

Should test the same when default value is explicitly set through the special syntax on the parameter.
Should test similar scenarios for VB too.

void m1([DateTimeConstant(-1)] DateTime x = default(DateTime))

In reply to: 64497023 [](ancestors = 64497023)

Copy link
Contributor

Choose a reason for hiding this comment

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

What we need to watch out for in cases when values are also provided through special syntax is that we wouldn't try to emit two or more DateTimeConstantValue attributes.


In reply to: 64497476 [](ancestors = 64497476,64497023)

Copy link
Member Author

Choose a reason for hiding this comment

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

I already manually verified the behavior when referencing metadata. The behavior is consistent, an error is reported (C# used to report error CS1501: No overload for method 'Method' takes 0 arguments and VB used to report error BC30455: Argument not specified for parameter 'p' of 'Public Function Method(p As Date = #12:00:00 AM#) As Date').
I cannot do compilation reference with native compiler.

I added a test for both scenarios (C# and VB).
I'll add test for "double attribute" concern.

Copy link
Member Author

Choose a reason for hiding this comment

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

The "double attribute" scenario works fine in C# (only one is emitted), but VB emits two. I'll stop by.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, it looks like C# never synthesizes the attribute.


In reply to: 64610028 [](ancestors = 64610028)

}
";
#endregion

// The native C# compiler emits this:
// .param[1]
// .custom instance void[mscorlib] System.Runtime.CompilerServices.DateTimeConstantAttribute::.ctor(int64) = (
// 01 00 ff ff ff ff ff ff ff ff 00 00
// )
Action<IModuleSymbol> verifier = (module) =>
{
var bar = (NamedTypeSymbol)((ModuleSymbol)module).GlobalNamespace.GetMember("Bar");
var method = (MethodSymbol)bar.GetMember("Method");
var parameters = method.GetParameters();
var theParameter = (PEParameterSymbol)parameters[0];
var peModule = (PEModuleSymbol)module;

Assert.Equal(ParameterAttributes.None, theParameter.Flags);

// let's find the attribute in the PE metadata
var attributeInfo = PEModule.FindTargetAttribute(peModule.Module.MetadataReader, theParameter.Handle, AttributeDescription.DateTimeConstantAttribute);
Assert.True(attributeInfo.HasValue);

long attributeValue;
Assert.True(peModule.Module.TryExtractLongValueFromAttribute(attributeInfo.Handle, out attributeValue));
Assert.Equal(-1L, attributeValue); // check the attribute is constructed with a -1

// check .param has no value
var constantHandle = peModule.Module.MetadataReader.GetParameter(theParameter.Handle).GetDefaultValue();
Assert.True(constantHandle.IsNil);
};

var comp = CompileAndVerify(source, symbolValidator: verifier);
comp.VerifyDiagnostics();
}

[Fact]
[WorkItem(217740, "https://devdiv.visualstudio.com/DefaultCollection/DevDiv/_workitems?id=217740")]
public void DateTimeConstantAttributeReferencedViaRef()
{
#region "Source"
var source1 = @"
using System;
using System.Runtime.CompilerServices;

public class Bar
{
public void Method([DateTimeConstant(-1)]DateTime p1) { }
}
";

var source2 = @"
public class Consumer
{
public static void M()
{
new Bar().Method();
}
}
";
#endregion

var libComp = CreateCompilationWithMscorlib(source1);
var libCompRef = new CSharpCompilationReference(libComp);

var comp2 = CreateCompilationWithMscorlib(source2, new[] { libCompRef });
comp2.VerifyDiagnostics(
// (6,19): error CS7036: There is no argument given that corresponds to the required formal parameter 'p1' of 'Bar.Method(DateTime)'
// new Bar().Method();
Diagnostic(ErrorCode.ERR_NoCorrespondingArgument, "Method").WithArguments("p1", "Bar.Method(System.DateTime)").WithLocation(6, 19)
);

var libAssemblyRef = libComp.EmitToImageReference();
var comp3 = CreateCompilationWithMscorlib(source2, new[] { libAssemblyRef });
comp3.VerifyDiagnostics(
// (6,19): error CS7036: There is no argument given that corresponds to the required formal parameter 'p1' of 'Bar.Method(DateTime)'
// new Bar().Method();
Diagnostic(ErrorCode.ERR_NoCorrespondingArgument, "Method").WithArguments("p1", "Bar.Method(System.DateTime)").WithLocation(6, 19)
);
}

[Fact]
[WorkItem(217740, "https://devdiv.visualstudio.com/DefaultCollection/DevDiv/_workitems?id=217740")]
public void DateTimeConstantAttributeWithDefaultValue()
{
#region "Source"
var source = @"
using System;
using System.Runtime.CompilerServices;

public class Bar
{
public DateTime M1([DateTimeConstant(-1)] DateTime x = default(DateTime)) { return x; }
public static void Main()
{
Console.WriteLine(new Bar().M1().Ticks);
}
}
";
#endregion

// The native C# compiler emits this:
// .method public hidebysig instance void M1([opt] valuetype[mscorlib] System.DateTime x) cil managed
// {
// .param [1] = nullref
// .custom instance void[mscorlib] System.Runtime.CompilerServices.DateTimeConstantAttribute::.ctor(int64) = ( 01 00 FF FF FF FF FF FF FF FF 00 00 )
Action<IModuleSymbol> verifier = (module) =>
{
var bar = (NamedTypeSymbol)((ModuleSymbol)module).GlobalNamespace.GetMember("Bar");
var method = (MethodSymbol)bar.GetMember("M1");
var parameters = method.GetParameters();
var theParameter = (PEParameterSymbol)parameters[0];
var peModule = (PEModuleSymbol)module;

Assert.Equal(ParameterAttributes.Optional | ParameterAttributes.HasDefault, theParameter.Flags);

// let's find the attribute in the PE metadata
var attributeInfo = PEModule.FindTargetAttribute(peModule.Module.MetadataReader, theParameter.Handle, AttributeDescription.DateTimeConstantAttribute);
Assert.True(attributeInfo.HasValue);

long attributeValue;
Assert.True(peModule.Module.TryExtractLongValueFromAttribute(attributeInfo.Handle, out attributeValue));
Assert.Equal(-1L, attributeValue); // check the attribute is constructed with a -1

// check .param has no value
var constantValue = peModule.Module.GetParamDefaultValue(theParameter.Handle);
Assert.Equal(ConstantValue.Null, constantValue);
};

var comp = CompileAndVerify(source, expectedOutput: "0", symbolValidator: verifier);
comp.VerifyDiagnostics();
}

[Fact, WorkItem(217740, "https://devdiv.visualstudio.com/DefaultCollection/DevDiv/_workitems?id=217740")]
public void LoadingDateTimeConstantWithBadValue()
{
var ilsource = @"
.class public auto ansi beforefieldinit C
extends [mscorlib]System.Object
{
.method public hidebysig instance valuetype [mscorlib]System.DateTime
Method([opt] valuetype [mscorlib]System.DateTime p) cil managed
{
.param [1]
.custom instance void [mscorlib]System.Runtime.CompilerServices.DateTimeConstantAttribute::.ctor(int64) = ( 01 00 FF FF FF FF FF FF FF FF 00 00 )
// Code size 7 (0x7)
.maxstack 1
.locals init (valuetype [mscorlib]System.DateTime V_0)
IL_0000: nop
IL_0001: ldarg.1
IL_0002: stloc.0
IL_0003: br.s IL_0005

IL_0005: ldloc.0
IL_0006: ret
} // end of method C::Method

.method public hidebysig specialname rtspecialname
instance void .ctor() cil managed
{
// Code size 7 (0x7)
.maxstack 8
IL_0000: ldarg.0
IL_0001: call instance void [mscorlib]System.Object::.ctor()
IL_0006: ret
} // end of method C::.ctor

} // end of class C

";

var cssource = @"
public class D
{
public static void Main()
{
System.Console.WriteLine(new C().Method().Ticks);
}
}
";

var ilReference = CompileIL(ilsource);
CompileAndVerify(cssource, expectedOutput: "0", additionalRefs: new[] { ilReference });
// The native compiler would produce a working exe, but that exe would fail at runtime
}

[Fact]
public void TestDecimalConstantAttribute()
{
Expand Down Expand Up @@ -3176,7 +3375,7 @@ public static int Main ()

// the resulting code does not need to verify
// This is consistent with Dev10 behavior
CompileAndVerify(source, options: TestOptions.ReleaseDll, verify:false, sourceSymbolValidator: sourceValidator, symbolValidator: metadataValidator);
CompileAndVerify(source, options: TestOptions.ReleaseDll, verify: false, sourceSymbolValidator: sourceValidator, symbolValidator: metadataValidator);
}

[Fact, WorkItem(544507, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/544507")]
Expand Down
13 changes: 11 additions & 2 deletions src/Compilers/Core/Portable/MetadataReader/PEModule.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1046,7 +1046,16 @@ internal bool HasDateTimeConstantAttribute(EntityHandle token, out ConstantValue
AttributeInfo info = FindLastTargetAttribute(token, AttributeDescription.DateTimeConstantAttribute);
if (info.HasValue && TryExtractLongValueFromAttribute(info.Handle, out value))
{
defaultValue = ConstantValue.Create(new DateTime(value));
// if value is outside this range, DateTime would throw when constructed
if (value < DateTime.MinValue.Ticks || value > DateTime.MaxValue.Ticks)
{
defaultValue = ConstantValue.Bad;
}
else
{
defaultValue = ConstantValue.Create(new DateTime(value));
}

return true;
}

Expand Down Expand Up @@ -1242,7 +1251,7 @@ internal bool TryExtractStringValueFromAttribute(CustomAttributeHandle handle, o
return TryExtractValueFromAttribute(handle, out value, s_attributeStringValueExtractor);
}

private bool TryExtractLongValueFromAttribute(CustomAttributeHandle handle, out long value)
internal bool TryExtractLongValueFromAttribute(CustomAttributeHandle handle, out long value)
{
return TryExtractValueFromAttribute(handle, out value, s_attributeLongValueExtractor);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,16 @@ internal ConstantValue DecodeDecimalConstantValue()

internal ConstantValue DecodeDateTimeConstantValue()
{
return ConstantValue.Create(new DateTime(this.CommonConstructorArguments[0].DecodeValue<long>(SpecialType.System_Int64)));
long value = this.CommonConstructorArguments[0].DecodeValue<long>(SpecialType.System_Int64);

// if value is outside this range, DateTime would throw when constructed
if (value < DateTime.MinValue.Ticks || value > DateTime.MaxValue.Ticks)
{
// Treat as-is parameter doesn't have default
return ConstantValue.Unset;
}

return ConstantValue.Create(new DateTime(value));
}

#endregion
Expand Down