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

use MethodParameter attribute if code attribute is missing #19

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

char3210
Copy link

@char3210 char3210 commented May 3, 2023

this applies parameter names for abstract methods and interface methods

for (long i = 0; i < count; i++) {
int nameIndex = raw.getU2At(offset);
int accessFlags = raw.getU2At(offset + 2);
parameters.add(new MethodParameterEntry(cp.getEntry(nameIndex), accessFlags));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nameIndex may be 0, indicating null. getEntry does not accept 0 arguments:

if (index == 0) throw new ConfusedCFRException("Attempt to fetch element 0 from constant pool");


@Override
public boolean isGoodName() {
return true;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, no good if it's 0-name; it's totally legal to have null name in MethodParameters (storing parameter flags only) but valid name in LocalVariableTable/LocalVariableTypeTable

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure what exactly isGoodName is supposed to represent, but I don't see a very easy way to fix this currently since this doesn't know which parameter it is until dumpParameter is called...


@Override
public Dumper dumpParameter(Dumper d, MethodPrototype prototype, int index, boolean defines) {
return d.parameterName(forced ? name : parameters.get(index).name.getValue(), this, prototype, index, defines);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same, will NPE on .getValue

@liach
Copy link

liach commented May 3, 2023

On a side note, I don't know why cfr's author doesn't just implement the length calculation and attribute name reading in the base Attribute class. This brings visible confusion when reviewing the attribute changes, and requires extra overrides and fields for shared content.

@liach
Copy link

liach commented May 3, 2023

Also as a side note: javac actually does NOT generate MethodParameters attributes by default! So you can hope Mojang actually added -parameters flag when compiling their binaries, which I don't think is the case.

In Java 20:

public interface Apple {
	int my(int argumentName);
}

compiles with javac Apple.java to javap output:

Classfile /C:/code/test/Apple.class
  Last modified May 3, 2023; size 111 bytes
  SHA-256 checksum c0538a6bdbb501ff27f98571a00bfccf855d25aca7d63ec9a446be8ebb783451
  Compiled from "Apple.java"
public interface Apple
  minor version: 0
  major version: 64
  flags: (0x0601) ACC_PUBLIC, ACC_INTERFACE, ACC_ABSTRACT
  this_class: #1                          // Apple
  super_class: #3                         // java/lang/Object
  interfaces: 0, fields: 0, methods: 1, attributes: 1
Constant pool:
  #1 = Class              #2              // Apple
  #2 = Utf8               Apple
  #3 = Class              #4              // java/lang/Object
  #4 = Utf8               java/lang/Object
  #5 = Utf8               my
  #6 = Utf8               (I)I
  #7 = Utf8               SourceFile
  #8 = Utf8               Apple.java
{
  public abstract int my(int);
    descriptor: (I)I
    flags: (0x0401) ACC_PUBLIC, ACC_ABSTRACT
}
SourceFile: "Apple.java"

No MethodParameters is available.

@char3210
Copy link
Author

char3210 commented May 3, 2023

yeah that's expected, but this is mostly for decompiling remapped jars, and the remapping process (intermediary -> named) does emit the MethodParameters attributes if parameters are present, at least in loom

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

Successfully merging this pull request may close these issues.

3 participants