Bug 42529 - vbnc creates invalid recursion in Microsoft.VisualBasic.CompilerServices.Versioned
Summary: vbnc creates invalid recursion in Microsoft.VisualBasic.CompilerServices.Vers...
Status: RESOLVED FIXED
Alias: None
Product: Compilers
Classification: Mono
Component: VisualBasic ()
Version: unspecified
Hardware: PC Linux
: --- normal
Target Milestone: ---
Assignee: Rolf Bjarne Kvinge [MSFT]
URL:
Depends on:
Blocks:
 
Reported: 2016-07-13 17:24 UTC by Toni Spets
Modified: 2016-07-15 10:42 UTC (History)
2 users (show)

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

Notice (2018-05-24): bugzilla.xamarin.com is now in read-only mode.

Please join us on Visual Studio Developer Community and in the Xamarin and Mono organizations on GitHub to continue tracking issues. Bugzilla will remain available for reference in read-only mode. We will continue to work on open Bugzilla bugs, copy them to the new locations as needed for follow-up, and add the new items under Related Links.

Our sincere thanks to everyone who has contributed on this bug tracker over the years. Thanks also for your understanding as we make these adjustments and improvements for the future.


Please create a new report on GitHub or Developer Community with your current version information, steps to reproduce, and relevant error messages or log files if you are hitting an issue that looks similar to this resolved bug and you do not yet see a matching new report.

Related Links:
Status:
RESOLVED FIXED

Description Toni Spets 2016-07-13 17:24:40 UTC
Method CallByName in Versioned.vb is incorrectly compiled into a recursion.

Source of the method:

Public Shared Function CallByName(ByVal Instance As Object, ByVal MethodName As String, ByVal UseCallType As CallType, ByVal ParamArray Arguments As Object()) As Object
    Return Interaction.CallByName(Instance, MethodName, UseCallType, Arguments)
End Function

monodis output of the method:

    // method line 215
    .method public static 
           default object CallByName (object Instance, string MethodName, valuetype Microsoft.VisualBasic.CallType UseCallType, object[] Arguments)  cil managed 
    {
        .param [4]
        .custom instance void class [mscorlib]System.ParamArrayAttribute::'.ctor'() =  (01 00 00 00 ) // ....

        // Method begins at RVA 0x3aac
        // Code size 17 (0x11)
        .maxstack 4
        .locals init (
                object  V_0)
        IL_0000:  nop 
        IL_0001:  ldarg.0 
        IL_0002:  ldarg.1 
        IL_0003:  ldarg.2 
        IL_0004:  ldarg.3 
        IL_0005:  call object class Microsoft.VisualBasic.CompilerServices.Versioned::CallByName(object, string, valuetype Microsoft.VisualBasic.CallType, object[])
        IL_000a:  ret 
        IL_000b:  nop 
        IL_000c:  ldloc 0

        IL_0010:  ret 
    } // end of method Versioned::CallByName

As you can see, it is compiled into self recursion rather than calling Interaction.CallByName. Inlining Interaction.CallByName into Versioned.CallByName works around this problem.
Comment 1 Alexander Köplinger [MSFT] 2016-07-13 19:19:59 UTC
@Rolf could we workaround the compiler bug somehow (I guess we won't be making investments in vbnc anymore with Roslyn on the horizon)?
Comment 2 Toni Spets 2016-07-14 03:52:34 UTC
Since all recent packages of mono-basic suffer from this, it'd be neat if it could be workarounded for official Xamarin packages at least.

The oldest tag I could compile without downgrading mono was 4.0 and it had the same issue.
Comment 3 Rolf Bjarne Kvinge [MSFT] 2016-07-14 14:37:33 UTC
@Alexander, yeah, I won't be doing much work on the compiler.

It should be possible to work around this in function itself though (at the very least by c&p the code from Interaction.CallByName as Toni suggested)
Comment 4 Toni Spets 2016-07-14 18:03:41 UTC
Uh, well, this is a bit odd. I compiled Microsoft.VisualBasic.dll with vbc.exe available from Microsoft.Net.Compilers and I got the same result. Isn't that the Roslyn compiler?

Is there any off chance the code has something wonky going on and both compilers are correct?
Comment 5 Alexander Köplinger [MSFT] 2016-07-14 18:21:09 UTC
Yeah, that's Roslyn so it should be correct :)

Reading https://github.com/mono/mono-basic/blob/8a804fd8f12f2e6a002173bbbc0974197530ec2f/vbruntime/Microsoft.VisualBasic/Microsoft.VisualBasic.CompilerServices/Versioned.vb#L32-L35 makes me feel like this could indeed be the issue, but I know zero about how that code is supposed to work.
Comment 6 Rolf Bjarne Kvinge [MSFT] 2016-07-14 23:27:23 UTC
Yep, this is compiler magic:

https://github.com/mono/mono-basic/blob/8a804fd8f12f2e6a002173bbbc0974197530ec2f/vbnc/vbnc/source/Emit/Emitter.vb#L1940-L1950

It's replacing calls to Interaction.CallByName with Versioned.CallByName

A simple workaround would probably be to swap the implementations of these two methods.
Comment 7 Toni Spets 2016-07-15 04:24:47 UTC
Since Roslyn agrees with the magic, it's probably the right move.
Comment 8 Alexander Köplinger [MSFT] 2016-07-15 09:31:56 UTC
@Toni do you want to send a pull request to the mono-basic repository?
Comment 9 Toni Spets 2016-07-15 10:06:40 UTC
https://github.com/mono/mono-basic/pull/9

There you go. Swapped the bodies keeping argument names. Tested this works in practice and checked monodis output for correct IL in both of the bodies.
Comment 10 Rolf Bjarne Kvinge [MSFT] 2016-07-15 10:42:53 UTC
I've merged the PR, thanks a lot!