JustDecompile Revisited

This post is long overdue.  Several months ago I took a look at a beta release (2011.1.516.2) of JustDecompile, the upcoming decompilation solution from Telerik.  At the time I referred to it as unusable due to the number of problems I encountered with both the UI and, more importantly, its ability to reliably decompile an assembly.

Not long after I wrote that post Tsviatko Yovtchev from the JustDecompile team contacted me and through comments and email we discussed the problems I’d encountered.  I sent him some of the assemblies that JustDecompile failed to process correctly.  Since then the team has released several updates and addressed many of the problems I experienced.  I wanted to take another look and see how the project has evolved.  Has Telerik solved the problems I experienced?  Read on to find out.

For reference, the version I used for this post is 2011.1.829.2 and is still a beta.

User Interface

In my last review I discussed some aspects of the UI that I found to be either broken or could use some refinement.  For the most part the UI is the same but there have been some notable changes.

One of the first things I mentioned felt like a miss in functionality; we had to manually reload assemblies every time they changed.  I was really happy to see that this version detects changes and asks us if we want to reload.  I could be picky and complain that the dialog should use “Yes” and “No” buttons rather than “OK” and “Cancel” but I’m so glad that the prompt is there that I didn’t even notice it the first or second times.

I also noticed that the Go To Type and Go To Symbol searches have been combined into a single Search dialog.  I like that the functions have been brought together but I stand by my original comments on this aspect.  As it stands now, the searches are still separate functions since each search type has its own tab and is completely independent of the other.  I don’t see any real productivity gain by merely putting them in a common dialog but keeping them functionally independent.  That said though, Telerik has really improved the search experience.

In my last review I complained about two major problems with search:

  1. Crashes when switching between dialogs
  2. Duplicated results

I’m pleased to report that both of these problems have been resolved.  The search dialog still isn’t modal but clicking buttons on the main window no longer causes the application to crash.  The search results also aren’t duplicated.  I tried several combinations of actions that caused duplicate results before and never saw the behavior again.

On a related note, the searches are also much more responsive.  Gone is the one second wait between keystrokes and searching.  The revamped search now returns results nearly instantaneously resulting in an experience similar to that of my beloved Navigate To dialog in Visual Studio.

A feature that I don’t remember seeing in my previous trial was the Find Usages button.  The concept is similar to the Find References feature of Visual Studio but I didn’t really find this implementation to be particularly intuitive.  When the dialog is opened for the first time it displays a message that reads “Press CTRL + Left Mouse Button on a type or member in the assembly list tree to find its usages.”  To me it seems like the dialog would be more useful if it automatically used the item selected in the tree (if possible) but overall I think it’s a nice addition.

Code Tests

Even with fixes and enhancements to the UI I’d still consider JustDecompile useless if its decompilation results were still as untrustworthy as I found last time.  So how does this version do against those same tests?

Just as before:

  1. All assemblies tested were originally written in C#.
  2. All decompilation tested was to C#.
  3. The code I tested was nothing out of the ordinary.  I wasn’t trying to “trick” JustDecompile in any way.

Each of the following examples assumes the enumeration definition:

public enum SampleEnum
{
	Unknown,
	Item1,
	Item2
}

Switch Statements

My tests started with a few switch statements. The first test used a switch statement where each case returned a value.

public string SampleMethod(SampleEnum enumValue)
{
	switch (enumValue)
	{
		case SampleEnum.Item1:
			return "Item 1";

		case SampleEnum.Item2:
			return "Item 2";

		case SampleEnum.Unknown:
			return "Unknown";
	}

	return String.Empty;
}

Just as before, JustDecompile handled this test code quite well.

public string SampleMethod(SampleEnum enumValue)
{
	switch (enumValue)
	{
		case SampleEnum.Unknown:
		{
			return "Unknown";
		}
		case SampleEnum.Item1:
		{
			return "Item 1";
		}
		case SampleEnum.Item2:
		{
			return "Item 2";
		}
	}
	return string.Empty;
}

As you can see, the only real difference is that JustDecompile converted each case body to a block and changed the sequence but otherwise the code is functionally equivalent. In the last version I tried this was the only test that JustDecompile handled correctly.

The second test used a state variable that was set by each case rather than returning directly from the block.

public string SampleMethod(SampleEnum enumValue)
{
	var val = String.Empty;

	switch (enumValue)
	{
		case SampleEnum.Item1:
			val = "Item 1";
			break;

		case SampleEnum.Item2:
			val = "Item 2";
			break;

		case SampleEnum.Unknown:
			val = "Unknown";
			break;
	}

	return val;
}

Last time I tried this code JustDecompile generated code that wouldn’t compile because it omitted the break statements. This version handles it much better.

public string SampleMethod(SampleEnum enumValue)
{
	string val = string.Empty;
	switch (enumValue)
	{
		case SampleEnum.Unknown:
		{
			val = "Unknown";
			break;
		}
		case SampleEnum.Item1:
		{
			val = "Item 1";
			break;
		}
		case SampleEnum.Item2:
		{
			val = "Item 2";
			break;
		}
	}
	return val;
}

Aside from the conversion to blocks and the alternate sequence the code is functionally correct. It’s also worth mentioning here that in the last version I tried, JustDecompile made up variable names rather than using those defined in the .locals block in the MSIL. That problem has also been resolved.

My third test replaced the SampleEnum.Unknown case with a default case but was otherwise identical.

public string SampleMethod(SampleEnum enumValue)
{
	var val = String.Empty;

	switch (enumValue)
	{
		case SampleEnum.Item1:
			val = "Item 1";
			break;

		case SampleEnum.Item2:
			val = "Item 2";
			break;

		default:
			val = "Unknown";
			break;
	}

	return val;
}

This case was the one where I lost what little confidence in the product I had left last time around. What JustDecompile produced then didn’t represent the original source in any way in that it actually switched around the case bodies! This problem has been fixed and JustDecompile now produces:

public string SampleMethod(SampleEnum enumValue)
{
	string val = string.Empty;
	switch (enumValue)
	{
		case SampleEnum.Item1:
		{
			val = "Item 1";
			break;
		}
		case SampleEnum.Item2:
		{
			val = "Item 2";
			break;
		}
		default:
		{
			val = "Unknown";
			break;
		}
	}
	return val;
}

The code is now as it should be – virtually identical.

My final test with switches involved changing the body of the default case to do something different than the other cases:

public string SampleMethod(SampleEnum enumValue)
{
	var val = String.Empty;

	switch (enumValue)
	{
		case SampleEnum.Item1:
			val = "Item 1";
			break;

		case SampleEnum.Item2:
			val = "Item 2";
			break;

		default:
			Console.WriteLine("Unknown");
			break;
	}

	return val;
}

The result last time was laughable. Not only did it mix up the case bodies, it also included MSIL and labels in the generated C# code. That problem is now resolved as you can see below:

public string SampleMethod(SampleEnum enumValue)
{
	string val = string.Empty;
	switch (enumValue)
	{
		case SampleEnum.Item1:
		{
			val = "Item 1";
			break;
		}
		case SampleEnum.Item2:
		{
			val = "Item 2";
			break;
		}
		default:
		{
			Console.WriteLine("Unknown");
			break;
		}
	}
	return val;
}

Here the cases and bodies match up perfectly and there’s no MSIL anywhere to be found.

Conditional Operator

In the interest of fairness I want to mention that I also tried the conditional operator tests in ILSpy (v1.0.0.1000) and it actually performed worse. ILSpy generated code very similar to JustDecompile but also completely disregarded the enumeration, treating it exclusively as an int instead.

Switch statements weren’t all I tested though. I also ran a few tests using the conditional operator. With all of the improvements I’d observed around switch statements I had high hopes but unfortunately my dreams were soon crushed.

Just like before my first conditional test used a variable for the condition:

public void SampleMethod(int arg)
{
	var isEven = arg % 2 == 0;
	var val = isEven ? SampleEnum.Item1 : SampleEnum.Item2;
}

There’s nothing particularly tricky here but JustDecompile still seems to think otherwise:

public void SampleMethod(int arg)
{
	bool isEven = arg % 2 == 0;
	SampleEnum val = isEven || SampleEnum.Item2;
}

JustDecompile still produces code that won’t compile. Once again, not only does it throw out the true part (SampleEnum.Item1), it also tries to compare a bool with a SampleEnum and assign a bool result to a variable of type SampleEnum. The only real progress here is that the names from MSIL .locals were used.

My other conditional operator test eliminated the isEven variable by inlining it as the condition.

public void ConditionalOperatorWithInlineExpression(int arg)
{
	var val = arg % 2 == 0 ? SampleEnum.Item1 : SampleEnum.Item2;
}

JustDecompile choked on it before and unfortunately it still does.

public void ConditionalOperatorWithInlineExpression(int arg)
{
	SampleEnum val = !(arg % SampleEnum.Item2) || SampleEnum.Item2;
}

Here’s another example of code that won’t compile and I don’t even know where to begin dissecting this result. Negating the modulus of an int and an enum? Assigning a bool to a SampleEnum? This is still really, really bad.

Other Tests

For the record, ILSpy handles these correctly. The code ILSpy generates for the second example is unrecognizable but still functionally correct.

For those that were following along in the comments of the last review I mentioned that I tried an older production assembly in JustDecompile and found more than a few problems there too. I never posted the code I checked before but in just one of the production methods I checked JustDecompile produced several problems including:

  • Type conversion/assignment issues
  • “Phantom” if statements
  • Use of undefined variables
  • Invalid enum values
  • Not returning a value

With this new version of JustDecompile I rechecked that assembly hoping for positive results but once again came away disappointed.  I won’t be posting all of the code from the methods in question but I wanted to show a few highlights.

This first example is for a simple method I wrote before I knew about DataRow.IsNull:

public static bool IsNull(object value)
{
	return (value == null || value == DBNull.Value || value.ToString().Length == 0);
}

JustDecompile has only marginally improved handling that method and what it produces still doesn’t represent the original code:

public static bool IsNull(object value)
{
	return value != null || value == DBNull.Value || value.ToString().Length == 0;
}

It’s amazing what a difference a single character can make but here it’s everything.

The other method I tested was a lot more complex and JustDecompile still fails miserably decompiling it. Take this case for example:

Please forgive the GlobalConstants class and its poorly named member. This is old code that traces its roots back to VB5!
case ConversionPeriod.Weekly:
	if (frequency != 52)
		cvt = (value * frequency) * GlobalConstants.QY;
	break;

This is what JustDecompile gives us for that block:

case ConversionPeriod.Weekly:
{
	CS$4$0001 = frequency == 52;
	if (!CS$4$0001)
	{
		cvt = value * (double)frequency * GlobalConstants.QY;
	}
	cvt = cvt / GlobalConstants.WEEKSPERMONTH;
	cvt = cvt * (double)frequency / (double)fromPeriod;
	cvt = cvt * GlobalConstants.QY;
}

Aside from the undefined MSIL variable it starts off OK but where did those three cvt = cvt... lines come from? They’re actually code from two other cases in the same switch statement.

On a positive note JustDecompile didn’t honor the underlying int value of the ConversionPeriod enumeration in my last test.  In this version each of the custom values was included in the generated code.

The Bottom Line

It’s important to remember that JustDecompile is still a beta product and I want to like it – I really do.  The team has made nice progress toward making it usable particularly in the areas of user interface, enumeration handling, and switch statement processing.  Unfortunately I still encountered too many serious problems to consider it a viable alternative to the other decompilation solutions.

Advertisements

7 comments

  1. Hi Dave,

    Many thanks for the thorough feedback and the time spent to reevaluate JustDecompile. We much appreciate such reviews. It’s nice to see that we are moving in the right direction clearing several of the initial issues you’d reported. We are aware of the rest and this is part of what really makes us keep the Beta in JustDecompile’s title. We’ll make our best to move out of Beta till the end of the year and doing so fixing the issues you mention. Hope you will be willing to make another review when this happens. Meanwhile we continue to work and evolve the product by introducing improvements to the decompilation almost every week. So some of the reported issues will be tackled earlier.

    Thanks again and hope you will continue with the useful feedback,
    Vladi

    1. Thanks for the note – I’ll be glad to do another article when its ready. I have every confidence that your team can pull it off. Don’t worry, I still love Telerik!

  2. Hi Dave,

    I’d like to ask you to try the latest JustDecompile release, which is our official version (no more Beta :)) with fully rebuilt decompilation engine, and let us know what you think. We’ll greatly appreciate if you are willing to make new review of the tool.

    Kind regards,
    Vladi
    The Telerik team

    1. I saw a note about its release on twitter and have already downloaded it. I haven’t gotten very far with it but I already have a new post in progress. I’ll let you know when it’s up.

Comments are closed.