Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[mono] Stop exporting ICU symbols from Mono#99449

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation

@matouskozak
Copy link
Member

@matouskozakmatouskozak commentedMar 8, 2024
edited
Loading

Customer Impact

  • Customer reported
  • Found internally

When running against the latest XCode, the iOS 17.4 simulator causes our apps to crash due to a symbols clash between their version of ICU and our version. Prior to this change, by default we had been exporting all our symbols and been lucky due to the ICU versions matching. To fix, we hide our symbols via-hidden-lx so that there is no longer a clash.

NOTE: This does not impact .NET 9 as we have moved away from shipping our own version of ICU on iOS.

Regression

  • Yes
  • No

Testing

We verified the fix thru manually copying new versions oflibmonosgen-2.0.dylib andlibSystem.Globalization.Native.dylib to previously crashing app and re-running it on the simulator. Currently, we are unable to verify this E2E by using a template iOS app with custom runtime due to issues with trampolines.

Risk

Low, but since we were unable to verify E2E with a template app, there may be more churn required if the iOS SDK team finds any issues.


Contributed:@ivanpovazan
fyi:@vitek-karas@akoeplinger@steveisok@dalexsoto@mkhamoyan

ivanpovazan and dalexsoto reacted with heart emoji
@matouskozak

This comment was marked as duplicate.

@matouskozakmatouskozak added runtime-monospecific to the Mono runtime os-iosApple iOS labelsMar 8, 2024
@rolfbjarne
Copy link
Member

@rolfbjarne Do you have any idea on what could be the issue or another working approach on how to tests this with custom runtime and iOS template app?

I wonder if the dylib needs to be signed?

Do you get the same error if you copy:

  1. An unmodified dylib from a published NuGet.
  2. A lcoally built dylib, from the same hash has the published NuGet without any source code changes.

An alternative might be to try in the x64 simulator.

@ivanpovazan
Copy link
Member

ivanpovazan commentedMar 8, 2024
edited
Loading

A note regarding E2E testing and:

Thread 0 Crashed::  Dispatch queue: com.apple.main-thread0   libsystem_platform.dylib             0x10ec25828 _platform_memset_pattern16 + 2961   libmonosgen-2.0.dylib                0x1113f0fa8 mono_arch_create_sdb_trampoline + 762   libmonosgen-2.0.dylib                0x1113a9134 mini_get_breakpoint_trampoline + 1003   libmonosgen-2.0.dylib                0x1113da7a8 mono_arch_init + 444   libmonosgen-2.0.dylib                0x1113147f8 mini_init + 8485   libmonosgen-2.0.dylib                0x11136b7d0 mono_jit_init_version + 206   libxamarin-dotnet-debug.dylib        0x110095550 xamarin_bridge_initialize + 2167   libxamarin-dotnet-debug.dylib        0x11009b8e4 xamarin_main + 17808   simulator_ICU_issue                  0x106b2b868 main + 64 (main.arm64.mm:409)9   dyld_sim                             0x10f209544 start_sim + 2010  dyld                                 0x10ed060e0 start + 2360Thread 1:0   libsystem_pthread.dylib              0x10fbe66dc start_wqthread +

We are getting this even if we build the runtime locally and configure the project file to reference the locally built runtime like:

  <PropertyGroupCondition="$(LocalBuild) == 'true'">    <RestoreAdditionalProjectSources>/Users/ivan/repos/runtime-mono/artifacts/packages/Release/Shipping</RestoreAdditionalProjectSources>  </PropertyGroup>  <ItemGroupCondition="$(LocalBuild) == 'true'">    <FrameworkReferenceUpdate="Microsoft.NETCore.App"RuntimeFrameworkVersion="8.0.3-dev" />  </ItemGroup>

However, if we build the app and manually copylibmonosgen-2.0.dylib andlibSystem.Globalization.Native.dylib over to the built app bundle and run it on the simulator, themono_arch_create_sdb_trampoline problem is not triggered, and the app runs fine (with the changes locally introduced).


I remember@lambdageek experienced the same problem a while back:#93860 (comment)

@rolfbjarne
Copy link
Member

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0 libsystem_platform.dylib 0x10ec25828 _platform_memset_pattern16 + 296
1 libmonosgen-2.0.dylib 0x1113f0fa8 mono_arch_create_sdb_trampoline + 76

IIRC it's related to creating executable + writable memory (or failing to do so).

You get the same crash on Mac Catalyst if you enable the sandbox, but don't add the jit entitlement.

ivanpovazan reacted with thumbs up emoji

@ivanpovazan
Copy link
Member

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0 libsystem_platform.dylib 0x10ec25828 _platform_memset_pattern16 + 296
1 libmonosgen-2.0.dylib 0x1113f0fa8 mono_arch_create_sdb_trampoline + 76

IIRC it's related to creating executable + writable memory (or failing to do so).

You get the same crash on Mac Catalyst if you enable the sandbox, but don't add the jit entitlement.

If I understand the flow correctly, we shouldn't be getting to themono_arch_create_sdb_trampoline at all.

For the app we generate:

void xamarin_setup_impl (){mono_jit_set_aot_mode (MONO_AOT_MODE_FULL);

which should be invoked from xamarin's main:
https://github.com/xamarin/xamarin-macios/blob/58e382efb7abdb9cf88900bd8e87f27f7e586a51/runtime/monotouch-main.m#L276

that further gets here:

}
staticvoid
mono_runtime_set_execution_mode (intmode)
{
mono_runtime_set_execution_mode_full (mode, TRUE);
}
/**
* mono_jit_set_aot_mode:
*/
void
mono_jit_set_aot_mode (MonoAotModemode)
{
/* we don't want to set mono_aot_mode twice */
staticgbooleaninited;
g_assert (!inited);
mono_aot_mode=mode;
inited= TRUE;
mono_runtime_set_execution_mode (mode);
}

and finally here:

mono_runtime_set_execution_mode_full (intmode,gbooleanoverride)
{
staticgbooleanmode_initialized= FALSE;
if (mode_initialized&& !override)
return;
mode_initialized= TRUE;
memset (&mono_ee_features,0,sizeof (mono_ee_features));
switch (mode) {
caseMONO_AOT_MODE_FULL:
mono_aot_only= TRUE;
mono_ee_features.use_aot_trampolines= TRUE;

which sets the:mono_ee_features.use_aot_trampolines = TRUE; and should guard against trying to create executable code (sdb trampolines) here:

if (mono_ee_features.use_aot_trampolines) {
tramp=mono_aot_get_trampoline ("sdb_breakpoint_trampoline");
}else {
#ifdefMONO_ARCH_HAVE_SDB_TRAMPOLINES
MonoTrampInfo*info;
tramp=mono_arch_create_sdb_trampoline (FALSE,&info, FALSE);
mono_tramp_info_register (info,NULL);
#else
tramp=NULL;
g_assert_not_reached ();
#endif
}

Will have to set up debug builds of everything I suppose .. :)

matouskozakand others added2 commitsMarch 8, 2024 17:49
Co-authored-by: Alexander Köplinger <alex.koeplinger@outlook.com>
@matouskozak
Copy link
MemberAuthor

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@matouskozak
Copy link
MemberAuthor

matouskozak commentedMar 11, 2024
edited
Loading

This fix probably won't be enough.

I was able to re-produce the original issue (#98941) without Xamarin by trying the Hybrid Globalization testshttps://github.com/dotnet/runtime/blob/release/8.0-staging/src/libraries/System.Globalization/tests/Hybrid/System.Globalization.IOS.Tests.csproj. The error message is the same

System.Globalization.IOS.Tests(42002,0x16df7f000) malloc: *** error for object 0x16df7a695: pointer being freed was not allocatedSystem.Globalization.IOS.Tests(42002,0x16df7f000) malloc: *** set a breakpoint in malloc_error_break to debug

and it also only fails on the newest iOS simulator (17.2 works fine) + it works on the .NET 9 main. However, it doesn't get fixed by using this PR.

Do you know if we possibly export ICU symbols somewhere else in the Mono Runtime?@akoeplinger@mkhamoyan
Or any other idea what could be causing this?

@rolfbjarne
Copy link
Member

Do you know if we possibly export ICU symbols somewhere else in the Mono Runtime?

You can runnm on Mono's libraries to check which symbols they export.

matouskozak reacted with thumbs up emoji

@akoeplinger
Copy link
Member

The code in src/mono/mono/mini/CMakeLists.txtshould have fixed it but yeah check with nm.

matouskozak reacted with thumbs up emoji

@am11
Copy link
Member

I think#98495 has superseded this.

@vitek-karas
Copy link
Member

@am11 - that's true for .NET 9, but this change is trying to fix this for .NET 8.

am11, matouskozak, and mkhamoyan reacted with thumbs up emoji

@vitek-karasvitek-karas added this to the8.0.x milestoneMar 15, 2024
Copy link
Member

@steveisoksteveisok left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

LGTM!@matouskozak library mode should be ok because we only export symbols declared inUnmanagedCallersOnly and hide everything else.

mfloob and matouskozak reacted with thumbs up emoji
@matouskozak
Copy link
MemberAuthor

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

This was referencedMar 18, 2024
@matouskozakmatouskozak added the Servicing-considerIssue for next servicing release review labelMar 19, 2024
Copy link
Member

@jeffschwMSFTjeffschwMSFT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

approved. we will take for consideration in 8.0.x

@steveisoksteveisok added Servicing-approvedApproved for servicing release and removed Servicing-considerIssue for next servicing release review labelsMar 20, 2024
@matouskozakmatouskozak merged commitafb3fca intodotnet:release/8.0-stagingMar 20, 2024
@github-actionsgithub-actionsbot locked and limited conversation to collaboratorsApr 25, 2024
@matouskozakmatouskozak deleted the bug/IOS_simulator_ICU_crash branchOctober 3, 2024 13:14
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.

Reviewers

@rolfbjarnerolfbjarnerolfbjarne approved these changes

@steveisoksteveisoksteveisok approved these changes

@akoeplingerakoeplingerakoeplinger approved these changes

@jeffschwMSFTjeffschwMSFTjeffschwMSFT approved these changes

@vargazvargazAwaiting requested review from vargaz

@lambdageeklambdageekAwaiting requested review from lambdageeklambdageek is a code owner

@SamMonoRTSamMonoRTAwaiting requested review from SamMonoRTSamMonoRT is a code owner

Assignees

@matouskozakmatouskozak

Labels

area-System.Globalizationos-iosApple iOSruntime-monospecific to the Mono runtimeServicing-approvedApproved for servicing release

Projects

None yet

Milestone

8.0.x

Development

Successfully merging this pull request may close these issues.

8 participants

@matouskozak@rolfbjarne@ivanpovazan@akoeplinger@am11@vitek-karas@steveisok@jeffschwMSFT

[8]ページ先頭

©2009-2025 Movatter.jp