From 9cd31f570c82cdc861d17c5dd4b7dd8dac83deb3 Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Thu, 19 Jul 2018 16:51:58 +0200 Subject: [PATCH] [Xamarin.Android.Build.Tasks] Fix ILRepack target rebuilds (#1979) Xamarin.Android uses the [ILRepack task][0] to repack/merge the `Xamarin.Android.Build.Tasks.dll` assembly (610ade7f), a step that is performed after the `Xamarin.Android.Build.Tasks.csproj` project's `Build` target completes. The step takes the project's output assembly -- `Xamarin.Android.Build.Tasks.dll` -- and several other dependencies, then merges them into one, *replacing* the original output assembly. The majority of time it works flawlessly, however when building the `monodroid` repo we observe a Mono runtime crash, similar-but-not- identical to [mono issue 9613][m9613]. Not all details are understood, but the high level overview of the issue is as follows: Mono runtime uses the **mmap**(2) system call on Unix to map the on-disk image of the assembly into memory. The `mmapp`ed area is used whenever the same assembly is loaded (e.g. `Assembly.LoadFrom()`) into the application to save time and memory. The memory is mapped as private which means that anything written to the mapped area by the Mono process is effectively discarded, that is not reflected in the file on disk. The protection, however, doesn't work in the other direction: when an external process (or even the same Mono process!) writes to the file that was `mmapp`ed. Unfortunately, the OS kernel behavior in such instance is left undefined by the POSIX standard: the kernel can choose to reflect the on-disk change in the mapping or ignore it. On macOS the write is ignored; on Linux, the write leads either to the `SIGBUS` signal being sent to the process or corruption to the `mmap`ped memory area (details of how this happens are, as of yet, unknown; it's just a theory at this point). The problem occurs in our case when the `` task overwrites the `Xamarin.Android.Build.Tasks.dll` assembly with its merged/repackaged form and that, somehow, corrupts Mono runtime's memory mapping of that assembly, eventually crashing: ## native crash #0 0x00007f974fd1b23a in __waitpid (pid=pid@entry=120561, stat_loc=stat_loc@entry=0x7f9712162b9c, options=options@entry=0) at ../sysdeps/unix/sysv/linux/waitpid.c:30 #1 0x000055b471f36c96 in dump_native_stacktrace (ctx=0x7f97121637c0, signal=0x55b472147cd3 "SIGSEGV") at mini-posix.c:719 #2 0x000055b471f36df2 in mono_dump_native_crash_info (signal=signal@entry=0x55b472147cd3 "SIGSEGV", ctx=ctx@entry=0x7f97121637c0, info=info@entry=0x7f97121638f0) at mini-posix.c:742 #3 0x000055b471ecc620 in mono_handle_native_crash (signal=signal@entry=0x55b472147cd3 "SIGSEGV", ctx=ctx@entry=0x7f97121637c0, info=info@entry=0x7f97121638f0) at mini-exceptions.c:2938 #4 0x000055b471e4c82c in mono_sigsegv_signal_handler (_dummy=7, _info=0x7f97121638f0, context=0x7f97121637c0) at mini-runtime.c:3441 #5 #6 __strcasecmp_l_avx () at ../sysdeps/x86_64/multiarch/strcmp-sse42.S:199 #7 0x000055b471fb2105 in mono_assembly_names_equal_flags (l=l@entry=0x7f96e0004370, r=r@entry=0x7f97307edb60, flags=flags@entry=MONO_ANAME_EQ_IGNORE_CASE) at assembly.c:663 #8 0x000055b471fa7baf in mono_domain_assembly_search (aname=0x7f96e0004370, user_data=0x0) at appdomain.c:2242 #9 0x000055b471fb10f7 in mono_assembly_invoke_search_hook_internal (aname=aname@entry=0x7f96e0004370, requesting=requesting@entry=0x0, refonly=0, postload=postload@entry=0) at assembly.c:1755 #10 0x000055b471fb4526 in mono_assembly_load_from_predicate (image=image@entry=0x7f96e001aae0, fname=fname@entry=0x7f96e000bc50 "/home/grendel/vc/xamarin/monodroid/monodroid/external/xamarin-android/src/Xamarin.Android.Build.Tasks/../../packages/ILRepack.Lib.MSBuild.Task.2.0.15.4/build/ILRepack.Lib.MSBuild.Task.dll", asmctx=asmctx@entry=MONO_ASMCTX_LOADFROM, predicate=predicate@entry=0x0, user_data=user_data@entry=0x0, status=status@entry=0x7f9712163fd4) at assembly.c:2638 #11 0x000055b471fb635c in mono_assembly_open_predicate (filename=, filename@entry=0x7f96e000ae40 "/home/grendel/vc/xamarin/monodroid/monodroid/external/xamarin-android/src/Xamarin.Android.Build.Tasks/../../packages/ILRepack.Lib.MSBuild.Task.2.0.15.4/build/ILRepack.Lib.MSBuild.Task.dll", asmctx=MONO_ASMCTX_LOADFROM, predicate=predicate@entry=0x0, user_data=user_data@entry=0x0, status=status@entry=0x7f9712163fd4) at assembly.c:2206 #12 0x000055b471fabd42 in ves_icall_System_Reflection_Assembly_LoadFrom (fname=..., refOnly=, error=0x7f9712164050) at appdomain.c:2272 ... ## managed stacktrace at <0xffffffff> at (wrapper managed-to-native) System.Reflection.Assembly.LoadFrom (string,bool) [0x0001b] in <79002cfb0e6e431d8465d6dcea08995e>:0 at System.Reflection.Assembly.LoadFrom (string) [0x00000] in /home/grendel/vc/mono/mono/mcs/class/corlib/System.Reflection/Assembly.cs:539 at System.Reflection.Assembly.UnsafeLoadFrom (string) [0x00000] in /home/grendel/vc/mono/mono/mcs/class/corlib/System.Reflection/Assembly.cs:571 at Microsoft.Build.Shared.TypeLoader.LoadAssembly (Microsoft.Build.Shared.AssemblyLoadInfo) [0x0001e] in :0 ... The crash occurs in frame 7, because the `r` parameter (an instance of the `MonoAssembly` structure) contains pointers that used to point to valid places in the `Xamarin.Android.Build.Tasks.dll`s `mmap`ed area, but now point to garbage data. One of those corrupted pointers is the ASCII assembly name - which is used by the function in frame 7. The crash this commit fixes occurs in a very specific scenario: when building the `OpenTK.csproj` project *directly* (e.g. `msbuild OpenTK.csproj`), which causes the `Xamarin.Android.Build.Tasks.dll` assembly to be loaded for use in the `` MSBuild statement, but at the same time `OpenTK.csproj` has a `@(ProjectReference)` to `Xamarin.Android.Build.Tasks.csproj`, which caused the `` task to run unconditionally, attempting to repack the assemblies again and leading to the crash described above. The fix, courtesy of @jonpryor, is to make the `ILRepacker` target not run every time but only whenever the assembly has to be re-packed because it has just been rebuilt. This also improves rebuild times. [0]: https://www.nuget.org/packages/ILRepack.Lib.MSBuild.Task [m9613]: https://github.com/mono/mono/issues/9613 --- .../ILRepack.targets | 25 ++++++++++++------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/ILRepack.targets b/src/Xamarin.Android.Build.Tasks/ILRepack.targets index 64c17aa32c6..5844888eec6 100644 --- a/src/Xamarin.Android.Build.Tasks/ILRepack.targets +++ b/src/Xamarin.Android.Build.Tasks/ILRepack.targets @@ -1,7 +1,13 @@  - + + <_ILRepacker_stamp>$(IntermediateOutputPath)ILRepacker.stamp + + @@ -16,15 +22,16 @@ +