enigma icon indicating copy to clipboard operation
enigma copied to clipboard

[feature request] Local variable remapping

Open sschr15 opened this issue 2 years ago • 6 comments

We here at @DuvetMC have no need to worry about trying to keep up-to-date with mappings since we're mapping all the old versions instead. As such, the extra work to ensure local variables are maintained is more of a priority because it can allow us to better describe what the code is doing in a way that could help a lot more given how inconsistently Notch was between the various integer types in literally anything

sschr15 avatar Feb 06 '23 01:02 sschr15

  • [ ] Implement --(no-)edit-locals options
  • [ ] Don't allow javadoc for local variables
  • [ ] Show the type of a local variable on its identifier panel
  • [ ] Implement proper local variable validation
    • In the simplest implementation, local variables would be dropped when the jar file is changed
      • With a more complex implementation, they should only be dropped if their containing method was changed
    • Ideally, local variables would only be dropped if their types don't match
    • [ ] Invalidate out-of-bounds local variables
      • [ ] Invalidate out-of-bounds parameters using the method descriptor
      • [ ] Invalidate out-of-bounds locals using Code#max_locals (JVMS§4.7.3)
  • [ ] Update the mapping format to support local variables (does it work as-is?)
    • Should we save them as the current ARG <index> <name> or should we add a LOCAL <index> <name> (<descriptor>)? entry type?
      • I'm not really sure, but I think the same variable can have different types in different frames
    • Maybe we need to keep something like an LVT to validate names and types
  • [ ] Ensure remappers work with local variable renaming
  • [ ] Decompiler support for local variable tokens

IotaBread avatar Feb 07 '24 00:02 IotaBread

Invalidate out-of-bounds local variables

I'd say that in the case of OOB and type mismatches simultaneously occurring, try to find a match keeping the order the same but dropping some entries in the middle, but I'm not sure how computationally challenging that is.

Update the mapping format to support local variables

My suggested format (for both params and local vars) is either LVT <index> <name> <descriptor> <start> <end>, essentially matching that of an LVT entry, or prohibiting duplicate local var entries and moving entries around as necessary to reuse the existing format

Decompiler support for local variable tokens

In the case of Vineflower, this is already supported via TextTokenVisitor.visitLocal

sschr15 avatar Feb 07 '24 01:02 sschr15

I'd say that in the case of OOB and type mismatches simultaneously occurring...

I think that's a bit out of scope for enigma, and more of something to be implemented in a matcher-like tool

My suggested format (...) is LVT <index> <name> <descriptor> <start> <end>

That looks good, though it becomes a lot more dependent on the method bytecode and harder to compute, since, from what I remember, if you don't have the original LVT, you have to look at all the frames in the code to find the scope of each variable

or prohibiting duplicate local var entries and moving entries around as necessary to reuse the existing format

I think reusing the existing format is prone to cause us issues... although now that I think about it, if two variables have the same index, (at least in java) aren't they expected to have the same name? If so, it'd make stuff a lot easier

Also, in regards to the format: @OroArmor, does your format have support for local variables?

In the case of Vineflower, this is already supported

I know, I wrote that code :P. I'm more worried about whether procyon and cfr have such a feature and if it's implemented. I know procyon provides an AST that we can index, so it's likely to be easy to implement (or already be done), but I don't know about cfr

IotaBread avatar Feb 07 '24 01:02 IotaBread

The lvt table extends from the argument index right? If so then I do.

OroArmor avatar Feb 07 '24 02:02 OroArmor

if two variables have the same index, aren't they expected to have the same name?

javac (and probably Eclipse's JDT) does not allow name shadowing. If in separate levels, Kotlin does allow name shadowing, but each is given its own local variable.

However, there is no requirement on specific slots being used for specific items. On a method being run, slot 0 starts as this if needed, with parameters taking the following slots as needed, but the JVM's only requirement is that no variable is stored as one type and loaded as a different type, and that frames and such declare the stack / variables as the state of the JVM must be when it encounters the equivalent of a frame declaration.

In certain old versions of Minecraft, the obfuscation used repeats local var usage and has some amount of variable shuffling to the point where such an assumption leads to instant issues for many decompilers.

sschr15 avatar Feb 07 '24 02:02 sschr15