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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for MCS 2.6.4 pinned region with array variable #3110

Closed

Conversation

ElektroKill
Copy link
Contributor

Link to issue(s) this covers:
N/A

Problem

The decompiler did not support decompiling pinned regions with array variables when the compiler used was MCS 2.6.4.

Before:

public unsafe int MultipleExitsOutOfFixedBlock(int[] arr)
{
	fixed (int* ptr = &System.Runtime.CompilerServices.Unsafe.AsRef<int>((int*)System.Runtime.CompilerServices.Unsafe.AsPointer(ref arr[0])))
	{
		if (*ptr < 0)
		{
			return *ptr;
		}
		if (*ptr == 21)
		{
			return 42;
		}
		if (*ptr == 42)
		{
			Console.WriteLine("outside");
			return 2;
		}
	}
	ptr = null;
	return 1;
}

After:

public unsafe int MultipleExitsOutOfFixedBlock(int[] arr)
{
	fixed (int* ptr = &arr[0])
	{
		if (*ptr < 0)
		{
			return *ptr;
		}
		if (*ptr == 21)
		{
			return 42;
		}
		if (*ptr == 42)
		{
			Console.WriteLine("outside");
			return 2;
		}
	}
	return 1;
}

Solution

  • Added additional code to remove the conv instruction present in the initialization part of the pinned region.
  • Extended the code responsible for removing the unpin stloc to correctly match the inverted condition found in MCS 2.6.4 compiled code.
  • Enabled already present correctness test to run for MCS 2.6.4.

trueInstr is Branch trueBr && trueBr.TargetContainer == sourceContainer &&
reachedEdgesPerBlock[trueBr.TargetBlock.ChildIndex] == 0)
{
unpinBlock = trueBr.TargetBlock;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand correctly, the point of this change is to check both outgoing edges for an if block?
Should innerBlock.Instructions.Count == 2 be innerBlock.Instructions.Count >= 2 instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I used == 2 since in my testing there were always two instructions with one being an if and the second being the fallthrough br. If we were to extend the limit to more than 2 instructions I think it would be necessary to check other instructions for potential outgoing edges which would mean additional logic. though in my testing it seems that the current code is enough.

@dgrunwald dgrunwald closed this in 38e7ab4 Apr 1, 2024
mattsh247 pushed a commit to mattsh247/ILSpy that referenced this pull request Jul 30, 2024
…ray variable

* Added additional code to remove the conv instruction present in the initialization part of the pinned region.
* Extended the code responsible for removing the unpin stloc to correctly match the inverted condition found in MCS 2.6.4 compiled code.
* Enabled already present correctness test to run for MCS 2.6.4.

This is a more generalized version of the fix on PR icsharpcode#3110 proposed by @ElektroKill.
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 this pull request may close these issues.

2 participants