[class-parse] Local variable mismatch issue

[jpobst]

Context: dotnet/android#5580

When running class-parse on the attached .jar file (compiled in Kotlin), it is unable to parse the HttpRequest$$serializer type.

class-parse: method com/vmware/chameleon/http/HttpRequest$$serializer.patch(Lkotlinx/serialization/Decoder;
Lcom/vmware/chameleon/http/HttpRequest;)Lcom/vmware/chameleon/http/HttpRequest;: Local variables array 
has 3 entries ('LocalVariableTableAttribute(LocalVariableTableEntry(Name='this', Descriptor='Lkotlinx/serialization/
KSerializer;', StartPC=0, Index=0), LocalVariableTableEntry(Name='decoder', Descriptor='Lkotlinx/serialization/
Decoder;', StartPC=0, Index=1), LocalVariableTableEntry(Name='old', Descriptor='Lcom/vmware/chameleon/
http/HttpRequest;', StartPC=0, Index=2))'); descriptor has 2 entries!

class-parse: method com/vmware/chameleon/http/HttpRequest$$serializer.patch(Lkotlinx/serialization/Decoder;
Lcom/vmware/chameleon/http/HttpRequest;)Lcom/vmware/chameleon/http/HttpRequest;: Local variable type 
descriptor mismatch! Got 'Lkotlinx/serialization/Decoder;'; expected 'Lkotlinx/serialization/KSerializer;'.

class-parse: method com/vmware/chameleon/http/HttpRequest$$serializer.patch(Lkotlinx/serialization/Decoder;
Lcom/vmware/chameleon/http/HttpRequest;)Lcom/vmware/chameleon/http/HttpRequest;: Local variable type 
descriptor mismatch! Got 'Lcom/vmware/chameleon/http/HttpRequest;'; expected 'Lkotlinx/serialization/Decoder;'.

classes.zip (Renamed from classes.jar.)

[jonpryor]

As discussed here, these messages are not responsible for the HttpRequest$$serializer type not being bound. Instead, that type implements an unresolvable interface, and thus we hit issues #359 and #371, so our binding process skips the type entirely.

Then we hit (new!) issue #789, wherein "bringing it back" is annoying, along with "issue #789 part b", wherein types with .. in the name are seemingly implicitly skipped (which still makes no sense to me).

These class-parse messages are debug messages:

https://github.com/xamarin/java.interop/blob/52082e20098eb87305c3e425cef412e8b1e65b7f/src/Xamarin.Android.Tools.Bytecode/Methods.cs#L107-L123

They will not prevent types or methods from being bound.

(Which is not to say I won't fix it; I will. See also 4b266fa, which is the same-but-different.)

[jpobst]

Doh! I didn't realize they were just debug messages and didn't actually prevent stuff from getting emitted. I would have dug deeper if I had noticed that they were present in the api.xml.class-parse. 😦

[jonpryor]

I don't think it's worth fixing this issue, as I think it's actually a Kotlin compiler "bug".

But first! The patch method which is causing the warning messages was deprecated and later removed from kotlinx: Kotlin/kotlinx.serialization@ef53d75

Consequently, I suspect that when developers use an updated kotlinx library, these class-parse messages won't be emitted any more.

What's the cause of the bug? Consider classes.zip, and our old favorite javap:

% javap -cp . -l -private -c -s -constants com/vmware/chameleon/http/HttpRequest\$\$serializer.class
…
  public com.vmware.chameleon.http.HttpRequest patch(kotlinx.serialization.Decoder, com.vmware.chameleon.http.HttpRequest);
    descriptor: (Lkotlinx/serialization/Decoder;Lcom/vmware/chameleon/http/HttpRequest;)Lcom/vmware/chameleon/http/HttpRequest;
    Code:
      …
    LineNumberTable:
      line 7: 12
    LocalVariableTable:
      Start  Length  Slot  Name   Signature
          0      22     0  this   Lkotlinx/serialization/KSerializer;
          0      22     1 decoder   Lkotlinx/serialization/Decoder;
          0      22     2   old   Lcom/vmware/chameleon/http/HttpRequest;
…

This method is weird, which is why class-parse is complaining about it.

First, the JNI method signature:

(Lkotlinx/serialization/Decoder;Lcom/vmware/chameleon/http/HttpRequest;)Lcom/vmware/chameleon/http/HttpRequest;

The method signature contains the parameters and return type; parameters are within the ( and ). There are thus two parameters, of type:

• kotlinx.serialization.Decoder
• com.vmware.chameleon.http.HttpRequest

So far, so good.

Next, this patch method also contains a local variable table:

    LocalVariableTable:
      Start  Length  Slot  Name   Signature
          0      22     0  this   Lkotlinx/serialization/KSerializer;
          0      22     1 decoder   Lkotlinx/serialization/Decoder;
          0      22     2   old   Lcom/vmware/chameleon/http/HttpRequest;

Local variables with a Start of 0 are parameters. This two-parameter method contains three parameters. Possibly weird, but hold that thought.

class-parse uses the LocalVariableTable in order to determine method parameter names, if/when javac -g is used: https://github.com/xamarin/java.interop/blob/52082e20098eb87305c3e425cef412e8b1e65b7f/src/Xamarin.Android.Tools.Bytecode/Methods.cs#L90-L125

Note line 91: local parameters are all entries in the LocalVariableTable with StartPC == 0.

Then class-parse skips the first parameter when:

1. names.Count != parameters.Length; parameters matches the method descriptor/JNI signature. If these counts don't match, the expectation is that there are additional local variables in addition to the parameters.
2. The method is an instance method
3. There is at least one name. (names.Count could be 0, if there is no LocalVariableTable or it's empty.)
4. The descriptor of the first name matches that of the declaring class.

Why does it do this? Because the JavaType default constructor hits this codepath as well: https://github.com/xamarin/java.interop/blob/52082e20098eb87305c3e425cef412e8b1e65b7f/tests/Xamarin.Android.Tools.Bytecode-Tests/java/com/xamarin/JavaType.java#L137-L138

% javap -l -private -c -s -constants  -cp . tests/Xamarin.Android.Tools.Bytecode-Tests/obj/Debug/classes/com/xamarin/JavaType.class
  public com.xamarin.JavaType();
    descriptor: ()V
    Code:
      …
    LocalVariableTable:
      Start  Length  Slot  Name   Signature
          0      21     0  this   Lcom/xamarin/JavaType;

No parameters -- it's a default constructor -- yet it has an entry for this in the LocalVariableTable.

Thus, there is a plausible expectation that the LocalVariableTable will have more entries than the method descriptor.

We now return to the patch method:

    LocalVariableTable:
      Start  Length  Slot  Name   Signature
          0      22     0  this   Lkotlinx/serialization/KSerializer;
          0      22     1 decoder   Lkotlinx/serialization/Decoder;
          0      22     2   old   Lcom/vmware/chameleon/http/HttpRequest;

1. names.Count != parameters.Length: 3 != 2; Check.
2. The method is an instance method? Check.
3. There is at least one name? Check -- there are three.
4. The descriptor of the first name matches that of the declaring class?

(4) is where things fail: the first parameter -- the one we're willing to skip over -- has a type of kotlinx.serialization.KSerializer, while the declaring class is com.vmware.chameleon.http.HttpRequest$$serializer. Because the types don't match as expected, we don't skip the first parameter, which causes us to hit the debug message on line 107.

Again, though, this is really weird; we basically have:

class `$serializer` /* : kotlinx.serialization.internal.GeneratedSerializer<com.vmware.chameleon.http.HttpRequest> */ {
    fun patch(decoder: Decoder, old: HttpRequest): HttpRequest {
        …
    }
}

except the this variable, instead of being of type $serializer -- the declaring type! -- is instead of type KSerializer.

In fact, I cannot figure out a way to create this "mismatch" using "normal" Kotlin code. The only way I've found to get this "mismatch", wherein the compile-time/run-time type must be of the declaring type, while the LocalVariableTable uses some other type, was to:

1. Use an older version of kotlinx which still contains these patch methods, and
2. Use the @Serializable annotation:

https://github.com/Kotlin/kotlinx.serialization/blob/f7f1bccdd03f4612d372385aee9c2f3a9147c7fc/integration-test/src/commonTest/kotlin/sample/MultiFileHierarchyModuleB.kt#L6-L7

The nested $serializable type and patch method and everything else are all "compiler magic" to implement the @Serializable annotation, which is why I call it a compiler "bug". With current kotlinx and the removal of the patch method, no samples within the kotlinx.serialization repo cause class-parse to emit the …descriptor has… debug message.

[jonpryor]

The only other "fix" I can think of would be to actually check the values within names, and e.g. skip the first entry if the first entry is this. While plausible, such a heuristic is not without drawbacks, e.g. other languages which compile to Java bytecode using this as a "normal" parameter name. I'd prefer to avoid that.

[jonpryor]

Scratch.Gji788.zip