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

test: difference code between Eclipse and Gradle #536

Closed
asashour opened this issue Mar 29, 2019 · 7 comments
Closed

test: difference code between Eclipse and Gradle #536

asashour opened this issue Mar 29, 2019 · 7 comments

Comments

@asashour
Copy link
Contributor

asashour commented Mar 29, 2019

This is the second time, I notice there is a difference in a test result between Eclipse and Gradle on Windows.

The first one was in #522, and I ended up adding noDebugInfo() for all relevant tests, because somehow the debug info contained boolean types however in obfuscated code, it is int.

The purpose of this issue, is not to particularly fix a specific test, but rather to know why Eclipse tests differ from Gradle.

I know Eclipse has an internal compiler, but I think we use the system java compiler, which is javac (through ToolProvider.getSystemJavaCompiler()).

Test case which passes in Gradle, but fails in Eclipse

	public static class TestCls {

		public byte[] test(int a) {
			return new byte[]{0, 1, 2};
		}
	}

	@Test
	public void test() {
		ClassNode cls = getClassNode(TestCls.class);
		String code = cls.getCode().toString();

		assertThat(code, containsString("return new byte[]"));
	}

The generated code in Eclipse is:

public class TestArrayFill3$TestCls {
    public byte[] test(int a) {
        byte[] bArr = new byte[3];
        bArr[1] = (byte) 1;
        bArr[2] = (byte) 2;
        return bArr;
    }
}

@skylot
Copy link
Owner

skylot commented Mar 29, 2019

@asashour looks like I found the cause of this issue. Here method getClassFilesWithInners load class using classloader, without compiling it, I think I do this for speed, so we don't need compile already compiled class.
After forcing compile of all classes there is some speed degradation like +3 seconds for all core test (which is very small and acceptable), but also 2 tests failed. I will try to fix them.

And yes, I know that Eclipse compiler in some cases generate very different bytecode, especially for syntax sugar like enums (see post in CFR decompiler blog).

@asashour
Copy link
Contributor Author

Thanks for the hint.

Another suggestion, is that we actually add other compilers, so we support different bytecode flavors.

Obviously this would double or triple the build time, but I guess it is worth it.

@skylot
Copy link
Owner

skylot commented Mar 29, 2019

I think the difference is very small and running all tests again not worth it, just adding some smali tests is enough. Also, there are tons of other tools for change bytecode like obfuscators, optimizers, packers, etc and we can't run tests again for all of them and in my opinion jadx is general enough to handle them good.

@asashour
Copy link
Contributor Author

Thanks, test passes now in Eclipse.

@skylot
Copy link
Owner

skylot commented Mar 29, 2019

I add test option to allow write test with Eclipse compiler. Maybe it will be useful.
But in your case Eclipse is very smart here: if array contains zeros Eclipse instead FILL_ARRAY make one by one element put excluding zeros.
For this code:

new byte[]{1, 0, 2, 0, 0, 0, 0, 0, 0, 3}

Eclipse compiler generate:

        byte[] bArr = new byte[10];
        bArr[0] = 1;
        bArr[2] = 2;
        bArr[9] = 3;

I like this 'zeros elimination'. Jadx can revert it, but only if exists all sequential puts without skips, and I think for this case I will not fix that because it looks good. And also it is hard to determine how many skips are allowed. Like

        byte[] arr = new byte[10];
        arr[9] = 1;

must be converted to new byte[]{0, 0, 0, 0, 0, 0, 0, 0, 0, 1} ?

@asashour
Copy link
Contributor Author

I have thought a little about it, but thought it doesn't worth the effort, since byte array starting with 0 wouldn't be a common case, and in both ways, the code is correct. After major issues are fixes, we can think about those enhancements :)

What was annoying is that eclipse tests gives different results, thanks for fixing it :)

@asashour
Copy link
Contributor Author

I see the Eclipse compiler option, very nice.

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

No branches or pull requests

2 participants