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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

ArgumentException Type must not be ByRef under dotcover #172

Closed
hazzik opened this issue Feb 4, 2021 · 9 comments 路 Fixed by #174
Closed

ArgumentException Type must not be ByRef under dotcover #172

hazzik opened this issue Feb 4, 2021 · 9 comments 路 Fixed by #174

Comments

@hazzik
Copy link
Owner

hazzik commented Feb 4, 2021

@hazzik I'm afraid to say that it is not quite fixed 馃槥I'm seeing a different error based on the changes in 0.29.0 as follows

System.ArgumentException: Type must not be ByRef (Parameter 'type')
  Stack Trace:
      at System.Dynamic.Utils.TypeUtils.ValidateType(Type type, String paramName, Boolean allowByRef, Boolean allowPointer)
   at System.Dynamic.Utils.TypeUtils.ValidateType(Type type, String paramName)
   at System.Linq.Expressions.Expression.Convert(Expression expression, Type type, MethodInfo method)
   at System.Linq.Expressions.Expression.Convert(Expression expression, Type type)
   at DelegateDecompiler.Processor.AdjustType(Expression expression, Type type)
   at DelegateDecompiler.Processor.GetArguments(ProcessorState state, MethodBase m)
   at DelegateDecompiler.Processor.Call(ProcessorState state, MethodInfo m)
   at DelegateDecompiler.Processor.Process()
   at DelegateDecompiler.Processor.Process(VariableInfo[] locals, IList`1 args, Instruction instruction, Type returnType)
   at DelegateDecompiler.MethodBodyDecompiler.DecompileConcrete(MethodInfo method, IList`1 args)
   at DelegateDecompiler.MethodBodyDecompiler.Decompile(MethodInfo method, Type declaringType)

Originally posted by @mbrookson in #171 (comment)

@hazzik
Copy link
Owner Author

hazzik commented Feb 4, 2021

@mbrookson can you show the debug trace?

@mbrookson
Copy link

Debug Trace:
IL_0000: ldloca.s JetBrains.Profiler.Core.Instrumentation.DataOnStack (0)
 IL_0002: ldc.i4.1
 IL_0003: stfld UInt32 StatementCount
 IL_0008: ldloca.s JetBrains.Profiler.Core.Instrumentation.DataOnStack (0)
 IL_000a: ldc.i4.s 75
 IL_000c: stfld UInt32 MetadataIndex
 IL_0011: ldloca.s JetBrains.Profiler.Core.Instrumentation.DataOnStack (0)
 IL_0013: ldc.i4 100663895
 IL_0018: stfld UInt32 MethodToken
 IL_001d: ldloca.s JetBrains.Profiler.Core.Instrumentation.DataOnStack (0)
 IL_001f: call Void JetBrains_Profiler_Core_Instrumentation_Begin(JetBrains.Profiler.Core.Instrumentation.DataOnStack ByRef)

@hazzik Here's what gets logged by default. Let me know if you require more info. Thanks.

@mbrookson
Copy link

Happy to test stuff in a separate branch if that helps to diagnose and fix the issue.

@hazzik
Copy link
Owner Author

hazzik commented Feb 4, 2021

It's 0:47 here. I would test it tomorrow my time. I think the problem within AdjustType. It should not try to adjust type into ref-type (Type.IsByRef).

@mbrookson
Copy link

No problem. I will do some more investigation and try some things. Thanks!

hazzik added a commit that referenced this issue Feb 4, 2021
@hazzik
Copy link
Owner Author

hazzik commented Feb 4, 2021

Ok I've managed to fix this issue. Can you please test this branch?

@mbrookson
Copy link

Apologies. I didn't see you'd already implemented a fix. Your solution in this branch also works and is definitely better than mine.

Appreciate you spending time on this. Thank you for resolving it! Please update in Nuget when you get a chance.

@hazzik
Copy link
Owner Author

hazzik commented Feb 4, 2021

Released in 0.29.1

@mbrookson
Copy link

@hazzik Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants