Skip to content
This repository has been archived by the owner on Jul 15, 2023. It is now read-only.

NullReferenceException in ccrewrite-generated code #191

Closed
fedotovalex opened this issue Aug 7, 2015 · 16 comments
Closed

NullReferenceException in ccrewrite-generated code #191

fedotovalex opened this issue Aug 7, 2015 · 16 comments

Comments

@fedotovalex
Copy link
Contributor

To reproduce, compile and run the following application in VS2015:

using System;
using System.Diagnostics.Contracts;

namespace Application
{
    public class Program
    {
        static void Main(string[] args)
        {
            new Program("Hello, world");
        }

        private Program(string a)
        {
            Contract.Requires(a != null);

            Delayed(() => Process(a));
        }

        private void Delayed(Action action)
        {
        }

        private static void Process(string s)
        {
        }

        public bool Flag = true;
    }
}

Observe a NullReferenceException thrown on the line with Contract.Requires(a != null). (Initializing the Flag field is crucial for reproducing it; without the field or if the field is left uninitialized, the problem does not reproduce).

When opened in a decompiler, the constructor shows the following code:

    private Program(string a)
    {
      Program.Program_<>c__DisplayClass1_0_0 cDisplayClass100;
      __ContractsRuntime.Requires(cDisplayClass100.a != null, (string) null, "a != null");
      this.Flag = true;
      Program.<>c__DisplayClass1_0 cDisplayClass10 = new Program.<>c__DisplayClass1_0();
      cDisplayClass10.a = a;
      base..ctor();
      this.Delayed(new Action((object) cDisplayClass10, __methodptr(<.ctor>b__0)));
    }

which explains the NullReferenceException.

FWIW, without the Flag field, the constructor looks like this:

    private Program(string a)
    {
      Program.Program_<>c__DisplayClass1_0_0 cDisplayClass100 = new Program.Program_<>c__DisplayClass1_0_0();
      cDisplayClass100.a = a;
      __ContractsRuntime.Requires(cDisplayClass100.a != null, (string) null, "a != null");
      Program.<>c__DisplayClass1_0 cDisplayClass10 = new Program.<>c__DisplayClass1_0();
      cDisplayClass10.a = a;
      base..ctor();
      this.Delayed(new Action((object) cDisplayClass10, __methodptr(<.ctor>b__0)));
    }
@SergeyTeplyakov
Copy link
Contributor

Thanks a lot! This is very useful bug report!

@SergeyTeplyakov
Copy link
Contributor

Grabbed.

@sharwell
Copy link
Member

sharwell commented Aug 8, 2015

Thanks a lot! This is very useful bug report!

Agreed 👍

@maw136
Copy link

maw136 commented Aug 14, 2015

I'd like to kindly ask, if there will be some fast-track updated release with that bug in mind?

@SergeyTeplyakov
Copy link
Contributor

I'm still working on it, it was slightly more complicated that I've expexted. We can plan next release when this and other fixes would be ready.

@a-vzhik
Copy link

a-vzhik commented Sep 2, 2015

Win 7 / VS2015
Got the similar issue with the next code:

    public sealed class CodeContractsCracker
    {
        private List<Action> _actions = new List<Action>();

        public CodeContractsCracker(Action action)
        {
            Contract.Requires<ArgumentNullException>(action != null);

            this._actions.Add(() => action());
        }
    }

Rewriter produces this output:

public CodeContractsCracker(Action testAction)
{
    CodeContractsCracker.CodeContractsCracker_<>c__DisplayClass1_0_0 codeContractsCracker_<>c__DisplayClass1_0_;
    __ContractsRuntime.Requires<ArgumentNullException>(codeContractsCracker_<>c__DisplayClass1_0_.testAction != null, null, "testAction != null");
    this._actions = new List<Action>();
    base..ctor();
    this._actions.Add(delegate
    {
        testAction();
    });
}

I saw that @SergeyTeplyakov is working on the problem. It would be great if you test updated CodeContracts against my example too.

@mgaffigan
Copy link

Also affecting our project. Repro similar to @a-vzhik on VS2015 against .Net 4.5.1.

class A
{
    object c = new object();

    public A(object o)
    {
        Contract.Requires(o != null);

        Action a = () => o.ToString();
    }
}

@SergeyTeplyakov, do you know what would be needed for a fix? We would consider putting a dev on it since it is showing up in multiple spots in our solution.

@SergeyTeplyakov
Copy link
Contributor

@mgaffigan I'm working on this fix right now.

The bug is subtle but the fix should be relatively straightforward.

I'll keep updating everyone. Hope to fix this stuff this week.

@mgaffigan
Copy link

@SergeyTeplyakov, excellent. Thank you.

@mgaffigan
Copy link

@SergeyTeplyakov, any luck?

@SergeyTeplyakov
Copy link
Contributor

@mgaffigan Found root cause, working on the fix.

@SergeyTeplyakov
Copy link
Contributor

And, just in case, if someone interested, the reason of this break is following:

"Old" C# compiler has following code generation pattern for constructors:

// VS2013-
Foo.ctor()
{
  // Closure initialization
  // Field initialization
  // Base Constructor call
  // Body of the constructor
}

But in C# 6.0 the pattern was changed to:

Foo.ctor()
{
  // Field initialization
  // Closure initialization
  // Base Constructor call
  // Body of the constructor
}

So the method ExtractClosureInitializationAndLocalThis should respect this change...

@SergeyTeplyakov
Copy link
Contributor

@mgaffigan I've made a dirty fix, will play around a little bit for more proper solution. Otherwise will create a PR with that...

SergeyTeplyakov added a commit to SergeyTeplyakov/CodeContracts that referenced this issue Nov 7, 2015
@SergeyTeplyakov
Copy link
Contributor

Found appropriate solution and real root cause of the issue.

It so funny that after couple of hours of investigation you need to change 5 lines of code to fix some nasty bug:)

PR submitted. I've added all test cases related to this issue. Everything works.

@tom-englert
Copy link
Contributor

@SergeyTeplyakov we recently faced this issue in our code.
Has this fix ever been merged? I could not find a PR for the changes.

@hubuk
Copy link
Contributor

hubuk commented Sep 2, 2016

@tom-englert
PR is here: #293

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

No branches or pull requests

8 participants