Skip to content

Commit

Permalink
[Xamarin.Android.Build.Tasks] Fix ILRepack target rebuilds (#1979)
Browse files Browse the repository at this point in the history
Xamarin.Android uses the [ILRepack task][0] to repack/merge the
`Xamarin.Android.Build.Tasks.dll` assembly (610ade7), 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
`<ILRepacker/>` 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  <signal handler called>
        #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=<optimized out>, 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=<optimized out>, error=0x7f9712164050) at appdomain.c:2272
        ...

        ## managed stacktrace
        at <unknown> <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 <bdc5207a22bb42ae9fe3f3d07e82871e>: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 `<UsingTask/>` MSBuild statement, but at the same time 
`OpenTK.csproj` has a `@(ProjectReference)` to
`Xamarin.Android.Build.Tasks.csproj`, which caused the
`<ILRepacker/>` 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]: mono/mono#9613
  • Loading branch information
grendello authored and jonpryor committed Aug 3, 2018
1 parent a404031 commit 9cd31f5
Showing 1 changed file with 16 additions and 9 deletions.
25 changes: 16 additions & 9 deletions src/Xamarin.Android.Build.Tasks/ILRepack.targets
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
<?xml version="1.0" encoding="UTF-8"?>
<Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
<UsingTask AssemblyFile="$(MSBuildThisFileDirectory)..\..\packages\ILRepack.Lib.MSBuild.Task.2.0.15.4\build\ILRepack.Lib.MSBuild.Task.dll" TaskName="ILRepack" />
<Target Name="ILRepacker" AfterTargets="Build">
<PropertyGroup>
<_ILRepacker_stamp>$(IntermediateOutputPath)ILRepacker.stamp</_ILRepacker_stamp>
</PropertyGroup>
<Target Name="ILRepacker"
AfterTargets="AfterBuild"
Inputs="$(OutputPath)\$(AssemblyName).dll"
Outputs="$(_ILRepacker_stamp)">
<ItemGroup>
<InputAssemblies Include="$(OutputPath)\NuGet.ProjectModel.dll" />
<InputAssemblies Include="$(OutputPath)\Newtonsoft.Json.dll" />
Expand All @@ -16,15 +22,16 @@
<InputAssemblies Include="$(OutputPath)\NuGet.Configuration.dll" />
</ItemGroup>
<ILRepack Parallel="true"
Internalize="true"
Verbose="true"
DebugInfo="true"
InternalizeExclude="@(DoNotInternalizeAssemblies)"
InputAssemblies="$(OutputPath)\$(AssemblyName).dll;@(InputAssemblies)"
LibraryPath="$(OutputPath)"
TargetKind="Dll"
OutputFile="$(OutputPath)\$(AssemblyName).dll"
Internalize="true"
Verbose="true"
DebugInfo="true"
InternalizeExclude="@(DoNotInternalizeAssemblies)"
InputAssemblies="$(OutputPath)\$(AssemblyName).dll;@(InputAssemblies)"
LibraryPath="$(OutputPath)"
TargetKind="Dll"
OutputFile="$(OutputPath)\$(AssemblyName).dll"
/>
<Delete Files="@(InputAssemblies)" />
<Touch AlwaysCreate="True" Files="$(_ILRepacker_stamp)" />
</Target>
</Project>

0 comments on commit 9cd31f5

Please sign in to comment.