search

dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.42k stars 4.76k forks source link

AppContext.GetData always return null for iOS #50290

Closed spouliot closed 3 years ago

spouliot commented 3 years ago

Description

We initialize the runtime with some keys, e.g.

        const char *propertyKeys[] = {
               "APP_PATHS",
                "PINVOKE_OVERRIDE",
        };
        const char *propertyValues[] = {
                xamarin_get_bundle_path (),
                pinvokeOverride,
        };
        static_assert (sizeof (propertyKeys) == sizeof (propertyValues), "The number of keys and values must be the same.");

AFAIK those features works fine at runtime.

However if you try to query the keys from managed code then the returned values are always null. For example that test would fail on the first assert

        [Test]
        public void ___AppContext ()
        {
            Assert.IsNotNull (AppContext.GetData ("APP_PATHS"), "APP_PATHS");
            Assert.IsNotNull (AppContext.GetData ("PINVOKE_OVERRIDE"), "PINVOKE_OVERRIDE");
        }

I have a feeling that AppContext.Setup is never called on the native properties/values so nothing gets exposed to managed code.

Configuration

Regression?

Not sure. We never had to query the values from managed code before.

Other information

This is blocking our work to enable ICU with iOS [1] since the runtime code calls AppContext.GetData https://github.com/dotnet/runtime/pull/49406/files#diff-4a770bb138400e296d16d4bf1e745c2f3a4eafcba26b7b602e87baa623d30fffR10

[1] https://github.com/xamarin/xamarin-macios/issues/8906

dotnet-issue-labeler[bot] commented 3 years ago

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

spouliot commented 3 years ago

c.c. @steveisok @lambdageek @CoffeeFlux

CoffeeFlux commented 3 years ago

Is there any chance that AppContext.Setup is getting linked out? And if not, how would I reproduce this locally?

spouliot commented 3 years ago

Is there any chance that AppContext.Setup is getting linked out?

No, linker is disabled in my test and I confirmed the version of System.Private.CoreLib.dll was identical between the .app and the runtime pack (and that AppContext.Setup was present inside them).

how would I reproduce this locally?

Add the test above to any of the unit test we have in our repo (or create your own unit test project).

filipnavara commented 3 years ago

It's an incorrect initialisation order, likely on Xamarin side. This is where AppContext.Setup gets called:

* thread #1, name = 'tid_403', queue = 'com.apple.main-thread', stop reason = breakpoint 1.1
  * frame #0: 0x0000000108a5ad20 libcoreclr.dylib`mono_runtime_install_appctx_properties
    frame #1: 0x0000000108a5a297 libcoreclr.dylib`mono_runtime_init_checked + 1079
    frame #2: 0x0000000108bcc769 libcoreclr.dylib`mini_init + 6249
    frame #3: 0x0000000108c3168e libcoreclr.dylib`mono_jit_init_version + 14
    frame #4: 0x0000000100042bf6 MyCocoaApp`xamarin_bridge_initialize at monovm-bridge.m:74:2
    frame #5: 0x00000001000486a9 MyCocoaApp`xamarin_main(argc=1, argv=0x0000000304553838, launch_mode=XamarinLaunchModeApp) at monotouch-main.m:401:2
    frame #6: 0x00000001000d19f7 MyCocoaApp`main(argc=1, argv=0x0000000304553838) at main.x86_64.mm:49:11
    frame #7: 0x00007fff20345621 libdyld.dylib`start + 1
    frame #8: 0x00007fff20345621 libdyld.dylib`start + 1

It happens before monovm_initialize is called.

filipnavara commented 3 years ago

Switching these two lines may help (but has to be tested, both on MonoVM and CoreCLR): https://github.com/xamarin/xamarin-macios/blob/cfe0a309dc97aaf0cbd196b366c18aa7d96f0a5a/runtime/monotouch-main.m#L405-L409

spouliot commented 3 years ago

@filipnavara thanks, that's worth a try

I stopped looking after reading https://github.com/xamarin/xamarin-macios/issues/10504#issuecomment-765540717

filipnavara commented 3 years ago

Yeah, that comment was not accurate: https://github.com/dotnet/runtime/issues/37240

spouliot commented 3 years ago

A quick local test (mono/iOS sim) seems to work

ghost commented 3 years ago

Tagging subscribers to this area: @CoffeeFlux See info in area-owners.md if you want to be subscribed.

Issue Details
### Description We initialize the runtime with some keys, e.g. ``` const char *propertyKeys[] = { "APP_PATHS", "PINVOKE_OVERRIDE", }; const char *propertyValues[] = { xamarin_get_bundle_path (), pinvokeOverride, }; static_assert (sizeof (propertyKeys) == sizeof (propertyValues), "The number of keys and values must be the same."); ``` AFAIK those features works fine at runtime. However if you try to query the keys from **managed** code then the returned values are always `null`. For example that test would fail on the first assert ```csharp [Test] public void ___AppContext () { Assert.IsNotNull (AppContext.GetData ("APP_PATHS"), "APP_PATHS"); Assert.IsNotNull (AppContext.GetData ("PINVOKE_OVERRIDE"), "PINVOKE_OVERRIDE"); } ``` I have a _feeling_ that `AppContext.Setup` is never called on the native properties/values so nothing gets exposed to managed code. ### Configuration * xamarin-macios https://github.com/xamarin/xamarin-macios/pull/10890 * using `6.0.100-preview.3.21161.23` ### Regression? Not sure. We never had to query the values from managed code before. ### Other information This is blocking our work to enable ICU with iOS [1] since the runtime code calls `AppContext.GetData` https://github.com/dotnet/runtime/pull/49406/files#diff-4a770bb138400e296d16d4bf1e745c2f3a4eafcba26b7b602e87baa623d30fffR10 [1] https://github.com/xamarin/xamarin-macios/issues/8906
Author: spouliot
Assignees: CoffeeFlux
Labels: `area-VM-meta-mono`, `os-ios`, `runtime-mono`, `untriaged`
Milestone: 6.0.0
steveisok commented 3 years ago

A quick local test (mono/iOS sim) seems to work

@spouliot Is it safe to close the issue?

CoffeeFlux commented 3 years ago

Given that Xamarin seems to be calling these in the wrong order, I'm closing since this isn't a runtime issue. If it later turns out to be one, we can always reopen.

spouliot commented 3 years ago

local tests were fine, it's on the bots now... https://github.com/xamarin/xamarin-macios/pull/11014 will ping if there are issues, but I don't expect any thanks!

spouliot commented 3 years ago

I'll confirm in the morning (if that was an issue before the call order change) but we are now failing on device (AOT builds) with:

00:32:58.1764370 00:32:58.1763870 xamarin_vm_initialize (3, 0x16f992e10, 0x16f992df8): rv: 0
00:32:58.2256090 00:32:58.2255640 2021-03-30 00:32:58.196 link sdk[592:571713] Unhandled managed exception: Attempting to JIT compile method '(wrapper managed-to-native) object object:__icall_wrapper_(null) (object,intptr,intptr)' while running in aot-only mode. See https://docs.microsoft.com/xamarin/ios/internals/limitations for more information.
00:32:58.2256360 00:32:58.2256320  (System.ExecutionEngineException)
00:32:58.2256500 00:32:58.2256460    at System.Collections.Generic.Dictionary`2[[System.String, System.Private.CoreLib, Version=6.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e],[System.Object, System.Private.CoreLib, Version=6.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]]..ctor(Int32 capacity, IEqualityComparer`1 comparer)
00:32:58.2256640 00:32:58.2256600    at System.Collections.Generic.Dictionary`2[[System.String, System.Private.CoreLib, Version=6.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e],[System.Object, System.Private.CoreLib, Version=6.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]]..ctor(Int32 capacity)
00:32:58.2256910 00:32:58.2256860    at System.AppContext.Setup(Char** pNames, Char** pValues, Int32 count)
00:32:58.2257040 00:32:58.2257000 
00:32:58.2257440 00:32:58.2257370 =================================================================
00:32:58.2257610 00:32:58.2257570   Native Crash Reporting
00:32:58.2257730 00:32:58.2257690 =================================================================
00:32:58.2257840 00:32:58.2257810 Got a SIGABRT while executing native code. This usually indicates
00:32:58.2257960 00:32:58.2257920 a fatal error in the mono runtime or one of the native libraries 
00:32:58.2258080 00:32:58.2258040 used by your application.
00:32:58.2258200 00:32:58.2258160 =================================================================
00:32:58.2258310 00:32:58.2258270 
00:32:58.2258430 00:32:58.2258380 =================================================================
00:32:58.2258540 00:32:58.2258500   Native stacktrace:
00:32:58.2258650 00:32:58.2258620 =================================================================
00:32:58.3083250 00:32:58.3083030   0x101f59144 - /private/var/containers/Bundle/Application/749C70E8-CD87-4963-9A37-52840577E1B7/link sdk.app/link sdk : mono_dump_native_crash_info
00:32:58.3133940 00:32:58.3133780   0x101f428e8 - /private/var/containers/Bundle/Application/749C70E8-CD87-4963-9A37-52840577E1B7/link sdk.app/link sdk : mono_handle_native_crash
00:32:58.3150660 00:32:58.3150480   0x101f58518 - /private/var/containers/Bundle/Application/749C70E8-CD87-4963-9A37-52840577E1B7/link sdk.app/link sdk : sigabrt_signal_handler
00:32:58.3152900 00:32:58.3152750   0x1f54d7298 - /usr/lib/system/libsystem_platform.dylib : <redacted>
00:32:58.3153190 00:32:58.3153110   0x1f54dcb50 - /usr/lib/system/libsystem_pthread.dylib : pthread_kill
00:32:58.3155190 00:32:58.3155100   0x1b2e5bb74 - /usr/lib/system/libsystem_c.dylib : abort
00:32:58.3180820 00:32:58.3180700   0x101c4b208 - /private/var/containers/Bundle/Application/749C70E8-CD87-4963-9A37-52840577E1B7/link sdk.app/link sdk : xamarin_find_protocol_wrapper_type
00:32:58.3195660 00:32:58.3195410   0x101e02754 - /private/var/containers/Bundle/Application/749C70E8-CD87-4963-9A37-52840577E1B7/link sdk.app/link sdk : mono_invoke_unhandled_exception_hook
00:32:58.3207310 00:32:58.3207080   0x101f422f8 - /private/var/containers/Bundle/Application/749C70E8-CD87-4963-9A37-52840577E1B7/link sdk.app/link sdk : mono_handle_exception_internal
00:32:58.3221710 00:32:58.3221600   0x101f40c44 - /private/var/containers/Bundle/Application/749C70E8-CD87-4963-9A37-52840577E1B7/link sdk.app/link sdk : mono_handle_exception
00:32:58.3243040 00:32:58.3242890   0x101f5694c - /private/var/containers/Bundle/Application/749C70E8-CD87-4963-9A37-52840577E1B7/link sdk.app/link sdk : mono_arm_throw_exception
00:32:58.3250850 00:32:58.3250730   0x10089719c - /private/var/containers/Bundle/Application/749C70E8-CD87-4963-9A37-52840577E1B7/link sdk.app/link sdk : rethrow_preserve_exception
00:32:58.3267710 00:32:58.3267460   0x100820b98 - /private/var/containers/Bundle/Application/749C70E8-CD87-4963-9A37-52840577E1B7/link sdk.app/link sdk : wrapper_castclass_object___castclass_with_cache_object_intptr_intptr
00:32:58.3284160 00:32:58.3284040   0x1006e4864 - /private/var/containers/Bundle/Application/749C70E8-CD87-4963-9A37-52840577E1B7/link sdk.app/link sdk : System_Collections_Generic_Dictionary_2_TKey_REF_TValue_REF__ctor_int_System_Collections_Generic_IEqualityComparer_1_TKey_REF
00:32:58.3297500 00:32:58.3297390   0x1006e4638 - /private/var/containers/Bundle/Application/749C70E8-CD87-4963-9A37-52840577E1B7/link sdk.app/link sdk : System_Collections_Generic_Dictionary_2_TKey_REF_TValue_REF__ctor_int
00:32:58.3314160 00:32:58.3314050   0x1004c5274 - /private/var/containers/Bundle/Application/749C70E8-CD87-4963-9A37-52840577E1B7/link sdk.app/link sdk : System_AppContext_Setup_char___char___int
00:32:58.3330360 00:32:58.3330130   0x10081f628 - /private/var/containers/Bundle/Application/749C70E8-CD87-4963-9A37-52840577E1B7/link sdk.app/link sdk : wrapper_runtime_invoke_object_runtime_invoke_dynamic_intptr_intptr_intptr_intptr
00:32:58.3342950 00:32:58.3342850   0x101f24318 - /private/var/containers/Bundle/Application/749C70E8-CD87-4963-9A37-52840577E1B7/link sdk.app/link sdk : mono_jit_runtime_invoke
00:32:58.3352700 00:32:58.3352600   0x101e49a64 - /private/var/containers/Bundle/Application/749C70E8-CD87-4963-9A37-52840577E1B7/link sdk.app/link sdk : mono_runtime_invoke_checked
00:32:58.3363060 00:32:58.3362960   0x101ddd734 - /private/var/containers/Bundle/Application/749C70E8-CD87-4963-9A37-52840577E1B7/link sdk.app/link sdk : mono_runtime_install_appctx_properties
00:32:58.3375930 00:32:58.3375820   0x101ddcca0 - /private/var/containers/Bundle/Application/749C70E8-CD87-4963-9A37-52840577E1B7/link sdk.app/link sdk : mono_runtime_init_checked
00:32:58.3385810 00:32:58.3385710   0x101f23a8c - /private/var/containers/Bundle/Application/749C70E8-CD87-4963-9A37-52840577E1B7/link sdk.app/link sdk : mini_init
00:32:58.3395930 00:32:58.3395730   0x101f2ce24 - /private/var/containers/Bundle/Application/749C70E8-CD87-4963-9A37-52840577E1B7/link sdk.app/link sdk : mono_jit_init_version
00:32:58.3407550 00:32:58.3407370   0x101c525c0 - /private/var/containers/Bundle/Application/749C70E8-CD87-4963-9A37-52840577E1B7/link sdk.app/link sdk : xamarin_bridge_initialize
00:32:58.3416990 00:32:58.3416860   0x101c53124 - /private/var/containers/Bundle/Application/749C70E8-CD87-4963-9A37-52840577E1B7/link sdk.app/link sdk : xamarin_main
00:32:58.3434190 00:32:58.3434040   0x101fa8f5c - /private/var/containers/Bundle/Application/749C70E8-CD87-4963-9A37-52840577E1B7/link sdk.app/link sdk : main
00:32:58.3436810 00:32:58.3436740   0x1a96626b0 - /usr/lib/system/libdyld.dylib : <redacted>
00:32:58.3436970 00:32:58.3436930 
00:32:58.3437190 00:32:58.3437150 =================================================================
00:32:58.3437300 00:32:58.3437270   Basic Fault Address Reporting
00:32:58.3437410 00:32:58.3437380 =================================================================
00:32:58.3437540 00:32:58.3437500 Memory around native instruction pointer (0x1d7982414):0x1d7982404  ff 0f 5f d6 c0 03 5f d6 10 29 80 d2 01 10 00 d4  .._..._..)......
00:32:58.3437650 00:32:58.3437620 0x1d7982414  03 01 00 54 7f 23 03 d5 fd 7b bf a9 fd 03 00 91  ...T.#...{......
00:32:58.3437770 00:32:58.3437730 0x1d7982424  ff 71 ff 97 bf 03 00 91 fd 7b c1 a8 ff 0f 5f d6  .q.......{...._.
00:32:58.3438470 00:32:58.3438360 0x1d7982434  c0 03 5f d6 90 29 80 d2 01 10 00 d4 03 01 00 54  .._..).........T
00:32:58.3438760 00:32:58.3438720 
00:32:58.3439270 00:32:58.3439170 =================================================================
00:32:58.3439420 00:32:58.3439380   Managed Stacktrace:
00:32:58.3439540 00:32:58.3439500 =================================================================
00:32:58.3439670 00:32:58.3439620 =================================================================

It might also be related to the linker (it's enabled on this project). I'm not sure if AppContext.Setup preserve what it needs... I don't recall seeing any attribute when I decompiled the code.

filipnavara commented 3 years ago

It doesn't have linker attribute but it's listed in the ILLink descriptor file: https://github.com/dotnet/runtime/blob/ec0bb49776bd03330356556de88bb8456b4f667c/src/mono/System.Private.CoreLib/src/ILLink/ILLink.Descriptors.xml#L630

spouliot commented 3 years ago

Might be related to https://github.com/xamarin/xamarin-macios/issues/10726

filipnavara commented 3 years ago

I have the crashing sample running under lldb (https://gist.github.com/filipnavara/a77f99b312716a2fc76ce357f111ab27) but it's way beyond my understanding of the code. Looks like the AOT process didn't generate wrapper for the mono_marshal_isinst_with_cache JIT icall. In fact, it didn't generate wrappers for any JIT icalls registered in mono_marshal_init.

cc @vargaz anything I can look for since I already have the whole thing set up?

vargaz commented 3 years ago

The AOT compiler generates icall wrappers for all icalls registered in the cross compiler. This is done at aot-compiler.c:4733.

filipnavara commented 3 years ago

Yeah, that's what I thought. I'll try to run the AOT process under LLDB and figure out more.

filipnavara commented 3 years ago

Actually, on second thought there's wrapper_managed_to_native_object___icall_wrapper_mono_marshal_isinst_with_cache_object_intptr_intptr in the linked executable and all the other icalls have similar names. Should that still go through the trampolines?

  * frame #0: 0x00000001063cc52c link sdk`mono_marshal_get_icall_wrapper(callinfo=0x00000001067f61b8, check_exceptions=1) at marshal.c:2756:39 [opt]
    frame #1: 0x00000001064be72c link sdk`mono_icall_get_wrapper_full [inlined] mono_icall_get_wrapper_method(callinfo=0x00000001067f61b8) at mini-runtime.c:642:9 [opt]
    frame #2: 0x00000001064be70c link sdk`mono_icall_get_wrapper_full(callinfo=0x00000001067f61b8, do_compile=0) at mini-runtime.c:655 [opt]
    frame #3: 0x00000001064bf410 link sdk`mono_resolve_patch_target [inlined] mono_icall_get_wrapper(callinfo=<unavailable>) at mini-runtime.c:683:9 [opt]
    frame #4: 0x00000001064bf408 link sdk`mono_resolve_patch_target(method=0x0000000000000000, code=0x0000000000000000, patch_info=0x000000016b3ea6d0, run_cctors=1, error=0x000000016b3ea760) at mini-runtime.c:1396 [opt]
    frame #5: 0x00000001064d6ed8 link sdk`mono_aot_plt_resolve(aot_module=0x000000010d808a00, regs=0x000000016b3ea7f0, code="`", error=0x000000016b3ea760) at aot-runtime.c:5010:22 [opt]
    frame #6: 0x00000001064e57ec link sdk`mono_aot_plt_trampoline(regs=<unavailable>, code=<unavailable>, aot_module=<unavailable>, tramp=<unavailable>) at mini-trampolines.c:928:8 [opt]
    frame #7: 0x0000000104e3c2ec link sdk`generic_trampoline_aot_plt + 252

If the answer is yes, then I guess the cache check in mono_marshal_get_icall_wrapper should be hit but it's not. The remaining code path has no chance of working properly since callinfo->name == null. This is result of DISABLE_JIT being defined and thus register_icall not setting the name:

#ifndef DISABLE_JIT
#define register_icall(func, sig, no_wrapper) \
    (mono_register_jit_icall_info (&mono_get_jit_icall_info ()->func, func, #func, (sig), (no_wrapper), #func))
#else
/* No need for the name/C symbol */
#define register_icall(func, sig, no_wrapper) \
    (mono_register_jit_icall_info (&mono_get_jit_icall_info ()->func, func, NULL, (sig), (no_wrapper), NULL))
#endif
filipnavara commented 3 years ago

The removal of string literals is quite recent change: https://github.com/dotnet/runtime/commit/4cbb1e6b09f0c99e0b50b21e7013ea4ef6e2b5f7