Safety check for "diamond dependencies" on netmodules
authorAndi McClure <andi.mcclure@xamarin.com>
Mon, 19 Sep 2016 21:55:33 +0000 (17:55 -0400)
committerAndi McClure <andi.mcclure@xamarin.com>
Mon, 19 Sep 2016 21:55:33 +0000 (17:55 -0400)
commit1dc438307e2a890f582d7c2802d99ff7e7af645c
tree8609026f7609b57a28cadba149dfd47e15bcafc7
parentf822445711c103e5aab649725a34f4e2ff73f32c
Safety check for "diamond dependencies" on netmodules

This is something I noticed while following up on an issue with
mcs/tests/test-418.exe. The ECMA-335 spec contains the text:

    "Usually, a module belongs only to one assembly, but it is
possible to share it across assemblies. When assembly A is loaded at
runtime, an instance of M3 will be loaded for it. When assembly B is
loaded into the same application domain, possibly simultaneously with
assembly A, M3 will be shared for both assemblies. Both assemblies
also reference F2, for which similar rules apply."

What happens in Mono when two assemblies both load the same module? We
never planned for that possibility in our architecture. Looking at our
current code, when an assembly image loads a module image into its
modules[] or files[] array, it immediately sets the "assembly" field
of that module to its own MonoAssembly. This field is a plain C
pointer with no safety on it whatsoever, and is references in various
places including by icalls. Consider the following scenario:

- Assembly A is loaded, references module M3, sets M3->assembly to
  A->assembly.
- Assembly B is loaded, references module M3, sets M3->assembly to
  b->assembly.
- Assembly B is unloaded.

At this point the image for B is unloaded, but M3 is still alive
because A keeps it alive, and its image contains a C pointer to freed
memory. There is likely some way to trigger a crash from this point.

Because the above scenario is fairly exotic, I dealt with it by just
causing the module loader to detect when multiple assemblies are
referencing the same module and throwing an exception in this case. I
also added a test to verify the exception is thrown. Allowing the
module load to raise an exception required adding _checked versions of
several functions.
13 files changed:
mono/metadata/assembly-internals.h [new file with mode: 0644]
mono/metadata/assembly.c
mono/metadata/assembly.h
mono/metadata/class.c
mono/metadata/icall.c
mono/metadata/image-internals.h
mono/metadata/image.c
mono/metadata/image.h
mono/tests/Makefile.am
mono/tests/test-multi-netmodule-1-netmodule.cs [new file with mode: 0644]
mono/tests/test-multi-netmodule-2-dll1.cs [new file with mode: 0644]
mono/tests/test-multi-netmodule-3-dll2.cs [new file with mode: 0644]
mono/tests/test-multi-netmodule-4-exe.cs [new file with mode: 0644]