Skip to content

Commit

Permalink
[Xamarin.Android.Tools.Bytecode] Ignore synthetic ctor params (#1091)
Browse files Browse the repository at this point in the history
Fixes: #1090

Attempting to parse [TensorFlow 1.13.1][0] with a Debug build of
`class-parse` results in an assertion failure:

	% 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 bin/Debug-net7.0/class-parse.dll classes.jar
	Unexpected number of method parameters; expected 0, got 1
	   at Xamarin.Android.Tools.Bytecode.MethodInfo.UpdateParametersFromMethodParametersAttribute(ParameterInfo[] parameters)
	   at Xamarin.Android.Tools.Bytecode.MethodInfo.GetParameters()
	   at Xamarin.Android.Tools.Bytecode.XmlClassDeclarationBuilder.GetMethodParameters(MethodInfo method)+MoveNext()
	   at System.Xml.Linq.XContainer.AddContentSkipNotify(Object content)
	   at System.Xml.Linq.XContainer.AddContentSkipNotify(Object content)
	   at Xamarin.Android.Tools.Bytecode.XmlClassDeclarationBuilder.GetMethod(String element, String name, MethodInfo method, String returns)
	   at Xamarin.Android.Tools.Bytecode.XmlClassDeclarationBuilder.<GetConstructors>b__23_2(MethodInfo c)

Note: this will only happen for Debug builds of `class-parse`, which
we do not ship.  Thus, this will only happen if you are using a local
build of xamarin/Java.Interop.

The assertion occurs when processing the constructor for
`org/tensorflow/Session$Runner$Reference.class`, which is a non-static
inner class:

	% unzip classes.jar 'org/tensorflow/Session$Runner$Reference.class'
	% dotnet bin/Debug-net7.0/class-parse.dll --dump 'org/tensorflow/Session$Runner$Reference.class'
	…
	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))

Note the `MethodParameterInfo.AccessFlags` value for the parameter:
`AccessFlags=Final, Synthetic`.  This differs from the
`Final, Mandated` value that is checked for by
`MethodInfo.UpdateParametersFromMethodParametersAttribute()`:

	var OuterThis = MethodParameterAccessFlags.Final | MethodParameterAccessFlags.Mandated;
	// …
	while (startIndex < pinfo.Count &&
	    (pinfo [startIndex].AccessFlags & OuterThis) == OuterThis) {
	  startIndex++;
	}

Because the parameter is `Final, Synthetic` and not `Final, Mandated`,
the parameter is *not* skipped, which results in the assertion message
`expected 0, got 1`.

@jonpryor does not know how or why `Final, Synthetic` is being used
by `Session$Runner$Reference.class`, and is unable to reproduce this
particular Java bytecode output with the JDK 1.8 and JDK 11 compilers.

Update `MethodInfo.UpdateParametersFromMethodParametersAttribute()`
to also skip `Final, Synthetic` parameters.  This allows
`Session$Runner$Reference.class` to be processed without assertions:

	% dotnet bin/Debug-net7.0/class-parse.dll classes.jar
	# no errors

Additionally, update the assertion message so that it includes the
declaring class name, method name, and method descriptor.

[0]: https://mvnrepository.com/artifact/org.tensorflow/tensorflow-android/1.13.1
  • Loading branch information
jonpryor authored Mar 30, 2023
1 parent 909239d commit 0355acf
Showing 1 changed file with 7 additions and 4 deletions.
11 changes: 7 additions & 4 deletions src/Xamarin.Android.Tools.Bytecode/Methods.cs
Original file line number Diff line number Diff line change
Expand Up @@ -294,17 +294,20 @@ void UpdateParametersFromMethodParametersAttribute (ParameterInfo[] parameters)
if (methodParams == null)
return;

const MethodParameterAccessFlags OuterThis =
MethodParameterAccessFlags.Mandated | MethodParameterAccessFlags.Final;
const MethodParameterAccessFlags OuterThis1 =
MethodParameterAccessFlags.Final | MethodParameterAccessFlags.Mandated;
const MethodParameterAccessFlags OuterThis2 =
MethodParameterAccessFlags.Final | MethodParameterAccessFlags.Synthetic;
var pinfo = methodParams.ParameterInfo;
int startIndex = 0;
while (startIndex < pinfo.Count &&
(pinfo [startIndex].AccessFlags & OuterThis) == OuterThis) {
((pinfo [startIndex].AccessFlags & OuterThis1) == OuterThis1 ||
(pinfo [startIndex].AccessFlags & OuterThis2) == OuterThis2)) {
startIndex++;
}
Debug.Assert (
parameters.Length == pinfo.Count - startIndex,
$"Unexpected number of method parameters; expected {parameters.Length}, got {pinfo.Count - startIndex}");
$"Unexpected number of method parameters in `{DeclaringType.FullJniName}.{Name}{Descriptor}`: expected {parameters.Length}, got {pinfo.Count - startIndex}");
int end = Math.Min (parameters.Length, pinfo.Count - startIndex);
for (int i = 0; i < end; ++i) {
var p = pinfo [i + startIndex];
Expand Down

0 comments on commit 0355acf

Please sign in to comment.