Bug 58291 - Mono.CSharp Evaluator does not compile enums and throws an exception
Summary: Mono.CSharp Evaluator does not compile enums and throws an exception
Status: RESOLVED FIXED
Alias: None
Product: Class Libraries
Classification: Mono
Component: mscorlib (show other bugs)
Version: 5.2 (2017-04)
Hardware: PC Windows
: --- normal
Target Milestone: 15.4
Assignee: Marek Safar
URL:
Depends on:
Blocks:
 
Reported: 2017-07-21 11:55 UTC by Michael Stoyke
Modified: 2017-08-28 12:59 UTC (History)
4 users (show)

See Also:
Tags:
Is this bug a regression?: ---
Last known good build:


Attachments
Diff for a potential bug fix (1.57 KB, application/mbox)
2017-07-21 11:55 UTC, Michael Stoyke
Details
Better fix for enums in Mono.CSharp (1.89 KB, patch)
2017-07-24 19:50 UTC, Michael Stoyke
Details | Diff

Description Michael Stoyke 2017-07-21 11:55:49 UTC
Created attachment 23713 [details]
Diff for a potential bug fix

When using the evaluator in Mono.CSharp we noticed a problem with enums. Enums were not compiled correctly and the compiler threw and exception in Mono.CSharp.Const.Emit() when calling FieldBuilder.SetConstant (c.GetValue());

Attached to this issue is a diff for a potential bugfix. But I don't understand enough of the mcs codebase to judge the implication of this change. I don't know if it's a good fix or just a workaround that treats the symptoms.

Please review the diff and let me know if this is a proper bugfix so that I can issue a pull request for it. Until then I'll be using this in our project as it seems it WorksForMe(tm)

To reproduce the issue, create an instance of the Mono.CSharp Evaluator and call Run() with something like:

"using System; namespace Test { public class TestClass { private TestEnum _te; public string Get() { return _te.ToString(); } } public enum TestEnum { First, Second } }"
Comment 1 Michael Stoyke 2017-07-24 19:50:05 UTC
Created attachment 23761 [details]
Better fix for enums in Mono.CSharp

I don't know if this new patch is a proper fix for the problem, but the first diff was certainly wrong and this one works for all my use cases. I'm looking forward to get some feedback if this is actually fixing the problems or just hiding the symptoms.
Comment 2 Marek Safar 2017-07-25 14:41:54 UTC
This is actually not mcs issue but SRE.

PR with the fix https://github.com/mono/mono/pull/5262
Comment 3 Michael Stoyke 2017-07-25 15:08:26 UTC
I don't think that this is the right solution to it. The issue is not just related to Mono, it also fails when using .NET (I used 4.6.2 in my tests).
Comment 4 Marek Safar 2017-07-25 15:13:22 UTC
Could you attach the exception you get on .NET ?
Comment 5 Michael Stoyke 2017-07-25 15:17:36 UTC
System.ArgumentException: Constant does not match the defined type.
   at System.Reflection.Emit.TypeBuilder.SetConstantValue(ModuleBuilder module, Int32 tk, Type destType, Object value)
   at System.Reflection.Emit.FieldBuilder.SetConstant(Object defaultValue)
   at Mono.CSharp.Const.Emit() in D:\Projects\Tests\TestMonoCompiler\TestMonoCompiler\mcs\const.cs:line 96
   at Mono.CSharp.TypeDefinition.Emit() in D:\Projects\Tests\TestMonoCompiler\TestMonoCompiler\mcs\class.cs:line 2228
   at Mono.CSharp.TypeDefinition.EmitContainer() in D:\Projects\Tests\TestMonoCompiler\TestMonoCompiler\mcs\class.cs:line 2261
   at Mono.CSharp.TypeContainer.EmitContainer() in D:\Projects\Tests\TestMonoCompiler\TestMonoCompiler\mcs\class.cs:line 339
   at Mono.CSharp.NamespaceContainer.EmitContainer() in D:\Projects\Tests\TestMonoCompiler\TestMonoCompiler\mcs\namespace.cs:line 876
   at Mono.CSharp.TypeContainer.EmitContainer() in D:\Projects\Tests\TestMonoCompiler\TestMonoCompiler\mcs\class.cs:line 339
   at Mono.CSharp.NamespaceContainer.EmitContainer() in D:\Projects\Tests\TestMonoCompiler\TestMonoCompiler\mcs\namespace.cs:line 876
   at Mono.CSharp.TypeContainer.EmitContainer() in D:\Projects\Tests\TestMonoCompiler\TestMonoCompiler\mcs\class.cs:line 339
   at Mono.CSharp.ModuleContainer.EmitContainer() in D:\Projects\Tests\TestMonoCompiler\TestMonoCompiler\mcs\module.cs:line 602
   at Mono.CSharp.Evaluator.CompileBlock(Class host, Undo undo, Report Report) in D:\Projects\Tests\TestMonoCompiler\TestMonoCompiler\mcs\eval.cs:line 783
   at Mono.CSharp.Evaluator.Compile(String input, CompiledMethod& compiled) in D:\Projects\Tests\TestMonoCompiler\TestMonoCompiler\mcs\eval.cs:line 268
   at Mono.CSharp.Evaluator.Evaluate(String input, Object& result, Boolean& result_set) in D:\Projects\Tests\TestMonoCompiler\TestMonoCompiler\mcs\eval.cs:line 359
   at Mono.CSharp.Evaluator.Run(String statement) in D:\Projects\Tests\TestMonoCompiler\TestMonoCompiler\mcs\eval.cs:line 464
   at Interface.Systems.ScriptingHost.Run(String code) in D:\Projects\Tests\TestMonoCompiler\TestMonoCompiler\Scriptinghost.cs:line 39
   at TestMonoCompiler.Program.Main(String[] args) in D:\Projects\Tests\TestMonoCompiler\TestMonoCompiler\Program.cs:line 44
Comment 6 Marek Safar 2017-07-26 11:35:17 UTC
The proposed patch is not really correct as it defines enum member with underlying type and not the enum type itself.

I made different fix for it instead
Comment 7 Michael Stoyke 2017-07-26 12:09:53 UTC
Awesome. Thanks a lot. I just saw the commit and it looks indeed much better than my hack.

I've also investigated a big more with a debugger. Apparently the main problem is that the Fieldbuilder is incompletely initialized for an enum. It looks like the IsEnum property returns true, but the underlying type property throws an exception.

I also looked at the .NET SRE code and it has proper handling for enums in the SetConstant code. The fact that the Fieldbuilder is throwing the exception when the underlying type property is accessed seems to be the main issue that amde this fail.
Comment 8 Prasad Raghorte 2017-08-02 07:22:39 UTC
@Marek Safar @Michael Stoyke
Can you please provide the fix version for verification.
Comment 9 Marek Safar 2017-08-02 15:40:27 UTC
It has been fixed in mono master only
Comment 10 Michael Stoyke 2017-08-28 12:59:51 UTC
It seems that the emitted type information is still not correct with this fix, should I open a new ticket or reuse this one?

Details:
If we try to iterate the fields (reflection) of a class that is using an enum compiled with this fix, we get an exception:

System.TypeLoadException: Field 'MyEnumParameter' is an enum type with a bad underlying type
at (wrapper managed-to-native) System.RuntimeType:GetFields_native (System.RuntimeType,intptr,System.Reflection.BindingFlags)
at System.RuntimeType.GetFields_internal (System.String name, System.Reflection.BindingFlags bindingAttr, System.RuntimeType reflectedType) [0x0001a] in <8bab4e9291ac48d3a5b37cf4064ebe75>:0 
at System.RuntimeType.GetFieldCandidates (System.String name, System.Reflection.BindingFlags bindingAttr, System.Boolean allowPrefixLookup) [0x0000f] in <8bab4e9291ac48d3a5b37cf4064ebe75>:0 
at System.RuntimeType.GetFields (System.Reflection.BindingFlags bindingAttr) [0x00000] in <8bab4e9291ac48d3a5b37cf4064ebe75>:0

Note You need to log in before you can comment on or make changes to this bug.