Skip to content
This repository has been archived by the owner on Aug 24, 2022. It is now read-only.

JSILC generates JS, that uses uninitialized variable if it is being passed by ref to another method #44

Closed
matra774 opened this issue Dec 4, 2011 · 3 comments
Labels

Comments

@matra774
Copy link
Contributor

matra774 commented Dec 4, 2011

Here is the minimal test case (reduced version of IEnumerable.ToArray from MOno implementation):

public class RefArrayTest
{
 public void SomeRefFunction2(ref int[] x)
 {
 }
 public int[] MyTest(bool dummy)
 {
   int[] array;
   if (dummy) // if you remove this and the following 4 lines, then the generated code is OK
   {
     array = new int[3];
     return array;
   }
   array = null;
   SomeRefFunction2(ref array); //Array.Resize(ref array, pos * 2);
   return array;
 }

}

And here is the JS code generated:

JSIL.MakeClass(new JSIL.TypeRef($asm02, "System.Object"), "TestHelloWorld.RefArrayTest", true, [], function ($) {
    $.prototype.SomeRefFunction2 = function (/* ref */ x) {
    };
    $.prototype.MyTest = function (dummy) {
        if (dummy) {
            var array = new JSIL.Variable(JSIL.Array.New($asm02.System.Int32, 3));
            var result = array.value;
        } else {
            array.value = null;  // **** UNINITIALIZED array, so array.value fails ****
            this.SomeRefFunction2(/* ref */ array);
            result = array.value;
        }
        return result;
    };
    $.prototype._ctor = function () {
        $asm02.System.Object.prototype._ctor.call(this);
    };

});
@kg
Copy link
Member

kg commented Dec 8, 2011

Oof, looks like the logic for introducing variable declarations is placing them inside conditionals. I thought I had made sure it wouldn't do that.

I've had a lot of problems with that particular stage of the code gen. It might be better to just put every declaration at the top of the function body and be done with it...

kg added a commit that referenced this issue Dec 11, 2011
…lways do it inside the containing function's scope instead of the current block scope. This prevents the initialization from being erroneously introduced inside a loop or conditional. Fixes issue #44.
@kg
Copy link
Member

kg commented Dec 11, 2011

I had to expand on your test case some to detect a few other ways in which the generated JS could be wrong - my first attempt at a fix produced valid JS with invalid behavior. :) Let me know if you find any other forms of this bug that I missed.

@kg kg closed this as completed Dec 11, 2011
@matra774
Copy link
Contributor Author

Thanks, great work! I'll let you know if I find any problems.

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

No branches or pull requests

2 participants