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

ICodeAnnotation positions sometimes don't take code comments into account #2141

Closed
NebelNidas opened this issue Apr 7, 2024 · 4 comments
Closed
Assignees
Labels
bug Core Issues in jadx-core module jadx-lib

Comments

@NebelNidas
Copy link
Contributor

NebelNidas commented Apr 7, 2024

Running javaClass.getCodeInfo().getCodeMetadata().searchDown for the following decompilation result:

package net.minecraft;

/* loaded from: net/minecraft/class_6567 */
public class class_6567 {
    public static final double field_34584 = 0.0d;
    public static final double field_34585 = 64.0d;
    public static final double field_34586 = -64.0d;
}
results in the token positions 81, 125, 176, 228 (expand to see highlights generated by Enigma).

image


Then I used this code to add comments to classes/fields/methods:

token positions correctly offset by 25 (char count of // + comment + LF): 106, 150, 201, 253

image

The janky approach above was necessary because JADX 1.4 didn't offer better APIs. After having upgraded to the latest master build, I wanted to make this cleaner, so I switched to using a plugin instead (more or less a 1:1 copy of the rename-mappings plugin, reduced to just the code-comment providing code). Surprisingly however, this results in JADX reporting misplaced token positions, they don't seem to take into account the additional offset caused by the comments:

token positions unchanged at 81, 125, 176, 228

image

@NebelNidas NebelNidas added bug Core Issues in jadx-core module labels Apr 7, 2024
@NebelNidas NebelNidas changed the title ICodeAnnotation positions in CodeInfo sometimes don't adjust to code comments ICodeAnnotation positions sometimes don't take code comments into account Apr 7, 2024
@skylot
Copy link
Owner

skylot commented Apr 7, 2024

@NebelNidas looks like something not updated or cached.
Can you point me to a branch with last approach, so I can debug this issue and maybe provide fixes?

@skylot skylot self-assigned this Apr 7, 2024
@skylot skylot added the jadx-lib label Apr 7, 2024
@NebelNidas
Copy link
Contributor Author

NebelNidas commented Apr 7, 2024

Sure thing! The code is here: FabricMC/Enigma#522

@skylot
Copy link
Owner

skylot commented Apr 7, 2024

I found the issue.
In short: prefer to run decompilation only once, save ICodeInfo and use it later

			...
			JavaClass cls = jadx.getClasses().get(0);

			AtomicReference<ICodeInfo> codeInfoRef = new AtomicReference<>();
			runWithFixedLineSeparator(() -> {
				ICodeInfo codeInfo = cls.getCodeInfo(); // decompilation done here
				codeInfoRef.set(codeInfo);
				index = new SourceIndex(codeInfo.getCodeStr());
			});

			// Tokens
			ICodeInfo codeInfo = codeInfoRef.get();
			codeInfo.getCodeMetadata().searchDown(0, (pos, ann) -> {
				processAnnotatedElement(pos, ann, codeInfo);
				return null;
			});

I use AtomicReference to save variable inside lambda, you can use any other method if you want to.

Long version:
Because caching is disabled, decompilation executed with every cls.getCodeInfo() call.
But after decompilation, all data in ClassNode is unloaded (resetted), so next decompilation did not contain added comment, and this lead to new (and wrong) positions.
If you want to add comment for every decompilation it is possible to make a decompile pass instead prepare and add comment in class node visit method.

@NebelNidas
Copy link
Contributor Author

Thanks for the fix, seems to work fine!

If you want to add comment for every decompilation

I think caching the result is the best solution for this specific situation, decompiling every time would be a waste of resources :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Core Issues in jadx-core module jadx-lib
Projects
None yet
Development

No branches or pull requests

2 participants