Skip to content

Commit

Permalink
[monodroid] Fix SIGSEGV when using closed generic types (dotnet#4541)
Browse files Browse the repository at this point in the history 
Context: https://github.com/jfversluis/SwipeViewDemo
Context: https://github.com/xamarin/Xamarin.Forms/blob/fa33ca3b3ac5c7c875023db785b56c67015e13b1/Xamarin.Forms.Platform.Android/AppCompat/TabbedPageRenderer.cs#L512

Commit 7117414 introduced native code which queries the Mono runtime
to obtain an instance of a `MonoClass` for a given `MonoReflectionType`
as well as the `MonoImage` associated with the `MonoClass`:

	// EmbeddedAssemblies::typemap_managed_to_java(MonoReflectionType *reflection_type, const uint8_t *mvid)
	MonoReflectionType     *reflection_type = …
	MonoType               *type            = mono_reflection_type_get_type (reflection_type);
	MonoClass              *klass           = mono_type_get_class (type);   // PROBLEMATIC
	// EmbeddedAssemblies::typemap_managed_to_java(MonoType *type, MonoClass *klass, const uint8_t *mvid)
	MonoImage              *image           = mono_class_get_image (klass);

However, it appears that when dealing with closed generic types -- such
as with `new GenericHolder<int>()` -- the above call chain results in a
SIGSEGV instead of a valid `MonoImage*`:

	F libc    : Fatal signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x3000021 in tid 19030 (s.swipeviewdemo), pid 19030 (s.swipeviewdemo)
	I crash_dump32: obtaining output fd from tombstoned, type: kDebuggerdTombstone
	I /system/bin/tombstoned: received crash request for pid 19030
	I crash_dump32: performing dump of process 19030 (target tid = 19030)
	F DEBUG   : *** *** *** *** *** *** *** *** *** *** *** *** *** *** *** ***
	F DEBUG   : Build fingerprint: 'google/sdk_gphone_x86/generic_x86:10/QSR1.191030.002/5978551:userdebug/dev-keys'
	F DEBUG   : Revision: '0'
	F DEBUG   : ABI: 'x86'
	F DEBUG   : Timestamp: 2020-04-08 15:52:55+0200
	F DEBUG   : pid: 19030, tid: 19030, name: s.swipeviewdemo  >>> nl.versluis.swipeviewdemo <<<
	F DEBUG   : uid: 10135
	F DEBUG   : signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x3000021
	F DEBUG   :     eax 03000005  ebx cb9f4d24  ecx 7191c13b  edx 00000004
	F DEBUG   :     edi 03000005  esi ffb22080
	F DEBUG   :     ebp ffb22108  esp ffb2206c  eip cbcfc1e4
	F DEBUG   :
	F DEBUG   : backtrace:
	F DEBUG   :       #00 pc 001b41e4  /data/app/Mono.Android.DebugRuntime-umHhDz421s4-tshrHwha0w==/lib/x86/libmonosgen-32bit-2.0.so (mono_image_get_name+4)
	F DEBUG   :       #1 pc 0000cac7  /data/app/nl.versluis.swipeviewdemo-jCFH_bcCNuFx1tLUhaJ4nw==/lib/x86/libmonodroid.so (xamarin::android::internal::EmbeddedAssemblies::typemap_managed_to_java(_MonoType*, _MonoClass*, unsigned char const*)+263) (BuildId: a2585ad379f788049e463af58c8686e9cdc1e778)
	F DEBUG   :       #2 pc 0000c8cc  /data/app/nl.versluis.swipeviewdemo-jCFH_bcCNuFx1tLUhaJ4nw==/lib/x86/libmonodroid.so (xamarin::android::internal::EmbeddedAssemblies::typemap_managed_to_java(_MonoReflectionType*, unsigned char const*)+124) (BuildId: a2585ad379f788049e463af58c8686e9cdc1e778)
	F DEBUG   :       dotnet#3 pc 0001621a  /data/app/nl.versluis.swipeviewdemo-jCFH_bcCNuFx1tLUhaJ4nw==/lib/x86/libmonodroid.so (xamarin::android::internal::MonodroidRuntime::typemap_managed_to_java(_MonoReflectionType*, unsigned char const*)+42) (BuildId: a2585ad379f788049e463af58c8686e9cdc1e778)
	F DEBUG   :       dotnet#4 pc 000325af  <anonymous:c7858000>

Investigating the issue I discovered that one of two things happened:

 1. the returned `MonoImage` instance was invalid, *or*
 2. the instance was valid but image name stored in `MonoImage`
    was `null`

(1) would only happen in 32-bit builds, resulting in a SIGSEGV within
`mono_image_get_name()`, because the `image` pointer value of 0x3000005
was invalid:

	I monodroid: const char *xamarin::android::internal::EmbeddedAssemblies::typemap_managed_to_java(MonoType *, MonoClass *, const uint8_t *)
	I monodroid:   type == 0xc6e60d3c, klass == 0xc6e60cb8, mvid == 0xc8c02d90
	I monodroid:   type name == Xamarin.Forms.Platform.Android.AppCompat.FormsFragmentPagerAdapter`1[Xamarin.Forms.Page]
	I monodroid:   calling mono_class_get_image (0xc6e60cb8)
	I monodroid:     image == 0x3000005

(2) only happened in 64-bit builds, resulting in a SIGSEGV as
`strlen()` was passed a `null` pointer:

	I monodroid: const char *xamarin::android::internal::EmbeddedAssemblies::typemap_managed_to_java(MonoType *, MonoClass *, const uint8_t *)
	I monodroid:   type == 0x7ce3fecc78, klass == 0x7ce3fecb98, mvid == 0x7ce9404520
	I monodroid:   type name == Xamarin.Forms.Platform.Android.AppCompat.FormsFragmentPagerAdapter`1[Xamarin.Forms.Page]
	I monodroid:   calling mono_class_get_image (0x7ce3fecb98)
	I monodroid:     image == 0x7ce3fecbc8
	I monodroid:     image_name == <null>

I found out that the root cause was in the `mono_type_get_class()` call
which is supposed to be used with great caution as it eventually calls
the `mono_type_get_class_internal()` function which is supposed to be
called only for object types `MONO_TYPE_CLASS` and `MONO_TYPE_VALUETYPE`
but not `MONO_TYPE_GENERICINST`, and `MONO_TYPE_GENERICINST` is used
for closed generic types.

The fix is to call the `mono_class_from_mono_type()` function instead
which, albeit slower, is safer and works correctly in all cases:

	// EmbeddedAssemblies::typemap_managed_to_java(MonoReflectionType *reflection_type, const uint8_t *mvid)
	MonoReflectionType     *reflection_type = …
	MonoType               *type            = mono_reflection_type_get_type (reflection_type);
	MonoClass              *klass           = mono_class_from_mono_type (type);     // PART OF THE FIX
	// EmbeddedAssemblies::typemap_managed_to_java(MonoType *type, MonoClass *klass, const uint8_t *mvid)
	MonoImage              *image           = mono_class_get_image (klass)

Fixing this led to the next issue, a managed exception thrown after the
runtime failed to map a managed type name to Java type name:

	W monodroid-assembly: typemap: unable to find mapping to a Java type from managed type 'Xamarin.Forms.Platform.Android.AppCompat.FormsFragmentPagerAdapter`1[T], Xamarin.Forms.Platform.Android'
	I monodroid-timing: Typemap.managed_to_java: end, total time; elapsed: 0s:0::33177
	D Mono    : DllImport attempting to load: '/system/lib64/liblog.so'.
	D Mono    : DllImport loaded library '/system/lib64/liblog.so'.
	D Mono    : DllImport searching in: '/system/lib64/liblog.so' ('/system/lib64/liblog.so').
	D Mono    : Searching for '__android_log_print'.
	D Mono    : Probing '__android_log_print'.
	D Mono    : Found as '__android_log_print'.
	I MonoDroid: UNHANDLED EXCEPTION:
	I MonoDroid: System.NotSupportedException: Cannot create instance of type 'Xamarin.Forms.Platform.Android.AppCompat.FormsFragmentPagerAdapter`1[[Xamarin.Forms.Page, Xamarin.Forms.Core, Version=2.0.0.0, Culture=neutral, PublicKeyToken=null]]': no Java peer type found.
	I MonoDroid:   at Java.Interop.JniPeerMembers+JniInstanceMethods..ctor (System.Type declaringType) [0x0004b] in <514e1249792e47a180b3f1293306b972>:0
	I MonoDroid:   at Java.Interop.JniPeerMembers+JniInstanceMethods.GetConstructorsForType (System.Type declaringType) [0x00031] in <514e1249792e47a180b3f1293306b972>:0
	I MonoDroid:   at Java.Interop.JniPeerMembers+JniInstanceMethods.StartCreateInstance (System.String constructorSignature, System.Type declaringType, Java.Interop.JniArgumentValue* parameters) [0x00038] in <514e1249792e47a180b3f1293306b972>:0
	I MonoDroid:   at Android.Support.V4.App.FragmentPagerAdapter..ctor (Android.Support.V4.App.FragmentManager fm) [0x0005b] in <fefee6c2c695459088a9df092723e052>:0
	I MonoDroid:   at Xamarin.Forms.Platform.Android.AppCompat.FormsFragmentPagerAdapter`1[T]..ctor (Xamarin.Forms.MultiPage`1[T] page, Android.Support.V4.App.FragmentManager fragmentManager) [0x00000] in <9d12bb15abb54c508c4bee636d1b3a42>:0
	I MonoDroid:   at Xamarin.Forms.Platform.Android.AppCompat.TabbedPageRenderer.CreateFormsViewPager (Android.Content.Context context, Xamarin.Forms.TabbedPage tabbedPage) [0x00033] in <9d12bb15abb54c508c4bee636d1b3a42>:0
	I MonoDroid:   at Xamarin.Forms.Platform.Android.AppCompat.TabbedPageRenderer.OnElementChanged (Xamarin.Forms.Platform.Android.ElementChangedEventArgs`1[TElement] e) [0x001cd] in <9d12bb15abb54c508c4bee636d1b3a42>:0
	I MonoDroid:   at Xamarin.Forms.Platform.Android.VisualElementRenderer`1[TElement].SetElement (TElement element) [0x000c0] in <9d12bb15abb54c508c4bee636d1b3a42>:0
	I MonoDroid:   at Xamarin.Forms.Platform.Android.VisualElementRenderer`1[TElement].Xamarin.Forms.Platform.Android.IVisualElementRenderer.SetElement (Xamarin.Forms.VisualElement element) [0x00033] in <9d12bb15abb54c508c4bee636d1b3a42>:0
	I MonoDroid:   at Xamarin.Forms.Platform.Android.Platform.CreateRenderer (Xamarin.Forms.VisualElement element, Android.Content.Context context) [0x0001f] in <9d12bb15abb54c508c4bee636d1b3a42>:0
	I MonoDroid:   at Xamarin.Forms.Platform.Android.AppCompat.Platform.AddChild (Xamarin.Forms.Page page, System.Boolean layout) [0x0000d] in <9d12bb15abb54c508c4bee636d1b3a42>:0
	I MonoDroid:   at Xamarin.Forms.Platform.Android.AppCompat.Platform.SetPageInternal (Xamarin.Forms.Page newRoot) [0x00061] in <9d12bb15abb54c508c4bee636d1b3a42>:0
	I MonoDroid:   at Xamarin.Forms.Platform.Android.AppCompat.Platform.SetPage (Xamarin.Forms.Page newRoot) [0x000e6] in <9d12bb15abb54c508c4bee636d1b3a42>:0
	I MonoDroid:   at Xamarin.Forms.Platform.Android.FormsAppCompatActivity.InternalSetPage (Xamarin.Forms.Page page) [0x0003f] in <9d12bb15abb54c508c4bee636d1b3a42>:0
	I MonoDroid:   at Xamarin.Forms.Platform.Android.FormsAppCompatActivity.SetMainPage () [0x0000c] in <9d12bb15abb54c508c4bee636d1b3a42>:0
	I MonoDroid:   at Xamarin.Forms.Platform.Android.FormsAppCompatActivity.LoadApplication (Xamarin.Forms.Application application) [0x00140] in <9d12bb15abb54c508c4bee636d1b3a42>:0
	I MonoDroid:   at SwipeViewDemo.Droid.MainActivity.OnCreate (Android.OS.Bundle savedInstanceState) [0x00035] in <15e30af50bb64ff6b6d20ac6fd546763>:0
	I MonoDroid:   at Android.App.Activity.n_OnCreate_Landroid_os_Bundle_ (System.IntPtr jnienv, System.IntPtr native__this, System.IntPtr native_savedInstanceState) [0x0000f] in <515e813169e54876823978ab785f569a>:0
	I MonoDroid:   at (wrapper dynamic-method) Android.Runtime.DynamicMethodNameCounter.7(intptr,intptr,intptr)

Note that the type name as seen by the native code via
`mono_get_type_name_full()` is:

	Xamarin.Forms.Platform.Android.AppCompat.FormsFragmentPagerAdapter`1[T], Xamarin.Forms.Platform.Android

while `Mono.Android.dll` expects to be looking for:

	Xamarin.Forms.Platform.Android.AppCompat.FormsFragmentPagerAdapter`1, Xamarin.Forms.Platform.Android

This discrepancy was caused by calling `mono_get_type_name_full()`
with its format parameter set to `MONO_TYPE_NAME_FORMAT_REFLECTION`,
while we needed the format value of `MONO_TYPE_NAME_FORMAT_FULL_NAME`.
  • Loading branch information
@grendello @jonpryor
grendello authored and jonpryor committed Apr 9, 2020
1 parent 0188f6f commit de9267f
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 3 deletions.
10 changes: 9 additions & 1 deletion src/Mono.Android/Test/Java.Interop/JnienvTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,15 +85,23 @@ public void InvokingNullInstanceDoesNotCrashDalvik ()
}

[Test]
public void NewGenericTypeThrows ()
public void NewOpenGenericTypeThrows ()
{
try {
var lrefInstance = JNIEnv.StartCreateInstance (typeof (GenericHolder<>), "()V");
JNIEnv.FinishCreateInstance (lrefInstance, "()V");
Assert.Fail ("SHOULD NOT BE REACHED: creation of open generic types is not supported");
} catch (NotSupportedException) {
}
}

[Test]
public void NewClosedGenericTypeWorks ()
{
using (var holder = new GenericHolder<int>()) {
}
}

[Test]
public void NewObjectArrayWithNullArray ()
{
Expand Down
4 changes: 2 additions & 2 deletions src/monodroid/jni/embedded-assemblies.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ EmbeddedAssemblies::typemap_managed_to_java ([[maybe_unused]] MonoType *type, Mo
{
constexpr char error_message[] = "typemap: unable to find mapping to a Java type from managed type '%s'";

simple_pointer_guard<char[], false> type_name (mono_type_get_name_full (type, MONO_TYPE_NAME_FORMAT_REFLECTION));
simple_pointer_guard<char[], false> type_name (mono_type_get_name_full (type, MONO_TYPE_NAME_FORMAT_FULL_NAME));
MonoImage *image = mono_class_get_image (klass);
const char *image_name = mono_image_get_name (image);
size_t type_name_len = strlen (type_name.get ());
Expand Down Expand Up @@ -490,7 +490,7 @@ EmbeddedAssemblies::typemap_managed_to_java (MonoReflectionType *reflection_type
return nullptr;
}

const char *ret = typemap_managed_to_java (type, mono_type_get_class (type), mvid);
const char *ret = typemap_managed_to_java (type, mono_class_from_mono_type (type), mvid);

if (XA_UNLIKELY (utils.should_log (LOG_TIMING))) {
total_time.mark_end ();
Expand Down

0 comments on commit de9267f

Please sign in to comment.