Assert matching method parameters in class-parse #1090

Activity

1. jpobst

   added

   bugComponent does not function as intendedComponent does not function as intended

   generatorIssues binding a Java library (generator, class-parse, etc.)Issues binding a Java library (generator, class-parse, etc.)

   on Mar 30, 2023
2. jonpryor commented on Mar 30, 2023

   

   jonpryor

   on Mar 30, 2023

   Contributor

   More actions

   Oddly, commit 8ccb837 is in the release/6.0.3xx branch, but was reverted in the release/6.0.1xx-preview10 branch.

   @jonathanpeppers: do you remember the background for the revert in 6.0.1xx-preview10? 2229ccc

   xamarin-android on release/6.0.4xx is using commit 2a882d2d, which does contain 8ccb837 and does not contain the revert in 2229ccc, so as far as I can tell .NET 6 currently contains 8ccb837.
3. jonathanpeppers commented on Mar 30, 2023

   

   jonathanpeppers

   on Mar 30, 2023

   Member

   More actions

   I'm not sure why I didn't put that information here: dotnet/android#6426

   My memory is there was some error caused by it, and we just wanted to revert for this one release.

   But then I don't see a java.interop commit that "fixes" something caused by 8ccb837.
4. jonpryor commented on Mar 30, 2023

   

   jonpryor

   on Mar 30, 2023

   Contributor

   More actions

   "Repro steps":

   curl -o tensorflow-android-1.13.1.aar https://repo1.maven.org/maven2/org/tensorflow/tensorflow-android/1.13.1/tensorflow-android-1.13.1.aar
   unzip tensorflow-android-1.13.1.aar classes.jar
   dotnet path/to/class-parse.dll classes.jar
   # boom

   The assert is triggered when processing the constructor for org/tensorflow/Session$Runner$Reference. class-parse.exe --dump shows:

   …
   ThisClass: Utf8("org/tensorflow/Session$Runner$Reference")
   …
   Methods Count: 2
   	0: <init> (Lorg/tensorflow/Session$Runner;)V Public
   		Code(60, Unknown[LineNumberTable](30), LocalVariableTableAttribute(LocalVariableTableEntry(Name='this', Descriptor='Lorg/tensorflow/Session$Runner$Reference;', StartPC=0, Index=0)), Xamarin.Android.Tools.Bytecode.StackMapTableAttribute)
   		MethodParametersAttribute(MethodParameterInfo(Name='this$1', AccessFlags=Final, Synthetic))
   

   which is to say that the only constructor accepts 1 parameter, but that parameter is named this. What is (probably) happening is Session.Runner.Reference is a non-static inner class of Session.Runner, and as such its constructor requires a reference of its enclosing type.

   % javap -cp . 'org/tensorflow/Session$Runner$Reference.class'
   Compiled from "Session.java"
   class org.tensorflow.Session$Runner$Reference implements java.lang.AutoCloseable {
     final org.tensorflow.Session$Runner this$1;
     public org.tensorflow.Session$Runner$Reference(org.tensorflow.Session$Runner);
     public void close();
   }
   
   % dotnet path/to/class-parse.exe --dump 'org/tensorflow/Session$Runner.class'
   .class version: 52.0
   …
   Attributes Count: 2
           0: SourceFile('Session.java')
           1: InnerClasses(Count=3,
             InnerClass(InnerClass='org/tensorflow/Session$Runner', OuterClass='org/tensorflow/Session', InnerName='Runner', InnerClassAccessFlags=Public, Final),
             InnerClass(InnerClass='org/tensorflow/Session$Runner$Reference', OuterClass='org/tensorflow/Session$Runner', InnerName='Reference', InnerClassAccessFlags=Private),
             InnerClass(InnerClass='org/tensorflow/Session$Run', OuterClass='org/tensorflow/Session', InnerName='Run', InnerClassAccessFlags=Public, Static, Final))
   …
   

   Aside: .class version 52.0 is JDK 1.8.

   So we look at this block of code: https://github.com/xamarin/java.interop/blob/909239d4b70e5367e2d47e6ce60a2b399830324a/src/Xamarin.Android.Tools.Bytecode/Methods.cs#L297-L307

   Turns Out™ that for the "offending" method, pinfo[0].AccessFlags is Final, Synthetic (0x1010), while what we were looking for -- in the form of OuterThis -- was Final, Mandated (0x8010). The actual value is what we expected for a "outer this reference", so we didn't skip it, which caused the assert to fire.
5. jonpryor commented on Mar 30, 2023

   

   jonpryor

   on Mar 30, 2023

   Contributor

   More actions

   I don't know why tensorflow is using Final, Synthetic instead of Final, Mandated. I suspect a compiler bug of some form?
6. jpobst commented on Mar 30, 2023

   

   jpobst

   on Mar 30, 2023

   ContributorAuthor

   More actions

   Another example is https://mvnrepository.com/artifact/com.google.crypto.tink/tink-android/1.8.0.

   However, this appears to be the same issue, as it is also Final, Synthetic.
7. jonpryor

   added a commit that references this issue on Mar 30, 2023

   [Xamarin.Android.Tools.Bytecode] Ignore synthetic ctor params

   ...

   0d5cddb
8. jonpryor

   closed this as completedin #1091on Mar 30, 2023
9. jonpryor

   added 2 commits that reference this issue on Mar 30, 2023

   [Xamarin.Android.Tools.Bytecode] Ignore synthetic ctor params (#1091)

   ...

   0355acf

   Bump to xamarin/Java.Interop/main@0355acf (#7931)

   ...

   9766e87
10. github-actions

    locked and limited conversation to collaborators on Apr 12, 2024
11. pull

    added a commit that references this issue on Jul 1, 2026

    [Xamarin.Android.Tools.Bytecode] Ignore synthetic ctor params (#1091)

    ...

    ab09ae0