From 01adacaf75003a59af3f39e33886d25fd009daab Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Thu, 9 Apr 2020 00:44:05 +0000 Subject: [PATCH] [monodroid] Fix SIGSEGV when using closed generic types (#4541) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Context: https://github.com/jfversluis/SwipeViewDemo Context: https://github.com/xamarin/Xamarin.Forms/blob/fa33ca3b3ac5c7c875023db785b56c67015e13b1/Xamarin.Forms.Platform.Android/AppCompat/TabbedPageRenderer.cs#L512 Commit 7117414c 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()` -- 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 : #01 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 : #02 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 : #03 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 : #04 pc 000325af 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 == 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 :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`. --- src/Mono.Android/Test/Java.Interop/JnienvTest.cs | 10 +++++++++- src/monodroid/jni/embedded-assemblies.cc | 4 ++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/Mono.Android/Test/Java.Interop/JnienvTest.cs b/src/Mono.Android/Test/Java.Interop/JnienvTest.cs index 75bdedac0e5..11ac6e04b18 100644 --- a/src/Mono.Android/Test/Java.Interop/JnienvTest.cs +++ b/src/Mono.Android/Test/Java.Interop/JnienvTest.cs @@ -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()) { + } + } + [Test] public void NewObjectArrayWithNullArray () { diff --git a/src/monodroid/jni/embedded-assemblies.cc b/src/monodroid/jni/embedded-assemblies.cc index 018289db9cd..f50277f24f9 100644 --- a/src/monodroid/jni/embedded-assemblies.cc +++ b/src/monodroid/jni/embedded-assemblies.cc @@ -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 type_name (mono_type_get_name_full (type, MONO_TYPE_NAME_FORMAT_REFLECTION)); + simple_pointer_guard 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 ()); @@ -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 ();