Add Line Numbers to Dex Dump #19
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
romainguy
requested changes
May 1, 2024
There was a problem hiding this comment.
Thanks, I left a couple of comments. A few more things:
- There are now 2 empty lines between methods in the output instead of just 1. I remember I had to do something to deal with this in the original implementation as well.
- There seems to be a bug with naming classes. For instance I got this in a dump:
class IntOffset..ExternalSyntheticBackport0(note the two dots) - Could you add a menu to turn debug line numbers on/off? It would make it more obvious to users what those numbers are, but more importantly it would make copy/pasting out of the tool easier when line numbers are disabled.
- Your change breaks another feature: when clicking on a line with a jump (goto/if/etc.) in the DEX output, the tool will draw a line to show where the jump will go. If the target line has a debug line number in the output, the feature stops working (prob. because the regex I use assumes whitespace at the beginning; one of the reasons why I wanted to change the parsers to output a list of class/methods/instructions)
build.gradle.kts
Outdated
| implementation("org.jetbrains.compose.components:components-splitpane-desktop:${extra["compose.version"] as String}") | ||
| implementation("com.fifesoft:rsyntaxtextarea:${extra["rsyntaxtextarea.version"] as String}") | ||
| implementation("com.fifesoft:rstaui:${extra["rstaui.version"] as String}") | ||
| implementation("net.java.dev.jna:jna:${extra["jna.version"] as String}") | ||
| implementation("androidx.collection:collection:${extra["collections.version"] as String}") | ||
| implementation("org.jetbrains.androidx.lifecycle:lifecycle-runtime:${extra["lifecycle.version"] as String}") | ||
| runtimeOnly("org.jetbrains.skiko:skiko-awt-runtime-linux-x64:${extra["skiko.version"] as String}") | ||
| runtimeOnly("org.jetbrains.skiko:skiko-awt-runtime-macos-arm64:${extra["skiko.version"] as String}") |
There was a problem hiding this comment.
This breaks on macOS, the native library can't be found if it's not implementation() :/
|
|
||
| private val PositionRegex = Regex("^\\s*0x(?<address>[0-9a-f]+) line=(?<line>\\d+)$") | ||
|
|
||
| private const val CLASS_START = "Class #" |
There was a problem hiding this comment.
I've been following the SnakeCase style for constants here.
| } | ||
| } | ||
| return classes | ||
| .filter { it.methods.isNotEmpty() } |
There was a problem hiding this comment.
You can combine those two filters in a single one
| val code = buildString { | ||
| instructions.forEach { | ||
| val lineNumber = positions[it.address] | ||
| val prefix = when (lineNumber != null) { |
There was a problem hiding this comment.
Use an if/else here instead of when/true/false
There was a problem hiding this comment.
Done, though as an aside, I really like using when-true-false in these cases because is more readable. At least IMO.
alonalbert
force-pushed
the
class-parser
branch
2 times, most recently
from
0d0343f to
7d0370d
Compare
|
Thanks for the updates! One last issue, there are still 2 empty lines between methods in the dump instead of 1. |
alonalbert
force-pushed
the
class-parser
branch
2 times, most recently
from
2c556be to
4f9ec31
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I rewrote the DexDump parsing into a dedicated class.
I changed the implementation to be (IMO) less brittle and more readable.