Skip to content

Commit

Permalink
Fix ILRepack target on Linux
Browse files Browse the repository at this point in the history
Xamarin.Android uses the ILRepack
task (https://www.nuget.org/packages/ILRepack.Lib.MSBuild.Task/) to repack/merge
the `Xamarin.Android.Build.Tasks` assembly, a step that is performed after the
`Xamarin.Android.Build.Tasks` project build completes. The step takes the
project's output assembly and several other dependencies and merges them into
one, replacing the original output assembly. For the majority of time it works
flawlessly, however the scenario described below causes a Mono runtime
crash (similar to this one mono/mono#9613, although
not identical).

The crash causes aren't yet know in every detail, 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 mmapped area is used
whenever the same assembly is loaded (e.g. via `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 mmapped. 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, however, the write leads either to the SIGBUS
signal being sent to the process or corruption to the mmapped 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 leading to a segfault:

  #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
  dotnet#4  0x000055b471e4c82c in mono_sigsegv_signal_handler (_dummy=7, _info=0x7f97121638f0, context=0x7f97121637c0) at mini-runtime.c:3441
  dotnet#5  <signal handler called>
  dotnet#6  __strcasecmp_l_avx () at ../sysdeps/x86_64/multiarch/strcmp-sse42.S:199
  dotnet#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
  dotnet#8  0x000055b471fa7baf in mono_domain_assembly_search (aname=0x7f96e0004370, user_data=0x0) at appdomain.c:2242
  dotnet#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
  dotnet#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
  dotnet#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
  dotnet#12 0x000055b471fabd42 in ves_icall_System_Reflection_Assembly_LoadFrom (fname=..., refOnly=<optimized out>, error=0x7f9712164050) at appdomain.c:2272
  dotnet#13 0x00000000412f4007 in ?? ()
  dotnet#14 0x00007f97440e1918 in ?? ()
  dotnet#15 0x00007f9749bd1800 in ?? ()
  dotnet#16 0x00000000e36e75af in ?? ()
  dotnet#17 0x00007f974434fda8 in ?? ()
  dotnet#18 0x00007f9749bd18e0 in ?? ()
  dotnet#19 0x00007f96e0002820 in ?? ()
  dotnet#20 0x00007f9712164160 in ?? ()
  dotnet#21 0x00007f9712164010 in ?? ()
  dotnet#22 0x0000000000000000 in ?? ()

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` assembbly's mmaped 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 situation can be detected by the runtime by using advisory locks and
reporting the attempt to overwrite the mmapped area, however a fix would require
creating a copy of all the assembly's in-memory data before the write is allowed
and that might be too expensive. None of those measures are currently
implemented by Mono (and it isn't sure if any of them will ever be implemented).

The crash this commit fixes occurs in a very specific scenario - when building
the `OpenTK.csproj` project *directly* (that is passing it as parameter to
`msbuild`), which causes a the `Xamarin.Android.Build.Tasks` assembly to be
loaded for use in the `<UsingTask/>` msbuild statement, but at the same time
`OpenTK.csproj` references the `Xamarin.Android.Build.Tasks` *project* which
causes 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 whenver the assembly has to be re-packed because it has just been
rebuilt.
  • Loading branch information
grendello committed Jul 18, 2018
1 parent 5995f0c commit 798de2e
Showing 1 changed file with 9 additions and 1 deletion.
10 changes: 9 additions & 1 deletion 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="Build"
Inputs="$(OutputPath)\$(AssemblyName).dll"
Outputs="$(_ILRepacker_stamp)">
<ItemGroup>
<InputAssemblies Include="$(OutputPath)\NuGet.ProjectModel.dll" />
<InputAssemblies Include="$(OutputPath)\Newtonsoft.Json.dll" />
Expand All @@ -26,5 +32,7 @@
OutputFile="$(OutputPath)\$(AssemblyName).dll"
/>
<Delete Files="@(InputAssemblies)" />
<Touch Files="$(_ILRepacker_stamp)"
AlwaysCreate="True"/>
</Target>
</Project>

0 comments on commit 798de2e

Please sign in to comment.