Skip to content

Commit

Permalink
[typemaps] Handle Java to managed type maps properly
Browse files Browse the repository at this point in the history
Fixes: #4596
Fixes: #4660
Context: xamarin/monodroid@192b1f1
Context: https://github.com/xamarin/java.interop/blob/fc18c54b2ccf14f444fadac0a1e7e81f837cc470/src/Java.Interop.Tools.JavaCallableWrappers/Java.Interop.Tools.JavaCallableWrappers/TypeNameMapGenerator.cs#L149-L186
Context: https://github.com/xamarin/java.interop/blob/010161d1ef6625b762429334f02e7b15e1f7caae/src/Java.Interop.Tools.JavaCallableWrappers/Java.Interop.Tools.JavaCallableWrappers/TypeNameMapGenerator.cs#L155

~~ Debug Mode ~~

In some environments, `BundleTest.TestBundleIntegerArrayList2()` fails
when  using the commercial shared runtime:

	Test 'Xamarin.Android.RuntimeTests.BundleTest.TestBundleIntegerArrayList2' failed: System.MemberAccessException : Cannot create an instance of Android.Runtime.JavaList`1[T] because Type.ContainsGenericParameters is true.
	  at System.Reflection.RuntimeConstructorInfo.DoInvoke (System.Object obj, System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture)
	  at System.Reflection.RuntimeConstructorInfo.Invoke (System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture)
	  at System.Reflection.ConstructorInfo.Invoke (System.Object[] parameters)
	  at Java.Interop.TypeManager.CreateProxy (System.Type type, System.IntPtr handle, Android.Runtime.JniHandleOwnership transfer)
	  at Java.Interop.TypeManager.CreateInstance (System.IntPtr handle, Android.Runtime.JniHandleOwnership transfer, System.Type targetType)
	  at Java.Lang.Object.GetObject (System.IntPtr handle, Android.Runtime.JniHandleOwnership transfer, System.Type type)
	  at Java.Lang.Object._GetObject[T] (System.IntPtr handle, Android.Runtime.JniHandleOwnership transfer)
	  at Java.Lang.Object.GetObject[T] (System.IntPtr handle, Android.Runtime.JniHandleOwnership transfer)
	  at Android.OS.Bundle.Get (System.String key)
	  at Xamarin.Android.RuntimeTests.BundleTest.TestBundleIntegerArrayList2 ()
	  at (wrapper managed-to-native) System.Reflection.RuntimeMethodInfo.InternalInvoke(System.Reflection.RuntimeMethodInfo,object,object[],System.Exception&)
	  at System.Reflection.RuntimeMethodInfo.Invoke (System.Object obj, System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture)

The problem appears to be rooted in ce2bc68.  In a pre-ce2bc689 world,
there were two separate sets of "typemap" files that the application
could use:

  * Java-to-managed mappings, in `typemap.jm`
  * Managed-to-Java mappings, in `typemap.mj`.

These files did *not* need to have the same number of entries!

	% unzip /Library/Frameworks/Xamarin.Android.framework/Versions/10.2.0.100/lib/xamarin.android/xbuild/Xamarin/Android/Mono.Android.DebugRuntime-debug.apk typemap.jm
	% unzip /Library/Frameworks/Xamarin.Android.framework/Versions/10.2.0.100/lib/xamarin.android/xbuild/Xamarin/Android/Mono.Android.DebugRuntime-debug.apk typemap.mj
	% strings typemap.jm | wc -l
	   10318
	% strings typemap.mj | wc -l
	   11956

The reason why they have a different number of entries is *aliasing*:
there can be *multiple* bindings for a given Java type.  The
managed-to-Java mappings can contain *all* of them; the
Java-to-managed mapping *must* pick Only One.

For example, [`java.util.ArrayList`][0] contains (at least?) *three* bindings:

  * [`Android.Runtime.JavaList`][1]
  * [`Android.Runtime.JavaList<T>`][2]
  * `Java.Util.ArrayList` (generated at build time)

In a pre-ce2bc689 world, this was "fine": we'd binary search each file
*separately* to find type mappings.

This changed in ce2bc68, as the typemap information was merged into a
*single* array in order to de-duplicate information.  This reduced
`.apk` size.

An unanticipated result of this is that the Java-to-managed mappings
can now contain *duplicate* keys, "as if" the original `typemap.jm`
had the contents:

  java/util/ArrayList
  Android.Runtime.JavaList, Mono.Android
  java/util/ArrayList
  Android.Runtime.JavaList`1, Mono.Android
  java/util/ArrayList
  Java.Util.ArrayLlist, Mono.Android

Whereas pre-ce2bc689 there would have only been the first entry.

"Sometimes" this duplication is "fine": the typemap search
"Just happens" to return the first entry, or the 3rd entry, and apps
continue to work as intended.

"Sometimes" it isn't: the typemap binary search finds the 2nd entry,
which is a generic type.  This results in attempting to instantiate a
generic type *without appropriate type parameters*, which results in
the aforementioned `MemberAccessException`.

Update typemap generation code in `Xamarin.Android.Build.Tasks` so that
all the duplicate Java type names will point to the same managed type
name.  Additionally, make sure we select the managed type in the same
fashion the old typemap generator in `Java.Interop` worked - prefer base
types to the derived ones if the type is an interface or an abstract
class.  The effect of this change is that no matter which entry
`binary_search` ends up selecting it will always return the same managed
type name for all the aliased Java types.

~~ Release mode ~~

Another issue introduced in ce2bc68 is that we no longer ignore
interface and generic types when generating the Java-to-managed maps.
This adversely affects the `Release` mode in which it is possible that
an attempt to instantiate an open generic type (or an interface) will
happen, leading to error similar to:

	FATAL EXCEPTION: main
	  Process: com.companyname.androidsingleviewapp1, PID: 24068
	  android.runtime.JavaProxyThrowable: System.MemberAccessException: Cannot create an instance of Com.Contoso.Javalibrary1.Class0`1[T] because Type.ContainsGenericParameters is true.
	    at System.Reflection.RuntimeConstructorInfo.DoInvoke (System.Object obj, System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture)
	    at System.Reflection.RuntimeConstructorInfo.Invoke (System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture)
	    at System.Reflection.ConstructorInfo.Invoke (System.Object[] parameters)
	    at Java.Interop.TypeManager.CreateProxy (System.Type type, System.IntPtr handle, Android.Runtime.JniHandleOwnership transfer)
	    at Java.Interop.TypeManager.CreateInstance (System.IntPtr handle, Android.Runtime.JniHandleOwnership transfer, System.Type targetType)
	    at Java.Lang.Object.GetObject (System.IntPtr handle, Android.Runtime.JniHandleOwnership transfer, System.Type type)
	    at Java.Lang.Object._GetObject[T] (System.IntPtr handle, Android.Runtime.JniHandleOwnership transfer)
	    at Java.Lang.Object.GetObject[T] (System.IntPtr handle, Android.Runtime.JniHandleOwnership transfer)
	    at Com.Contoso.Javalibrary1.Class1.Create ()
	    at AndroidSingleViewApp1.MainActivity.OnCreate (Android.OS.Bundle savedInstanceState)
	    at Android.App.Activity.n_OnCreate_Landroid_os_Bundle_ (System.IntPtr jnienv, System.IntPtr native__this, System.IntPtr native_savedInstanceState)
	    at (wrapper dynamic-method) Android.Runtime.DynamicMethodNameCounter.1(intptr,intptr,intptr)
	         at crc647d7dcab8da8ee782.MainActivity.n_onCreate(Native Method)
	         at crc647d7dcab8da8ee782.MainActivity.onCreate(MainActivity.java:29)
	         at android.app.Activity.performCreate(Activity.java:7144)
	         at android.app.Activity.performCreate(Activity.java:7135)
	         at android.app.Instrumentation.callActivityOnCreate(Instrumentation.java:1271)
	         at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:2931)
	         at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:3086)
	         at android.app.servertransaction.LaunchActivityItem.execute(LaunchActivityItem.java:78)
	         at android.app.servertransaction.TransactionExecutor.executeCallbacks(TransactionExecutor.java:108)
	         at android.app.servertransaction.TransactionExecutor.execute(TransactionExecutor.java:68)
	         at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1816)
	         at android.os.Handler.dispatchMessage(Handler.java:106)
	         at android.os.Looper.loop(Looper.java:193)
	         at android.app.ActivityThread.main(ActivityThread.java:6718)
	         at java.lang.reflect.Method.invoke(Native Method)
	         at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:493)
	         at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:858)

Update typemap generation code to also ignore generic types and
interfaces when generating the Java to Managed map in `Release` mode. We
must not simply skip outputting entries for such types because we need
to maintain element count parity between java-to-managed and
managed-to-java lookup tables.  Therefore, the way we skip such types is
to output a type token ID of `0` instead of the actual one.  This will
cause the runtime code to fail to find a mapping, thus allowing the
managed code to deal with such a type properly.

~~ Test Updates ~~

Update `TestBundleIntegerArrayList2()` so that it asserts
the runtime *type* of `obj` returned, asserting that it is in fact a
`JavaList` and not e.g. `Java.Util.ArrayList` or something.
(The linker could otherwise "change" things, and this assertion will
ensure that `JavaList` won't be linked away…)

Add a unit test for issue #4660 (by @brendanzagaeski)

It's very cute. :-)

We sort the Java-to-managed type name table, so it can be binary
searched to find the Managed type.

Additionally, the "preferred" managed type is the *first* type in
`monodis --typedef` output; the first type that we encounter when
going through all types in an assembly.

Brendan's cute way of provoking the bug is to add a new *generic* type
which will come *before* the "normally preferred" (and correct!) type.
Thus, even if/when binary search returns the first entry, that'll
still be the *wrong entry*, because that'll be a generic type!

Very cute.

[0]: https://developer.android.com/reference/java/util/ArrayList
[1]: https://github.com/xamarin/xamarin-android/blob/61f09bf2ef0c8f09550e2d77eb97f417432b315b/src/Mono.Android/Android.Runtime/JavaList.cs#L11-L13
[2]: https://github.com/xamarin/xamarin-android/blob/61f09bf2ef0c8f09550e2d77eb97f417432b315b/src/Mono.Android/Android.Runtime/JavaList.cs#L667-L668
  • Loading branch information
grendello committed May 7, 2020
1 parent 7dab4ee commit aabda15
Show file tree
Hide file tree
Showing 9 changed files with 116 additions and 21 deletions.
1 change: 1 addition & 0 deletions src/Mono.Android/Test/Android.OS/BundleTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ public void TestBundleIntegerArrayList()
Assert.NotNull (list, "'key' doesn't refer to a list of integers");
var obj = b.Get ("key");
Assert.NotNull (obj, "Missing 'key' in bundle");
Assert.IsTrue (obj is global::Android.Runtime.JavaList, "`obj` should be a JavaList!");
try {
list = b.GetIntegerArrayList ("key");
Assert.NotNull (list, "'key' doesn't refer to a list of integers after non-generic call");
Expand Down
6 changes: 3 additions & 3 deletions src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ void Run (DirectoryAssemblyResolver res)

// Step 2 - Generate type maps
// Type mappings need to use all the assemblies, always.
WriteTypeMappings (allJavaTypes);
WriteTypeMappings (allJavaTypes, cache);

var javaTypes = new List<TypeDefinition> ();
foreach (TypeDefinition td in allJavaTypes) {
Expand Down Expand Up @@ -399,10 +399,10 @@ void SaveResource (string resource, string filename, string destDir, Func<string
MonoAndroidHelper.CopyIfStringChanged (template, Path.Combine (destDir, filename));
}

void WriteTypeMappings (List<TypeDefinition> types)
void WriteTypeMappings (List<TypeDefinition> types, TypeDefinitionCache cache)
{
var tmg = new TypeMapGenerator ((string message) => Log.LogDebugMessage (message), SupportedAbis);
if (!tmg.Generate (Debug, SkipJniAddNativeMethodRegistrationAttributeScan, types, TypemapOutputDirectory, GenerateNativeAssembly, out ApplicationConfigTaskState appConfState))
if (!tmg.Generate (Debug, SkipJniAddNativeMethodRegistrationAttributeScan, types, cache, TypemapOutputDirectory, GenerateNativeAssembly, out ApplicationConfigTaskState appConfState))
throw new XamarinAndroidException (4308, Properties.Resources.XA4308);
GeneratedBinaryTypeMaps = tmg.GeneratedBinaryTypeMaps.ToArray ();
BuildEngine4.RegisterTaskObject (ApplicationConfigTaskState.RegisterTaskObjectKey, appConfState, RegisteredTaskObjectLifetime.Build, allowEarlyCollection: false);
Expand Down
97 changes: 82 additions & 15 deletions src/Xamarin.Android.Build.Tasks/Utilities/TypeMapGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System.Linq;
using System.Text;

using Java.Interop.Tools.Cecil;
using Mono.Cecil;

namespace Xamarin.Android.Tasks
Expand Down Expand Up @@ -52,6 +53,7 @@ internal sealed class TypeMapReleaseEntry
public uint Token;
public int AssemblyNameIndex = -1;
public int ModuleIndex = -1;
public bool SkipInJavaToManaged;
}

internal sealed class ModuleReleaseData
Expand All @@ -76,6 +78,8 @@ internal sealed class TypeMapDebugEntry
public string ManagedLabel;
public int JavaIndex;
public int ManagedIndex;
public TypeDefinition TypeDefinition;
public bool SkipInJavaToManaged;
}

// Widths include the terminating nul character but not the padding!
Expand All @@ -87,6 +91,7 @@ internal sealed class ModuleDebugData
public List<TypeMapDebugEntry> JavaToManagedMap;
public List<TypeMapDebugEntry> ManagedToJavaMap;
public string OutputFilePath;
public string ModuleName;
public byte[] ModuleNameBytes;
}

Expand Down Expand Up @@ -125,7 +130,7 @@ void UpdateApplicationConfig (TypeDefinition javaType, ApplicationConfigTaskStat
}
}

public bool Generate (bool debugBuild, bool skipJniAddNativeMethodRegistrationAttributeScan, List<TypeDefinition> javaTypes, string outputDirectory, bool generateNativeAssembly, out ApplicationConfigTaskState appConfState)
public bool Generate (bool debugBuild, bool skipJniAddNativeMethodRegistrationAttributeScan, List<TypeDefinition> javaTypes, TypeDefinitionCache cache, string outputDirectory, bool generateNativeAssembly, out ApplicationConfigTaskState appConfState)
{
if (String.IsNullOrEmpty (outputDirectory))
throw new ArgumentException ("must not be null or empty", nameof (outputDirectory));
Expand All @@ -140,25 +145,26 @@ public bool Generate (bool debugBuild, bool skipJniAddNativeMethodRegistrationAt
string typemapsOutputDirectory = Path.Combine (outputDirectory, "typemaps");

if (debugBuild) {
return GenerateDebug (skipJniAddNativeMethodRegistrationAttributeScan, javaTypes, typemapsOutputDirectory, generateNativeAssembly, appConfState);
return GenerateDebug (skipJniAddNativeMethodRegistrationAttributeScan, javaTypes, cache, typemapsOutputDirectory, generateNativeAssembly, appConfState);
}

return GenerateRelease (skipJniAddNativeMethodRegistrationAttributeScan, javaTypes, typemapsOutputDirectory, appConfState);
}

bool GenerateDebug (bool skipJniAddNativeMethodRegistrationAttributeScan, List<TypeDefinition> javaTypes, string outputDirectory, bool generateNativeAssembly, ApplicationConfigTaskState appConfState)
bool GenerateDebug (bool skipJniAddNativeMethodRegistrationAttributeScan, List<TypeDefinition> javaTypes, TypeDefinitionCache cache, string outputDirectory, bool generateNativeAssembly, ApplicationConfigTaskState appConfState)
{
if (generateNativeAssembly)
return GenerateDebugNativeAssembly (skipJniAddNativeMethodRegistrationAttributeScan, javaTypes, outputDirectory, appConfState);
return GenerateDebugFiles (skipJniAddNativeMethodRegistrationAttributeScan, javaTypes, outputDirectory, appConfState);
return GenerateDebugNativeAssembly (skipJniAddNativeMethodRegistrationAttributeScan, javaTypes, cache, outputDirectory, appConfState);
return GenerateDebugFiles (skipJniAddNativeMethodRegistrationAttributeScan, javaTypes, cache, outputDirectory, appConfState);
}

bool GenerateDebugFiles (bool skipJniAddNativeMethodRegistrationAttributeScan, List<TypeDefinition> javaTypes, string outputDirectory, ApplicationConfigTaskState appConfState)
bool GenerateDebugFiles (bool skipJniAddNativeMethodRegistrationAttributeScan, List<TypeDefinition> javaTypes, TypeDefinitionCache cache, string outputDirectory, ApplicationConfigTaskState appConfState)
{
var modules = new Dictionary<string, ModuleDebugData> (StringComparer.Ordinal);
int maxModuleFileNameWidth = 0;
int maxModuleNameWidth = 0;

var javaDuplicates = new Dictionary<string, List<TypeMapDebugEntry>> (StringComparer.Ordinal);
foreach (TypeDefinition td in javaTypes) {
UpdateApplicationConfig (td, appConfState);
string moduleName = td.Module.Assembly.Name.Name;
Expand All @@ -173,7 +179,8 @@ bool GenerateDebugFiles (bool skipJniAddNativeMethodRegistrationAttributeScan, L
JavaToManagedMap = new List<TypeMapDebugEntry> (),
ManagedToJavaMap = new List<TypeMapDebugEntry> (),
OutputFilePath = Path.Combine (outputDirectory, outputFileName),
ModuleNameBytes = outputEncoding.GetBytes (moduleName)
ModuleName = moduleName,
ModuleNameBytes = outputEncoding.GetBytes (moduleName),
};

if (module.ModuleNameBytes.Length > maxModuleNameWidth)
Expand All @@ -186,6 +193,7 @@ bool GenerateDebugFiles (bool skipJniAddNativeMethodRegistrationAttributeScan, L
}

TypeMapDebugEntry entry = GetDebugEntry (td);
HandleDebugDuplicates (javaDuplicates, entry, td, cache);
if (entry.JavaName.Length > module.JavaNameWidth)
module.JavaNameWidth = (uint)entry.JavaName.Length + 1;

Expand All @@ -195,6 +203,7 @@ bool GenerateDebugFiles (bool skipJniAddNativeMethodRegistrationAttributeScan, L
module.JavaToManagedMap.Add (entry);
module.ManagedToJavaMap.Add (entry);
}
SyncDebugDuplicates (javaDuplicates);

foreach (ModuleDebugData module in modules.Values) {
PrepareDebugMaps (module);
Expand All @@ -217,18 +226,22 @@ bool GenerateDebugFiles (bool skipJniAddNativeMethodRegistrationAttributeScan, L
return true;
}

bool GenerateDebugNativeAssembly (bool skipJniAddNativeMethodRegistrationAttributeScan, List<TypeDefinition> javaTypes, string outputDirectory, ApplicationConfigTaskState appConfState)
bool GenerateDebugNativeAssembly (bool skipJniAddNativeMethodRegistrationAttributeScan, List<TypeDefinition> javaTypes, TypeDefinitionCache cache, string outputDirectory, ApplicationConfigTaskState appConfState)
{
var javaToManaged = new List<TypeMapDebugEntry> ();
var managedToJava = new List<TypeMapDebugEntry> ();

var javaDuplicates = new Dictionary<string, List<TypeMapDebugEntry>> (StringComparer.Ordinal);
foreach (TypeDefinition td in javaTypes) {
UpdateApplicationConfig (td, appConfState);

TypeMapDebugEntry entry = GetDebugEntry (td);
HandleDebugDuplicates (javaDuplicates, entry, td, cache);

javaToManaged.Add (entry);
managedToJava.Add (entry);
}
SyncDebugDuplicates (javaDuplicates);

var data = new ModuleDebugData {
EntryCount = (uint)javaToManaged.Count,
Expand All @@ -246,6 +259,39 @@ bool GenerateDebugNativeAssembly (bool skipJniAddNativeMethodRegistrationAttribu
return true;
}

void SyncDebugDuplicates (Dictionary<string, List<TypeMapDebugEntry>> javaDuplicates)
{
foreach (List<TypeMapDebugEntry> duplicates in javaDuplicates.Values) {
if (duplicates.Count < 2) {
continue;
}

TypeMapDebugEntry template = duplicates [0];
for (int i = 1; i < duplicates.Count; i++) {
duplicates[i].TypeDefinition = template.TypeDefinition;
duplicates[i].ManagedName = template.ManagedName;
}
}
}

void HandleDebugDuplicates (Dictionary<string, List<TypeMapDebugEntry>> javaDuplicates, TypeMapDebugEntry entry, TypeDefinition td, TypeDefinitionCache cache)
{
List<TypeMapDebugEntry> duplicates;

if (!javaDuplicates.TryGetValue (entry.JavaName, out duplicates)) {
javaDuplicates.Add (entry.JavaName, new List<TypeMapDebugEntry> { entry });
} else {
duplicates.Add (entry);
TypeMapDebugEntry oldEntry = duplicates[0];
if (td.IsAbstract || td.IsInterface || oldEntry.TypeDefinition.IsAbstract || oldEntry.TypeDefinition.IsInterface) {
if (td.IsAssignableFrom (oldEntry.TypeDefinition, cache)) {
oldEntry.TypeDefinition = td;
oldEntry.ManagedName = GetManagedTypeName (td);
}
}
}
}

void PrepareDebugMaps (ModuleDebugData module)
{
module.JavaToManagedMap.Sort ((TypeMapDebugEntry a, TypeMapDebugEntry b) => String.Compare (a.JavaName, b.JavaName, StringComparison.Ordinal));
Expand All @@ -261,6 +307,16 @@ void PrepareDebugMaps (ModuleDebugData module)
}

TypeMapDebugEntry GetDebugEntry (TypeDefinition td)
{
return new TypeMapDebugEntry {
JavaName = Java.Interop.Tools.TypeNameMappings.JavaNativeTypeManager.ToJniName (td),
ManagedName = GetManagedTypeName (td),
TypeDefinition = td,
SkipInJavaToManaged = ShouldSkipInJavaToManaged (td),
};
}

string GetManagedTypeName (TypeDefinition td)
{
// This is necessary because Mono runtime will return to us type name with a `.` for nested types (not a
// `/` or a `+`. So, for instance, a type named `DefaultRenderer` found in the
Expand All @@ -277,17 +333,13 @@ TypeMapDebugEntry GetDebugEntry (TypeDefinition td)
//
string managedTypeName = td.FullName.Replace ('/', '+');

return new TypeMapDebugEntry {
JavaName = Java.Interop.Tools.TypeNameMappings.JavaNativeTypeManager.ToJniName (td),
ManagedName = $"{managedTypeName}, {td.Module.Assembly.Name.Name}",
};
return $"{managedTypeName}, {td.Module.Assembly.Name.Name}";
}

bool GenerateRelease (bool skipJniAddNativeMethodRegistrationAttributeScan, List<TypeDefinition> javaTypes, string outputDirectory, ApplicationConfigTaskState appConfState)
{
int assemblyId = 0;
int maxJavaNameLength = 0;
int maxModuleFileNameLength = 0;
var knownAssemblies = new Dictionary<string, int> (StringComparer.Ordinal);
var tempModules = new Dictionary<byte[], ModuleReleaseData> ();
Dictionary <AssemblyDefinition, int> moduleCounter = null;
Expand Down Expand Up @@ -329,12 +381,18 @@ bool GenerateRelease (bool skipJniAddNativeMethodRegistrationAttributeScan, List
}

string javaName = Java.Interop.Tools.TypeNameMappings.JavaNativeTypeManager.ToJniName (td);
// We will ignore generic types and interfaces when generating the Java to Managed map, but we must not
// omit them from the table we output - we need the same number of entries in both java-to-managed and
// managed-to-java tables. `SkipInJavaToManaged` set to `true` will cause the native assembly generator
// to output `0` as the token id for the type, thus effectively causing the runtime unable to match such
// a Java type name to a managed type. This fixes https://github.com/xamarin/xamarin-android/issues/4660
var entry = new TypeMapReleaseEntry {
JavaName = javaName,
JavaNameLength = outputEncoding.GetByteCount (javaName),
ManagedTypeName = td.FullName,
Token = td.MetadataToken.ToUInt32 (),
AssemblyNameIndex = knownAssemblies [assemblyName]
AssemblyNameIndex = knownAssemblies [assemblyName],
SkipInJavaToManaged = ShouldSkipInJavaToManaged (td),
};

if (entry.JavaNameLength > maxJavaNameLength)
Expand Down Expand Up @@ -376,6 +434,11 @@ bool GenerateRelease (bool skipJniAddNativeMethodRegistrationAttributeScan, List
return true;
}

bool ShouldSkipInJavaToManaged (TypeDefinition td)
{
return td.IsInterface || td.HasGenericParameters;
}

void GenerateNativeAssembly (Func<NativeAssemblerTargetProvider, bool, bool, NativeAssemblyGenerator> getGenerator)
{
NativeAssemblerTargetProvider asmTargetProvider;
Expand Down Expand Up @@ -491,6 +554,10 @@ void OutputModule (ModuleDebugData moduleData)
//
void OutputModule (BinaryWriter bw, ModuleDebugData moduleData)
{
if ((uint)moduleData.JavaToManagedMap.Count == UInt32.MaxValue) {
throw new InvalidOperationException ($"Too many types in module {moduleData.ModuleName}");
}

bw.Write (moduleMagicString);
bw.Write (TypeMapFormatVersion);
bw.Write (moduleData.JavaToManagedMap.Count);
Expand All @@ -502,7 +569,7 @@ void OutputModule (BinaryWriter bw, ModuleDebugData moduleData)
foreach (TypeMapDebugEntry entry in moduleData.JavaToManagedMap) {
bw.Write (outputEncoding.GetBytes (entry.JavaName));
PadField (bw, entry.JavaName.Length, (int)moduleData.JavaNameWidth);
bw.Write (entry.ManagedIndex);
bw.Write (entry.SkipInJavaToManaged ? UInt32.MaxValue : (uint)entry.ManagedIndex);
}

foreach (TypeMapDebugEntry entry in moduleData.ManagedToJavaMap) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ protected override void WriteSymbols (StreamWriter output)
if (haveJavaToManaged) {
foreach (TypeMapGenerator.TypeMapDebugEntry entry in data.JavaToManagedMap) {
size += WritePointer (output, entry.JavaLabel);
size += WritePointer (output, entry.ManagedLabel);
size += WritePointer (output, entry.SkipInJavaToManaged ? null : entry.ManagedLabel);
}
}
WriteStructureSize (output, JavaToManagedSymbol, size, alwaysWriteSize: true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ uint WriteJavaMapEntry (StreamWriter output, TypeMapGenerator.TypeMapReleaseEntr
size += WriteData (output, entry.ModuleIndex);

WriteCommentLine (output, "type_token_id");
size += WriteData (output, entry.Token);
size += WriteData (output, entry.SkipInJavaToManaged ? 0 : entry.Token);

WriteCommentLine (output, "java_name");
size += WriteAsciiData (output, entry.JavaName, mappingData.JavaNameWidth);
Expand Down
12 changes: 11 additions & 1 deletion src/monodroid/jni/embedded-assemblies.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include <libgen.h>
#include <errno.h>
#include <unistd.h>
#include <climits>

#include <mono/metadata/assembly.h>
#include <mono/metadata/image.h>
Expand Down Expand Up @@ -208,6 +209,10 @@ EmbeddedAssemblies::typemap_java_to_managed (const char *java_type_name)
}

const char *managed_type_name = entry->to;
if (managed_type_name == nullptr) {
log_debug (LOG_DEFAULT, "typemap: Java type '%s' maps either to an open generic type or an interface type.");
return nullptr;
}
log_debug (LOG_DEFAULT, "typemap: Java type '%s' corresponds to managed type '%s'", java_type_name, managed_type_name);

MonoType *type = mono_reflection_type_from_name (const_cast<char*>(managed_type_name), nullptr);
Expand Down Expand Up @@ -736,7 +741,12 @@ EmbeddedAssemblies::typemap_load_file (BinaryTypeMapHeader &header, const char *
cur->from = reinterpret_cast<char*>(java_pos);

uint32_t idx = *(reinterpret_cast<uint32_t*>(java_pos + header.java_name_width));
cur->to = reinterpret_cast<char*>(managed_start + (managed_entry_size * idx));
if (idx < UINT32_MAX) {
cur->to = reinterpret_cast<char*>(managed_start + (managed_entry_size * idx));
} else {
// Ignore the type mapping
cur->to = nullptr;
}
java_pos += java_entry_size;

cur = &module.managed_to_java[i];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,13 @@ namespace Xamarin.Android.JcwGenTests {
[TestFixture]
public class BindingTests {

[Test]
public void TestTimingCreateTimingIsCorrectType ()
{
var t = Com.Xamarin.Android.Timing.CreateTiming ();
Assert.IsTrue (t is Com.Xamarin.Android.Timing);
}

[Test]
public void TestResourceId ()
{
Expand Down
6 changes: 6 additions & 0 deletions tests/CodeGen-Binding/Xamarin.Android.McwGen-Tests/Timing.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,12 @@

namespace Com.Xamarin.Android {

// Provoke https://github.com/xamarin/xamarin-android/issues/4660
// We need this type to appear *before* `Timing` in `monodis --typedef` output
[Register ("com/xamarin/android/Timing", DoNotGenerateAcw = true)]
public partial class Timinf<T> : Timing {
}

public partial class Timing {

static IntPtr jonp_id_VirtualVoidMethod;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@

public class Timing {

public static Timing createTiming () {
return new Timing ();
}

public static void StaticVoidMethod ()
{
}
Expand Down

0 comments on commit aabda15

Please sign in to comment.